Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Foo:
T = typing.TypeVar(*args)
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)
x: typing.TypeAlias = list[T]

Expand Down Expand Up @@ -90,9 +90,9 @@ class Foo:
"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)
AnyList = TypeAliasType("AnyList", list[T], typep_params=(T,))
AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))

# unsafe fix if comments within the fix
T = TypeVar("T")
Expand Down Expand Up @@ -128,3 +128,7 @@ class Foo:
str # comment6
# comment7
) # 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]
10 changes: 9 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/pyupgrade/UP046_0.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,22 @@ def more_generic(u: U, t: T) -> tuple[U, T]:
return (u, t)


# TODO(brent) default requires 3.13
# default requires 3.13
V = TypeVar("V", default=Any, bound=str)


class DefaultTypeVar(Generic[V]): # -> [V: str = Any]
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
class Outer:
class Inner(Generic[T]):
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP047.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ def any_str_param(s: AnyStr) -> AnyStr:
return s


# these cases are not handled

# TODO(brent) default requires 3.13
# default requires 3.13
V = TypeVar("V", default=Any, bound=str)


def default_var(v: V) -> V:
return v


# these cases are not handled

def outer():
def inner(t: T) -> T:
return t
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ pub(crate) const fn is_refined_submodule_import_match_enabled(settings: &LinterS
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
pub(crate) const fn is_b006_check_guaranteed_mutable_expr_enabled(
settings: &LinterSettings,
Expand Down
24 changes: 23 additions & 1 deletion crates/ruff_linter/src/rules/pyupgrade/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ mod tests {
use crate::rules::{isort, pyupgrade};
use crate::settings::types::PreviewMode;
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::ConvertTypedDictFunctionalToClass, Path::new("UP013.py"))]
Expand Down Expand Up @@ -139,6 +139,28 @@ mod tests {
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_1.py"))]
#[test_case(Rule::QuotedAnnotation, Path::new("UP037_2.pyi"))]
Expand Down
127 changes: 65 additions & 62 deletions crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ use ruff_python_ast::{
use ruff_python_semantic::SemanticModel;
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_function::*;
pub(crate) use non_pep695_type_alias::*;
pub(crate) use private_type_parameter::*;

use crate::checkers::ast::Checker;

mod non_pep695_generic_class;
mod non_pep695_generic_function;
mod non_pep695_type_alias;
Expand Down Expand Up @@ -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(())
}
Expand All @@ -133,66 +138,63 @@ impl<'a> From<&'a TypeVar<'a>> for TypeParam {
name,
restriction,
kind,
default: _, // TODO(brent) see below
default,
}: &'a TypeVar<'a>,
) -> Self {
let default = default.map(|expr| Box::new(expr.clone()));
match kind {
TypeParamKind::TypeVar => {
TypeParam::TypeVar(TypeParamTypeVar {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()),
bound: match restriction {
Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())),
Some(TypeVarRestriction::Constraint(constraints)) => {
Some(Box::new(Expr::Tuple(ast::ExprTuple {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
elts: constraints.iter().map(|expr| (*expr).clone()).collect(),
ctx: ast::ExprContext::Load,
parenthesized: true,
})))
}
Some(TypeVarRestriction::AnyStr) => {
Some(Box::new(Expr::Tuple(ast::ExprTuple {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
elts: vec![
Expr::Name(ExprName {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
id: Name::from("str"),
ctx: ast::ExprContext::Load,
}),
Expr::Name(ExprName {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
id: Name::from("bytes"),
ctx: ast::ExprContext::Load,
}),
],
ctx: ast::ExprContext::Load,
parenthesized: true,
})))
}
None => None,
},
// We don't handle defaults here yet. Should perhaps be a different rule since
// defaults are only valid in 3.13+.
default: None,
})
}
TypeParamKind::TypeVar => TypeParam::TypeVar(TypeParamTypeVar {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()),
bound: match restriction {
Some(TypeVarRestriction::Bound(bound)) => Some(Box::new((*bound).clone())),
Some(TypeVarRestriction::Constraint(constraints)) => {
Some(Box::new(Expr::Tuple(ast::ExprTuple {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
elts: constraints.iter().map(|expr| (*expr).clone()).collect(),
ctx: ast::ExprContext::Load,
parenthesized: true,
})))
}
Some(TypeVarRestriction::AnyStr) => {
Some(Box::new(Expr::Tuple(ast::ExprTuple {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
elts: vec![
Expr::Name(ExprName {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
id: Name::from("str"),
ctx: ast::ExprContext::Load,
}),
Expr::Name(ExprName {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
id: Name::from("bytes"),
ctx: ast::ExprContext::Load,
}),
],
ctx: ast::ExprContext::Load,
parenthesized: true,
})))
}
None => None,
},
default,
}),
TypeParamKind::TypeVarTuple => TypeParam::TypeVarTuple(TypeParamTypeVarTuple {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()),
default: None,
default,
}),
TypeParamKind::ParamSpec => TypeParam::ParamSpec(TypeParamParamSpec {
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
name: Identifier::new(*name, TextRange::default()),
default: None,
default,
}),
}
}
Expand Down Expand Up @@ -318,8 +320,8 @@ pub(crate) fn expr_name_to_type_var<'a>(
.first()
.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
// generic type parameters before that
// `default` was added in PEP 696 and Python 3.13. We now support converting
// TypeVars with defaults to PEP 695 type parameters.
//
// ```python
// T = TypeVar("T", default=Any, bound=str)
Expand Down Expand Up @@ -367,21 +369,22 @@ fn in_nested_context(checker: &Checker) -> bool {
}

/// 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() {
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
// can't predict what the user wanted. also bail out if any Python 3.13+ default values are
// found on the type parameters
(vars
.iter()
.unique_by(|tvar| tvar.name)
.filter(|tvar| tvar.default.is_none())
.count()
== vars.len())
.then_some(vars)
// can't predict what the user wanted.
(vars.iter().unique_by(|tvar| tvar.name).count() == vars.len()).then_some(vars)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to preview gate this as I mentioned on the issue. I think this might be the easiest place to do that, assuming every usage goes through this code path.

}

/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`
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();
return;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use ruff_python_ast::{Expr, ExprCall, ExprName, Keyword, StmtAnnAssign, StmtAssi
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
use crate::preview::is_type_var_default_enabled;
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
use ruff_python_ast::PythonVersion;

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ UP040 [*] Type alias `x` uses `TypeAlias` annotation instead of the `type` keywo
44 | x: typing.TypeAlias = list[T]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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
41 |
Expand All @@ -226,7 +226,7 @@ help: Use the `type` keyword
- x: typing.TypeAlias = list[T]
44 + type x = list[T]
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)
note: This is an unsafe fix and may change runtime behavior

Expand Down Expand Up @@ -355,6 +355,26 @@ help: Use the `type` keyword
87 | # OK: Other name
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.py:99:1
|
Expand Down Expand Up @@ -469,6 +489,8 @@ UP040 [*] Type alias `T` uses `TypeAlias` annotation instead of the `type` keywo
129 | | # comment7
130 | | ) # comment8
| |_^
131 |
132 | # Test case for TypeVar with default - should be converted when preview mode is enabled
|
help: Use the `type` keyword
119 | | str
Expand Down
Loading