Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
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]
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ 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
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
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 @@ -469,6 +469,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