[flake8-tidy-imports] autofix relative imports (#2891)

Previous fix was bugged. This one is only fixing when the `module_path` is present, making it far more robust.

Closes #2764 and closes #2869

crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252.py

@@ -18,8 +18,6 @@ from \
     ..parent\
     import \
     world_hello
-
-# TID252 (without autofix; too many levels up)
 from ..... import ultragrantparent
 from ...... import ultragrantparent
 from ....... import ultragrantparent

crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/api/application.py

@@ -0,0 +1,7 @@
+from typing import TYPE_CHECKING, Any, ClassVar
+ 
+import attrs
+
+from ..protocol import commands, definitions, responses
+from ..server import example
+from . import logger, models

crates/ruff/src/checkers/ast.rs

@@ -1320,8 +1320,8 @@ where
                                 stmt,
                                 level.as_ref(),
                                 module.as_deref(),
+                                self.module_path.as_ref(),
                                 &self.settings.flake8_tidy_imports.ban_relative_imports,
-                                self.path,
                             )
                         {
                             self.diagnostics.push(diagnostic);

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

@@ -1,5 +1,3 @@
-use std::path::Path;
-
 use rustpython_parser::ast::{Stmt, StmtKind};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
@@ -7,7 +5,7 @@ use serde::{Deserialize, Serialize};
 use ruff_macros::{define_violation, derive_message_formats};
 use ruff_python::string::is_lower_with_underscore;
 
-use crate::ast::helpers::{create_stmt, unparse_stmt};
+use crate::ast::helpers::{create_stmt, from_relative_import, unparse_stmt};
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::fix::Fix;
@@ -32,7 +30,7 @@ define_violation!(
     /// Checks for relative imports.
     ///
     /// ## Why is this bad?
-    /// Absolute imports, or relative imports from siblings, are recommended by [PEP 8](https://peps.python.org/pep-0008/#imports):
+    /// Absolute imports, or relative imports from siblings, are recommended by [PEP 8]:
     ///
     /// > Absolute imports are recommended, as they are usually more readable and tend to be better behaved...
     /// > ```python
@@ -60,6 +58,8 @@ define_violation!(
     /// ```python
     /// from mypkg import foo
     /// ```
+    ///
+    /// [PEP 8]: https://peps.python.org/pep-0008/#imports
     pub struct RelativeImports {
         pub strictness: Strictness,
     }
@@ -89,51 +89,45 @@ fn fix_banned_relative_import(
     stmt: &Stmt,
     level: Option<&usize>,
     module: Option<&str>,
-    path: &Path,
+    module_path: Option<&Vec<String>>,
     stylist: &Stylist,
 ) -> Option<Fix> {
-    let base = if let Some(module) = module {
-        module.to_string()
+    // Only fix is the module path is known
+    if let Some(mut parts) = module_path.cloned() {
+        // Remove relative level from module path
+        for _ in 0..*level? {
+            parts.pop();
+        }
+
+        let call_path = from_relative_import(&parts, module.unwrap());
+        let module_name = call_path.as_slice();
+
+        // Require import to be a valid PEP 8 module:
+        // https://python.org/dev/peps/pep-0008/#package-and-module-names
+        if module_name.iter().any(|f| !is_lower_with_underscore(f)) {
+            return None;
+        }
+
+        let content = match &stmt.node {
+            StmtKind::ImportFrom { names, .. } => unparse_stmt(
+                &create_stmt(StmtKind::ImportFrom {
+                    module: Some(module_name.join(".")),
+                    names: names.clone(),
+                    level: Some(0),
+                }),
+                stylist,
+            ),
+            _ => return None,
+        };
+
+        Some(Fix::replacement(
+            content,
+            stmt.location,
+            stmt.end_location.unwrap(),
+        ))
     } else {
-        String::new()
-    };
-
-    let mut parent = path.parent()?;
-    for _ in 0..*level? {
-        parent = parent.parent()?;
+        None
     }
-
-    let module_name = parent.file_name()?.to_string_lossy().to_string();
-
-    // Require import to be a valid PEP 8 module:
-    // https://python.org/dev/peps/pep-0008/#package-and-module-names
-    if !is_lower_with_underscore(module_name.as_str()) {
-        return None;
-    }
-
-    let new_import = if base.is_empty() {
-        module_name
-    } else {
-        format!("{}.{}", module_name, base)
-    };
-
-    let content = match &stmt.node {
-        StmtKind::ImportFrom { names, .. } => unparse_stmt(
-            &create_stmt(StmtKind::ImportFrom {
-                module: Some(new_import),
-                names: names.clone(),
-                level: Some(0),
-            }),
-            stylist,
-        ),
-        _ => return None,
-    };
-
-    Some(Fix::replacement(
-        content,
-        stmt.location,
-        stmt.end_location.unwrap(),
-    ))
 }
 
 /// TID252
@@ -142,8 +136,8 @@ pub fn banned_relative_import(
     stmt: &Stmt,
     level: Option<&usize>,
     module: Option<&str>,
+    module_path: Option<&Vec<String>>,
     strictness: &Strictness,
-    path: &Path,
 ) -> Option<Diagnostic> {
     let strictness_level = match strictness {
         Strictness::All => 0,
@@ -158,7 +152,7 @@ pub fn banned_relative_import(
         );
         if checker.patch(diagnostic.kind.rule()) {
             if let Some(fix) =
-                fix_banned_relative_import(stmt, level, module, path, checker.stylist)
+                fix_banned_relative_import(stmt, level, module, module_path, checker.stylist)
             {
                 diagnostic.amend(fix);
             };
@@ -213,4 +207,21 @@ mod tests {
         assert_yaml_snapshot!(diagnostics);
         Ok(())
     }
+
+    #[test]
+    fn ban_parent_imports_package() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("flake8_tidy_imports/TID252/my_package/sublib/api/application.py"),
+            &Settings {
+                flake8_tidy_imports: super::super::Settings {
+                    ban_relative_imports: Strictness::Parents,
+                    ..Default::default()
+                },
+                namespace_packages: vec![Path::new("my_package").to_path_buf()],
+                ..Settings::for_rules(vec![Rule::RelativeImports])
+            },
+        )?;
+        assert_yaml_snapshot!(diagnostics);
+        Ok(())
+    }
 }

crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_all_imports.snap

@@ -11,15 +11,7 @@ expression: diagnostics
   end_location:
     row: 7
     column: 21
-  fix:
-    content:
-      - from fixtures import sibling
-    location:
-      row: 7
-      column: 0
-    end_location:
-      row: 7
-      column: 21
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -30,15 +22,7 @@ expression: diagnostics
   end_location:
     row: 8
     column: 28
-  fix:
-    content:
-      - from fixtures.sibling import example
-    location:
-      row: 8
-      column: 0
-    end_location:
-      row: 8
-      column: 28
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -49,15 +33,7 @@ expression: diagnostics
   end_location:
     row: 9
     column: 21
-  fix:
-    content:
-      - from test import parent
-    location:
-      row: 9
-      column: 0
-    end_location:
-      row: 9
-      column: 21
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -68,15 +44,7 @@ expression: diagnostics
   end_location:
     row: 10
     column: 28
-  fix:
-    content:
-      - from test.parent import example
-    location:
-      row: 10
-      column: 0
-    end_location:
-      row: 10
-      column: 28
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -87,15 +55,7 @@ expression: diagnostics
   end_location:
     row: 11
     column: 27
-  fix:
-    content:
-      - from resources import grandparent
-    location:
-      row: 11
-      column: 0
-    end_location:
-      row: 11
-      column: 27
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -106,15 +66,7 @@ expression: diagnostics
   end_location:
     row: 12
     column: 34
-  fix:
-    content:
-      - from resources.grandparent import example
-    location:
-      row: 12
-      column: 0
-    end_location:
-      row: 12
-      column: 34
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -125,15 +77,7 @@ expression: diagnostics
   end_location:
     row: 13
     column: 26
-  fix:
-    content:
-      - from fixtures.parent import hello
-    location:
-      row: 13
-      column: 0
-    end_location:
-      row: 13
-      column: 26
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -144,15 +88,7 @@ expression: diagnostics
   end_location:
     row: 16
     column: 19
-  fix:
-    content:
-      - from fixtures.parent import hello_world
-    location:
-      row: 14
-      column: 0
-    end_location:
-      row: 16
-      column: 19
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -163,24 +99,16 @@ expression: diagnostics
   end_location:
     row: 20
     column: 15
-  fix:
-    content:
-      - from test.parent import world_hello
-    location:
-      row: 17
-      column: 0
-    end_location:
-      row: 20
-      column: 15
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
       strictness: all
   location:
-    row: 23
+    row: 21
     column: 0
   end_location:
-    row: 23
+    row: 21
     column: 34
   fix: ~
   parent: ~
@@ -188,10 +116,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 24
+    row: 22
     column: 0
   end_location:
-    row: 24
+    row: 22
     column: 35
   fix: ~
   parent: ~
@@ -199,10 +127,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 25
+    row: 23
     column: 0
   end_location:
-    row: 25
+    row: 23
     column: 36
   fix: ~
   parent: ~
@@ -210,10 +138,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 26
+    row: 24
     column: 0
   end_location:
-    row: 26
+    row: 24
     column: 38
   fix: ~
   parent: ~
@@ -221,10 +149,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 27
+    row: 25
     column: 0
   end_location:
-    row: 27
+    row: 25
     column: 56
   fix: ~
   parent: ~
@@ -232,10 +160,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 28
+    row: 26
     column: 0
   end_location:
-    row: 28
+    row: 26
     column: 40
   fix: ~
   parent: ~
@@ -243,10 +171,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 29
+    row: 27
     column: 0
   end_location:
-    row: 29
+    row: 27
     column: 44
   fix: ~
   parent: ~
@@ -254,10 +182,10 @@ expression: diagnostics
     RelativeImports:
       strictness: all
   location:
-    row: 30
+    row: 28
     column: 0
   end_location:
-    row: 30
+    row: 28
     column: 62
   fix: ~
   parent: ~

crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports.snap

@@ -11,15 +11,7 @@ expression: diagnostics
   end_location:
     row: 9
     column: 21
-  fix:
-    content:
-      - from test import parent
-    location:
-      row: 9
-      column: 0
-    end_location:
-      row: 9
-      column: 21
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -30,15 +22,7 @@ expression: diagnostics
   end_location:
     row: 10
     column: 28
-  fix:
-    content:
-      - from test.parent import example
-    location:
-      row: 10
-      column: 0
-    end_location:
-      row: 10
-      column: 28
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -49,15 +33,7 @@ expression: diagnostics
   end_location:
     row: 11
     column: 27
-  fix:
-    content:
-      - from resources import grandparent
-    location:
-      row: 11
-      column: 0
-    end_location:
-      row: 11
-      column: 27
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -68,15 +44,7 @@ expression: diagnostics
   end_location:
     row: 12
     column: 34
-  fix:
-    content:
-      - from resources.grandparent import example
-    location:
-      row: 12
-      column: 0
-    end_location:
-      row: 12
-      column: 34
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
@@ -87,24 +55,16 @@ expression: diagnostics
   end_location:
     row: 20
     column: 15
-  fix:
-    content:
-      - from test.parent import world_hello
-    location:
-      row: 17
-      column: 0
-    end_location:
-      row: 20
-      column: 15
+  fix: ~
   parent: ~
 - kind:
     RelativeImports:
       strictness: parents
   location:
-    row: 23
+    row: 21
     column: 0
   end_location:
-    row: 23
+    row: 21
     column: 34
   fix: ~
   parent: ~
@@ -112,10 +72,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 24
+    row: 22
     column: 0
   end_location:
-    row: 24
+    row: 22
     column: 35
   fix: ~
   parent: ~
@@ -123,10 +83,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 25
+    row: 23
     column: 0
   end_location:
-    row: 25
+    row: 23
     column: 36
   fix: ~
   parent: ~
@@ -134,10 +94,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 26
+    row: 24
     column: 0
   end_location:
-    row: 26
+    row: 24
     column: 38
   fix: ~
   parent: ~
@@ -145,10 +105,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 27
+    row: 25
     column: 0
   end_location:
-    row: 27
+    row: 25
     column: 56
   fix: ~
   parent: ~
@@ -156,10 +116,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 28
+    row: 26
     column: 0
   end_location:
-    row: 28
+    row: 26
     column: 40
   fix: ~
   parent: ~
@@ -167,10 +127,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 29
+    row: 27
     column: 0
   end_location:
-    row: 29
+    row: 27
     column: 44
   fix: ~
   parent: ~
@@ -178,10 +138,10 @@ expression: diagnostics
     RelativeImports:
       strictness: parents
   location:
-    row: 30
+    row: 28
     column: 0
   end_location:
-    row: 30
+    row: 28
     column: 62
   fix: ~
   parent: ~

crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap

@@ -0,0 +1,81 @@
+---
+source: crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs
+expression: diagnostics
+---
+- kind:
+    RelativeImports:
+      strictness: parents
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 55
+  fix:
+    content:
+      - "from my_package.sublib.protocol import commands, definitions, responses"
+    location:
+      row: 5
+      column: 0
+    end_location:
+      row: 5
+      column: 55
+  parent: ~
+- kind:
+    RelativeImports:
+      strictness: parents
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 55
+  fix:
+    content:
+      - "from my_package.sublib.protocol import commands, definitions, responses"
+    location:
+      row: 5
+      column: 0
+    end_location:
+      row: 5
+      column: 55
+  parent: ~
+- kind:
+    RelativeImports:
+      strictness: parents
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 55
+  fix:
+    content:
+      - "from my_package.sublib.protocol import commands, definitions, responses"
+    location:
+      row: 5
+      column: 0
+    end_location:
+      row: 5
+      column: 55
+  parent: ~
+- kind:
+    RelativeImports:
+      strictness: parents
+  location:
+    row: 6
+    column: 0
+  end_location:
+    row: 6
+    column: 28
+  fix:
+    content:
+      - from my_package.sublib.server import example
+    location:
+      row: 6
+      column: 0
+    end_location:
+      row: 6
+      column: 28
+  parent: ~
+

resources/test/popmon

@@ -0,0 +1 @@
+Subproject commit 0949e0fce929528413ceeb29484db94d9a237d69