From 26989726997611e7578bd8e2fa7467812855d80a Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Wed, 25 Sep 2024 15:05:15 +0300
Subject: [PATCH 01/10] [pyflakes] Fix check of unused imports

Fix error when importing modules without submodules.
Add attribute check for similar imports with submodule.
---
 crates/ruff_linter/src/checkers/ast/mod.rs    |  16 +
 crates/ruff_linter/src/rules/pyflakes/mod.rs  |   6 +-
 .../src/rules/pyflakes/rules/unused_import.rs | 139 +++---
 ...ules__pyflakes__tests__F401_F401_0.py.snap |  40 ++
 crates/ruff_python_semantic/src/model.rs      | 450 ++++++++++++------
 5 files changed, 445 insertions(+), 206 deletions(-)

diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs
index ca5afee26ceb78..22dd8d9d35e99e 100644
--- a/crates/ruff_linter/src/checkers/ast/mod.rs
+++ b/crates/ruff_linter/src/checkers/ast/mod.rs
@@ -1130,6 +1130,15 @@ impl<'a> Visitor<'a> for Checker<'a> {
                     }
                 }
             }
+            Expr::Attribute(ast::ExprAttribute {
+                value: _,
+                range: _,
+                ctx,
+                attr: _,
+            }) => match 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 +2055,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();
 
diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs
index 7fb0f32673a8d2..0c962ffef8702f 100644
--- a/crates/ruff_linter/src/rules/pyflakes/mod.rs
+++ b/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],
         );
     }
 
diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
index 09e4149b32c98b..2763c41d7bed9c 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
@@ -1,8 +1,9 @@
 use std::borrow::Cow;
 use std::iter;
+use unicode_normalization::UnicodeNormalization;
 
 use anyhow::{anyhow, bail, Result};
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 
 use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
@@ -276,75 +277,95 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
     // Collect all unused imports by statement.
     let mut unused: BTreeMap<(NodeId, Exceptions), Vec<ImportBinding>> = BTreeMap::default();
     let mut ignored: BTreeMap<(NodeId, Exceptions), Vec<ImportBinding>> = BTreeMap::default();
+    let mut already_checked_imports: HashSet<String> = HashSet::new();
 
     for binding_id in scope.binding_ids() {
-        let binding = checker.semantic().binding(binding_id);
+        let top_binding = checker.semantic().binding(binding_id);
 
-        if binding.is_used()
-            || binding.is_explicit_export()
-            || binding.is_nonlocal()
-            || binding.is_global()
-        {
-            continue;
-        }
-
-        let Some(import) = binding.as_any_import() else {
+        let Some(_) = top_binding.as_any_import() else {
             continue;
         };
 
-        let Some(node_id) = binding.source else {
-            continue;
-        };
-
-        let name = binding.name(checker.source());
+        let binding_name = top_binding.name(checker.source()).split(".").next().unwrap_or("");
 
-        // If an import is marked as required, avoid treating it as unused, regardless of whether
-        // it was _actually_ used.
-        if checker
-            .settings
-            .isort
-            .required_imports
-            .iter()
-            .any(|required_import| required_import.matches(name, &import))
+        for binding in scope
+            .get_all(&binding_name.nfkc().collect::<String>())
+            .map(|binding_id| checker.semantic().binding(binding_id))
         {
-            continue;
-        }
+            let name = binding.name(checker.source());
 
-        // If an import was marked as allowed, avoid treating it as unused.
-        if checker
-            .settings
-            .pyflakes
-            .allowed_unused_imports
-            .iter()
-            .any(|allowed_unused_import| {
-                let allowed_unused_import = QualifiedName::from_dotted_name(allowed_unused_import);
-                import.qualified_name().starts_with(&allowed_unused_import)
-            })
-        {
-            continue;
-        }
+            if already_checked_imports.contains(name)
+            {
+                continue;
+            }
+            {
+                already_checked_imports.insert(name.to_string());
+            }
 
-        let import = ImportBinding {
-            name,
-            import,
-            range: binding.range(),
-            parent_range: binding.parent_range(checker.semantic()),
-        };
+            if binding.is_used()
+                || binding.is_explicit_export()
+                || binding.is_nonlocal()
+                || binding.is_global()
+            {
+                continue;
+            }
 
-        if checker.rule_is_ignored(Rule::UnusedImport, import.start())
-            || import.parent_range.is_some_and(|parent_range| {
-                checker.rule_is_ignored(Rule::UnusedImport, parent_range.start())
-            })
-        {
-            ignored
-                .entry((node_id, binding.exceptions))
-                .or_default()
-                .push(import);
-        } else {
-            unused
-                .entry((node_id, binding.exceptions))
-                .or_default()
-                .push(import);
+            let Some(import) = binding.as_any_import() else {
+                continue;
+            };
+
+            let Some(stmt_id) = binding.source else {
+                continue;
+            };
+
+            // If an import is marked as required, avoid treating it as unused, regardless of whether
+            // it was _actually_ used.
+            if checker
+                .settings
+                .isort
+                .required_imports
+                .iter()
+                .any(|required_import| required_import.matches(name, &import))
+            {
+                continue;
+            }
+
+            // If an import was marked as allowed, avoid treating it as unused.
+            if checker
+                .settings
+                .pyflakes
+                .allowed_unused_imports
+                .iter()
+                .any(|allowed_unused_import| {
+                    let allowed_unused_import = QualifiedName::from_dotted_name(allowed_unused_import);
+                    import.qualified_name().starts_with(&allowed_unused_import)
+                })
+            {
+                continue;
+            }
+
+            let import = ImportBinding {
+                name,
+                import,
+                range: binding.range(),
+                parent_range: binding.parent_range(checker.semantic()),
+            };
+
+            if checker.rule_is_ignored(Rule::UnusedImport, import.start())
+                || import.parent_range.is_some_and(|parent_range| {
+                    checker.rule_is_ignored(Rule::UnusedImport, parent_range.start())
+                })
+            {
+                ignored
+                    .entry((stmt_id, binding.exceptions))
+                    .or_default()
+                    .push(import);
+            } else {
+                unused
+                    .entry((stmt_id, binding.exceptions))
+                    .or_default()
+                    .push(import);
+            }
         }
     }
 
diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F401_F401_0.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F401_F401_0.py.snap
index b77ae8d19f8e36..adf6b6942cb5f6 100644
--- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F401_F401_0.py.snap
+++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F401_F401_0.py.snap
@@ -40,6 +40,46 @@ F401_0.py:6:5: F401 [*] `collections.OrderedDict` imported but unused
 8 7 | )
 9 8 | import multiprocessing.pool
 
+F401_0.py:10:8: F401 [*] `multiprocessing.process` imported but unused
+   |
+ 8 | )
+ 9 | import multiprocessing.pool
+10 | import multiprocessing.process
+   |        ^^^^^^^^^^^^^^^^^^^^^^^ F401
+11 | import logging.config
+12 | import logging.handlers
+   |
+   = help: Remove unused import: `multiprocessing.process`
+
+ℹ Safe fix
+7  7  |     namedtuple,
+8  8  | )
+9  9  | import multiprocessing.pool
+10    |-import multiprocessing.process
+11 10 | import logging.config
+12 11 | import logging.handlers
+13 12 | from typing import (
+
+F401_0.py:11:8: F401 [*] `logging.config` imported but unused
+   |
+ 9 | import multiprocessing.pool
+10 | import multiprocessing.process
+11 | import logging.config
+   |        ^^^^^^^^^^^^^^ F401
+12 | import logging.handlers
+13 | from typing import (
+   |
+   = help: Remove unused import: `logging.config`
+
+ℹ Safe fix
+8  8  | )
+9  9  | import multiprocessing.pool
+10 10 | import multiprocessing.process
+11    |-import logging.config
+12 11 | import logging.handlers
+13 12 | from typing import (
+14 13 |     TYPE_CHECKING,
+
 F401_0.py:12:8: F401 [*] `logging.handlers` imported but unused
    |
 10 | import multiprocessing.process
diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 21d8c41f87140f..27968ba9e29582 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::path::Path;
 
 use bitflags::bitflags;
@@ -5,7 +6,7 @@ use rustc_hash::FxHashMap;
 
 use ruff_python_ast::helpers::from_relative_import;
 use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
-use ruff_python_ast::{self as ast, Expr, ExprContext, PySourceType, Stmt};
+use ruff_python_ast::{self as ast, Expr, ExprContext, ExprName, PySourceType, Stmt};
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
 use crate::binding::{
@@ -352,10 +353,274 @@ impl<'a> SemanticModel<'a> {
         }
     }
 
+    fn resolve_binding(
+        &mut self,
+        binding_id: BindingId,
+        name_expr: &ExprName,
+        scope_id: &ScopeId,
+    ) -> Option<ReadResult>
+    {
+
+        let reference_id = self.resolved_references.push(
+            self.scope_id,
+            self.node_id,
+            ExprContext::Load,
+            self.flags,
+            name_expr.range,
+        );
+
+        self.bindings[binding_id].references.push(reference_id);
+
+        if let Some(binding_id) = self.resolve_submodule(
+            name_expr.id.as_str(),
+            *scope_id,
+            binding_id,
+        ) {
+            let reference_id = self.resolved_references.push(
+                self.scope_id,
+                self.node_id,
+                ExprContext::Load,
+                self.flags,
+                name_expr.range,
+            );
+            self.bindings[binding_id].references.push(reference_id);
+        }
+
+        match self.bindings[binding_id].kind {
+            // If it's a type annotation, don't treat it as resolved. For example, given:
+            //
+            // ```python
+            // name: str
+            // print(name)
+            // ```
+            //
+            // The `name` in `print(name)` should be treated as unresolved, but the `name` in
+            // `name: str` should be treated as used.
+            //
+            // Stub files are an exception. In a stub file, it _is_ considered valid to
+            // resolve to a type annotation.
+            BindingKind::Annotation if !self.in_stub_file() => None,
+
+            // If it's a deletion, don't treat it as resolved, since the name is now
+            // unbound. For example, given:
+            //
+            // ```python
+            // x = 1
+            // del x
+            // print(x)
+            // ```
+            //
+            // The `x` in `print(x)` should be treated as unresolved.
+            //
+            // Similarly, given:
+            //
+            // ```python
+            // try:
+            //     pass
+            // except ValueError as x:
+            //     pass
+            //
+            // print(x)
+            //
+            // The `x` in `print(x)` should be treated as unresolved.
+            BindingKind::Deletion | BindingKind::UnboundException(None) => {
+                self.unresolved_references.push(
+                    name_expr.range,
+                    self.exceptions(),
+                    UnresolvedReferenceFlags::empty(),
+                );
+                Some(ReadResult::UnboundLocal(binding_id))
+            }
+
+            BindingKind::ConditionalDeletion(binding_id) => {
+                self.unresolved_references.push(
+                    name_expr.range,
+                    self.exceptions(),
+                    UnresolvedReferenceFlags::empty(),
+                );
+                Some(ReadResult::UnboundLocal(binding_id))
+            }
+
+            // If we hit an unbound exception that shadowed a bound name, resole to the
+            // bound name. For example, given:
+            //
+            // ```python
+            // x = 1
+            //
+            // try:
+            //     pass
+            // except ValueError as x:
+            //     pass
+            //
+            // print(x)
+            // ```
+            //
+            // The `x` in `print(x)` should resolve to the `x` in `x = 1`.
+            BindingKind::UnboundException(Some(binding_id)) => {
+                // Mark the binding as used.
+                let reference_id = self.resolved_references.push(
+                    self.scope_id,
+                    self.node_id,
+                    ExprContext::Load,
+                    self.flags,
+                    name_expr.range,
+                );
+                self.bindings[binding_id].references.push(reference_id);
+
+                // Mark any submodule aliases as used.
+                if let Some(binding_id) = self.resolve_submodule(
+                    name_expr.id.as_str(),
+                    *scope_id,
+                    binding_id,
+                ) {
+                    let reference_id = self.resolved_references.push(
+                        self.scope_id,
+                        self.node_id,
+                        ExprContext::Load,
+                        self.flags,
+                        name_expr.range,
+                    );
+                    self.bindings[binding_id].references.push(reference_id);
+                }
+
+                self.resolved_names
+                    .insert(name_expr.into(), binding_id);
+                Some(ReadResult::Resolved(binding_id))
+            }
+
+            BindingKind::Global(Some(binding_id))
+            | BindingKind::Nonlocal(binding_id, _) => {
+                // Mark the shadowed binding as used.
+                let reference_id = self.resolved_references.push(
+                    self.scope_id,
+                    self.node_id,
+                    ExprContext::Load,
+                    self.flags,
+                    name_expr.range,
+                );
+                self.bindings[binding_id].references.push(reference_id);
+
+                // Treat it as resolved.
+                self.resolved_names
+                    .insert(name_expr.into(), binding_id);
+                Some(ReadResult::Resolved(binding_id))
+            }
+
+            _ => {
+                // Otherwise, treat it as resolved.
+                self.resolved_names
+                    .insert(name_expr.into(), binding_id);
+                Some(ReadResult::Resolved(binding_id))
+            }
+        }
+    }
+
+    /// Resolve a `load` reference to an [`ast::ExprAttribute`].
+    pub fn resolve_attribute_load(&mut self, attribute: &ast::ExprAttribute) -> ReadResult {
+        let name_expr;
+
+        let mut full_name = format!("{}", attribute.attr.id);
+        let mut current_expr = &*attribute.value;
+        let mut result = None;
+        let mut is_name_exist = false;
+        let mut already_checked_imports: HashSet<String> = HashSet::new();
+
+        while let Expr::Attribute(expr_attr) = &current_expr {
+            full_name = format!("{}.{}", expr_attr.attr.id, full_name);
+            current_expr = &*expr_attr.value;
+        }
+
+        if let Expr::Name(ref expr_name) = current_expr {
+            full_name = format!("{}.{}", expr_name.id, full_name);
+            name_expr = Some(expr_name);
+        } else {
+            return ReadResult::NotFound;
+        }
+
+        if name_expr.is_none() {
+            return ReadResult::NotFound;
+        }
+
+        full_name = full_name.trim_end_matches('.').to_string();
+
+        let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
+        let mut binding_ids: Vec<(BindingId, ScopeId)> = vec![];
+
+        for (_index, scope_id) in ancestor_scope_ids.into_iter().enumerate() {
+            for binding_id in self.scopes[scope_id].get_all(name_expr.unwrap().id.as_str()){
+                binding_ids.push((binding_id, scope_id));
+            }
+        }
+
+        for (binding_id, scope_id) in binding_ids.iter() {
+            if let BindingKind::SubmoduleImport(binding_kind) = &self.binding(*binding_id).kind
+            {
+                if binding_kind.qualified_name.to_string() == full_name {
+                    if let Some(result) = self.resolve_binding(
+                        *binding_id,
+                        &name_expr.unwrap(),
+                        scope_id,
+                    ) {
+                        return result;
+                    }
+                }
+            }
+            if let BindingKind::Import(_) = &self.binding(*binding_id).kind {
+                is_name_exist = true;
+            }
+        }
+
+        // TODO: need to move the block implementation to resolve_load, but carefully
+        // start check module import
+        for (binding_id, scope_id) in binding_ids.iter() {
+            let Some(import) = self.binding(*binding_id).as_any_import() else {
+                continue;
+            };
+            let name = &import.qualified_name().to_string()
+                .split(".").next().unwrap_or("").to_owned();
+
+            match self.bindings[*binding_id].kind {
+                BindingKind::SubmoduleImport(_) if !is_name_exist => continue,
+                BindingKind::WithItemVar => continue,
+                BindingKind::SubmoduleImport(_) => {
+                    result = self.resolve_binding(
+                        *binding_id,
+                        &name_expr.unwrap(),
+                        scope_id,
+                    );
+                },
+                BindingKind::Import(_) => {
+
+                    if already_checked_imports.contains(&name.to_string())
+                    {
+                        continue;
+                    } else {
+                        already_checked_imports.insert(name.to_string());
+                    }
+
+                    result = self.resolve_binding(
+                        *binding_id,
+                        &name_expr.unwrap(),
+                        scope_id,
+                    );
+                }
+                _ => {}
+            }
+        }
+        // end check module import
+
+        if result.is_none() {
+            ReadResult::NotFound
+        } else {
+            result.unwrap()
+        }
+    }
+
     /// Resolve a `load` reference to an [`ast::ExprName`].
     pub fn resolve_load(&mut self, name: &ast::ExprName) -> ReadResult {
         // PEP 563 indicates that if a forward reference can be resolved in the module scope, we
         // should prefer it over local resolutions.
+
         if self.in_forward_reference() {
             if let Some(binding_id) = self.scopes.global().get(name.id.as_str()) {
                 if !self.bindings[binding_id].is_unbound() {
@@ -392,9 +657,18 @@ impl<'a> SemanticModel<'a> {
         let mut seen_function = false;
         let mut import_starred = false;
         let mut class_variables_visible = true;
-        for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() {
+        let mut result = None;
+
+        let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
+
+        for (index, scope_id) in ancestor_scope_ids.into_iter().enumerate() {
             let scope = &self.scopes[scope_id];
-            if scope.kind.is_class() {
+            let is_class = scope.kind.is_class();
+            let is_function = scope.kind.is_function();
+            let uses_star_imports = scope.uses_star_imports();
+            let mut is_name = false;
+
+            if is_class {
                 // Allow usages of `__class__` within methods, e.g.:
                 //
                 // ```python
@@ -430,154 +704,42 @@ impl<'a> SemanticModel<'a> {
             class_variables_visible = scope.kind.is_type() && index == 0;
 
             if let Some(binding_id) = scope.get(name.id.as_str()) {
-                // Mark the binding as used.
-                let reference_id = self.resolved_references.push(
-                    self.scope_id,
-                    self.node_id,
-                    ExprContext::Load,
-                    self.flags,
-                    name.range,
-                );
-                self.bindings[binding_id].references.push(reference_id);
-
-                // Mark any submodule aliases as used.
-                if let Some(binding_id) =
-                    self.resolve_submodule(name.id.as_str(), scope_id, binding_id)
-                {
-                    let reference_id = self.resolved_references.push(
-                        self.scope_id,
-                        self.node_id,
-                        ExprContext::Load,
-                        self.flags,
-                        name.range,
-                    );
-                    self.bindings[binding_id].references.push(reference_id);
-                }
-
-                match self.bindings[binding_id].kind {
-                    // If it's a type annotation, don't treat it as resolved. For example, given:
-                    //
-                    // ```python
-                    // name: str
-                    // print(name)
-                    // ```
-                    //
-                    // The `name` in `print(name)` should be treated as unresolved, but the `name` in
-                    // `name: str` should be treated as used.
-                    //
-                    // Stub files are an exception. In a stub file, it _is_ considered valid to
-                    // resolve to a type annotation.
-                    BindingKind::Annotation if !self.in_stub_file() => continue,
-
-                    // If it's a deletion, don't treat it as resolved, since the name is now
-                    // unbound. For example, given:
-                    //
-                    // ```python
-                    // x = 1
-                    // del x
-                    // print(x)
-                    // ```
-                    //
-                    // The `x` in `print(x)` should be treated as unresolved.
-                    //
-                    // Similarly, given:
-                    //
-                    // ```python
-                    // try:
-                    //     pass
-                    // except ValueError as x:
-                    //     pass
-                    //
-                    // print(x)
-                    //
-                    // The `x` in `print(x)` should be treated as unresolved.
-                    BindingKind::Deletion | BindingKind::UnboundException(None) => {
-                        self.unresolved_references.push(
-                            name.range,
-                            self.exceptions(),
-                            UnresolvedReferenceFlags::empty(),
-                        );
-                        return ReadResult::UnboundLocal(binding_id);
+                // Return solved if there is at least one import with a submodule
+                for temp_binding_id in scope.get_all(name.id.as_str()) {
+                    if let BindingKind::Import(_) = &self.bindings[temp_binding_id].kind {
+                        is_name = true;
                     }
+                }
 
-                    BindingKind::ConditionalDeletion(binding_id) => {
-                        self.unresolved_references.push(
-                            name.range,
-                            self.exceptions(),
-                            UnresolvedReferenceFlags::empty(),
-                        );
-                        return ReadResult::UnboundLocal(binding_id);
-                    }
+                // Todo: Move the implementation here
+                for temp_binding_id in scope.get_all(name.id.as_str()) {
+                    if let BindingKind::SubmoduleImport(_) = &self
+                        .bindings[temp_binding_id].kind {
 
-                    // If we hit an unbound exception that shadowed a bound name, resole to the
-                    // bound name. For example, given:
-                    //
-                    // ```python
-                    // x = 1
-                    //
-                    // try:
-                    //     pass
-                    // except ValueError as x:
-                    //     pass
-                    //
-                    // print(x)
-                    // ```
-                    //
-                    // The `x` in `print(x)` should resolve to the `x` in `x = 1`.
-                    BindingKind::UnboundException(Some(binding_id)) => {
-                        // Mark the binding as used.
-                        let reference_id = self.resolved_references.push(
-                            self.scope_id,
-                            self.node_id,
-                            ExprContext::Load,
-                            self.flags,
-                            name.range,
-                        );
-                        self.bindings[binding_id].references.push(reference_id);
-
-                        // Mark any submodule aliases as used.
-                        if let Some(binding_id) =
-                            self.resolve_submodule(name.id.as_str(), scope_id, binding_id)
-                        {
-                            let reference_id = self.resolved_references.push(
-                                self.scope_id,
-                                self.node_id,
-                                ExprContext::Load,
-                                self.flags,
-                                name.range,
-                            );
-                            self.bindings[binding_id].references.push(reference_id);
+                        if !is_name {
+                            return ReadResult::NotFound;
                         }
 
-                        self.resolved_names.insert(name.into(), binding_id);
-                        return ReadResult::Resolved(binding_id);
+                        for reference_id in self.bindings[temp_binding_id].references() {
+                            if self.resolved_references[reference_id].range()
+                                .contains_range(self.bindings[temp_binding_id].range)
+                            {
+                                return ReadResult::Resolved(temp_binding_id);
+                            }
+                        }
                     }
+                }
 
-                    BindingKind::Global(Some(binding_id))
-                    | BindingKind::Nonlocal(binding_id, _) => {
-                        // Mark the shadowed binding as used.
-                        let reference_id = self.resolved_references.push(
-                            self.scope_id,
-                            self.node_id,
-                            ExprContext::Load,
-                            self.flags,
-                            name.range,
-                        );
-                        self.bindings[binding_id].references.push(reference_id);
-
-                        // Treat it as resolved.
-                        self.resolved_names.insert(name.into(), binding_id);
-                        return ReadResult::Resolved(binding_id);
-                    }
+                result = self.resolve_binding(
+                    binding_id,
+                    &name,
+                    &scope_id,
+                );
 
-                    _ => {
-                        // Otherwise, treat it as resolved.
-                        self.resolved_names.insert(name.into(), binding_id);
-                        return ReadResult::Resolved(binding_id);
-                    }
+                if !result.is_none() {
+                    return result.unwrap();
                 }
             }
-
             // Allow usages of `__module__` and `__qualname__` within class scopes, e.g.:
             //
             // ```python
@@ -593,14 +755,14 @@ impl<'a> SemanticModel<'a> {
             //     __qualname__ = "Bar"
             //     print(__qualname__)
             // ```
-            if index == 0 && scope.kind.is_class() {
+            if index == 0 && is_class {
                 if matches!(name.id.as_str(), "__module__" | "__qualname__") {
                     return ReadResult::ImplicitGlobal;
                 }
             }
 
-            seen_function |= scope.kind.is_function();
-            import_starred = import_starred || scope.uses_star_imports();
+            seen_function |= is_function;
+            import_starred = import_starred || uses_star_imports;
         }
 
         if import_starred {

From 60281d3f16b6f1e4459182c87c792c6751163c35 Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Tue, 29 Oct 2024 15:06:54 +0300
Subject: [PATCH 02/10] [pyflakes] Ignore submodule_imports in F823

---
 .../src/rules/pyflakes/rules/undefined_local.rs           | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
index d13aa29107864c..4f43b17c301179 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
@@ -3,6 +3,7 @@ use std::string::ToString;
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_semantic::{Scope, ScopeId};
+use ruff_python_semantic::BindingKind::SubmoduleImport;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -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
                     }

From 9c27ead3d619f847d1a405a45a15e0f85eaee265 Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Fri, 29 Nov 2024 14:31:02 +0300
Subject: [PATCH 03/10] fixup! [pyflakes] Ignore submodule_imports in F823

---
 .../rules/pyflakes/rules/undefined_local.rs   |  2 +-
 .../src/rules/pyflakes/rules/unused_import.rs | 24 +++---
 crates/ruff_python_semantic/src/model.rs      | 85 +++++++------------
 3 files changed, 44 insertions(+), 67 deletions(-)

diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
index 4f43b17c301179..8cea6984088f1b 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/undefined_local.rs
@@ -2,8 +2,8 @@ use std::string::ToString;
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
-use ruff_python_semantic::{Scope, ScopeId};
 use ruff_python_semantic::BindingKind::SubmoduleImport;
+use ruff_python_semantic::{Scope, ScopeId};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
index 2763c41d7bed9c..73dff06862115f 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
@@ -286,7 +286,11 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
             continue;
         };
 
-        let binding_name = top_binding.name(checker.source()).split(".").next().unwrap_or("");
+        let binding_name = top_binding
+            .name(checker.source())
+            .split(".")
+            .next()
+            .unwrap_or("");
 
         for binding in scope
             .get_all(&binding_name.nfkc().collect::<String>())
@@ -294,8 +298,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
         {
             let name = binding.name(checker.source());
 
-            if already_checked_imports.contains(name)
-            {
+            if already_checked_imports.contains(name) {
                 continue;
             }
             {
@@ -331,16 +334,13 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
             }
 
             // If an import was marked as allowed, avoid treating it as unused.
-            if checker
-                .settings
-                .pyflakes
-                .allowed_unused_imports
-                .iter()
-                .any(|allowed_unused_import| {
-                    let allowed_unused_import = QualifiedName::from_dotted_name(allowed_unused_import);
+            if checker.settings.pyflakes.allowed_unused_imports.iter().any(
+                |allowed_unused_import| {
+                    let allowed_unused_import =
+                        QualifiedName::from_dotted_name(allowed_unused_import);
                     import.qualified_name().starts_with(&allowed_unused_import)
-                })
-            {
+                },
+            ) {
                 continue;
             }
 
diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 27968ba9e29582..4e5755760cdd04 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -358,9 +358,7 @@ impl<'a> SemanticModel<'a> {
         binding_id: BindingId,
         name_expr: &ExprName,
         scope_id: &ScopeId,
-    ) -> Option<ReadResult>
-    {
-
+    ) -> Option<ReadResult> {
         let reference_id = self.resolved_references.push(
             self.scope_id,
             self.node_id,
@@ -371,11 +369,9 @@ impl<'a> SemanticModel<'a> {
 
         self.bindings[binding_id].references.push(reference_id);
 
-        if let Some(binding_id) = self.resolve_submodule(
-            name_expr.id.as_str(),
-            *scope_id,
-            binding_id,
-        ) {
+        if let Some(binding_id) =
+            self.resolve_submodule(name_expr.id.as_str(), *scope_id, binding_id)
+        {
             let reference_id = self.resolved_references.push(
                 self.scope_id,
                 self.node_id,
@@ -468,11 +464,9 @@ impl<'a> SemanticModel<'a> {
                 self.bindings[binding_id].references.push(reference_id);
 
                 // Mark any submodule aliases as used.
-                if let Some(binding_id) = self.resolve_submodule(
-                    name_expr.id.as_str(),
-                    *scope_id,
-                    binding_id,
-                ) {
+                if let Some(binding_id) =
+                    self.resolve_submodule(name_expr.id.as_str(), *scope_id, binding_id)
+                {
                     let reference_id = self.resolved_references.push(
                         self.scope_id,
                         self.node_id,
@@ -483,13 +477,11 @@ impl<'a> SemanticModel<'a> {
                     self.bindings[binding_id].references.push(reference_id);
                 }
 
-                self.resolved_names
-                    .insert(name_expr.into(), binding_id);
+                self.resolved_names.insert(name_expr.into(), binding_id);
                 Some(ReadResult::Resolved(binding_id))
             }
 
-            BindingKind::Global(Some(binding_id))
-            | BindingKind::Nonlocal(binding_id, _) => {
+            BindingKind::Global(Some(binding_id)) | BindingKind::Nonlocal(binding_id, _) => {
                 // Mark the shadowed binding as used.
                 let reference_id = self.resolved_references.push(
                     self.scope_id,
@@ -501,15 +493,13 @@ impl<'a> SemanticModel<'a> {
                 self.bindings[binding_id].references.push(reference_id);
 
                 // Treat it as resolved.
-                self.resolved_names
-                    .insert(name_expr.into(), binding_id);
+                self.resolved_names.insert(name_expr.into(), binding_id);
                 Some(ReadResult::Resolved(binding_id))
             }
 
             _ => {
                 // Otherwise, treat it as resolved.
-                self.resolved_names
-                    .insert(name_expr.into(), binding_id);
+                self.resolved_names.insert(name_expr.into(), binding_id);
                 Some(ReadResult::Resolved(binding_id))
             }
         }
@@ -547,20 +537,17 @@ impl<'a> SemanticModel<'a> {
         let mut binding_ids: Vec<(BindingId, ScopeId)> = vec![];
 
         for (_index, scope_id) in ancestor_scope_ids.into_iter().enumerate() {
-            for binding_id in self.scopes[scope_id].get_all(name_expr.unwrap().id.as_str()){
+            for binding_id in self.scopes[scope_id].get_all(name_expr.unwrap().id.as_str()) {
                 binding_ids.push((binding_id, scope_id));
             }
         }
 
         for (binding_id, scope_id) in binding_ids.iter() {
-            if let BindingKind::SubmoduleImport(binding_kind) = &self.binding(*binding_id).kind
-            {
+            if let BindingKind::SubmoduleImport(binding_kind) = &self.binding(*binding_id).kind {
                 if binding_kind.qualified_name.to_string() == full_name {
-                    if let Some(result) = self.resolve_binding(
-                        *binding_id,
-                        &name_expr.unwrap(),
-                        scope_id,
-                    ) {
+                    if let Some(result) =
+                        self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id)
+                    {
                         return result;
                     }
                 }
@@ -576,33 +563,28 @@ impl<'a> SemanticModel<'a> {
             let Some(import) = self.binding(*binding_id).as_any_import() else {
                 continue;
             };
-            let name = &import.qualified_name().to_string()
-                .split(".").next().unwrap_or("").to_owned();
+            let name = &import
+                .qualified_name()
+                .to_string()
+                .split(".")
+                .next()
+                .unwrap_or("")
+                .to_owned();
 
             match self.bindings[*binding_id].kind {
                 BindingKind::SubmoduleImport(_) if !is_name_exist => continue,
                 BindingKind::WithItemVar => continue,
                 BindingKind::SubmoduleImport(_) => {
-                    result = self.resolve_binding(
-                        *binding_id,
-                        &name_expr.unwrap(),
-                        scope_id,
-                    );
-                },
+                    result = self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id);
+                }
                 BindingKind::Import(_) => {
-
-                    if already_checked_imports.contains(&name.to_string())
-                    {
+                    if already_checked_imports.contains(&name.to_string()) {
                         continue;
                     } else {
                         already_checked_imports.insert(name.to_string());
                     }
 
-                    result = self.resolve_binding(
-                        *binding_id,
-                        &name_expr.unwrap(),
-                        scope_id,
-                    );
+                    result = self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id);
                 }
                 _ => {}
             }
@@ -713,15 +695,14 @@ impl<'a> SemanticModel<'a> {
 
                 // Todo: Move the implementation here
                 for temp_binding_id in scope.get_all(name.id.as_str()) {
-                    if let BindingKind::SubmoduleImport(_) = &self
-                        .bindings[temp_binding_id].kind {
-
+                    if let BindingKind::SubmoduleImport(_) = &self.bindings[temp_binding_id].kind {
                         if !is_name {
                             return ReadResult::NotFound;
                         }
 
                         for reference_id in self.bindings[temp_binding_id].references() {
-                            if self.resolved_references[reference_id].range()
+                            if self.resolved_references[reference_id]
+                                .range()
                                 .contains_range(self.bindings[temp_binding_id].range)
                             {
                                 return ReadResult::Resolved(temp_binding_id);
@@ -730,11 +711,7 @@ impl<'a> SemanticModel<'a> {
                     }
                 }
 
-                result = self.resolve_binding(
-                    binding_id,
-                    &name,
-                    &scope_id,
-                );
+                result = self.resolve_binding(binding_id, &name, &scope_id);
 
                 if !result.is_none() {
                     return result.unwrap();

From 4f20c21308df40096cdfdd06a1e683f593468575 Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Thu, 19 Dec 2024 17:01:15 +0300
Subject: [PATCH 04/10] [pyflakes] Fix incorrect resolving of submodules with
 aliases

---
 crates/ruff_python_semantic/src/model.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 4e5755760cdd04..fad2373c5e93af 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -995,6 +995,14 @@ impl<'a> SemanticModel<'a> {
             return None;
         }
 
+        if !submodule
+            .qualified_name()
+            .to_string()
+            .starts_with(&import.qualified_name().to_string())
+        {
+            return None;
+        }
+
         Some(binding_id)
     }
 

From 1c443cb0136515982473e9c8ba8d04ae725a655f Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Thu, 19 Dec 2024 17:24:19 +0300
Subject: [PATCH 05/10] fixup! [pyflakes] Fix check of unused imports

---
 crates/ruff_linter/src/checkers/ast/mod.rs    |  9 ++---
 .../src/rules/pyflakes/rules/unused_import.rs |  2 +-
 crates/ruff_python_semantic/src/model.rs      | 36 +++++++++----------
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs
index 22dd8d9d35e99e..21fd277794cc58 100644
--- a/crates/ruff_linter/src/checkers/ast/mod.rs
+++ b/crates/ruff_linter/src/checkers/ast/mod.rs
@@ -1135,10 +1135,11 @@ impl<'a> Visitor<'a> for Checker<'a> {
                 range: _,
                 ctx,
                 attr: _,
-            }) => match ctx {
-                ExprContext::Load => self.handle_attribute_load(expr),
-                _ => {}
-            },
+            }) => {
+                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),
diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
index 73dff06862115f..32bcbdc8d6302f 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
@@ -288,7 +288,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
 
         let binding_name = top_binding
             .name(checker.source())
-            .split(".")
+            .split('.')
             .next()
             .unwrap_or("");
 
diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index fad2373c5e93af..5329417fec6f0e 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -357,7 +357,7 @@ impl<'a> SemanticModel<'a> {
         &mut self,
         binding_id: BindingId,
         name_expr: &ExprName,
-        scope_id: &ScopeId,
+        scope_id: ScopeId,
     ) -> Option<ReadResult> {
         let reference_id = self.resolved_references.push(
             self.scope_id,
@@ -370,7 +370,7 @@ impl<'a> SemanticModel<'a> {
         self.bindings[binding_id].references.push(reference_id);
 
         if let Some(binding_id) =
-            self.resolve_submodule(name_expr.id.as_str(), *scope_id, binding_id)
+            self.resolve_submodule(name_expr.id.as_str(), scope_id, binding_id)
         {
             let reference_id = self.resolved_references.push(
                 self.scope_id,
@@ -465,7 +465,7 @@ impl<'a> SemanticModel<'a> {
 
                 // Mark any submodule aliases as used.
                 if let Some(binding_id) =
-                    self.resolve_submodule(name_expr.id.as_str(), *scope_id, binding_id)
+                    self.resolve_submodule(name_expr.id.as_str(), scope_id, binding_id)
                 {
                     let reference_id = self.resolved_references.push(
                         self.scope_id,
@@ -536,17 +536,17 @@ impl<'a> SemanticModel<'a> {
         let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
         let mut binding_ids: Vec<(BindingId, ScopeId)> = vec![];
 
-        for (_index, scope_id) in ancestor_scope_ids.into_iter().enumerate() {
+        for scope_id in ancestor_scope_ids {
             for binding_id in self.scopes[scope_id].get_all(name_expr.unwrap().id.as_str()) {
                 binding_ids.push((binding_id, scope_id));
             }
         }
 
-        for (binding_id, scope_id) in binding_ids.iter() {
+        for (binding_id, scope_id) in &binding_ids {
             if let BindingKind::SubmoduleImport(binding_kind) = &self.binding(*binding_id).kind {
                 if binding_kind.qualified_name.to_string() == full_name {
                     if let Some(result) =
-                        self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id)
+                        self.resolve_binding(*binding_id, name_expr.unwrap(), *scope_id)
                     {
                         return result;
                     }
@@ -559,14 +559,14 @@ impl<'a> SemanticModel<'a> {
 
         // TODO: need to move the block implementation to resolve_load, but carefully
         // start check module import
-        for (binding_id, scope_id) in binding_ids.iter() {
+        for (binding_id, scope_id) in &binding_ids {
             let Some(import) = self.binding(*binding_id).as_any_import() else {
                 continue;
             };
             let name = &import
                 .qualified_name()
                 .to_string()
-                .split(".")
+                .split('.')
                 .next()
                 .unwrap_or("")
                 .to_owned();
@@ -575,26 +575,25 @@ impl<'a> SemanticModel<'a> {
                 BindingKind::SubmoduleImport(_) if !is_name_exist => continue,
                 BindingKind::WithItemVar => continue,
                 BindingKind::SubmoduleImport(_) => {
-                    result = self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id);
+                    result = self.resolve_binding(*binding_id, name_expr.unwrap(), *scope_id);
                 }
                 BindingKind::Import(_) => {
                     if already_checked_imports.contains(&name.to_string()) {
                         continue;
-                    } else {
-                        already_checked_imports.insert(name.to_string());
                     }
+                    already_checked_imports.insert(name.to_string());
 
-                    result = self.resolve_binding(*binding_id, &name_expr.unwrap(), scope_id);
+                    result = self.resolve_binding(*binding_id, name_expr.unwrap(), *scope_id);
                 }
                 _ => {}
             }
         }
         // end check module import
 
-        if result.is_none() {
-            ReadResult::NotFound
+        if let Some(result) = result {
+            result
         } else {
-            result.unwrap()
+            ReadResult::NotFound
         }
     }
 
@@ -639,7 +638,6 @@ impl<'a> SemanticModel<'a> {
         let mut seen_function = false;
         let mut import_starred = false;
         let mut class_variables_visible = true;
-        let mut result = None;
 
         let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
 
@@ -711,10 +709,8 @@ impl<'a> SemanticModel<'a> {
                     }
                 }
 
-                result = self.resolve_binding(binding_id, &name, &scope_id);
-
-                if !result.is_none() {
-                    return result.unwrap();
+                if let Some(res) = self.resolve_binding(binding_id, name, scope_id) {
+                    return res;
                 }
             }
             // Allow usages of `__module__` and `__qualname__` within class scopes, e.g.:

From 8d0d50cdf6f8b32e5641ac71c42b0c4fceea951f Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Tue, 14 Jan 2025 16:27:50 +0300
Subject: [PATCH 06/10] fixup! fixup! [pyflakes] Fix check of unused imports

---
 .../src/rules/pyflakes/rules/unused_import.rs |  6 ++---
 crates/ruff_python_semantic/src/model.rs      | 24 +++++++++----------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
index 32bcbdc8d6302f..db2c3d24274a6e 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
@@ -277,7 +277,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
     // Collect all unused imports by statement.
     let mut unused: BTreeMap<(NodeId, Exceptions), Vec<ImportBinding>> = BTreeMap::default();
     let mut ignored: BTreeMap<(NodeId, Exceptions), Vec<ImportBinding>> = BTreeMap::default();
-    let mut already_checked_imports: HashSet<String> = HashSet::new();
+    let mut already_checked_imports: HashSet<&str> = HashSet::new();
 
     for binding_id in scope.binding_ids() {
         let top_binding = checker.semantic().binding(binding_id);
@@ -298,11 +298,11 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
         {
             let name = binding.name(checker.source());
 
-            if already_checked_imports.contains(name) {
+            if already_checked_imports.contains(&name) {
                 continue;
             }
             {
-                already_checked_imports.insert(name.to_string());
+                already_checked_imports.insert(name);
             }
 
             if binding.is_used()
diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 5329417fec6f0e..833e7204f5a9dc 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -507,31 +507,30 @@ impl<'a> SemanticModel<'a> {
 
     /// Resolve a `load` reference to an [`ast::ExprAttribute`].
     pub fn resolve_attribute_load(&mut self, attribute: &ast::ExprAttribute) -> ReadResult {
-        let name_expr;
-
-        let mut full_name = format!("{}", attribute.attr.id);
+        let mut name_segments = vec![attribute.attr.id.as_str()];
         let mut current_expr = &*attribute.value;
         let mut result = None;
         let mut is_name_exist = false;
         let mut already_checked_imports: HashSet<String> = HashSet::new();
 
-        while let Expr::Attribute(expr_attr) = &current_expr {
-            full_name = format!("{}.{}", expr_attr.attr.id, full_name);
-            current_expr = &*expr_attr.value;
+        while let Expr::Attribute(expr_attr) = current_expr {
+            name_segments.push(expr_attr.attr.id.as_str());
+            current_expr = &expr_attr.value;
         }
 
-        if let Expr::Name(ref expr_name) = current_expr {
-            full_name = format!("{}.{}", expr_name.id, full_name);
-            name_expr = Some(expr_name);
+        let name_expr = if let Expr::Name(ref expr_name) = current_expr {
+            name_segments.push(expr_name.id.as_str());
+            Some(expr_name)
         } else {
             return ReadResult::NotFound;
-        }
+        };
 
         if name_expr.is_none() {
             return ReadResult::NotFound;
         }
 
-        full_name = full_name.trim_end_matches('.').to_string();
+        name_segments.reverse();
+        let full_name = name_segments.join(".");
 
         let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
         let mut binding_ids: Vec<(BindingId, ScopeId)> = vec![];
@@ -993,8 +992,7 @@ impl<'a> SemanticModel<'a> {
 
         if !submodule
             .qualified_name()
-            .to_string()
-            .starts_with(&import.qualified_name().to_string())
+            .starts_with(import.qualified_name())
         {
             return None;
         }

From 2f238e13b6d8a55b653c11f4e0f62d4e58f4906d Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Fri, 28 Mar 2025 16:26:30 +0300
Subject: [PATCH 07/10] fixup! fixup! fixup! [pyflakes] Fix check of unused
 imports

---
 crates/ruff_python_semantic/src/model.rs | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 833e7204f5a9dc..d0d9833b14b000 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -525,21 +525,19 @@ impl<'a> SemanticModel<'a> {
             return ReadResult::NotFound;
         };
 
-        if name_expr.is_none() {
-            return ReadResult::NotFound;
-        }
-
         name_segments.reverse();
         let full_name = name_segments.join(".");
 
-        let ancestor_scope_ids: Vec<_> = self.scopes.ancestor_ids(self.scope_id).collect();
-        let mut binding_ids: Vec<(BindingId, ScopeId)> = vec![];
-
-        for scope_id in ancestor_scope_ids {
-            for binding_id in self.scopes[scope_id].get_all(name_expr.unwrap().id.as_str()) {
-                binding_ids.push((binding_id, scope_id));
-            }
-        }
+        let binding_ids: Vec<_> = self
+            .scopes
+            .ancestor_ids(self.scope_id)
+            .flat_map(|scope_id| {
+                self.scopes[scope_id]
+                    .get_all(name_expr.unwrap().id.as_str())
+                    .into_iter()
+                    .map(move |binding_id| (binding_id, scope_id))
+            })
+            .collect();
 
         for (binding_id, scope_id) in &binding_ids {
             if let BindingKind::SubmoduleImport(binding_kind) = &self.binding(*binding_id).kind {

From ed03bb80a79d9163fa7a3a4665832c123ceb2470 Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Mon, 12 May 2025 14:59:13 +0300
Subject: [PATCH 08/10] [flake8-type-checking] Add test cases for TC004

---
 .../src/rules/flake8_type_checking/mod.rs     | 36 +++++++++++++++++++
 ...ts__github_issue_15723_false_negative.snap |  4 +++
 ...github_issue_15723_ideal_import_order.snap | 24 +++++++++++++
 ...s__github_issue_15723_regression_test.snap |  4 +++
 4 files changed, 68 insertions(+)
 create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap
 create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_ideal_import_order.snap
 create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_regression_test.snap

diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
index ab95fe3ac56fd9..b09455737e2fe5 100644
--- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+++ b/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,
diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap
new file mode 100644
index 00000000000000..6c5ead27428cec
--- /dev/null
+++ b/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,4 @@
+---
+source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+---
+
diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_ideal_import_order.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_ideal_import_order.snap
new file mode 100644
index 00000000000000..595576e5fd53c4
--- /dev/null
+++ b/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: ...
diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_regression_test.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_regression_test.snap
new file mode 100644
index 00000000000000..6c5ead27428cec
--- /dev/null
+++ b/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
+---
+

From 923381790a91d4b9ee787ea0674515c24e169a70 Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Thu, 22 May 2025 16:56:27 +0300
Subject: [PATCH 09/10] [flake8-type-checking] Add
 runtime_import_in_type_checking_block check for all bindings

---
 .../runtime_import_in_type_checking_block.rs  | 107 ++++++++++--------
 1 file changed, 62 insertions(+), 45 deletions(-)

diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs
index f5f9114b71d6aa..a0407c9dabba1b 100644
--- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs
+++ b/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);
+                    }
                 }
             }
         }

From 996c5d7250a1e6e4338ebab4637a26c7782e8e9b Mon Sep 17 00:00:00 2001
From: Gleb Pilikin <gpilikin@croc.ru>
Date: Thu, 22 May 2025 17:01:44 +0300
Subject: [PATCH 10/10] [flake8-type-checking] Fix false negative test of TC004

---
 ...ts__github_issue_15723_false_negative.snap | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap
index 6c5ead27428cec..ae13107db3a1ff 100644
--- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap
+++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__github_issue_15723_false_negative.snap
@@ -1,4 +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: ...