[`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660)

## Summary

Fixes #20656

---------

Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
Dan Parizher 2025-10-15 10:51:55 -04:00 committed by GitHub
parent 9e404a30c3
commit c06c3f9505
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 289 additions and 76 deletions

View File

@ -43,7 +43,7 @@ class Foo:
T = typing.TypeVar(*args) T = typing.TypeVar(*args)
x: typing.TypeAlias = list[T] x: typing.TypeAlias = list[T]
# `default` should be skipped for now, added in Python 3.13 # `default` was added in Python 3.13
T = typing.TypeVar("T", default=Any) T = typing.TypeVar("T", default=Any)
x: typing.TypeAlias = list[T] x: typing.TypeAlias = list[T]
@ -90,9 +90,9 @@ PositiveList = TypeAliasType(
"PositiveList2", list[Annotated[T, Gt(0)]], type_params=(T,) "PositiveList2", list[Annotated[T, Gt(0)]], type_params=(T,)
) )
# `default` should be skipped for now, added in Python 3.13 # `default` was added in Python 3.13
T = typing.TypeVar("T", default=Any) T = typing.TypeVar("T", default=Any)
AnyList = TypeAliasType("AnyList", list[T], typep_params=(T,)) AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
# unsafe fix if comments within the fix # unsafe fix if comments within the fix
T = TypeVar("T") T = TypeVar("T")
@ -128,3 +128,7 @@ T: TypeAlias = ( # comment0
str # comment6 str # comment6
# comment7 # comment7
) # comment8 ) # comment8
# Test case for TypeVar with default - should be converted when preview mode is enabled
T_default = TypeVar("T_default", default=int)
DefaultList: TypeAlias = list[T_default]

View File

@ -122,7 +122,7 @@ class MixedGenerics[U]:
return (u, t) return (u, t)
# TODO(brent) default requires 3.13 # default requires 3.13
V = TypeVar("V", default=Any, bound=str) V = TypeVar("V", default=Any, bound=str)
@ -130,6 +130,14 @@ class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
var: V var: V
# Test case for TypeVar with default but no bound
W = TypeVar("W", default=int)
class DefaultOnlyTypeVar(Generic[W]): # -> [W = int]
var: W
# nested classes and functions are skipped # nested classes and functions are skipped
class Outer: class Outer:
class Inner(Generic[T]): class Inner(Generic[T]):

View File

@ -44,9 +44,7 @@ def any_str_param(s: AnyStr) -> AnyStr:
return s return s
# these cases are not handled # default requires 3.13
# TODO(brent) default requires 3.13
V = TypeVar("V", default=Any, bound=str) V = TypeVar("V", default=Any, bound=str)
@ -54,6 +52,8 @@ def default_var(v: V) -> V:
return v return v
# these cases are not handled
def outer(): def outer():
def inner(t: T) -> T: def inner(t: T) -> T:
return t return t

View File

@ -242,6 +242,11 @@ pub(crate) const fn is_refined_submodule_import_match_enabled(settings: &LinterS
settings.preview.is_enabled() settings.preview.is_enabled()
} }
// https://github.com/astral-sh/ruff/pull/20660
pub(crate) const fn is_type_var_default_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}
// github.com/astral-sh/ruff/issues/20004 // github.com/astral-sh/ruff/issues/20004
pub(crate) const fn is_b006_check_guaranteed_mutable_expr_enabled( pub(crate) const fn is_b006_check_guaranteed_mutable_expr_enabled(
settings: &LinterSettings, settings: &LinterSettings,

View File

@ -19,7 +19,7 @@ mod tests {
use crate::rules::{isort, pyupgrade}; use crate::rules::{isort, pyupgrade};
use crate::settings::types::PreviewMode; use crate::settings::types::PreviewMode;
use crate::test::{test_path, test_snippet}; use crate::test::{test_path, test_snippet};
use crate::{assert_diagnostics, settings}; use crate::{assert_diagnostics, assert_diagnostics_diff, settings};
#[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))] #[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))]
#[test_case(Rule::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))] #[test_case(Rule::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))]
@ -140,6 +140,28 @@ mod tests {
Ok(()) 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"))]
#[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))]
#[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))]
fn type_var_default_preview(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}__preview_diff", path.to_string_lossy());
assert_diagnostics_diff!(
snapshot,
Path::new("pyupgrade").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Disabled,
..settings::LinterSettings::for_rule(rule_code)
},
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
);
Ok(())
}
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_0.py"))] #[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_1.py"))]
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))] #[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))]

View File

@ -14,13 +14,14 @@ use ruff_python_ast::{
use ruff_python_semantic::SemanticModel; use ruff_python_semantic::SemanticModel;
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::preview::is_type_var_default_enabled;
pub(crate) use non_pep695_generic_class::*; pub(crate) use non_pep695_generic_class::*;
pub(crate) use non_pep695_generic_function::*; pub(crate) use non_pep695_generic_function::*;
pub(crate) use non_pep695_type_alias::*; pub(crate) use non_pep695_type_alias::*;
pub(crate) use private_type_parameter::*; pub(crate) use private_type_parameter::*;
use crate::checkers::ast::Checker;
mod non_pep695_generic_class; mod non_pep695_generic_class;
mod non_pep695_generic_function; mod non_pep695_generic_function;
mod non_pep695_type_alias; mod non_pep695_type_alias;
@ -122,6 +123,10 @@ impl Display for DisplayTypeVar<'_> {
} }
} }
} }
if let Some(default) = self.type_var.default {
f.write_str(" = ")?;
f.write_str(&self.source[default.range()])?;
}
Ok(()) Ok(())
} }
@ -133,66 +138,63 @@ impl<'a> From<&'a TypeVar<'a>> for TypeParam {
name, name,
restriction, restriction,
kind, kind,
default: _, // TODO(brent) see below default,
}: &'a TypeVar<'a>, }: &'a TypeVar<'a>,
) -> Self { ) -> Self {
let default = default.map(|expr| Box::new(expr.clone()));
match kind { match kind {
TypeParamKind::TypeVar => { TypeParamKind::TypeVar => TypeParam::TypeVar(TypeParamTypeVar {
TypeParam::TypeVar(TypeParamTypeVar { range: TextRange::default(),
range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE,
node_index: ruff_python_ast::AtomicNodeIndex::NONE, name: Identifier::new(*name, TextRange::default()),
name: Identifier::new(*name, TextRange::default()), bound: match restriction {
bound: match restriction { Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())),
Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())), Some(TypeVarRestriction::Constraint(constraints)) => {
Some(TypeVarRestriction::Constraint(constraints)) => { Some(Box::new(Expr::Tuple(ast::ExprTuple {
Some(Box::new(Expr::Tuple(ast::ExprTuple { range: TextRange::default(),
range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE,
node_index: ruff_python_ast::AtomicNodeIndex::NONE, elts: constraints.iter().map(|expr| (*expr).clone()).collect(),
elts: constraints.iter().map(|expr| (*expr).clone()).collect(), ctx: ast::ExprContext::Load,
ctx: ast::ExprContext::Load, parenthesized: true,
parenthesized: true, })))
}))) }
} Some(TypeVarRestriction::AnyStr) => {
Some(TypeVarRestriction::AnyStr) => { Some(Box::new(Expr::Tuple(ast::ExprTuple {
Some(Box::new(Expr::Tuple(ast::ExprTuple { range: TextRange::default(),
range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE,
node_index: ruff_python_ast::AtomicNodeIndex::NONE, elts: vec![
elts: vec![ Expr::Name(ExprName {
Expr::Name(ExprName { range: TextRange::default(),
range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE,
node_index: ruff_python_ast::AtomicNodeIndex::NONE, id: Name::from("str"),
id: Name::from("str"), ctx: ast::ExprContext::Load,
ctx: ast::ExprContext::Load, }),
}), Expr::Name(ExprName {
Expr::Name(ExprName { range: TextRange::default(),
range: TextRange::default(), node_index: ruff_python_ast::AtomicNodeIndex::NONE,
node_index: ruff_python_ast::AtomicNodeIndex::NONE, id: Name::from("bytes"),
id: Name::from("bytes"), ctx: ast::ExprContext::Load,
ctx: ast::ExprContext::Load, }),
}), ],
], ctx: ast::ExprContext::Load,
ctx: ast::ExprContext::Load, parenthesized: true,
parenthesized: true, })))
}))) }
} None => None,
None => None, },
}, default,
// We don't handle defaults here yet. Should perhaps be a different rule since }),
// defaults are only valid in 3.13+.
default: None,
})
}
TypeParamKind::TypeVarTuple => TypeParam::TypeVarTuple(TypeParamTypeVarTuple { TypeParamKind::TypeVarTuple => TypeParam::TypeVarTuple(TypeParamTypeVarTuple {
range: TextRange::default(), range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE, node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()), name: Identifier::new(*name, TextRange::default()),
default: None, default,
}), }),
TypeParamKind::ParamSpec => TypeParam::ParamSpec(TypeParamParamSpec { TypeParamKind::ParamSpec => TypeParam::ParamSpec(TypeParamParamSpec {
range: TextRange::default(), range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE, node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()), name: Identifier::new(*name, TextRange::default()),
default: None, default,
}), }),
} }
} }
@ -318,8 +320,8 @@ pub(crate) fn expr_name_to_type_var<'a>(
.first() .first()
.is_some_and(Expr::is_string_literal_expr) .is_some_and(Expr::is_string_literal_expr)
{ {
// TODO(brent) `default` was added in PEP 696 and Python 3.13 but can't be used in // `default` was added in PEP 696 and Python 3.13. We now support converting
// generic type parameters before that // TypeVars with defaults to PEP 695 type parameters.
// //
// ```python // ```python
// T = TypeVar("T", default=Any, bound=str) // T = TypeVar("T", default=Any, bound=str)
@ -367,21 +369,22 @@ fn in_nested_context(checker: &Checker) -> bool {
} }
/// Deduplicate `vars`, returning `None` if `vars` is empty or any duplicates are found. /// Deduplicate `vars`, returning `None` if `vars` is empty or any duplicates are found.
fn check_type_vars(vars: Vec<TypeVar<'_>>) -> Option<Vec<TypeVar<'_>>> { /// Also returns `None` if any `TypeVar` has a default value and preview mode is not enabled.
fn check_type_vars<'a>(vars: Vec<TypeVar<'a>>, checker: &Checker) -> Option<Vec<TypeVar<'a>>> {
if vars.is_empty() { if vars.is_empty() {
return None; return None;
} }
// If any type variables have defaults and preview mode is not enabled, skip the rule
if vars.iter().any(|tv| tv.default.is_some())
&& !is_type_var_default_enabled(checker.settings())
{
return None;
}
// If any type variables were not unique, just bail out here. this is a runtime error and we // If any type variables were not unique, just bail out here. this is a runtime error and we
// can't predict what the user wanted. also bail out if any Python 3.13+ default values are // can't predict what the user wanted.
// found on the type parameters (vars.iter().unique_by(|tvar| tvar.name).count() == vars.len()).then_some(vars)
(vars
.iter()
.unique_by(|tvar| tvar.name)
.filter(|tvar| tvar.default.is_none())
.count()
== vars.len())
.then_some(vars)
} }
/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if /// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if

View File

@ -186,7 +186,7 @@ pub(crate) fn non_pep695_generic_class(checker: &Checker, class_def: &StmtClassD
// //
// just because we can't confirm that `SomethingElse` is a `TypeVar` // just because we can't confirm that `SomethingElse` is a `TypeVar`
if !visitor.any_skipped { if !visitor.any_skipped {
let Some(type_vars) = check_type_vars(visitor.vars) else { let Some(type_vars) = check_type_vars(visitor.vars, checker) else {
diagnostic.defuse(); diagnostic.defuse();
return; return;
}; };

View File

@ -154,7 +154,7 @@ pub(crate) fn non_pep695_generic_function(checker: &Checker, function_def: &Stmt
} }
} }
let Some(type_vars) = check_type_vars(type_vars) else { let Some(type_vars) = check_type_vars(type_vars, checker) else {
return; return;
}; };

View File

@ -8,6 +8,7 @@ use ruff_python_ast::{Expr, ExprCall, ExprName, Keyword, StmtAnnAssign, StmtAssi
use ruff_text_size::{Ranged, TextRange}; use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::preview::is_type_var_default_enabled;
use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
use ruff_python_ast::PythonVersion; use ruff_python_ast::PythonVersion;
@ -232,8 +233,10 @@ pub(crate) fn non_pep695_type_alias(checker: &Checker, stmt: &StmtAnnAssign) {
.unique_by(|tvar| tvar.name) .unique_by(|tvar| tvar.name)
.collect::<Vec<_>>(); .collect::<Vec<_>>();
// TODO(brent) handle `default` arg for Python 3.13+ // Skip if any TypeVar has defaults and preview mode is not enabled
if vars.iter().any(|tv| tv.default.is_some()) { if vars.iter().any(|tv| tv.default.is_some())
&& !is_type_var_default_enabled(checker.settings())
{
return; return;
} }

View File

@ -217,7 +217,7 @@ UP040 [*] Type alias `x` uses `TypeAlias` annotation instead of the `type` keywo
44 | x: typing.TypeAlias = list[T] 44 | x: typing.TypeAlias = list[T]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
45 | 45 |
46 | # `default` should be skipped for now, added in Python 3.13 46 | # `default` was added in Python 3.13
| |
help: Use the `type` keyword help: Use the `type` keyword
41 | 41 |
@ -226,7 +226,7 @@ help: Use the `type` keyword
- x: typing.TypeAlias = list[T] - x: typing.TypeAlias = list[T]
44 + type x = list[T] 44 + type x = list[T]
45 | 45 |
46 | # `default` should be skipped for now, added in Python 3.13 46 | # `default` was added in Python 3.13
47 | T = typing.TypeVar("T", default=Any) 47 | T = typing.TypeVar("T", default=Any)
note: This is an unsafe fix and may change runtime behavior note: This is an unsafe fix and may change runtime behavior
@ -355,6 +355,26 @@ help: Use the `type` keyword
87 | # OK: Other name 87 | # OK: Other name
88 | T = TypeVar("T", bound=SupportGt) 88 | T = TypeVar("T", bound=SupportGt)
UP040 [*] Type alias `AnyList` uses `TypeAliasType` assignment instead of the `type` keyword
--> UP040.py:95:1
|
93 | # `default` was added in Python 3.13
94 | T = typing.TypeVar("T", default=Any)
95 | AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
96 |
97 | # unsafe fix if comments within the fix
|
help: Use the `type` keyword
92 |
93 | # `default` was added in Python 3.13
94 | T = typing.TypeVar("T", default=Any)
- AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
95 + type AnyList[T = Any] = list[T]
96 |
97 | # unsafe fix if comments within the fix
98 | T = TypeVar("T")
UP040 [*] Type alias `PositiveList` uses `TypeAliasType` assignment instead of the `type` keyword UP040 [*] Type alias `PositiveList` uses `TypeAliasType` assignment instead of the `type` keyword
--> UP040.py:99:1 --> UP040.py:99:1
| |
@ -469,6 +489,8 @@ UP040 [*] Type alias `T` uses `TypeAlias` annotation instead of the `type` keywo
129 | | # comment7 129 | | # comment7
130 | | ) # comment8 130 | | ) # comment8
| |_^ | |_^
131 |
132 | # Test case for TypeVar with default - should be converted when preview mode is enabled
| |
help: Use the `type` keyword help: Use the `type` keyword
119 | | str 119 | | str

View File

@ -0,0 +1,49 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
--- Linter settings ---
-linter.preview = disabled
+linter.preview = enabled
--- Summary ---
Removed: 0
Added: 2
--- Added ---
UP040 [*] Type alias `x` uses `TypeAlias` annotation instead of the `type` keyword
--> UP040.py:48:1
|
46 | # `default` was added in Python 3.13
47 | T = typing.TypeVar("T", default=Any)
48 | x: typing.TypeAlias = list[T]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
49 |
50 | # OK
|
help: Use the `type` keyword
45 |
46 | # `default` was added in Python 3.13
47 | T = typing.TypeVar("T", default=Any)
- x: typing.TypeAlias = list[T]
48 + type x[T = Any] = list[T]
49 |
50 | # OK
51 | x: TypeAlias
note: This is an unsafe fix and may change runtime behavior
UP040 [*] Type alias `DefaultList` uses `TypeAlias` annotation instead of the `type` keyword
--> UP040.py:134:1
|
132 | # Test case for TypeVar with default - should be converted when preview mode is enabled
133 | T_default = TypeVar("T_default", default=int)
134 | DefaultList: TypeAlias = list[T_default]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: Use the `type` keyword
131 |
132 | # Test case for TypeVar with default - should be converted when preview mode is enabled
133 | T_default = TypeVar("T_default", default=int)
- DefaultList: TypeAlias = list[T_default]
134 + type DefaultList[T_default = int] = list[T_default]
note: This is an unsafe fix and may change runtime behavior

View File

@ -0,0 +1,10 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
--- Linter settings ---
-linter.preview = disabled
+linter.preview = enabled
--- Summary ---
Removed: 0
Added: 0

View File

@ -0,0 +1,48 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
--- Linter settings ---
-linter.preview = disabled
+linter.preview = enabled
--- Summary ---
Removed: 0
Added: 2
--- Added ---
UP046 [*] Generic class `DefaultTypeVar` uses `Generic` subclass instead of type parameters
--> UP046_0.py:129:22
|
129 | class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
| ^^^^^^^^^^
130 | var: V
|
help: Use type parameters
126 | V = TypeVar("V", default=Any, bound=str)
127 |
128 |
- class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
129 + class DefaultTypeVar[V: str = Any]: # -> [V: str = Any]
130 | var: V
131 |
132 |
note: This is an unsafe fix and may change runtime behavior
UP046 [*] Generic class `DefaultOnlyTypeVar` uses `Generic` subclass instead of type parameters
--> UP046_0.py:137:26
|
137 | class DefaultOnlyTypeVar(Generic[W]): # -> [W = int]
| ^^^^^^^^^^
138 | var: W
|
help: Use type parameters
134 | W = TypeVar("W", default=int)
135 |
136 |
- class DefaultOnlyTypeVar(Generic[W]): # -> [W = int]
137 + class DefaultOnlyTypeVar[W = int]: # -> [W = int]
138 | var: W
139 |
140 |
note: This is an unsafe fix and may change runtime behavior

View File

@ -0,0 +1,10 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
--- Linter settings ---
-linter.preview = disabled
+linter.preview = enabled
--- Summary ---
Removed: 0
Added: 0

View File

@ -0,0 +1,29 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
--- Linter settings ---
-linter.preview = disabled
+linter.preview = enabled
--- Summary ---
Removed: 0
Added: 1
--- Added ---
UP047 [*] Generic function `default_var` should use type parameters
--> UP047.py:51:5
|
51 | def default_var(v: V) -> V:
| ^^^^^^^^^^^^^^^^^
52 | return v
|
help: Use type parameters
48 | V = TypeVar("V", default=Any, bound=str)
49 |
50 |
- def default_var(v: V) -> V:
51 + def default_var[V: str = Any](v: V) -> V:
52 | return v
53 |
54 |
note: This is an unsafe fix and may change runtime behavior