Skip to content

Commit

Permalink
Avoid attributing runtime references to module-level imports (#4942)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Jun 7, 2023
1 parent 20240fc commit ae75b30
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 4 deletions.
32 changes: 32 additions & 0 deletions crates/ruff/resources/test/fixtures/pyflakes/F401_17.py
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -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.


27 changes: 23 additions & 4 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit ae75b30

Please sign in to comment.