From 1c7ea690a820deaa0e17ecf72593ebc4781f3752 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Thu, 30 Oct 2025 14:14:29 -0400 Subject: [PATCH] [`flake8-type-checking`] Fix `TC003` false positive with `future-annotations` (#21125) Summary -- Fixes #21121 by upgrading `RuntimeEvaluated` annotations like `dataclasses.KW_ONLY` to `RuntimeRequired`. We already had special handling for `TypingOnly` annotations in this context but not `RuntimeEvaluated`. Combining that with the `future-annotations` setting, which allowed ignoring the `RuntimeEvaluated` flag, led to the reported bug where we would try to move `KW_ONLY` into a `TYPE_CHECKING` block. Test Plan -- A new test based on the issue --- .../fixtures/flake8_type_checking/TC003.py | 11 ++++++ .../test/fixtures/pyupgrade/UP037_3.py | 17 +++++++++ crates/ruff_linter/src/checkers/ast/mod.rs | 8 +++++ .../src/rules/flake8_type_checking/mod.rs | 20 +++++++++++ ...future_import_kw_only__TC003_TC003.py.snap | 28 +++++++++++++++ crates/ruff_linter/src/rules/pyupgrade/mod.rs | 15 ++++++++ ...__rules__pyupgrade__tests__UP037_3.py.snap | 36 +++++++++++++++++++ ...grade__tests__rules_py313__UP037_3.py.snap | 4 +++ 8 files changed, 139 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_3.py create mode 100644 crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__add_future_import_kw_only__TC003_TC003.py.snap create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_3.py.snap create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__rules_py313__UP037_3.py.snap 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 d9dd926e42..9ea93ac378 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,14 @@ def f(): import os print(os) + + +# regression test for https://github.com/astral-sh/ruff/issues/21121 +from dataclasses import KW_ONLY, dataclass + + +@dataclass +class DataClass: + a: int + _: KW_ONLY # should be an exception to TC003, even with future-annotations + b: int diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_3.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_3.py new file mode 100644 index 0000000000..929486d6da --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP037_3.py @@ -0,0 +1,17 @@ +""" +Regression test for an ecosystem hit on +https://github.com/astral-sh/ruff/pull/21125. + +We should mark all of the components of special dataclass annotations as +runtime-required, not just the first layer. +""" + +from dataclasses import dataclass +from typing import ClassVar, Optional + + +@dataclass(frozen=True) +class EmptyCell: + _singleton: ClassVar[Optional["EmptyCell"]] = None + # the behavior of _singleton above should match a non-ClassVar + _doubleton: "EmptyCell" diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 2321cfbb7c..2280dc33cb 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -1400,6 +1400,14 @@ impl<'a> Visitor<'a> for Checker<'a> { AnnotationContext::RuntimeRequired => { self.visit_runtime_required_annotation(annotation); } + AnnotationContext::RuntimeEvaluated + if flake8_type_checking::helpers::is_dataclass_meta_annotation( + annotation, + self.semantic(), + ) => + { + self.visit_runtime_required_annotation(annotation); + } AnnotationContext::RuntimeEvaluated => { self.visit_runtime_evaluated_annotation(annotation); } 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 fd8f08626d..337c420621 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs @@ -98,6 +98,26 @@ mod tests { Ok(()) } + #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("TC003.py"))] + fn add_future_import_dataclass_kw_only_py313(rule: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "add_future_import_kw_only__{}_{}", + rule.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("flake8_type_checking").join(path).as_path(), + &settings::LinterSettings { + future_annotations: true, + // The issue in #21121 also didn't trigger on Python 3.14 + unresolved_target_version: PythonVersion::PY313.into(), + ..settings::LinterSettings::for_rule(rule) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + // we test these rules as a pair, since they're opposites of one another // so we want to make sure their fixes are not going around in circles. #[test_case(Rule::UnquotedTypeAlias, Path::new("TC007.py"))] diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__add_future_import_kw_only__TC003_TC003.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__add_future_import_kw_only__TC003_TC003.py.snap new file mode 100644 index 0000000000..c2f1077e90 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__add_future_import_kw_only__TC003_TC003.py.snap @@ -0,0 +1,28 @@ +--- +source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +--- +TC003 [*] Move standard library import `os` into a type-checking block + --> TC003.py:8:12 + | + 7 | def f(): + 8 | import os + | ^^ + 9 | +10 | x: os + | +help: Move into type-checking block +2 | +3 | For typing-only import detection tests, see `TC002.py`. +4 | """ +5 + from typing import TYPE_CHECKING +6 + +7 + if TYPE_CHECKING: +8 + import os +9 | +10 | +11 | def f(): + - import os +12 | +13 | x: os +14 | +note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index c8919fe1b0..c933de5ee8 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -64,6 +64,7 @@ mod tests { #[test_case(Rule::QuotedAnnotation, Path::new("UP037_0.py"))] #[test_case(Rule::QuotedAnnotation, Path::new("UP037_1.py"))] #[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))] + #[test_case(Rule::QuotedAnnotation, Path::new("UP037_3.py"))] #[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"))] #[test_case(Rule::RedundantOpenModes, Path::new("UP015_1.py"))] #[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))] @@ -156,6 +157,20 @@ mod tests { Ok(()) } + #[test_case(Rule::QuotedAnnotation, Path::new("UP037_3.py"))] + fn rules_py313(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("rules_py313__{}", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("pyupgrade").join(path).as_path(), + &settings::LinterSettings { + unresolved_target_version: PythonVersion::PY313.into(), + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.py"))] #[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.pyi"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))] diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_3.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_3.py.snap new file mode 100644 index 0000000000..19cdea494d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP037_3.py.snap @@ -0,0 +1,36 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP037 [*] Remove quotes from type annotation + --> UP037_3.py:15:35 + | +13 | @dataclass(frozen=True) +14 | class EmptyCell: +15 | _singleton: ClassVar[Optional["EmptyCell"]] = None + | ^^^^^^^^^^^ +16 | # the behavior of _singleton above should match a non-ClassVar +17 | _doubleton: "EmptyCell" + | +help: Remove quotes +12 | +13 | @dataclass(frozen=True) +14 | class EmptyCell: + - _singleton: ClassVar[Optional["EmptyCell"]] = None +15 + _singleton: ClassVar[Optional[EmptyCell]] = None +16 | # the behavior of _singleton above should match a non-ClassVar +17 | _doubleton: "EmptyCell" + +UP037 [*] Remove quotes from type annotation + --> UP037_3.py:17:17 + | +15 | _singleton: ClassVar[Optional["EmptyCell"]] = None +16 | # the behavior of _singleton above should match a non-ClassVar +17 | _doubleton: "EmptyCell" + | ^^^^^^^^^^^ + | +help: Remove quotes +14 | class EmptyCell: +15 | _singleton: ClassVar[Optional["EmptyCell"]] = None +16 | # the behavior of _singleton above should match a non-ClassVar + - _doubleton: "EmptyCell" +17 + _doubleton: EmptyCell diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__rules_py313__UP037_3.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__rules_py313__UP037_3.py.snap new file mode 100644 index 0000000000..2bacb5d540 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__rules_py313__UP037_3.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +