From ae75b303f05c829d8551e8e4a48d41f2d543dfdb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Jun 2023 17:56:03 -0400 Subject: [PATCH] Avoid attributing runtime references to module-level imports (#4942) --- .../test/fixtures/pyflakes/F401_17.py | 32 ++++++++++++++ crates/ruff/src/rules/pyflakes/mod.rs | 1 + ...les__pyflakes__tests__F401_F401_17.py.snap | 42 +++++++++++++++++++ crates/ruff_python_semantic/src/model.rs | 27 ++++++++++-- 4 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pyflakes/F401_17.py create mode 100644 crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_17.py.snap diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F401_17.py b/crates/ruff/resources/test/fixtures/pyflakes/F401_17.py new file mode 100644 index 0000000000000..0d243eac55195 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyflakes/F401_17.py @@ -0,0 +1,32 @@ +"""Test that runtime typing references are properly attributed to scoped imports.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, cast + +if TYPE_CHECKING: + from threading import Thread + + +def fn(thread: Thread): + from threading import Thread + + # The `Thread` on the left-hand side should resolve to the `Thread` imported at the + # top level. + x: Thread + + +def fn(thread: Thread): + from threading import Thread + + # The `Thread` on the left-hand side should resolve to the `Thread` imported at the + # top level. + cast("Thread", thread) + + +def fn(thread: Thread): + from threading import Thread + + # The `Thread` on the right-hand side should resolve to the`Thread` imported within + # `fn`. + cast(Thread, thread) diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index ed4690d9edaee..a27d8c190d6d9 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/crates/ruff/src/rules/pyflakes/mod.rs @@ -42,6 +42,7 @@ mod tests { #[test_case(Rule::UnusedImport, Path::new("F401_14.py"))] #[test_case(Rule::UnusedImport, Path::new("F401_15.py"))] #[test_case(Rule::UnusedImport, Path::new("F401_16.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_17.py"))] #[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"))] #[test_case(Rule::UndefinedLocalWithImportStar, Path::new("F403.py"))] #[test_case(Rule::LateFutureImport, Path::new("F404.py"))] diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_17.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_17.py.snap new file mode 100644 index 0000000000000..c8f566d1c4d9a --- /dev/null +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_17.py.snap @@ -0,0 +1,42 @@ +--- +source: crates/ruff/src/rules/pyflakes/mod.rs +--- +F401_17.py:12:27: F401 [*] `threading.Thread` imported but unused + | +12 | def fn(thread: Thread): +13 | from threading import Thread + | ^^^^^^ F401 +14 | +15 | # The `Thread` on the left-hand side should resolve to the `Thread` imported at the + | + = help: Remove unused import: `threading.Thread` + +ℹ Fix +9 9 | +10 10 | +11 11 | def fn(thread: Thread): +12 |- from threading import Thread +13 12 | +14 13 | # The `Thread` on the left-hand side should resolve to the `Thread` imported at the +15 14 | # top level. + +F401_17.py:20:27: F401 [*] `threading.Thread` imported but unused + | +20 | def fn(thread: Thread): +21 | from threading import Thread + | ^^^^^^ F401 +22 | +23 | # The `Thread` on the left-hand side should resolve to the `Thread` imported at the + | + = help: Remove unused import: `threading.Thread` + +ℹ Fix +17 17 | +18 18 | +19 19 | def fn(thread: Thread): +20 |- from threading import Thread +21 20 | +22 21 | # The `Thread` on the left-hand side should resolve to the `Thread` imported at the +23 22 | # top level. + + diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 7b23c81b6a7cb..949a461f21996 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -178,7 +178,7 @@ impl<'a> SemanticModel<'a> { pub fn resolve_reference(&mut self, symbol: &str, range: TextRange) -> ResolvedReference { // 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_deferred_type_definition() { + if self.in_forward_reference() { if let Some(binding_id) = self.scopes.global().get(symbol) { // Mark the binding as used. let context = self.execution_context(); @@ -239,9 +239,7 @@ impl<'a> SemanticModel<'a> { // // The `name` in `print(name)` should be treated as unresolved, but the `name` in // `name: str` should be treated as used. - if !self.in_deferred_type_definition() - && self.bindings[binding_id].kind.is_annotation() - { + if !self.in_forward_reference() && self.bindings[binding_id].kind.is_annotation() { continue; } @@ -756,6 +754,27 @@ impl<'a> SemanticModel<'a> { || self.in_future_type_definition() } + /// Return `true` if the context is in a forward type reference. + /// + /// Includes deferred string types, and future types in annotations. + /// + /// ## Examples + /// ```python + /// from __future__ import annotations + /// + /// from threading import Thread + /// + /// + /// x: Thread # Forward reference + /// cast("Thread", x) # Forward reference + /// cast(Thread, x) # Non-forward reference + /// ``` + pub const fn in_forward_reference(&self) -> bool { + self.in_simple_string_type_definition() + || self.in_complex_string_type_definition() + || (self.in_future_type_definition() && self.in_annotation()) + } + /// Return `true` if the context is in an exception handler. pub const fn in_exception_handler(&self) -> bool { self.flags.contains(SemanticModelFlags::EXCEPTION_HANDLER)