fix(isort): comply with isort when sorting sub-package modules

e.g. packages named "foo.bar"

additionally make sure `__future__` imports are always sorted at the
very top.

Issue #3059

crates/ruff/resources/test/fixtures/isort/pyproject.toml

@@ -4,5 +4,3 @@ line-length = 88
 [tool.ruff.isort]
 lines-after-imports = 3
 lines-between-types = 2
-known-local-folder = ["ruff"]
-force-to-top = ["lib1", "lib3", "lib5", "lib3.lib4", "z"]

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

@@ -3,3 +3,8 @@
 import numpy as np
 import os
 from leading_prefix import Class
+import baz
+from foo import bar, baz
+from foo.bar import blah, blub
+import foo
+import foo.bar

crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs

@@ -92,7 +92,7 @@ fn fix_banned_relative_import(
     module_path: Option<&Vec<String>>,
     stylist: &Stylist,
 ) -> Option<Fix> {
-    // Only fix is the module path is known.
+    // Only fix if the module path is known.
     if let Some(mut parts) = module_path.cloned() {
         // Remove relative level from module path.
         for _ in 0..*level? {

crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

@@ -150,15 +150,13 @@ pub fn typing_only_runtime_import(
         && binding.runtime_usage.is_none()
         && binding.synthetic_usage.is_none()
     {
-        // Extract the module base and level from the full name.
-        // Ex) `foo.bar.baz` -> `foo`, `0`
+        // Extract the module level from the full name.
         // Ex) `.foo.bar.baz` -> `foo`, `1`
-        let module_base = full_name.split('.').next().unwrap();
         let level = full_name.chars().take_while(|c| *c == '.').count();
 
         // Categorize the import.
         match categorize(
-            module_base,
+            full_name,
             Some(&level),
             &settings.src,
             package,

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

@@ -38,7 +38,7 @@ enum Reason<'a> {
 
 #[allow(clippy::too_many_arguments)]
 pub fn categorize(
-    module_base: &str,
+    module_name: &str,
     level: Option<&usize>,
     src: &[PathBuf],
     package: Option<&Path>,
@@ -48,19 +48,20 @@ pub fn categorize(
     extra_standard_library: &BTreeSet<String>,
     target_version: PythonVersion,
 ) -> ImportType {
+    let module_base = module_name.split('.').next().unwrap();
     let (import_type, reason) = {
         if level.map_or(false, |level| *level > 0) {
             (ImportType::LocalFolder, Reason::NonZeroLevel)
-        } else if known_first_party.contains(module_base) {
+        } else if module_base == "__future__" {
+            (ImportType::Future, Reason::Future)
+        } else if known_first_party.contains(module_name) {
             (ImportType::FirstParty, Reason::KnownFirstParty)
-        } else if known_third_party.contains(module_base) {
+        } else if known_third_party.contains(module_name) {
             (ImportType::ThirdParty, Reason::KnownThirdParty)
-        } else if known_local_folder.contains(module_base) {
+        } else if known_local_folder.contains(module_name) {
             (ImportType::LocalFolder, Reason::KnownLocalFolder)
-        } else if extra_standard_library.contains(module_base) {
+        } else if extra_standard_library.contains(module_name) {
             (ImportType::StandardLibrary, Reason::ExtraStandardLibrary)
-        } else if module_base == "__future__" {
-            (ImportType::Future, Reason::Future)
         } else if KNOWN_STANDARD_LIBRARY
             .get(&target_version.as_tuple())
             .unwrap()
@@ -77,7 +78,7 @@ pub fn categorize(
     };
     debug!(
         "Categorized '{}' as {:?} ({:?})",
-        module_base, import_type, reason
+        module_name, import_type, reason
     );
     import_type
 }
@@ -117,7 +118,7 @@ pub fn categorize_imports<'a>(
     // Categorize `StmtKind::Import`.
     for (alias, comments) in block.import {
         let import_type = categorize(
-            &alias.module_base(),
+            &alias.module_name(),
             None,
             src,
             package,
@@ -136,7 +137,7 @@ pub fn categorize_imports<'a>(
     // Categorize `StmtKind::ImportFrom` (without re-export).
     for (import_from, aliases) in block.import_from {
         let classification = categorize(
-            &import_from.module_base(),
+            &import_from.module_name(),
             import_from.level,
             src,
             package,
@@ -155,7 +156,7 @@ pub fn categorize_imports<'a>(
     // Categorize `StmtKind::ImportFrom` (with re-export).
     for ((import_from, alias), comments) in block.import_from_as {
         let classification = categorize(
-            &import_from.module_base(),
+            &import_from.module_name(),
             import_from.level,
             src,
             package,
@@ -174,7 +175,7 @@ pub fn categorize_imports<'a>(
     // Categorize `StmtKind::ImportFrom` (with star).
     for (import_from, comments) in block.import_from_star {
         let classification = categorize(
-            &import_from.module_base(),
+            &import_from.module_name(),
             import_from.level,
             src,
             package,

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

@@ -375,10 +375,6 @@ mod tests {
     #[test_case(Path::new("preserve_tabs_2.py"))]
     #[test_case(Path::new("relative_imports_order.py"))]
     #[test_case(Path::new("reorder_within_section.py"))]
-    #[test_case(Path::new("separate_first_party_imports.py"))]
-    #[test_case(Path::new("separate_future_imports.py"))]
-    #[test_case(Path::new("separate_local_folder_imports.py"))]
-    #[test_case(Path::new("separate_third_party_imports.py"))]
     #[test_case(Path::new("skip.py"))]
     #[test_case(Path::new("skip_file.py"))]
     #[test_case(Path::new("sort_similar_imports.py"))]
@@ -399,6 +395,31 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Path::new("separate_first_party_imports.py"))]
+    #[test_case(Path::new("separate_future_imports.py"))]
+    #[test_case(Path::new("separate_local_folder_imports.py"))]
+    #[test_case(Path::new("separate_third_party_imports.py"))]
+    fn separate_modules(path: &Path) -> Result<()> {
+        let snapshot = format!("{}", path.to_string_lossy());
+        let diagnostics = test_path(
+            Path::new("isort").join(path).as_path(),
+            &Settings {
+                isort: super::settings::Settings {
+                    known_first_party: BTreeSet::from(["foo.bar".to_string(), "baz".to_string()]),
+                    known_third_party: BTreeSet::from([
+                        "foo".to_string(),
+                        "__future__".to_string(),
+                    ]),
+                    ..super::settings::Settings::default()
+                },
+                src: vec![test_resource_path("fixtures/isort")],
+                ..Settings::for_rule(Rule::UnsortedImports)
+            },
+        )?;
+        assert_yaml_snapshot!(snapshot, diagnostics);
+        Ok(())
+    }
+
     // Test currently disabled as line endings are automatically converted to
     // platform-appropriate ones in CI/CD #[test_case(Path::new("
     // line_ending_crlf.py"))] #[test_case(Path::new("line_ending_lf.py"))]

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

@@ -1,5 +1,5 @@
 ---
-source: src/rules/isort/mod.rs
+source: crates/ruff/src/rules/isort/mod.rs
 expression: diagnostics
 ---
 - kind:
@@ -8,23 +8,28 @@ expression: diagnostics
     row: 1
     column: 0
   end_location:
-    row: 6
+    row: 11
     column: 0
   fix:
     content:
       - import os
       - import sys
       - ""
+      - import foo
       - import numpy as np
+      - "from foo import bar, baz"
       - ""
+      - import baz
+      - import foo.bar
       - import leading_prefix
+      - "from foo.bar import blah, blub"
       - from leading_prefix import Class
       - ""
     location:
       row: 1
       column: 0
     end_location:
-      row: 6
+      row: 11
       column: 0
   parent: ~