Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid attributing runtime references to module-level imports #4942

Merged
merged 1 commit into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading