Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine the warnings about incompatible linter options #8196

Merged
merged 5 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 69 additions & 21 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ use ruff_diagnostics::SourceMap;
use ruff_linter::fs;
use ruff_linter::logging::LogLevel;
use ruff_linter::registry::Rule;
use ruff_linter::settings::rule_table::RuleTable;
use ruff_linter::rules::flake8_quotes::settings::Quote;
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::warn_user_once;
use ruff_python_ast::{PySourceType, SourceType};
use ruff_python_formatter::{format_module_source, FormatModuleError};
use ruff_python_formatter::{format_module_source, FormatModuleError, QuoteStyle};
use ruff_text_size::{TextLen, TextRange, TextSize};
use ruff_workspace::resolver::{
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile, Resolver,
Expand Down Expand Up @@ -700,31 +700,79 @@ pub(super) fn warn_incompatible_formatter_settings(
{
let mut incompatible_rules = Vec::new();

for incompatible_rule in RuleTable::from_iter([
Rule::LineTooLong,
Rule::TabIndentation,
Rule::IndentationWithInvalidMultiple,
Rule::IndentationWithInvalidMultipleComment,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I these think these are redundant when using tabs in the formatter.

Rule::OverIndented,
Rule::IndentWithSpaces,
for rule in [
// The formatter might collapse implicit string concatenation on a single line.
Rule::SingleLineImplicitStringConcatenation,
Copy link
Member Author

@MichaReiser MichaReiser Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm undecided if we should warn about this rule. The formatter might collapse implicit string concatenated lines which then triggers this rule. However, it might be something users want to fix manually.

Same for MissingTrailingComma. The rule might require manual intervention (and magic-trailing-comma set to respect), but once fixed it remains fixed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the formatter collapses the strings, I would love to know about it to fix it afterward.

// Flags missing trailing commas when all arguments are on its own line:
// ```python
// def args(
// aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh
// ):
// pass
// ```
Rule::MissingTrailingComma,
Rule::ProhibitedTrailingComma,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed because the formatter will never output code that triggers it? Is that right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe this is the case.

Rule::BadQuotesInlineString,
Rule::BadQuotesMultilineString,
Rule::BadQuotesDocstring,
Rule::AvoidableEscapedQuote,
])
.iter_enabled()
{
if setting.linter.rules.enabled(incompatible_rule) {
incompatible_rules.push(format!("'{}'", incompatible_rule.noqa_code()));
] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}

// Rules asserting for space indentation
if setting.formatter.indent_style.is_tab() {
for rule in [Rule::TabIndentation, Rule::IndentWithSpaces] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}
}

// Rules asserting for indent-width=4
if setting.formatter.indent_width.value() != 4 {
for rule in [
Rule::IndentationWithInvalidMultiple,
Rule::IndentationWithInvalidMultipleComment,
] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}
}

if !incompatible_rules.is_empty() {
incompatible_rules.sort();
warn!("The following rules may cause conflicts when used with the formatter: {}. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.", incompatible_rules.join(", "));
let mut rule_names: Vec<_> = incompatible_rules
.into_iter()
.map(|rule| format!("'{}'", rule.noqa_code()))
.collect();
rule_names.sort();
warn!("The following rules may cause conflicts when used with the formatter: {}. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.", rule_names.join(", "));
}

// Rules with different quote styles.
if setting
.linter
.rules
.any_enabled(&[Rule::BadQuotesInlineString, Rule::AvoidableEscapedQuote])
&& matches!(
(
setting.formatter.quote_style,
setting.linter.flake8_quotes.inline_quotes
),
(QuoteStyle::Single, Quote::Double) | (QuoteStyle::Double, Quote::Single)
)
{
warn!("The `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `\"single\"` or `\"double\"`.");
}

if setting.linter.rules.enabled(Rule::BadQuotesMultilineString)
&& setting.linter.flake8_quotes.multiline_quotes == Quote::Single
{
warn!("The `flake8-quotes.multiline-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling 'Q001' when using the formatter because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change the `flake8-quotes.multiline-quotes` option to `\"double\"`.`");
}

if setting.linter.rules.enabled(Rule::BadQuotesDocstring)
&& setting.linter.flake8_quotes.docstring_quotes == Quote::Single
{
warn!("The `flake8-quotes.docstring-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `\"double\"`.");
}

if setting.linter.rules.enabled(Rule::UnsortedImports) {
Expand Down
91 changes: 81 additions & 10 deletions crates/ruff_cli/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ fn format_option_inheritance() -> Result<()> {
&ruff_toml,
r#"
extend = "base.toml"
extend-select = ["Q000"]

[lint]
extend-select = ["COM812"]

[format]
quote-style = "single"
Expand Down Expand Up @@ -273,7 +275,7 @@ if condition:
print('Should change quotes')

----- stderr -----
warning: The following rules may cause conflicts when used with the formatter: 'Q000'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: The following rules may cause conflicts when used with the formatter: 'COM812'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
"###);
Ok(())
}
Expand Down Expand Up @@ -359,18 +361,27 @@ fn conflicting_options() -> Result<()> {
fs::write(
&ruff_toml,
r#"
indent-width = 2

[lint]
select = ["ALL"]
ignore = ["D203", "D212"]

[isort]
[lint.isort]
lines-after-imports = 3
lines-between-types = 2
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true

[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "single"
multiline-quotes = "single"

[format]
skip-magic-trailing-comma = true
indent-style = "tab"
"#,
)?;

Expand All @@ -392,7 +403,10 @@ def say_hy(name: str):
1 file reformatted

----- stderr -----
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'COM819', 'D206', 'E501', 'ISC001', 'Q000', 'Q001', 'Q002', 'Q003', 'W191'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'D206', 'ISC001', 'W191'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: The `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `"single"` or `"double"`.
warning: The `flake8-quotes.multiline-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q001' when using the formatter because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change the `flake8-quotes.multiline-quotes` option to `"double"`.`
warning: The `flake8-quotes.docstring-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `"double"`.
warning: The isort option 'isort.lines-after-imports' with a value other than `-1`, `1` or `2` is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `2`, `1`, or `-1` (default).
warning: The isort option 'isort.lines-between-types' with a value larger than 1 is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `1` or `0` (default).
warning: The isort option 'isort.force-wrap-aliases' is incompatible with the formatter 'format.skip-magic-trailing-comma=true' option. To avoid unexpected behavior, we recommend either setting 'isort.force-wrap-aliases=false' or 'format.skip-magic-trailing-comma=false'.
Expand All @@ -408,18 +422,27 @@ fn conflicting_options_stdin() -> Result<()> {
fs::write(
&ruff_toml,
r#"
indent-width = 2

[lint]
select = ["ALL"]
ignore = ["D203", "D212"]

[isort]
[lint.isort]
lines-after-imports = 3
lines-between-types = 2
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true

[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "single"
multiline-quotes = "single"

[format]
skip-magic-trailing-comma = true
indent-style = "tab"
"#,
)?;

Expand All @@ -434,10 +457,13 @@ def say_hy(name: str):
exit_code: 0
----- stdout -----
def say_hy(name: str):
print(f"Hy {name}")
print(f"Hy {name}")

----- stderr -----
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'COM819', 'D206', 'E501', 'ISC001', 'Q000', 'Q001', 'Q002', 'Q003', 'W191'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'D206', 'ISC001', 'W191'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: The `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `"single"` or `"double"`.
warning: The `flake8-quotes.multiline-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q001' when using the formatter because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change the `flake8-quotes.multiline-quotes` option to `"double"`.`
warning: The `flake8-quotes.docstring-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `"double"`.
warning: The isort option 'isort.lines-after-imports' with a value other than `-1`, `1` or `2` is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `2`, `1`, or `-1` (default).
warning: The isort option 'isort.lines-between-types' with a value larger than 1 is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `1` or `0` (default).
warning: The isort option 'isort.force-wrap-aliases' is incompatible with the formatter 'format.skip-magic-trailing-comma=true' option. To avoid unexpected behavior, we recommend either setting 'isort.force-wrap-aliases=false' or 'format.skip-magic-trailing-comma=false'.
Expand All @@ -453,18 +479,60 @@ fn valid_linter_options() -> Result<()> {
fs::write(
&ruff_toml,
r#"
[lint]
select = ["ALL"]
ignore = ["D203", "D212"]
ignore = ["D203", "D212", "COM812", "ISC001"]

[isort]
[lint.isort]
lines-after-imports = 2
lines-between-types = 1
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true

[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "double"
multiline-quotes = "double"

[format]
skip-magic-trailing-comma = false
quote-style = "single"
"#,
)?;

let test_path = tempdir.path().join("test.py");
fs::write(
&test_path,
r#"
def say_hy(name: str):
print(f"Hy {name}")"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["format", "--no-cache", "--config"])
.arg(&ruff_toml)
.arg(test_path), @r###"
success: true
exit_code: 0
----- stdout -----
1 file reformatted

----- stderr -----
"###);
Ok(())
}

#[test]
fn all_rules_default_options() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");

fs::write(
&ruff_toml,
r#"
[lint]
select = ["ALL"]
"#,
)?;

Expand All @@ -486,10 +554,13 @@ def say_hy(name: str):
1 file reformatted

----- stderr -----
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'COM819', 'D206', 'E501', 'ISC001', 'Q000', 'Q001', 'Q002', 'Q003', 'W191'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
warning: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'ISC001'. To avoid unexpected behavior, we recommend disabling these rules, either by removing them from the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.
"###);
Ok(())
}

#[test]
fn test_diff() {
let args = ["format", "--no-cache", "--isolated", "--diff"];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ use crate::settings::LinterSettings;
/// ```python
/// foo = "bar's"
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter automatically removes
/// unnecessary escapes, making this rule redundant.
#[violation]
pub struct AvoidableEscapedQuote;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ use super::super::settings::Quote;
///
/// ## Options
/// - `flake8-quotes.inline-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// quotes for inline strings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesInlineString {
preferred_quote: Quote,
Expand Down Expand Up @@ -81,6 +85,10 @@ impl AlwaysFixableViolation for BadQuotesInlineString {
///
/// ## Options
/// - `flake8-quotes.multiline-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces double quotes
/// for multiline strings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesMultilineString {
preferred_quote: Quote,
Expand Down Expand Up @@ -129,6 +137,10 @@ impl AlwaysFixableViolation for BadQuotesMultilineString {
///
/// ## Options
/// - `flake8-quotes.docstring-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces double
/// quotes for docstrings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesDocstring {
preferred_quote: Quote,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ use super::LogicalLine;
/// a = 1
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `indent-width` with a value other than `4`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
#[violation]
pub struct IndentationWithInvalidMultiple {
Expand Down Expand Up @@ -54,6 +58,9 @@ impl Violation for IndentationWithInvalidMultiple {
/// if True:
/// # a = 1
/// ```
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `indent-width` with a value other than `4`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
#[violation]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ use ruff_text_size::{TextLen, TextRange, TextSize};
/// a = 1
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `format.indent-style="tab"`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#tabs-or-spaces
#[violation]
pub struct TabIndentation;
Expand Down
8 changes: 8 additions & 0 deletions crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ use crate::registry::Rule;
/// """
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `format.indent-style="tab"`.
///
/// ## References
/// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/)
/// - [NumPy Style Guide](https://numpydoc.readthedocs.io/en/latest/format.html)
Expand Down Expand Up @@ -126,6 +130,10 @@ impl AlwaysFixableViolation for UnderIndentation {
/// """
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant.
///
/// ## References
/// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/)
/// - [NumPy Style Guide](https://numpydoc.readthedocs.io/en/latest/format.html)
Expand Down
Loading