From cb588d1d6da9c5409c91ce0ae2aa41eafdd566f6 Mon Sep 17 00:00:00 2001
From: Jonathan Plasse <13716151+JonathanPlasse@users.noreply.github.com>
Date: Wed, 29 Mar 2023 21:13:12 +0200
Subject: [PATCH] Allow `TID252` to fix all valid module paths (#3796)

---
 .../my_package/sublib/api/application.py      |  1 +
 .../sublib/protocol/UpperCaseModule.py        |  2 ++
 .../flake8_tidy_imports/relative_imports.rs   | 24 +++++++++++++------
 ...ts__tests__ban_parent_imports_package.snap | 21 ++++++++++++++++
 4 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100644 crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/protocol/UpperCaseModule.py

diff --git a/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/api/application.py b/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/api/application.py
index 03aa1b834cc96..be08607c3213d 100644
--- a/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/api/application.py
+++ b/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/api/application.py
@@ -7,3 +7,4 @@
 from ..server import example
 from .. import server
 from . import logger, models
+from ..protocol.UpperCaseModule import some_function
diff --git a/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/protocol/UpperCaseModule.py b/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/protocol/UpperCaseModule.py
new file mode 100644
index 0000000000000..11ffb986471eb
--- /dev/null
+++ b/crates/ruff/resources/test/fixtures/flake8_tidy_imports/TID252/my_package/sublib/protocol/UpperCaseModule.py
@@ -0,0 +1,2 @@
+def some_function():
+    pass
diff --git a/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs b/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs
index f0db904fd240f..c7c6e575128b3 100644
--- a/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs
+++ b/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs
@@ -7,7 +7,8 @@ use ruff_macros::{derive_message_formats, violation, CacheKey};
 use ruff_python_ast::helpers::{create_stmt, from_relative_import, unparse_stmt};
 use ruff_python_ast::source_code::Stylist;
 use ruff_python_ast::types::Range;
-use ruff_python_stdlib::identifiers::is_module_name;
+use ruff_python_stdlib::identifiers::is_identifier;
+use ruff_python_stdlib::keyword::KWLIST;
 
 use crate::checkers::ast::Checker;
 use crate::registry::AsRule;
@@ -106,25 +107,34 @@ fn fix_banned_relative_import(
 
         let module_name = if let Some(module) = module {
             let call_path = from_relative_import(&parts, module);
-            // Require import to be a valid PEP 8 module:
+            // Require import to be a valid module:
             // https://python.org/dev/peps/pep-0008/#package-and-module-names
-            if !call_path.iter().all(|part| is_module_name(part)) {
+            if !call_path
+                .iter()
+                .all(|part| is_identifier(part) && !KWLIST.contains(part))
+            {
                 return None;
             }
             call_path.as_slice().join(".")
         } else if parts.len() > 1 {
             let module = parts.pop().unwrap();
             let call_path = from_relative_import(&parts, &module);
-            // Require import to be a valid PEP 8 module:
+            // Require import to be a valid module:
             // https://python.org/dev/peps/pep-0008/#package-and-module-names
-            if !call_path.iter().all(|part| is_module_name(part)) {
+            if !call_path
+                .iter()
+                .all(|part| is_identifier(part) && !KWLIST.contains(part))
+            {
                 return None;
             }
             call_path.as_slice().join(".")
         } else {
-            // Require import to be a valid PEP 8 module:
+            // Require import to be a valid module:
             // https://python.org/dev/peps/pep-0008/#package-and-module-names
-            if !parts.iter().all(|part| is_module_name(part)) {
+            if !parts
+                .iter()
+                .all(|part| is_identifier(part) && !KWLIST.contains(&part.as_str()))
+            {
                 return None;
             }
             parts.join(".")
diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap
index 0b208b812ed51..8c84403bf3ecd 100644
--- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap
+++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap
@@ -121,4 +121,25 @@ expression: diagnostics
           row: 8
           column: 21
   parent: ~
+- kind:
+    name: RelativeImports
+    body: Relative imports from parent modules are banned
+    suggestion: Replace relative imports from parent modules with absolute imports
+    fixable: true
+  location:
+    row: 10
+    column: 0
+  end_location:
+    row: 10
+    column: 52
+  fix:
+    edits:
+      - content: from my_package.sublib.protocol.UpperCaseModule import some_function
+        location:
+          row: 10
+          column: 0
+        end_location:
+          row: 10
+          column: 52
+  parent: ~