From 894f6855e18766efa51e418c0fad899aa108733c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 29 Jan 2024 15:34:10 -0500 Subject: [PATCH] Avoid marking InitVar as typing-only --- .../fixtures/flake8_type_checking/init_var.py | 18 +++++++ crates/ruff_linter/src/checkers/ast/mod.rs | 15 ++++++ .../src/rules/flake8_type_checking/helpers.rs | 33 +++++++++++- .../src/rules/flake8_type_checking/mod.rs | 5 +- ...y-standard-library-import_init_var.py.snap | 50 +++++++++++++++++++ ...ng-only-third-party-import_strict.py.snap} | 0 ...y-standard-library-import_init_var.py.snap | 25 ++++++++++ 7 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_type_checking/init_var.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-standard-library-import_init_var.py.snap rename crates/ruff_linter/src/rules/flake8_type_checking/snapshots/{ruff_linter__rules__flake8_type_checking__tests__strict.snap => ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-third-party-import_strict.py.snap} (100%) create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_init_var.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/init_var.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/init_var.py new file mode 100644 index 0000000000000..93eda77699a74 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/init_var.py @@ -0,0 +1,18 @@ +"""Test: avoid marking an `InitVar` as typing-only.""" + +from __future__ import annotations + +from dataclasses import FrozenInstanceError, InitVar, dataclass +from pathlib import Path + + +@dataclass +class C: + i: int + j: int = None + database: InitVar[Path] = None + + err: FrozenInstanceError = None + + def __post_init__(self, database): + ... diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 540745e5f8e12..ac648ac64e942 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -721,6 +721,21 @@ where AnnotationContext::RuntimeEvaluated => { self.visit_runtime_evaluated_annotation(annotation); } + AnnotationContext::TypingOnly + if flake8_type_checking::helpers::is_dataclass_meta_annotation( + annotation, + self.semantic(), + ) => + { + if let Expr::Subscript(subscript) = &**annotation { + // Ex) `InitVar[str]` + self.visit_runtime_required_annotation(&subscript.value); + self.visit_annotation(&subscript.slice); + } else { + // Ex) `InitVar` + self.visit_runtime_required_annotation(annotation); + } + } AnnotationContext::TypingOnly => self.visit_annotation(annotation), } 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 a7abd8571e468..cd0d67b11183b 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -2,11 +2,11 @@ use anyhow::Result; use ruff_diagnostics::Edit; use ruff_python_ast::call_path::from_qualified_name; -use ruff_python_ast::helpers::map_callable; +use ruff_python_ast::helpers::{map_callable, map_subscript}; use ruff_python_ast::{self as ast, Decorator, Expr}; use ruff_python_codegen::{Generator, Stylist}; use ruff_python_semantic::{ - analyze, Binding, BindingKind, NodeId, ResolvedReference, SemanticModel, + analyze, Binding, BindingKind, NodeId, ResolvedReference, ScopeKind, SemanticModel, }; use ruff_source_file::Locator; use ruff_text_size::Ranged; @@ -104,6 +104,35 @@ fn runtime_required_decorators( }) } +/// Returns `true` if an annotation will be inspected at runtime by the `dataclasses` module. +/// +/// Specifically, detects whether an annotation is to either `dataclasses.InitVar` or +/// `typing.ClassVar` within a `@dataclass` class definition. +/// +/// See: +pub(crate) fn is_dataclass_meta_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool { + // Determine whether the assignment is in a `@dataclass` class definition. + if let ScopeKind::Class(class_def) = semantic.current_scope().kind { + if class_def.decorator_list.iter().any(|decorator| { + semantic + .resolve_call_path(map_callable(&decorator.expression)) + .is_some_and(|call_path| { + matches!(call_path.as_slice(), ["dataclasses", "dataclass"]) + }) + }) { + // Determine whether the annotation is `typing.ClassVar` or `dataclasses.InitVar`. + return semantic + .resolve_call_path(map_subscript(annotation)) + .is_some_and(|call_path| { + matches!(call_path.as_slice(), ["dataclasses", "InitVar"]) + || semantic.match_typing_call_path(&call_path, "ClassVar") + }); + } + } + + false +} + /// Returns `true` if a function is registered as a `singledispatch` interface. /// /// For example, `fun` below is a `singledispatch` interface: diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs index 700f27973f594..ef105e99ff29f 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -39,6 +39,7 @@ mod tests { #[test_case(Rule::RuntimeStringUnion, Path::new("TCH006_2.py"))] #[test_case(Rule::TypingOnlyFirstPartyImport, Path::new("TCH001.py"))] #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TCH003.py"))] + #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("init_var.py"))] #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("snapshot.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("TCH002.py"))] #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote.py"))] @@ -75,7 +76,9 @@ mod tests { } #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("strict.py"))] + #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("init_var.py"))] fn strict(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("strict_{}_{}", rule_code.as_ref(), path.to_string_lossy()); let diagnostics = test_path( Path::new("flake8_type_checking").join(path).as_path(), &settings::LinterSettings { @@ -86,7 +89,7 @@ mod tests { ..settings::LinterSettings::for_rule(rule_code) }, )?; - assert_messages!(diagnostics); + assert_messages!(snapshot, diagnostics); Ok(()) } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-standard-library-import_init_var.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-standard-library-import_init_var.py.snap new file mode 100644 index 0000000000000..64ba87ffb395b --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-standard-library-import_init_var.py.snap @@ -0,0 +1,50 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +init_var.py:5:25: TCH003 [*] Move standard library import `dataclasses.FrozenInstanceError` into a type-checking block + | +3 | from __future__ import annotations +4 | +5 | from dataclasses import FrozenInstanceError, InitVar, dataclass + | ^^^^^^^^^^^^^^^^^^^ TCH003 +6 | from pathlib import Path + | + = help: Move into type-checking block + +ℹ Unsafe fix +2 2 | +3 3 | from __future__ import annotations +4 4 | +5 |-from dataclasses import FrozenInstanceError, InitVar, dataclass + 5 |+from dataclasses import InitVar, dataclass +6 6 | from pathlib import Path + 7 |+from typing import TYPE_CHECKING + 8 |+ + 9 |+if TYPE_CHECKING: + 10 |+ from dataclasses import FrozenInstanceError +7 11 | +8 12 | +9 13 | @dataclass + +init_var.py:6:21: TCH003 [*] Move standard library import `pathlib.Path` into a type-checking block + | +5 | from dataclasses import FrozenInstanceError, InitVar, dataclass +6 | from pathlib import Path + | ^^^^ TCH003 + | + = help: Move into type-checking block + +ℹ Unsafe fix +3 3 | from __future__ import annotations +4 4 | +5 5 | from dataclasses import FrozenInstanceError, InitVar, dataclass +6 |-from pathlib import Path + 6 |+from typing import TYPE_CHECKING + 7 |+ + 8 |+if TYPE_CHECKING: + 9 |+ from pathlib import Path +7 10 | +8 11 | +9 12 | @dataclass + + diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-third-party-import_strict.py.snap similarity index 100% rename from crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict.snap rename to crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__strict_typing-only-third-party-import_strict.py.snap diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_init_var.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_init_var.py.snap new file mode 100644 index 0000000000000..08a45c92e134c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__typing-only-standard-library-import_init_var.py.snap @@ -0,0 +1,25 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +init_var.py:6:21: TCH003 [*] Move standard library import `pathlib.Path` into a type-checking block + | +5 | from dataclasses import FrozenInstanceError, InitVar, dataclass +6 | from pathlib import Path + | ^^^^ TCH003 + | + = help: Move into type-checking block + +ℹ Unsafe fix +3 3 | from __future__ import annotations +4 4 | +5 5 | from dataclasses import FrozenInstanceError, InitVar, dataclass +6 |-from pathlib import Path + 6 |+from typing import TYPE_CHECKING + 7 |+ + 8 |+if TYPE_CHECKING: + 9 |+ from pathlib import Path +7 10 | +8 11 | +9 12 | @dataclass + +