From a31e8db87fbd8570ff376dabbaa117ca7a24d066 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 6 Jul 2023 21:59:51 -0400 Subject: [PATCH] Differentiate between runtime and typing-time annotations --- crates/ruff/src/checkers/ast/mod.rs | 20 +++-- ...ture_annotations__tests__edge_case.py.snap | 8 ++ ...02_no_future_import_uses_lowercase.py.snap | 7 ++ ..._fa102_no_future_import_uses_union.py.snap | 14 ++++ ..._no_future_import_uses_union_inner.py.snap | 16 ++++ crates/ruff_python_semantic/src/model.rs | 74 +++++++++++++------ 6 files changed, 112 insertions(+), 27 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 7767129c5f5bdc..f4df5c2e8fcb03 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1811,7 +1811,7 @@ where { if let Some(expr) = &arg_with_default.def.annotation { if runtime_annotation { - self.visit_type_definition(expr); + self.visit_runtime_annotation(expr); } else { self.visit_annotation(expr); }; @@ -1823,7 +1823,7 @@ where if let Some(arg) = &args.vararg { if let Some(expr) = &arg.annotation { if runtime_annotation { - self.visit_type_definition(expr); + self.visit_runtime_annotation(expr); } else { self.visit_annotation(expr); }; @@ -1832,7 +1832,7 @@ where if let Some(arg) = &args.kwarg { if let Some(expr) = &arg.annotation { if runtime_annotation { - self.visit_type_definition(expr); + self.visit_runtime_annotation(expr); } else { self.visit_annotation(expr); }; @@ -1840,7 +1840,7 @@ where } for expr in returns { if runtime_annotation { - self.visit_type_definition(expr); + self.visit_runtime_annotation(expr); } else { self.visit_annotation(expr); }; @@ -1992,7 +1992,7 @@ where }; if runtime_annotation { - self.visit_type_definition(annotation); + self.visit_runtime_annotation(annotation); } else { self.visit_annotation(annotation); } @@ -2089,7 +2089,7 @@ where fn visit_annotation(&mut self, expr: &'b Expr) { let flags_snapshot = self.semantic.flags; - self.semantic.flags |= SemanticModelFlags::ANNOTATION; + self.semantic.flags |= SemanticModelFlags::TYPING_ANNOTATION; self.visit_type_definition(expr); self.semantic.flags = flags_snapshot; } @@ -4125,6 +4125,14 @@ impl<'a> Checker<'a> { self.semantic.flags = snapshot; } + /// Visit an [`Expr`], and treat it as a runtime-required type annotation. + fn visit_runtime_annotation(&mut self, expr: &'a Expr) { + let snapshot = self.semantic.flags; + self.semantic.flags |= SemanticModelFlags::RUNTIME_ANNOTATION; + self.visit_type_definition(expr); + self.semantic.flags = snapshot; + } + /// Visit an [`Expr`], and treat it as a type definition. fn visit_type_definition(&mut self, expr: &'a Expr) { let snapshot = self.semantic.flags; diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__edge_case.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__edge_case.py.snap index fc0cdbf8ad7782..ed1dc0f3df24ef 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__edge_case.py.snap +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__edge_case.py.snap @@ -1,6 +1,14 @@ --- source: crates/ruff/src/rules/flake8_future_annotations/mod.rs --- +edge_case.py:5:13: FA100 Missing `from __future__ import annotations`, but uses `typing.List` + | +5 | def main(_: List[int]) -> None: + | ^^^^ FA100 +6 | a_list: t.List[str] = [] +7 | a_list.append("hello") + | + edge_case.py:6:13: FA100 Missing `from __future__ import annotations`, but uses `typing.List` | 5 | def main(_: List[int]) -> None: diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap index 5fb0fe74447f19..14acba04f76907 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_lowercase.py.snap @@ -9,4 +9,11 @@ no_future_import_uses_lowercase.py:2:13: FA102 Missing `from __future__ import a 3 | a_list.append("hello") | +no_future_import_uses_lowercase.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +6 | def hello(y: dict[str, int]) -> None: + | ^^^^^^^^^^^^^^ FA102 +7 | del y + | + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap index cee83994c01145..6bf73cdcf13642 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union.py.snap @@ -17,4 +17,18 @@ no_future_import_uses_union.py:2:13: FA102 Missing `from __future__ import annot 3 | a_list.append("hello") | +no_future_import_uses_union.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union + | +6 | def hello(y: dict[str, int] | None) -> None: + | ^^^^^^^^^^^^^^^^^^^^^ FA102 +7 | del y + | + +no_future_import_uses_union.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +6 | def hello(y: dict[str, int] | None) -> None: + | ^^^^^^^^^^^^^^ FA102 +7 | del y + | + diff --git a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap index d1538618555444..7f5156463a6563 100644 --- a/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap +++ b/crates/ruff/src/rules/flake8_future_annotations/snapshots/ruff__rules__flake8_future_annotations__tests__fa102_no_future_import_uses_union_inner.py.snap @@ -17,6 +17,22 @@ no_future_import_uses_union_inner.py:2:18: FA102 Missing `from __future__ import 3 | a_list.append("hello") | +no_future_import_uses_union_inner.py:6:14: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection + | +6 | def hello(y: dict[str | None, int]) -> None: + | ^^^^^^^^^^^^^^^^^^^^^ FA102 +7 | z: tuple[str, str | None, str] = tuple(y) +8 | del z + | + +no_future_import_uses_union_inner.py:6:19: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union + | +6 | def hello(y: dict[str | None, int]) -> None: + | ^^^^^^^^^^ FA102 +7 | z: tuple[str, str | None, str] = tuple(y) +8 | del z + | + no_future_import_uses_union_inner.py:7:8: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection | 6 | def hello(y: dict[str | None, int]) -> None: diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 6773c973f202d1..27b15715def088 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -929,7 +929,7 @@ impl<'a> SemanticModel<'a> { /// Return the [`ExecutionContext`] of the current scope. pub const fn execution_context(&self) -> ExecutionContext { if self.in_type_checking_block() - || self.in_annotation() + || self.in_typing_annotation() || self.in_complex_string_type_definition() || self.in_simple_string_type_definition() { @@ -974,7 +974,17 @@ impl<'a> SemanticModel<'a> { /// Return `true` if the context is in a type annotation. pub const fn in_annotation(&self) -> bool { - self.flags.contains(SemanticModelFlags::ANNOTATION) + self.in_typing_annotation() || self.in_runtime_annotation() + } + + /// Return `true` if the context is in a typing-only type annotation. + pub const fn in_typing_annotation(&self) -> bool { + self.flags.contains(SemanticModelFlags::TYPING_ANNOTATION) + } + + /// Return `true` if the context is in a runtime-required type annotation. + pub const fn in_runtime_annotation(&self) -> bool { + self.flags.contains(SemanticModelFlags::RUNTIME_ANNOTATION) } /// Return `true` if the context is in a type definition. @@ -1025,7 +1035,7 @@ impl<'a> SemanticModel<'a> { 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()) + || (self.in_future_type_definition() && self.in_typing_annotation()) } /// Return `true` if the context is in an exception handler. @@ -1147,13 +1157,36 @@ bitflags! { /// Flags indicating the current context of the analysis. #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] pub struct SemanticModelFlags: u16 { - /// The context is in a type annotation. + /// The context is in a typing-time-only type annotation. /// /// For example, the context could be visiting `int` in: /// ```python - /// x: int = 1 + /// def foo() -> int: + /// x: int = 1 /// ``` - const ANNOTATION = 1 << 0; + /// + /// In this case, Python doesn't require that the type annotation be evaluated at runtime. + /// + /// Function argument annotations are always evaluated at runtime, unless + /// `from __future__ import annotations` is used. Annotated assignments are also evaluated + /// at runtime if they're within a module or class scope. + const TYPING_ANNOTATION = 1 << 0; + + /// The context is in a runtime type annotation. + /// + /// For example, the context could be visiting `int` in: + /// ```python + /// def foo(x: int) -> int: + /// ... + /// ``` + /// + /// In this case, Python requires that the type annotation be evaluated at runtime, + /// as it needs to be available on the function's `__annotations__` attribute. + /// + /// Function argument annotations are always evaluated at runtime, unless + /// `from __future__ import annotations` is used. Annotated assignments are also evaluated + /// at runtime if they're within a module or class scope. + const RUNTIME_ANNOTATION = 1 << 1; /// The context is in a type definition. /// @@ -1167,7 +1200,7 @@ bitflags! { /// All type annotations are also type definitions, but the converse is not true. /// In our example, `int` is a type definition but not a type annotation, as it /// doesn't appear in a type annotation context, but rather in a type definition. - const TYPE_DEFINITION = 1 << 1; + const TYPE_DEFINITION = 1 << 2; /// The context is in a (deferred) "simple" string type definition. /// @@ -1178,7 +1211,7 @@ bitflags! { /// /// "Simple" string type definitions are those that consist of a single string literal, /// as opposed to an implicitly concatenated string literal. - const SIMPLE_STRING_TYPE_DEFINITION = 1 << 2; + const SIMPLE_STRING_TYPE_DEFINITION = 1 << 3; /// The context is in a (deferred) "complex" string type definition. /// @@ -1189,7 +1222,7 @@ bitflags! { /// /// "Complex" string type definitions are those that consist of a implicitly concatenated /// string literals. These are uncommon but valid. - const COMPLEX_STRING_TYPE_DEFINITION = 1 << 3; + const COMPLEX_STRING_TYPE_DEFINITION = 1 << 4; /// The context is in a (deferred) `__future__` type definition. /// @@ -1202,7 +1235,7 @@ bitflags! { /// /// `__future__`-style type annotations are only enabled if the `annotations` feature /// is enabled via `from __future__ import annotations`. - const FUTURE_TYPE_DEFINITION = 1 << 4; + const FUTURE_TYPE_DEFINITION = 1 << 5; /// The context is in an exception handler. /// @@ -1213,7 +1246,7 @@ bitflags! { /// except Exception: /// x: int = 1 /// ``` - const EXCEPTION_HANDLER = 1 << 5; + const EXCEPTION_HANDLER = 1 << 6; /// The context is in an f-string. /// @@ -1221,7 +1254,7 @@ bitflags! { /// ```python /// f'{x}' /// ``` - const F_STRING = 1 << 6; + const F_STRING = 1 << 7; /// The context is in a nested f-string. /// @@ -1229,7 +1262,7 @@ bitflags! { /// ```python /// f'{f"{x}"}' /// ``` - const NESTED_F_STRING = 1 << 7; + const NESTED_F_STRING = 1 << 8; /// The context is in a boolean test. /// @@ -1241,7 +1274,7 @@ bitflags! { /// /// The implication is that the actual value returned by the current expression is /// not used, only its truthiness. - const BOOLEAN_TEST = 1 << 8; + const BOOLEAN_TEST = 1 << 9; /// The context is in a `typing::Literal` annotation. /// @@ -1250,7 +1283,7 @@ bitflags! { /// def f(x: Literal["A", "B", "C"]): /// ... /// ``` - const LITERAL = 1 << 9; + const LITERAL = 1 << 10; /// The context is in a subscript expression. /// @@ -1258,7 +1291,7 @@ bitflags! { /// ```python /// x["a"]["b"] /// ``` - const SUBSCRIPT = 1 << 10; + const SUBSCRIPT = 1 << 11; /// The context is in a type-checking block. /// @@ -1270,8 +1303,7 @@ bitflags! { /// if TYPE_CHECKING: /// x: int = 1 /// ``` - const TYPE_CHECKING_BLOCK = 1 << 11; - + const TYPE_CHECKING_BLOCK = 1 << 12; /// The context has traversed past the "top-of-file" import boundary. /// @@ -1284,7 +1316,7 @@ bitflags! { /// /// x: int = 1 /// ``` - const IMPORT_BOUNDARY = 1 << 12; + const IMPORT_BOUNDARY = 1 << 13; /// The context has traversed past the `__future__` import boundary. /// @@ -1299,7 +1331,7 @@ bitflags! { /// /// Python considers it a syntax error to import from `__future__` after /// any other non-`__future__`-importing statements. - const FUTURES_BOUNDARY = 1 << 13; + const FUTURES_BOUNDARY = 1 << 14; /// `__future__`-style type annotations are enabled in this context. /// @@ -1311,7 +1343,7 @@ bitflags! { /// def f(x: int) -> int: /// ... /// ``` - const FUTURE_ANNOTATIONS = 1 << 14; + const FUTURE_ANNOTATIONS = 1 << 15; } }