isort: fix bad interaction between force-sort-within-sections and force-to-top (#3645)

Author: bluetech

crates/ruff/resources/test/fixtures/isort/force_sort_within_sections.py

@@ -2,7 +2,9 @@
 from c import * # import_from_star
 import a  # import
 import c.d
+from z import z1
 import b as b1  # import_as
+import z
 
 from ..parent import *
 from .my import fn

crates/ruff/src/rules/isort/mod.rs

@@ -665,6 +665,7 @@ mod tests {
             &Settings {
                 isort: super::settings::Settings {
                     force_sort_within_sections: true,
+                    force_to_top: BTreeSet::from(["z".to_string()]),
                     ..super::settings::Settings::default()
                 },
                 src: vec![test_resource_path("fixtures/isort")],

crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__force_sort_within_sections.py.snap

@@ -11,15 +11,15 @@ expression: diagnostics
     row: 1
     column: 0
   end_location:
-    row: 12
+    row: 14
     column: 0
   fix:
-    content: "import a  # import\nimport b as b1  # import_as\nimport c.d\nfrom a import a1  # import_from\nfrom c import *  # import_from_star\n\nfrom ...grandparent import fn3\nfrom ..parent import *\nfrom . import my\nfrom .my import fn\nfrom .my.nested import fn2\n"
+    content: "import a  # import\nimport b as b1  # import_as\nimport c.d\nimport z\nfrom a import a1  # import_from\nfrom c import *  # import_from_star\nfrom z import z1\n\nfrom ...grandparent import fn3\nfrom ..parent import *\nfrom . import my\nfrom .my import fn\nfrom .my.nested import fn2\n"
     location:
       row: 1
       column: 0
     end_location:
-      row: 12
+      row: 14
       column: 0
   parent: ~
 

crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__force_sort_within_sections_force_sort_within_sections.py.snap

@@ -11,15 +11,15 @@ expression: diagnostics
     row: 1
     column: 0
   end_location:
-    row: 12
+    row: 14
     column: 0
   fix:
-    content: "import a  # import\nfrom a import a1  # import_from\nimport b as b1  # import_as\nfrom c import *  # import_from_star\nimport c.d\n\nfrom ...grandparent import fn3\nfrom ..parent import *\nfrom . import my\nfrom .my import fn\nfrom .my.nested import fn2\n"
+    content: "import z\nfrom z import z1\nimport a  # import\nfrom a import a1  # import_from\nimport b as b1  # import_as\nfrom c import *  # import_from_star\nimport c.d\n\nfrom ...grandparent import fn3\nfrom ..parent import *\nfrom . import my\nfrom .my import fn\nfrom .my.nested import fn2\n"
     location:
       row: 1
       column: 0
     end_location:
-      row: 12
+      row: 14
       column: 0
   parent: ~
 

crates/ruff/src/rules/isort/sorting.rs

@@ -44,29 +44,28 @@ fn prefix(
     }
 }
 
+/// Compare two module names' by their `force-to-top`ness.
+fn cmp_force_to_top(name1: &str, name2: &str, force_to_top: &BTreeSet<String>) -> Ordering {
+    let force_to_top1 = force_to_top.contains(name1);
+    let force_to_top2 = force_to_top.contains(name2);
+    force_to_top1.cmp(&force_to_top2).reverse()
+}
+
 /// Compare two top-level modules.
 pub fn cmp_modules(
     alias1: &AliasData,
     alias2: &AliasData,
     force_to_top: &BTreeSet<String>,
 ) -> Ordering {
-    (match (
-        force_to_top.contains(alias1.name),
-        force_to_top.contains(alias2.name),
-    ) {
-        (true, true) => Ordering::Equal,
-        (false, false) => Ordering::Equal,
-        (true, false) => Ordering::Less,
-        (false, true) => Ordering::Greater,
-    })
-    .then_with(|| natord::compare_ignore_case(alias1.name, alias2.name))
-    .then_with(|| natord::compare(alias1.name, alias2.name))
-    .then_with(|| match (alias1.asname, alias2.asname) {
-        (None, None) => Ordering::Equal,
-        (None, Some(_)) => Ordering::Less,
-        (Some(_), None) => Ordering::Greater,
-        (Some(asname1), Some(asname2)) => natord::compare(asname1, asname2),
-    })
+    cmp_force_to_top(alias1.name, alias2.name, force_to_top)
+        .then_with(|| natord::compare_ignore_case(alias1.name, alias2.name))
+        .then_with(|| natord::compare(alias1.name, alias2.name))
+        .then_with(|| match (alias1.asname, alias2.asname) {
+            (None, None) => Ordering::Equal,
+            (None, Some(_)) => Ordering::Less,
+            (Some(_), None) => Ordering::Greater,
+            (Some(asname1), Some(asname2)) => natord::compare(asname1, asname2),
+        })
 }
 
 /// Compare two member imports within `StmtKind::ImportFrom` blocks.
@@ -124,15 +123,11 @@ pub fn cmp_import_from(
         relative_imports_order,
     )
     .then_with(|| {
-        match (
-            force_to_top.contains(&import_from1.module_name()),
-            force_to_top.contains(&import_from2.module_name()),
-        ) {
-            (true, true) => Ordering::Equal,
-            (false, false) => Ordering::Equal,
-            (true, false) => Ordering::Less,
-            (false, true) => Ordering::Greater,
-        }
+        cmp_force_to_top(
+            &import_from1.module_name(),
+            &import_from2.module_name(),
+            force_to_top,
+        )
     })
     .then_with(|| match (&import_from1.module, import_from2.module) {
         (None, None) => Ordering::Equal,
@@ -143,6 +138,17 @@ pub fn cmp_import_from(
     })
 }
 
+/// Compare an import to an import-from.
+fn cmp_import_import_from(
+    import: &AliasData,
+    import_from: &ImportFromData,
+    force_to_top: &BTreeSet<String>,
+) -> Ordering {
+    cmp_force_to_top(import.name, &import_from.module_name(), force_to_top).then_with(|| {
+        natord::compare_ignore_case(import.name, import_from.module.unwrap_or_default())
+    })
+}
+
 /// Compare two [`EitherImport`] enums which may be [`Import`] or [`ImportFrom`]
 /// structs.
 pub fn cmp_either_import(
@@ -154,10 +160,10 @@ pub fn cmp_either_import(
     match (a, b) {
         (Import((alias1, _)), Import((alias2, _))) => cmp_modules(alias1, alias2, force_to_top),
         (ImportFrom((import_from, ..)), Import((alias, _))) => {
-            natord::compare_ignore_case(import_from.module.unwrap_or_default(), alias.name)
+            cmp_import_import_from(alias, import_from, force_to_top).reverse()
         }
         (Import((alias, _)), ImportFrom((import_from, ..))) => {
-            natord::compare_ignore_case(alias.name, import_from.module.unwrap_or_default())
+            cmp_import_import_from(alias, import_from, force_to_top)
         }
         (ImportFrom((import_from1, ..)), ImportFrom((import_from2, ..))) => cmp_import_from(
             import_from1,