Refine warning about incompatible isort settings (#8192)

crates/ruff_cli/src/commands/format.rs

@@ -19,7 +19,6 @@ use ruff_diagnostics::SourceMap;
 use ruff_linter::fs;
 use ruff_linter::logging::LogLevel;
 use ruff_linter::registry::Rule;
-use ruff_linter::rules::isort;
 use ruff_linter::settings::rule_table::RuleTable;
 use ruff_linter::source_kind::{SourceError, SourceKind};
 use ruff_linter::warn_user_once;
@@ -713,32 +712,28 @@ pub(super) fn warn_incompatible_formatter_settings(
             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 incompatible_options = Vec::new();
-
-        let isort_defaults = isort::settings::Settings::default();
-
-        if setting.linter.isort.force_single_line != isort_defaults.force_single_line {
-            incompatible_options.push("'isort.force-single-line'");
-        }
-
-        if setting.linter.isort.force_wrap_aliases != isort_defaults.force_wrap_aliases {
-            incompatible_options.push("'isort.force-wrap-aliases'");
-        }
-
-        if setting.linter.isort.lines_after_imports != isort_defaults.lines_after_imports {
-            incompatible_options.push("'isort.lines-after-imports'");
-        }
+        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).");
+            }
 
-        if setting.linter.isort.lines_between_types != isort_defaults.lines_between_types {
-            incompatible_options.push("'isort.lines_between_types'");
-        }
+            // 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).");
+            }
 
-        if setting.linter.isort.split_on_trailing_comma != isort_defaults.split_on_trailing_comma {
-            incompatible_options.push("'isort.split_on_trailing_comma'");
-        }
+            // 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'.");
+                }
 
-        if !incompatible_options.is_empty() {
-            warn!("The following isort options may cause conflicts when used with the formatter: {}. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.", incompatible_options.join(", "));
+                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'.");
+                }
+            }
         }
     }
 }

crates/ruff_cli/tests/format.rs

@@ -363,11 +363,14 @@ select = ["ALL"]
 ignore = ["D203", "D212"]
 
 [isort]
-force-single-line = true
-force-wrap-aliases = true
-lines-after-imports = 0
+lines-after-imports = 3
 lines-between-types = 2
+force-wrap-aliases = true
+combine-as-imports = true
 split-on-trailing-comma = true
+
+[format]
+skip-magic-trailing-comma = true
 "#,
     )?;
 
@@ -390,7 +393,10 @@ def say_hy(name: str):
 
     ----- 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 isort options may cause conflicts when used with the formatter: 'isort.force-single-line', 'isort.force-wrap-aliases', 'isort.lines-after-imports', 'isort.lines_between_types'. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.
+    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'.
+    warning: 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'.
     "###);
     Ok(())
 }
@@ -406,11 +412,14 @@ select = ["ALL"]
 ignore = ["D203", "D212"]
 
 [isort]
-force-single-line = true
-force-wrap-aliases = true
-lines-after-imports = 0
+lines-after-imports = 3
 lines-between-types = 2
+force-wrap-aliases = true
+combine-as-imports = true
 split-on-trailing-comma = true
+
+[format]
+skip-magic-trailing-comma = true
 "#,
     )?;
 
@@ -429,7 +438,55 @@ def say_hy(name: str):
 
     ----- 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 isort options may cause conflicts when used with the formatter: 'isort.force-single-line', 'isort.force-wrap-aliases', 'isort.lines-after-imports', 'isort.lines_between_types'. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.
+    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'.
+    warning: 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'.
+    "###);
+    Ok(())
+}
+
+#[test]
+fn valid_linter_options() -> Result<()> {
+    let tempdir = TempDir::new()?;
+    let ruff_toml = tempdir.path().join("ruff.toml");
+    fs::write(
+        &ruff_toml,
+        r#"
+select = ["ALL"]
+ignore = ["D203", "D212"]
+
+[isort]
+lines-after-imports = 2
+lines-between-types = 1
+force-wrap-aliases = true
+combine-as-imports = true
+split-on-trailing-comma = true
+
+[format]
+skip-magic-trailing-comma = false
+"#,
+    )?;
+
+    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 -----
+    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.
     "###);
     Ok(())
 }

crates/ruff_python_formatter/src/options.rs

@@ -250,6 +250,10 @@ impl MagicTrailingComma {
     pub const fn is_respect(self) -> bool {
         matches!(self, Self::Respect)
     }
+
+    pub const fn is_ignore(self) -> bool {
+        matches!(self, Self::Ignore)
+    }
 }
 
 impl FromStr for MagicTrailingComma {

crates/ruff_workspace/src/options.rs

@@ -1683,6 +1683,9 @@ pub struct IsortOptions {
     /// `combine-as-imports = true`. When `combine-as-imports` isn't
     /// enabled, every aliased `import from` will be given its own line, in
     /// which case, wrapping is not necessary.
+    ///
+    /// When using the formatter, ensure that `format.skip-magic-trailing-comma` is set to `false` (default)
+    /// when enabling `force-wrap-aliases` to avoid that the formatter collapses members if they all fit on a single line.
     #[option(
         default = r#"false"#,
         value_type = "bool",
@@ -1726,6 +1729,9 @@ pub struct IsortOptions {
     /// the imports will never be folded into one line.
     ///
     /// See isort's [`split-on-trailing-comma`](https://pycqa.github.io/isort/docs/configuration/options.html#split-on-trailing-comma) option.
+    ///
+    /// When using the formatter, ensure that `format.skip-magic-trailing-comma` is set to `false` (default) when enabling `split-on-trailing-comma`
+    /// to avoid that the formatter removes the trailing commas.
     #[option(
         default = r#"true"#,
         value_type = "bool",
@@ -1907,6 +1913,9 @@ pub struct IsortOptions {
 
     /// The number of blank lines to place after imports.
     /// Use `-1` for automatic determination.
+    ///
+    /// When using the formatter, only the values `-1`, `1`, and `2` are compatible because
+    /// it enforces at least one empty and at most two empty lines after imports.
     #[option(
         default = r#"-1"#,
         value_type = "int",
@@ -1918,6 +1927,9 @@ pub struct IsortOptions {
     pub lines_after_imports: Option<isize>,
 
     /// The number of lines to place between "direct" and `import from` imports.
+    ///
+    /// When using the formatter, only the values `0` and `1` are compatible because
+    /// it preserves up to one empty line after imports in nested blocks.
     #[option(
         default = r#"0"#,
         value_type = "int",