From 10bda3df00679c6808d8593470fe693ea8c9ae8f Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Thu, 30 Oct 2025 22:29:07 +0530 Subject: [PATCH] [`pyupgrade`] Fix false positive for `TypeVar` with default on Python <3.13 (`UP046`,`UP047`) (#21045) ## Summary Type default for Type parameter was added in Python 3.13 (PEP 696). `typing_extensions.TypeVar` backports the default argument to earlier versions. `UP046` & `UP047` were getting triggered when `typing_extensions.TypeVar` with `default` argument was used on python version < 3.13 It shouldn't be triggered for python version < 3.13 This commit fixes the bug by adding a python version check before triggering them. Fixes #20929. ## Test Plan ### Manual testing 1 As the issue author pointed out in https://github.com/astral-sh/ruff/issues/20929#issuecomment-3413194511, ran the following on `main` branch: > % cargo run -p ruff -- check ../efax/ --target-version py312 --no-cache
Output ```zsh Compiling ruff_linter v0.14.1 (/Users/prakhar/ruff/crates/ruff_linter) Compiling ruff v0.14.1 (/Users/prakhar/ruff/crates/ruff) Compiling ruff_graph v0.1.0 (/Users/prakhar/ruff/crates/ruff_graph) Compiling ruff_workspace v0.0.0 (/Users/prakhar/ruff/crates/ruff_workspace) Compiling ruff_server v0.2.2 (/Users/prakhar/ruff/crates/ruff_server) Finished `dev` profile [unoptimized + debuginfo] target(s) in 6.72s Running `target/debug/ruff check ../efax/ --target-version py312 --no-cache` UP046 Generic class `ExpectationParametrization` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/expectation_parametrization.py:17:48 | 17 | class ExpectationParametrization(Distribution, Generic[NP]): | ^^^^^^^^^^^ 18 | """The expectation parametrization of an exponential family distribution. | help: Use type parameters UP046 Generic class `ExpToNat` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/mixins/exp_to_nat/exp_to_nat.py:27:68 | 26 | @dataclass 27 | class ExpToNat(ExpectationParametrization[NP], SimpleDistribution, Generic[NP]): | ^^^^^^^^^^^ 28 | """This mixin implements the conversion from expectation to natural parameters. | help: Use type parameters UP046 Generic class `HasEntropyEP` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/mixins/has_entropy.py:25:20 | 23 | HasEntropy, 24 | JaxAbstractClass, 25 | Generic[NP]): | ^^^^^^^^^^^ 26 | @abstract_jit 27 | @abstractmethod | help: Use type parameters UP046 Generic class `HasEntropyNP` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/mixins/has_entropy.py:64:20 | 62 | class HasEntropyNP(NaturalParametrization[EP], 63 | HasEntropy, 64 | Generic[EP]): | ^^^^^^^^^^^ 65 | @jit 66 | @final | help: Use type parameters UP046 Generic class `NaturalParametrization` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/natural_parametrization.py:43:30 | 41 | class NaturalParametrization(Distribution, 42 | JaxAbstractClass, 43 | Generic[EP, Domain]): | ^^^^^^^^^^^^^^^^^^^ 44 | """The natural parametrization of an exponential family distribution. | help: Use type parameters UP046 Generic class `Structure` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/efax/_src/structure/structure.py:31:17 | 30 | @dataclass 31 | class Structure(Generic[P]): | ^^^^^^^^^^ 32 | """This class generalizes the notion of type for Distribution objects. | help: Use type parameters UP046 Generic class `DistributionInfo` uses `Generic` subclass instead of type parameters --> /Users/prakhar/efax/tests/distribution_info.py:20:24 | 20 | class DistributionInfo(Generic[NP, EP, Domain]): | ^^^^^^^^^^^^^^^^^^^^^^^ 21 | def __init__(self, dimensions: int = 1, safety: float = 0.0) -> None: 22 | super().__init__() | help: Use type parameters Found 7 errors. No fixes available (7 hidden fixes can be enabled with the `--unsafe-fixes` option). ```
Running it after the changes: ```zsh ruff % cargo run -p ruff -- check ../efax/ --target-version py312 --no-cache Compiling ruff_linter v0.14.1 (/Users/prakhar/ruff/crates/ruff_linter) Compiling ruff v0.14.1 (/Users/prakhar/ruff/crates/ruff) Compiling ruff_graph v0.1.0 (/Users/prakhar/ruff/crates/ruff_graph) Compiling ruff_workspace v0.0.0 (/Users/prakhar/ruff/crates/ruff_workspace) Compiling ruff_server v0.2.2 (/Users/prakhar/ruff/crates/ruff_server) Finished `dev` profile [unoptimized + debuginfo] target(s) in 7.86s Running `target/debug/ruff check ../efax/ --target-version py312 --no-cache` All checks passed! ``` --- ### Manual testing 2 Ran the check on the following script (mainly to verify `UP047`): ```py from __future__ import annotations from typing import Generic from typing_extensions import TypeVar T = TypeVar("T", default=int) def generic_function(var: T) -> T: return var Q = TypeVar("Q", default=str) class GenericClass(Generic[Q]): var: Q ``` On `main` branch: > ruff % cargo run -p ruff -- check ~/up046.py --target-version py312 --preview --no-cache
Output ```zsh Compiling ruff_linter v0.14.1 (/Users/prakhar/ruff/crates/ruff_linter) Compiling ruff v0.14.1 (/Users/prakhar/ruff/crates/ruff) Compiling ruff_graph v0.1.0 (/Users/prakhar/ruff/crates/ruff_graph) Compiling ruff_workspace v0.0.0 (/Users/prakhar/ruff/crates/ruff_workspace) Compiling ruff_server v0.2.2 (/Users/prakhar/ruff/crates/ruff_server) Finished `dev` profile [unoptimized + debuginfo] target(s) in 7.43s Running `target/debug/ruff check /Users/prakhar/up046.py --target-version py312 --preview --no-cache` UP047 Generic function `generic_function` should use type parameters --> /Users/prakhar/up046.py:10:5 | 10 | def generic_function(var: T) -> T: | ^^^^^^^^^^^^^^^^^^^^^^^^ 11 | return var | help: Use type parameters UP046 Generic class `GenericClass` uses `Generic` subclass instead of type parameters --> /Users/prakhar/up046.py:17:20 | 17 | class GenericClass(Generic[Q]): | ^^^^^^^^^^ 18 | var: Q | help: Use type parameters Found 2 errors. No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option). ```
After the fix (this branch): ```zsh ruff % cargo run -p ruff -- check ~/up046.py --target-version py312 --preview --no-cache Compiling ruff_linter v0.14.1 (/Users/prakhar/ruff/crates/ruff_linter) Compiling ruff v0.14.1 (/Users/prakhar/ruff/crates/ruff) Compiling ruff_graph v0.1.0 (/Users/prakhar/ruff/crates/ruff_graph) Compiling ruff_workspace v0.0.0 (/Users/prakhar/ruff/crates/ruff_workspace) Compiling ruff_server v0.2.2 (/Users/prakhar/ruff/crates/ruff_server) Finished `dev` profile [unoptimized + debuginfo] target(s) in 7.40s Running `target/debug/ruff check /Users/prakhar/up046.py --target-version py312 --preview --no-cache` All checks passed! ``` Signed-off-by: Prakhar Pratyush --- .../test/fixtures/pyupgrade/UP046_2.py | 13 ++++++++++++ .../pyupgrade/{UP047.py => UP047_0.py} | 0 .../test/fixtures/pyupgrade/UP047_1.py | 12 +++++++++++ crates/ruff_linter/src/rules/pyupgrade/mod.rs | 20 +++++++++++++++++-- .../src/rules/pyupgrade/rules/pep695/mod.rs | 14 ++++++++----- ...__rules__pyupgrade__tests__UP046_2.py.snap | 4 ++++ ..._rules__pyupgrade__tests__UP047_0.py.snap} | 12 +++++------ ...ade__tests__UP047_0.py__preview_diff.snap} | 2 +- ...__rules__pyupgrade__tests__UP047_1.py.snap | 4 ++++ 9 files changed, 67 insertions(+), 14 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_2.py rename crates/ruff_linter/resources/test/fixtures/pyupgrade/{UP047.py => UP047_0.py} (100%) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_1.py create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP046_2.py.snap rename crates/ruff_linter/src/rules/pyupgrade/snapshots/{ruff_linter__rules__pyupgrade__tests__UP047.py.snap => ruff_linter__rules__pyupgrade__tests__UP047_0.py.snap} (95%) rename crates/ruff_linter/src/rules/pyupgrade/snapshots/{ruff_linter__rules__pyupgrade__tests__UP047.py__preview_diff.snap => ruff_linter__rules__pyupgrade__tests__UP047_0.py__preview_diff.snap} (96%) create mode 100644 crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_1.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_2.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_2.py new file mode 100644 index 0000000000..aab7dce4d3 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_2.py @@ -0,0 +1,13 @@ +"""This is placed in a separate fixture as `TypeVar` needs to be imported +from `typing_extensions` to support default arguments in Python version < 3.13. +We verify that UP046 doesn't apply in this case. +""" + +from typing import Generic +from typing_extensions import TypeVar + +T = TypeVar("T", default=str) + + +class DefaultTypeVar(Generic[T]): + var: T diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_0.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047.py rename to crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_0.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_1.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_1.py new file mode 100644 index 0000000000..186c8305ef --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047_1.py @@ -0,0 +1,12 @@ +"""This is placed in a separate fixture as `TypeVar` needs to be imported +from `typing_extensions` to support default arguments in Python version < 3.13. +We verify that UP047 doesn't apply in this case. +""" + +from typing_extensions import TypeVar + +T = TypeVar("T", default=int) + + +def default_var(var: T) -> T: + return var diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index 044c90b3a2..c8919fe1b0 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -111,7 +111,7 @@ mod tests { #[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.pyi"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))] - #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))] + #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047_0.py"))] #[test_case(Rule::PrivateTypeParameter, Path::new("UP049_0.py"))] #[test_case(Rule::PrivateTypeParameter, Path::new("UP049_1.py"))] #[test_case(Rule::UselessClassMetaclassType, Path::new("UP050.py"))] @@ -125,6 +125,22 @@ mod tests { Ok(()) } + #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_2.py"))] + #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047_1.py"))] + fn rules_not_applied_default_typevar_backported(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = path.to_string_lossy().to_string(); + let diagnostics = test_path( + Path::new("pyupgrade").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + unresolved_target_version: PythonVersion::PY312.into(), + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_diagnostics!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))] #[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))] fn rules_preview(rule_code: Rule, path: &Path) -> Result<()> { @@ -144,7 +160,7 @@ mod tests { #[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.pyi"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))] #[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))] - #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))] + #[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047_0.py"))] fn type_var_default_preview(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}__preview_diff", path.to_string_lossy()); assert_diagnostics_diff!( diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs index c633c5b4ee..10f95ca180 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs @@ -6,8 +6,8 @@ use std::fmt::Display; use itertools::Itertools; use ruff_python_ast::{ - self as ast, Arguments, Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAssign, - TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, + self as ast, Arguments, Expr, ExprCall, ExprName, ExprSubscript, Identifier, PythonVersion, + Stmt, StmtAssign, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, name::Name, visitor::{self, Visitor}, }; @@ -369,15 +369,19 @@ fn in_nested_context(checker: &Checker) -> bool { } /// Deduplicate `vars`, returning `None` if `vars` is empty or any duplicates are found. -/// Also returns `None` if any `TypeVar` has a default value and preview mode is not enabled. +/// Also returns `None` if any `TypeVar` has a default value and the target Python version +/// is below 3.13 or preview mode is not enabled. Note that `typing_extensions` backports +/// the default argument, but the rule should be skipped in that case. fn check_type_vars<'a>(vars: Vec>, checker: &Checker) -> Option>> { if vars.is_empty() { return None; } - // If any type variables have defaults and preview mode is not enabled, skip the rule + // If any type variables have defaults, skip the rule unless + // running with preview mode enabled and targeting Python 3.13+. if vars.iter().any(|tv| tv.default.is_some()) - && !is_type_var_default_enabled(checker.settings()) + && (checker.target_version() < PythonVersion::PY313 + || !is_type_var_default_enabled(checker.settings())) { return None; } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP046_2.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP046_2.py.snap new file mode 100644 index 0000000000..2bacb5d540 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP046_2.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- + diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py.snap similarity index 95% rename from crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py.snap rename to crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py.snap index 9cbe736a7a..07a3869ecd 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py.snap @@ -2,7 +2,7 @@ source: crates/ruff_linter/src/rules/pyupgrade/mod.rs --- UP047 [*] Generic function `f` should use type parameters - --> UP047.py:12:5 + --> UP047_0.py:12:5 | 12 | def f(t: T) -> T: | ^^^^^^^ @@ -20,7 +20,7 @@ help: Use type parameters note: This is an unsafe fix and may change runtime behavior UP047 [*] Generic function `g` should use type parameters - --> UP047.py:16:5 + --> UP047_0.py:16:5 | 16 | def g(ts: tuple[*Ts]) -> tuple[*Ts]: | ^^^^^^^^^^^^^^^^^ @@ -38,7 +38,7 @@ help: Use type parameters note: This is an unsafe fix and may change runtime behavior UP047 [*] Generic function `h` should use type parameters - --> UP047.py:20:5 + --> UP047_0.py:20:5 | 20 | def h( | _____^ @@ -62,7 +62,7 @@ help: Use type parameters note: This is an unsafe fix and may change runtime behavior UP047 [*] Generic function `i` should use type parameters - --> UP047.py:29:5 + --> UP047_0.py:29:5 | 29 | def i(s: S) -> S: | ^^^^^^^ @@ -80,7 +80,7 @@ help: Use type parameters note: This is an unsafe fix and may change runtime behavior UP047 [*] Generic function `broken_fix` should use type parameters - --> UP047.py:39:5 + --> UP047_0.py:39:5 | 37 | # TypeVars with the new-style generic syntax and will be rejected by type 38 | # checkers @@ -100,7 +100,7 @@ help: Use type parameters note: This is an unsafe fix and may change runtime behavior UP047 [*] Generic function `any_str_param` should use type parameters - --> UP047.py:43:5 + --> UP047_0.py:43:5 | 43 | def any_str_param(s: AnyStr) -> AnyStr: | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py__preview_diff.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py__preview_diff.snap similarity index 96% rename from crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py__preview_diff.snap rename to crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py__preview_diff.snap index 2a11cd2e59..2d1ff52344 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047.py__preview_diff.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_0.py__preview_diff.snap @@ -11,7 +11,7 @@ Added: 1 --- Added --- UP047 [*] Generic function `default_var` should use type parameters - --> UP047.py:51:5 + --> UP047_0.py:51:5 | 51 | def default_var(v: V) -> V: | ^^^^^^^^^^^^^^^^^ diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_1.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_1.py.snap new file mode 100644 index 0000000000..2bacb5d540 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP047_1.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +