Skip to content

Commit a4dd1e5

Browse files
authored
Refine the warnings about incompatible linter options (#8196)
## Summary Avoid warning about incompatible rules except if their configuration directly conflicts with the formatter. This should reduce the noise and potentially the need for #8175 and #8185 I also extended the rule and option documentation to mention any potential formatter incompatibilities or whether they're redundant when using the formatter. * `LineTooLong`: This is a use case we explicitly want to support. Don't warn about it * `TabIndentation`, `IndentWithSpaces`: Only warn if `indent-style="tab"` * `IndentationWithInvalidMultiple`, `IndentationWithInvalidMultipleComment`: Only warn if `indent-width != 4` * `OverIndented`: Don't warn, but mention that the rule is redundant * `BadQuotesInlineString`: Warn if quote setting is different from `format.quote-style` * `BadQuotesMultilineString`, `BadQuotesDocstring`: Warn if `quote != "double"` ## Test Plan I added a new integration test for the default configuration with `ALL`. `ruff format` now only shows two incompatible rules, which feels more reasonable.
1 parent be3307e commit a4dd1e5

File tree

9 files changed

+247
-49
lines changed

9 files changed

+247
-49
lines changed

crates/ruff_cli/src/commands/format.rs

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ use ruff_diagnostics::SourceMap;
1818
use ruff_linter::fs;
1919
use ruff_linter::logging::LogLevel;
2020
use ruff_linter::registry::Rule;
21-
use ruff_linter::settings::rule_table::RuleTable;
21+
use ruff_linter::rules::flake8_quotes::settings::Quote;
2222
use ruff_linter::source_kind::{SourceError, SourceKind};
2323
use ruff_linter::warn_user_once;
2424
use ruff_python_ast::{PySourceType, SourceType};
25-
use ruff_python_formatter::{format_module_source, FormatModuleError};
25+
use ruff_python_formatter::{format_module_source, FormatModuleError, QuoteStyle};
2626
use ruff_text_size::{TextLen, TextRange, TextSize};
2727
use ruff_workspace::resolver::{
2828
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile, Resolver,
@@ -700,53 +700,105 @@ pub(super) fn warn_incompatible_formatter_settings(
700700
{
701701
let mut incompatible_rules = Vec::new();
702702

703-
for incompatible_rule in RuleTable::from_iter([
704-
Rule::LineTooLong,
705-
Rule::TabIndentation,
706-
Rule::IndentationWithInvalidMultiple,
707-
Rule::IndentationWithInvalidMultipleComment,
708-
Rule::OverIndented,
709-
Rule::IndentWithSpaces,
703+
for rule in [
704+
// The formatter might collapse implicit string concatenation on a single line.
710705
Rule::SingleLineImplicitStringConcatenation,
706+
// Flags missing trailing commas when all arguments are on its own line:
707+
// ```python
708+
// def args(
709+
// aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh
710+
// ):
711+
// pass
712+
// ```
711713
Rule::MissingTrailingComma,
712-
Rule::ProhibitedTrailingComma,
713-
Rule::BadQuotesInlineString,
714-
Rule::BadQuotesMultilineString,
715-
Rule::BadQuotesDocstring,
716-
Rule::AvoidableEscapedQuote,
717-
])
718-
.iter_enabled()
719-
{
720-
if setting.linter.rules.enabled(incompatible_rule) {
721-
incompatible_rules.push(format!("'{}'", incompatible_rule.noqa_code()));
714+
] {
715+
if setting.linter.rules.enabled(rule) {
716+
incompatible_rules.push(rule);
717+
}
718+
}
719+
720+
// Rules asserting for space indentation
721+
if setting.formatter.indent_style.is_tab() {
722+
for rule in [Rule::TabIndentation, Rule::IndentWithSpaces] {
723+
if setting.linter.rules.enabled(rule) {
724+
incompatible_rules.push(rule);
725+
}
726+
}
727+
}
728+
729+
// Rules asserting for indent-width=4
730+
if setting.formatter.indent_width.value() != 4 {
731+
for rule in [
732+
Rule::IndentationWithInvalidMultiple,
733+
Rule::IndentationWithInvalidMultipleComment,
734+
] {
735+
if setting.linter.rules.enabled(rule) {
736+
incompatible_rules.push(rule);
737+
}
722738
}
723739
}
724740

725741
if !incompatible_rules.is_empty() {
726-
incompatible_rules.sort();
727-
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(", "));
742+
let mut rule_names: Vec<_> = incompatible_rules
743+
.into_iter()
744+
.map(|rule| format!("`{}`", rule.noqa_code()))
745+
.collect();
746+
rule_names.sort();
747+
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(", "));
748+
}
749+
750+
// Rules with different quote styles.
751+
if setting
752+
.linter
753+
.rules
754+
.any_enabled(&[Rule::BadQuotesInlineString, Rule::AvoidableEscapedQuote])
755+
{
756+
match (
757+
setting.linter.flake8_quotes.inline_quotes,
758+
setting.formatter.quote_style,
759+
) {
760+
(Quote::Double, QuoteStyle::Single) => {
761+
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\"`.");
762+
}
763+
(Quote::Single, QuoteStyle::Double) => {
764+
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\"`.");
765+
}
766+
_ => {}
767+
}
768+
}
769+
770+
if setting.linter.rules.enabled(Rule::BadQuotesMultilineString)
771+
&& setting.linter.flake8_quotes.multiline_quotes == Quote::Single
772+
{
773+
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\"`.`");
774+
}
775+
776+
if setting.linter.rules.enabled(Rule::BadQuotesDocstring)
777+
&& setting.linter.flake8_quotes.docstring_quotes == Quote::Single
778+
{
779+
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\"`.`");
728780
}
729781

730782
if setting.linter.rules.enabled(Rule::UnsortedImports) {
731783
// The formatter removes empty lines if the value is larger than 2 but always inserts a empty line after imports.
732784
// Two empty lines are okay because `isort` only uses this setting for top-level imports (not in nested blocks).
733785
if !matches!(setting.linter.isort.lines_after_imports, 1 | 2 | -1) {
734-
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).");
786+
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).");
735787
}
736788

737789
// Values larger than two get reduced to one line by the formatter if the import is in a nested block.
738790
if setting.linter.isort.lines_between_types > 1 {
739-
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).");
791+
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).");
740792
}
741793

742794
// isort inserts a trailing comma which the formatter preserves, but only if `skip-magic-trailing-comma` isn't false.
743795
if setting.formatter.magic_trailing_comma.is_ignore() {
744796
if setting.linter.isort.force_wrap_aliases {
745-
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'.");
797+
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`.");
746798
}
747799

748800
if setting.linter.isort.split_on_trailing_comma {
749-
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'.");
801+
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`.");
750802
}
751803
}
752804
}

0 commit comments

Comments
 (0)