[flake8-type-checking] Add test cases for TC004

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -1130,6 +1130,16 @@ impl<'a> Visitor<'a> for Checker<'a> {
                     }
                 }
             }
+            Expr::Attribute(ast::ExprAttribute {
+                value: _,
+                range: _,
+                ctx,
+                attr: _,
+            }) => {
+                if ctx == &ExprContext::Load {
+                    self.handle_attribute_load(expr);
+                }
+            }
             Expr::Name(ast::ExprName { id, ctx, range: _ }) => match ctx {
                 ExprContext::Load => self.handle_node_load(expr),
                 ExprContext::Store => self.handle_node_store(id, expr),
@@ -2046,6 +2056,13 @@ impl<'a> Checker<'a> {
         self.semantic.resolve_load(expr);
     }
 
+    fn handle_attribute_load(&mut self, expr: &Expr) {
+        let Expr::Attribute(expr) = expr else {
+            return;
+        };
+        self.semantic.resolve_attribute_load(expr);
+    }
+
     fn handle_node_store(&mut self, id: &'a str, expr: &Expr) {
         let parent = self.semantic.current_statement();
 

crates/ruff_linter/src/rules/flake8_type_checking/mod.rs

@@ -471,6 +471,42 @@ mod tests {
     ",
         "tc010_precedence_over_tc008"
     )]
+    #[test_case(
+        r"
+        from __future__ import annotations
+        import importlib.abc
+        from typing import TYPE_CHECKING
+        if TYPE_CHECKING:
+            import importlib.machinery
+        class Foo(importlib.abc.MetaPathFinder):
+            def bar(self) -> importlib.machinery.ModuleSpec: ...
+    ",
+        "github_issue_15723_regression_test"
+    )]
+    #[test_case(
+        r"
+        from __future__ import annotations
+        from typing import TYPE_CHECKING
+        if TYPE_CHECKING:
+            import importlib.abc
+            import importlib.machinery
+        class Foo(importlib.abc.MetaPathFinder):
+            def bar(self) -> importlib.machinery.ModuleSpec: ...
+    ",
+        "github_issue_15723_false_negative"
+    )]
+    #[test_case(
+        r"
+        from __future__ import annotations
+        from typing import TYPE_CHECKING
+        if TYPE_CHECKING:
+            import importlib.machinery
+            import importlib.abc
+        class Foo(importlib.abc.MetaPathFinder):
+            def bar(self) -> importlib.machinery.ModuleSpec: ...
+    ",
+        "github_issue_15723_ideal_import_order"
+    )]
     fn contents(contents: &str, snapshot: &str) {
         let diagnostics = test_snippet(
             contents,

crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs

@@ -2,7 +2,7 @@ use std::borrow::Cow;
 
 use anyhow::Result;
 use rustc_hash::FxHashMap;
-
+use unicode_normalization::UnicodeNormalization;
 use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_semantic::{Imported, NodeId, Scope};
@@ -106,74 +106,91 @@ pub(crate) fn runtime_import_in_type_checking_block(
     let mut actions: FxHashMap<(NodeId, Action), Vec<ImportBinding>> = FxHashMap::default();
 
     for binding_id in scope.binding_ids() {
-        let binding = checker.semantic().binding(binding_id);
+        let top_binding = checker.semantic().binding(binding_id);
 
-        let Some(import) = binding.as_any_import() else {
+        let Some(_) = top_binding.as_any_import() else {
             continue;
         };
 
-        let Some(reference_id) = binding.references.first().copied() else {
+        let Some(reference_id) = top_binding.references.first().copied() else {
             continue;
         };
 
-        if binding.context.is_typing()
-            && binding.references().any(|reference_id| {
+        let binding_name = top_binding
+            .name(checker.source())
+            .split('.')
+            .next()
+            .unwrap_or("");
+
+        // NOTE: It’s necessary to go through all bindings, including shadowed ones,
+        //       using the module name.
+        for binding in scope
+            .get_all(&binding_name.nfkc().collect::<String>())
+            .map(|binding_id| checker.semantic().binding(binding_id))
+        {
+            if binding.context.is_typing()
+                && binding.references().any(|reference_id| {
                 checker
                     .semantic()
                     .reference(reference_id)
                     .in_runtime_context()
             })
-        {
-            let Some(node_id) = binding.source else {
-                continue;
-            };
+            {
+                let Some(import) = binding.as_any_import() else {
+                    continue;
+                };
 
-            let import = ImportBinding {
-                import,
-                reference_id,
-                binding,
-                range: binding.range(),
-                parent_range: binding.parent_range(checker.semantic()),
-            };
+                let Some(node_id) = binding.source else {
+                    continue;
+                };
 
-            if checker.rule_is_ignored(Rule::RuntimeImportInTypeCheckingBlock, import.start())
-                || import.parent_range.is_some_and(|parent_range| {
+                let import = ImportBinding {
+                    import,
+                    reference_id,
+                    binding,
+                    range: binding.range(),
+                    parent_range: binding.parent_range(checker.semantic()),
+                };
+
+                if checker.rule_is_ignored(Rule::RuntimeImportInTypeCheckingBlock, import.start())
+                    || import.parent_range.is_some_and(|parent_range| {
                     checker.rule_is_ignored(
                         Rule::RuntimeImportInTypeCheckingBlock,
                         parent_range.start(),
                     )
                 })
-            {
-                actions
-                    .entry((node_id, Action::Ignore))
-                    .or_default()
-                    .push(import);
-            } else {
-                // Determine whether the member should be fixed by moving the import out of the
-                // type-checking block, or by quoting its references.
-                // TODO: We should check `reference.in_annotated_type_alias()`
-                //       as well to match the behavior of the flake8 plugin
-                //       although maybe the best way forward is to add an
-                //       additional setting to configure whether quoting
-                //       or moving the import is preferred for type aliases
-                //       since some people will consistently use their
-                //       type aliases at runtimes, while others won't, so
-                //       the best solution is unclear.
-                if checker.settings.flake8_type_checking.quote_annotations
-                    && binding.references().all(|reference_id| {
-                        let reference = checker.semantic().reference(reference_id);
-                        reference.in_typing_context() || reference.in_runtime_evaluated_annotation()
-                    })
                 {
                     actions
-                        .entry((node_id, Action::Quote))
+                        .entry((node_id, Action::Ignore))
                         .or_default()
                         .push(import);
                 } else {
-                    actions
-                        .entry((node_id, Action::Move))
-                        .or_default()
-                        .push(import);
+                    // Determine whether the member should be fixed by moving the import out of the
+                    // type-checking block, or by quoting its references.
+                    // TODO: We should check `reference.in_annotated_type_alias()`
+                    //       as well to match the behavior of the flake8 plugin
+                    //       although maybe the best way forward is to add an
+                    //       additional setting to configure whether quoting
+                    //       or moving the import is preferred for type aliases
+                    //       since some people will consistently use their
+                    //       type aliases at runtimes, while others won't, so
+                    //       the best solution is unclear.
+                    if checker.settings.flake8_type_checking.quote_annotations
+                        && binding.references().all(|reference_id| {
+                        let reference = checker.semantic().reference(reference_id);
+                        reference.in_typing_context() || reference.in_runtime_evaluated_annotation()
+                    })
+                    {
+                        actions
+                            .entry((node_id, Action::Quote))
+                            .or_default()
+                            .push(import);
+                    } else {
+                        actions
+                            .entry((node_id, Action::Move))
+                            .or_default()
+                            .push(import);
+                    }
                 }
             }
         }

crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap

@@ -0,0 +1,24 @@
+---
+source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+---
+<filename>:5:12: TC004 [*] Move import `importlib.abc` out of type-checking block. Import is used for more than type hinting.
+  |
+3 | from typing import TYPE_CHECKING
+4 | if TYPE_CHECKING:
+5 |     import importlib.abc
+  |            ^^^^^^^^^^^^^ TC004
+6 |     import importlib.machinery
+7 | class Foo(importlib.abc.MetaPathFinder):
+  |
+  = help: Move out of type-checking block
+
+ℹ Unsafe fix
+1 1 | 
+2 2 | from __future__ import annotations
+3 3 | from typing import TYPE_CHECKING
+  4 |+import importlib.abc
+4 5 | if TYPE_CHECKING:
+5   |-    import importlib.abc
+6 6 |     import importlib.machinery
+7 7 | class Foo(importlib.abc.MetaPathFinder):
+8 8 |     def bar(self) -> importlib.machinery.ModuleSpec: ...

crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_ideal_import_order.snap

@@ -0,0 +1,24 @@
+---
+source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+---
+<filename>:6:12: TC004 [*] Move import `importlib.abc` out of type-checking block. Import is used for more than type hinting.
+  |
+4 | if TYPE_CHECKING:
+5 |     import importlib.machinery
+6 |     import importlib.abc
+  |            ^^^^^^^^^^^^^ TC004
+7 | class Foo(importlib.abc.MetaPathFinder):
+8 |     def bar(self) -> importlib.machinery.ModuleSpec: ...
+  |
+  = help: Move out of type-checking block
+
+ℹ Unsafe fix
+1 1 | 
+2 2 | from __future__ import annotations
+3 3 | from typing import TYPE_CHECKING
+  4 |+import importlib.abc
+4 5 | if TYPE_CHECKING:
+5 6 |     import importlib.machinery
+6   |-    import importlib.abc
+7 7 | class Foo(importlib.abc.MetaPathFinder):
+8 8 |     def bar(self) -> importlib.machinery.ModuleSpec: ...

crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_regression_test.snap

@@ -0,0 +1,4 @@
+---
+source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+---
+

crates/ruff_linter/src/rules/pyflakes/mod.rs

@@ -2654,7 +2654,7 @@ lambda: fu
         import foo.baz as foo
         foo
         ",
-            &[Rule::RedefinedWhileUnused],
+            &[Rule::UnusedImport, Rule::RedefinedWhileUnused],
         );
     }
 
@@ -2704,13 +2704,13 @@ lambda: fu
 
     #[test]
     fn unused_package_with_submodule_import() {
-        // When a package and its submodule are imported, only report once.
+        // When a package and its submodule are imported, report both.
         flakes(
             r"
         import fu
         import fu.bar
         ",
-            &[Rule::UnusedImport],
+            &[Rule::UnusedImport, Rule::UnusedImport],
         );
     }
 

crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs

@@ -2,6 +2,7 @@ use std::string::ToString;
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_semantic::BindingKind::SubmoduleImport;
 use ruff_python_semantic::{Scope, ScopeId};
 use ruff_text_size::Ranged;
 
@@ -61,7 +62,12 @@ pub(crate) fn undefined_local(
                 if let Some(range) = shadowed.references().find_map(|reference_id| {
                     let reference = checker.semantic().reference(reference_id);
                     if reference.scope_id() == scope_id {
-                        Some(reference.range())
+                        // FIXME: ignore submodules
+                        if let SubmoduleImport(..) = shadowed.kind {
+                            None
+                        } else {
+                            Some(reference.range())
+                        }
                     } else {
                         None
                     }