Skip to content

Commit c06c3f9

Browse files
danparizherntBre
danparizher
and
ntBre
authored
[pyupgrade] Fix false negative for TypeVar with default argument in non-pep695-generic-class (UP046) (#20660)
## Summary Fixes #20656 --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent 9e404a3 commit c06c3f9

15 files changed

+289
-76
lines changed

‎crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class Foo:
4343
T = typing.TypeVar(*args)
4444
x: typing.TypeAlias = list[T]
4545

46-
# `default` should be skipped for now, added in Python 3.13
46+
# `default` was added in Python 3.13
4747
T = typing.TypeVar("T", default=Any)
4848
x: typing.TypeAlias = list[T]
4949

@@ -90,9 +90,9 @@ class Foo:
9090
"PositiveList2", list[Annotated[T, Gt(0)]], type_params=(T,)
9191
)
9292

93-
# `default` should be skipped for now, added in Python 3.13
93+
# `default` was added in Python 3.13
9494
T = typing.TypeVar("T", default=Any)
95-
AnyList = TypeAliasType("AnyList", list[T], typep_params=(T,))
95+
AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
9696

9797
# unsafe fix if comments within the fix
9898
T = TypeVar("T")
@@ -128,3 +128,7 @@ class Foo:
128128
str # comment6
129129
# comment7
130130
) # comment8
131+
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]

‎crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_0.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,22 @@ def more_generic(u: U, t: T) -> tuple[U, T]:
122122
return (u, t)
123123

124124

125-
# TODO(brent) default requires 3.13
125+
# default requires 3.13
126126
V = TypeVar("V", default=Any, bound=str)
127127

128128

129129
class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
130130
var: V
131131

132132

133+
# Test case for TypeVar with default but no bound
134+
W = TypeVar("W", default=int)
135+
136+
137+
class DefaultOnlyTypeVar(Generic[W]): # -> [W = int]
138+
var: W
139+
140+
133141
# nested classes and functions are skipped
134142
class Outer:
135143
class Inner(Generic[T]):

‎crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,16 @@ def any_str_param(s: AnyStr) -> AnyStr:
4444
return s
4545

4646

47-
# these cases are not handled
48-
49-
# TODO(brent) default requires 3.13
47+
# default requires 3.13
5048
V = TypeVar("V", default=Any, bound=str)
5149

5250

5351
def default_var(v: V) -> V:
5452
return v
5553

5654

55+
# these cases are not handled
56+
5757
def outer():
5858
def inner(t: T) -> T:
5959
return t

‎crates/ruff_linter/src/preview.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,11 @@ pub(crate) const fn is_refined_submodule_import_match_enabled(settings: &LinterS
242242
settings.preview.is_enabled()
243243
}
244244

245+
// https://github.com/astral-sh/ruff/pull/20660
246+
pub(crate) const fn is_type_var_default_enabled(settings: &LinterSettings) -> bool {
247+
settings.preview.is_enabled()
248+
}
249+
245250
// github.com/astral-sh/ruff/issues/20004
246251
pub(crate) const fn is_b006_check_guaranteed_mutable_expr_enabled(
247252
settings: &LinterSettings,

‎crates/ruff_linter/src/rules/pyupgrade/mod.rs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ mod tests {
1919
use crate::rules::{isort, pyupgrade};
2020
use crate::settings::types::PreviewMode;
2121
use crate::test::{test_path, test_snippet};
22-
use crate::{assert_diagnostics, settings};
22+
use crate::{assert_diagnostics, assert_diagnostics_diff, settings};
2323

2424
#[test_case(Rule::ConvertNamedTupleFunctionalToClass, Path::new("UP014.py"))]
2525
#[test_case(Rule::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))]
@@ -140,6 +140,28 @@ mod tests {
140140
Ok(())
141141
}
142142

143+
#[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.py"))]
144+
#[test_case(Rule::NonPEP695TypeAlias, Path::new("UP040.pyi"))]
145+
#[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_0.py"))]
146+
#[test_case(Rule::NonPEP695GenericClass, Path::new("UP046_1.py"))]
147+
#[test_case(Rule::NonPEP695GenericFunction, Path::new("UP047.py"))]
148+
fn type_var_default_preview(rule_code: Rule, path: &Path) -> Result<()> {
149+
let snapshot = format!("{}__preview_diff", path.to_string_lossy());
150+
assert_diagnostics_diff!(
151+
snapshot,
152+
Path::new("pyupgrade").join(path).as_path(),
153+
&settings::LinterSettings {
154+
preview: PreviewMode::Disabled,
155+
..settings::LinterSettings::for_rule(rule_code)
156+
},
157+
&settings::LinterSettings {
158+
preview: PreviewMode::Enabled,
159+
..settings::LinterSettings::for_rule(rule_code)
160+
},
161+
);
162+
Ok(())
163+
}
164+
143165
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_0.py"))]
144166
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_1.py"))]
145167
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))]

‎crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs‎

Lines changed: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ use ruff_python_ast::{
1414
use ruff_python_semantic::SemanticModel;
1515
use ruff_text_size::{Ranged, TextRange};
1616

17+
use crate::checkers::ast::Checker;
18+
use crate::preview::is_type_var_default_enabled;
19+
1720
pub(crate) use non_pep695_generic_class::*;
1821
pub(crate) use non_pep695_generic_function::*;
1922
pub(crate) use non_pep695_type_alias::*;
2023
pub(crate) use private_type_parameter::*;
2124

22-
use crate::checkers::ast::Checker;
23-
2425
mod non_pep695_generic_class;
2526
mod non_pep695_generic_function;
2627
mod non_pep695_type_alias;
@@ -122,6 +123,10 @@ impl Display for DisplayTypeVar<'_> {
122123
}
123124
}
124125
}
126+
if let Some(default) = self.type_var.default {
127+
f.write_str(" = ")?;
128+
f.write_str(&self.source[default.range()])?;
129+
}
125130

126131
Ok(())
127132
}
@@ -133,66 +138,63 @@ impl<'a> From<&'a TypeVar<'a>> for TypeParam {
133138
name,
134139
restriction,
135140
kind,
136-
default: _, // TODO(brent) see below
141+
default,
137142
}: &'a TypeVar<'a>,
138143
) -> Self {
144+
let default = default.map(|expr| Box::new(expr.clone()));
139145
match kind {
140-
TypeParamKind::TypeVar => {
141-
TypeParam::TypeVar(TypeParamTypeVar {
142-
range: TextRange::default(),
143-
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
144-
name: Identifier::new(*name, TextRange::default()),
145-
bound: match restriction {
146-
Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())),
147-
Some(TypeVarRestriction::Constraint(constraints)) => {
148-
Some(Box::new(Expr::Tuple(ast::ExprTuple {
149-
range: TextRange::default(),
150-
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
151-
elts: constraints.iter().map(|expr| (*expr).clone()).collect(),
152-
ctx: ast::ExprContext::Load,
153-
parenthesized: true,
154-
})))
155-
}
156-
Some(TypeVarRestriction::AnyStr) => {
157-
Some(Box::new(Expr::Tuple(ast::ExprTuple {
158-
range: TextRange::default(),
159-
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
160-
elts: vec![
161-
Expr::Name(ExprName {
162-
range: TextRange::default(),
163-
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
164-
id: Name::from("str"),
165-
ctx: ast::ExprContext::Load,
166-
}),
167-
Expr::Name(ExprName {
168-
range: TextRange::default(),
169-
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
170-
id: Name::from("bytes"),
171-
ctx: ast::ExprContext::Load,
172-
}),
173-
],
174-
ctx: ast::ExprContext::Load,
175-
parenthesized: true,
176-
})))
177-
}
178-
None => None,
179-
},
180-
// We don't handle defaults here yet. Should perhaps be a different rule since
181-
// defaults are only valid in 3.13+.
182-
default: None,
183-
})
184-
}
146+
TypeParamKind::TypeVar => TypeParam::TypeVar(TypeParamTypeVar {
147+
range: TextRange::default(),
148+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
149+
name: Identifier::new(*name, TextRange::default()),
150+
bound: match restriction {
151+
Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())),
152+
Some(TypeVarRestriction::Constraint(constraints)) => {
153+
Some(Box::new(Expr::Tuple(ast::ExprTuple {
154+
range: TextRange::default(),
155+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
156+
elts: constraints.iter().map(|expr| (*expr).clone()).collect(),
157+
ctx: ast::ExprContext::Load,
158+
parenthesized: true,
159+
})))
160+
}
161+
Some(TypeVarRestriction::AnyStr) => {
162+
Some(Box::new(Expr::Tuple(ast::ExprTuple {
163+
range: TextRange::default(),
164+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
165+
elts: vec![
166+
Expr::Name(ExprName {
167+
range: TextRange::default(),
168+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
169+
id: Name::from("str"),
170+
ctx: ast::ExprContext::Load,
171+
}),
172+
Expr::Name(ExprName {
173+
range: TextRange::default(),
174+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
175+
id: Name::from("bytes"),
176+
ctx: ast::ExprContext::Load,
177+
}),
178+
],
179+
ctx: ast::ExprContext::Load,
180+
parenthesized: true,
181+
})))
182+
}
183+
None => None,
184+
},
185+
default,
186+
}),
185187
TypeParamKind::TypeVarTuple => TypeParam::TypeVarTuple(TypeParamTypeVarTuple {
186188
range: TextRange::default(),
187189
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
188190
name: Identifier::new(*name, TextRange::default()),
189-
default: None,
191+
default,
190192
}),
191193
TypeParamKind::ParamSpec => TypeParam::ParamSpec(TypeParamParamSpec {
192194
range: TextRange::default(),
193195
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
194196
name: Identifier::new(*name, TextRange::default()),
195-
default: None,
197+
default,
196198
}),
197199
}
198200
}
@@ -318,8 +320,8 @@ pub(crate) fn expr_name_to_type_var<'a>(
318320
.first()
319321
.is_some_and(Expr::is_string_literal_expr)
320322
{
321-
// TODO(brent) `default` was added in PEP 696 and Python 3.13 but can't be used in
322-
// generic type parameters before that
323+
// `default` was added in PEP 696 and Python 3.13. We now support converting
324+
// TypeVars with defaults to PEP 695 type parameters.
323325
//
324326
// ```python
325327
// T = TypeVar("T", default=Any, bound=str)
@@ -367,21 +369,22 @@ fn in_nested_context(checker: &Checker) -> bool {
367369
}
368370

369371
/// Deduplicate `vars`, returning `None` if `vars` is empty or any duplicates are found.
370-
fn check_type_vars(vars: Vec<TypeVar<'_>>) -> Option<Vec<TypeVar<'_>>> {
372+
/// Also returns `None` if any `TypeVar` has a default value and preview mode is not enabled.
373+
fn check_type_vars<'a>(vars: Vec<TypeVar<'a>>, checker: &Checker) -> Option<Vec<TypeVar<'a>>> {
371374
if vars.is_empty() {
372375
return None;
373376
}
374377

378+
// If any type variables have defaults and preview mode is not enabled, skip the rule
379+
if vars.iter().any(|tv| tv.default.is_some())
380+
&& !is_type_var_default_enabled(checker.settings())
381+
{
382+
return None;
383+
}
384+
375385
// If any type variables were not unique, just bail out here. this is a runtime error and we
376-
// can't predict what the user wanted. also bail out if any Python 3.13+ default values are
377-
// found on the type parameters
378-
(vars
379-
.iter()
380-
.unique_by(|tvar| tvar.name)
381-
.filter(|tvar| tvar.default.is_none())
382-
.count()
383-
== vars.len())
384-
.then_some(vars)
386+
// can't predict what the user wanted.
387+
(vars.iter().unique_by(|tvar| tvar.name).count() == vars.len()).then_some(vars)
385388
}
386389

387390
/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if

‎crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_generic_class.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ pub(crate) fn non_pep695_generic_class(checker: &Checker, class_def: &StmtClassD
186186
//
187187
// just because we can't confirm that `SomethingElse` is a `TypeVar`
188188
if !visitor.any_skipped {
189-
let Some(type_vars) = check_type_vars(visitor.vars) else {
189+
let Some(type_vars) = check_type_vars(visitor.vars, checker) else {
190190
diagnostic.defuse();
191191
return;
192192
};

‎crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_generic_function.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ pub(crate) fn non_pep695_generic_function(checker: &Checker, function_def: &Stmt
154154
}
155155
}
156156

157-
let Some(type_vars) = check_type_vars(type_vars) else {
157+
let Some(type_vars) = check_type_vars(type_vars, checker) else {
158158
return;
159159
};
160160

‎crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_type_alias.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use ruff_python_ast::{Expr, ExprCall, ExprName, Keyword, StmtAnnAssign, StmtAssi
88
use ruff_text_size::{Ranged, TextRange};
99

1010
use crate::checkers::ast::Checker;
11+
use crate::preview::is_type_var_default_enabled;
1112
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
1213
use ruff_python_ast::PythonVersion;
1314

@@ -232,8 +233,10 @@ pub(crate) fn non_pep695_type_alias(checker: &Checker, stmt: &StmtAnnAssign) {
232233
.unique_by(|tvar| tvar.name)
233234
.collect::<Vec<_>>();
234235

235-
// TODO(brent) handle `default` arg for Python 3.13+
236-
if vars.iter().any(|tv| tv.default.is_some()) {
236+
// Skip if any TypeVar has defaults and preview mode is not enabled
237+
if vars.iter().any(|tv| tv.default.is_some())
238+
&& !is_type_var_default_enabled(checker.settings())
239+
{
237240
return;
238241
}
239242

‎crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP040.py.snap‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ UP040 [*] Type alias `x` uses `TypeAlias` annotation instead of the `type` keywo
217217
44 | x: typing.TypeAlias = list[T]
218218
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
219219
45 |
220-
46 | # `default` should be skipped for now, added in Python 3.13
220+
46 | # `default` was added in Python 3.13
221221
|
222222
help: Use the `type` keyword
223223
41 |
@@ -226,7 +226,7 @@ help: Use the `type` keyword
226226
- x: typing.TypeAlias = list[T]
227227
44 + type x = list[T]
228228
45 |
229-
46 | # `default` should be skipped for now, added in Python 3.13
229+
46 | # `default` was added in Python 3.13
230230
47 | T = typing.TypeVar("T", default=Any)
231231
note: This is an unsafe fix and may change runtime behavior
232232
@@ -355,6 +355,26 @@ help: Use the `type` keyword
355355
87 | # OK: Other name
356356
88 | T = TypeVar("T", bound=SupportGt)
357357
358+
UP040 [*] Type alias `AnyList` uses `TypeAliasType` assignment instead of the `type` keyword
359+
--> UP040.py:95:1
360+
|
361+
93 | # `default` was added in Python 3.13
362+
94 | T = typing.TypeVar("T", default=Any)
363+
95 | AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
364+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
365+
96 |
366+
97 | # unsafe fix if comments within the fix
367+
|
368+
help: Use the `type` keyword
369+
92 |
370+
93 | # `default` was added in Python 3.13
371+
94 | T = typing.TypeVar("T", default=Any)
372+
- AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
373+
95 + type AnyList[T = Any] = list[T]
374+
96 |
375+
97 | # unsafe fix if comments within the fix
376+
98 | T = TypeVar("T")
377+
358378
UP040 [*] Type alias `PositiveList` uses `TypeAliasType` assignment instead of the `type` keyword
359379
--> UP040.py:99:1
360380
|
@@ -469,6 +489,8 @@ UP040 [*] Type alias `T` uses `TypeAlias` annotation instead of the `type` keywo
469489
129 | | # comment7
470490
130 | | ) # comment8
471491
| |_^
492+
131 |
493+
132 | # Test case for TypeVar with default - should be converted when preview mode is enabled
472494
|
473495
help: Use the `type` keyword
474496
119 | | str

0 commit comments

Comments
 (0)