-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from 4 commits
5aaa44d
75947a9
e2ab766
ab72767
36e9fa5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -700,53 +700,105 @@ 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, | ||
Rule::OverIndented, | ||
Rule::IndentWithSpaces, | ||
for rule in [ | ||
// The formatter might collapse implicit string concatenation on a single line. | ||
Rule::SingleLineImplicitStringConcatenation, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]) | ||
{ | ||
match ( | ||
setting.linter.flake8_quotes.inline_quotes, | ||
setting.formatter.quote_style, | ||
) { | ||
(Quote::Double, QuoteStyle::Single) => { | ||
warn!("The `flake8-quotes.inline-quotes=\"double\"` option is incompatible with the formatter's `format.quote-style=\"single\"`. We recommend disabling `Q000` and `Q003` when using the formatter, which enforces a consistent quote style. Alternatively, set both options to either `\"single\"` or `\"double\"`."); | ||
} | ||
(Quote::Single, QuoteStyle::Double) => { | ||
warn!("The `flake8-quotes.inline-quotes=\"single\"` option is incompatible with the formatter's `format.quote-style=\"double\"`. We recommend disabling `Q000` and `Q003` when using the formatter, which enforces a consistent quote style. Alternatively, set both options to either `\"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, which enforces double quotes for multiline strings. Alternatively, set 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.multiline-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling `Q002` when using the formatter, which enforces double quotes for docstrings. Alternatively, set the `flake8-quotes.docstring-quotes` option to `\"double\"`.`"); | ||
} | ||
|
||
if setting.linter.rules.enabled(Rule::UnsortedImports) { | ||
// The formatter removes empty lines if the value is larger than 2 but always inserts a empty line after imports. | ||
// Two empty lines are okay because `isort` only uses this setting for top-level imports (not in nested blocks). | ||
if !matches!(setting.linter.isort.lines_after_imports, 1 | 2 | -1) { | ||
warn!("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)."); | ||
warn!("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)."); | ||
} | ||
|
||
// Values larger than two get reduced to one line by the formatter if the import is in a nested block. | ||
if setting.linter.isort.lines_between_types > 1 { | ||
warn!("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)."); | ||
warn!("The isort option `isort.lines-between-types` with a value greater than 1 is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `1` or `0` (default)."); | ||
} | ||
|
||
// isort inserts a trailing comma which the formatter preserves, but only if `skip-magic-trailing-comma` isn't false. | ||
if setting.formatter.magic_trailing_comma.is_ignore() { | ||
if setting.linter.isort.force_wrap_aliases { | ||
warn!("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'."); | ||
warn!("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`."); | ||
} | ||
|
||
if setting.linter.isort.split_on_trailing_comma { | ||
warn!("The isort option 'isort.split-on-trailing-comma' is incompatible with the formatter 'format.skip-magic-trailing-comma=true' option. To avoid unexpected behavior, we recommend either setting 'isort.split-on-trailing-comma=false' or 'format.skip-magic-trailing-comma=false'."); | ||
warn!("The isort option `isort.split-on-trailing-comma` is incompatible with the formatter `format.skip-magic-trailing-comma=true` option. To avoid unexpected behavior, we recommend either setting `isort.split-on-trailing-comma=false` or `format.skip-magic-trailing-comma=false`."); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
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.