From 33e693e5497bec37551d0e7d8e53d8fc1a7e40cb Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Wed, 26 Mar 2025 13:59:31 +0100 Subject: [PATCH 1/2] [`flake8-type-checking`] Treat type statement value as typing references --- .../fixtures/flake8_type_checking/TC003.py | 24 +++++++ .../src/rules/flake8_type_checking/helpers.rs | 50 ++++++++++++++- .../rules/typing_only_runtime_import.rs | 6 +- ...only-standard-library-import_TC003.py.snap | 62 ++++++++++++++++++- crates/ruff_python_semantic/src/reference.rs | 8 +++ 5 files changed, 146 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC003.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC003.py index d9dd926e4272e..dab77fc65fddd 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC003.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC003.py @@ -14,3 +14,27 @@ def f(): import os print(os) + + +def f(): + import pathlib + + type Paths = list[pathlib.Path] + + +def f(): + import pathlib + + type Paths = list[pathlib.Path] + + print(Paths) + + +def f(): + import pathlib + + type Paths = list[pathlib.Path] + type PathsMapping = dict[str, Paths] + + # FIXME: false positive for indirect runtime use of Paths + print(PathsMapping) diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index 40e24076632fe..91cba51dc59c0 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -18,16 +18,62 @@ use crate::Locator; /// Returns `true` if the [`ResolvedReference`] is in a typing-only context _or_ a runtime-evaluated /// context (with quoting enabled). -pub(crate) fn is_typing_reference(reference: &ResolvedReference, settings: &Settings) -> bool { +pub(crate) fn is_typing_reference( + semantic: &SemanticModel, + reference: &ResolvedReference, + settings: &Settings, +) -> bool { reference.in_type_checking_block() // if we're not in a type checking block, we necessarily need to be within a // type definition to be considered a typing reference || (reference.in_type_definition() && (reference.in_typing_only_annotation() || reference.in_string_type_definition() + || (reference.in_deferred_type_alias_value() && !parent_type_alias_has_runtime_references(semantic, reference)) || (settings.quote_annotations && reference.in_runtime_evaluated_annotation()))) } +/// Find the [`Binding`] defined by the [PEP 695] type alias from a +/// [`Reference`] originating from its value expression and check +/// whether or not the binding has any any runtime references +/// +/// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias +pub(crate) fn parent_type_alias_has_runtime_references( + semantic: &SemanticModel, + reference: &ResolvedReference, +) -> bool { + // For now this check is non-recursive, i.e. if one of the references + // occurs in another type alias that itself has a runtime reference + // we would currently fail to detect that as a runtime reference. + // + // On the plus-side we don't have to worry about cyclic dependencies + // between type aliases this way. Otherwise we would need to detect + // and break cycles in order to not get caught in an infinite recursion. + let Some(expression_id) = reference.expression_id() else { + return false; + }; + // type statements are wrapped in an extra scope for the type parameters + // so if we want to look up the binding defined by the type alias we need + // to look at the parent scope + let Some(scope_id) = semantic.parent_scope_id(reference.scope_id()) else { + return false; + }; + let statement = semantic.statement(expression_id); + let scope = &semantic.scopes[scope_id]; + for binding_id in scope.binding_ids() { + let binding = semantic.binding(binding_id); + let Some(binding_statement) = binding.statement(semantic) else { + continue; + }; + if statement == binding_statement { + return binding + .references() + .any(|reference_id| semantic.reference(reference_id).in_runtime_context()); + } + } + false +} + /// Returns `true` if the [`Binding`] represents a runtime-required import. pub(crate) fn is_valid_runtime_import( binding: &Binding, @@ -42,7 +88,7 @@ pub(crate) fn is_valid_runtime_import( && binding .references() .map(|reference_id| semantic.reference(reference_id)) - .any(|reference| !is_typing_reference(reference, settings)) + .any(|reference| !is_typing_reference(semantic, reference, settings)) } else { false } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 777a7b123713b..f933db4472273 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -281,7 +281,11 @@ pub(crate) fn typing_only_runtime_import( .references() .map(|reference_id| checker.semantic().reference(reference_id)) .all(|reference| { - is_typing_reference(reference, &checker.settings.flake8_type_checking) + is_typing_reference( + checker.semantic(), + reference, + &checker.settings.flake8_type_checking, + ) }) { let qualified_name = import.qualified_name(); diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_TC003.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_TC003.py.snap index e7590ea5c254f..581991a20af20 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_TC003.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_TC003.py.snap @@ -25,4 +25,64 @@ TC003.py:8:12: TC003 [*] Move standard library import `os` into a type-checking 8 |- import os 9 12 | 10 13 | x: os -11 14 | +11 14 | + +TC003.py:20:12: TC003 [*] Move standard library import `pathlib` into a type-checking block + | +19 | def f(): +20 | import pathlib + | ^^^^^^^ TC003 +21 | +22 | type Paths = list[pathlib.Path] + | + = help: Move into type-checking block + +ℹ Unsafe fix +2 2 | +3 3 | For typing-only import detection tests, see `TC002.py`. +4 4 | """ + 5 |+from typing import TYPE_CHECKING + 6 |+ + 7 |+if TYPE_CHECKING: + 8 |+ import pathlib +5 9 | +6 10 | +7 11 | def f(): +-------------------------------------------------------------------------------- +17 21 | +18 22 | +19 23 | def f(): +20 |- import pathlib +21 24 | +22 25 | type Paths = list[pathlib.Path] +23 26 | + +TC003.py:34:12: TC003 [*] Move standard library import `pathlib` into a type-checking block + | +33 | def f(): +34 | import pathlib + | ^^^^^^^ TC003 +35 | +36 | type Paths = list[pathlib.Path] + | + = help: Move into type-checking block + +ℹ Unsafe fix +2 2 | +3 3 | For typing-only import detection tests, see `TC002.py`. +4 4 | """ + 5 |+from typing import TYPE_CHECKING + 6 |+ + 7 |+if TYPE_CHECKING: + 8 |+ import pathlib +5 9 | +6 10 | +7 11 | def f(): +-------------------------------------------------------------------------------- +31 35 | +32 36 | +33 37 | def f(): +34 |- import pathlib +35 38 | +36 39 | type Paths = list[pathlib.Path] +37 40 | type PathsMapping = dict[str, Paths] diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs index bee6413d4a9cd..44d5c2da9191f 100644 --- a/crates/ruff_python_semantic/src/reference.rs +++ b/crates/ruff_python_semantic/src/reference.rs @@ -86,6 +86,14 @@ impl ResolvedReference { .intersects(SemanticModelFlags::DUNDER_ALL_DEFINITION) } + /// Return `true` if the model is in the r.h.s. of a [PEP 695] type alias. + /// + /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias + pub const fn in_deferred_type_alias_value(&self) -> bool { + self.flags + .intersects(SemanticModelFlags::DEFERRED_TYPE_ALIAS) + } + /// Return `true` if the context is in the r.h.s. of a [PEP 613] type alias. /// /// [PEP 613]: https://peps.python.org/pep-0613/ From bfa19d13c0e1727c2b6a3c7234b01bc50e84eeab Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Wed, 26 Mar 2025 14:13:45 +0100 Subject: [PATCH 2/2] Fix typo --- crates/ruff_python_semantic/src/reference.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs index 44d5c2da9191f..69e1be1b44b89 100644 --- a/crates/ruff_python_semantic/src/reference.rs +++ b/crates/ruff_python_semantic/src/reference.rs @@ -86,7 +86,7 @@ impl ResolvedReference { .intersects(SemanticModelFlags::DUNDER_ALL_DEFINITION) } - /// Return `true` if the model is in the r.h.s. of a [PEP 695] type alias. + /// Return `true` if the context is in the r.h.s. of a [PEP 695] type alias. /// /// [PEP 695]: https://peps.python.org/pep-0695/#generic-type-alias pub const fn in_deferred_type_alias_value(&self) -> bool {