Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,55 @@ from typing_extensions import TypeAliasType, TypeVar
T = TypeVar("T", default=int)
Alias = TypeAliasType("Alias", list[T], type_params=(T,))
```

## `TypeVar` with unpacked keyword arguments

When a `TypeVar` uses unpacked keyword arguments (e.g. `**{"default": Any}`), the fix cannot
safely inline it into PEP 695 syntax and should not be offered.

```toml
target-version = "py314"
[lint]
preview = true
select = ["UP040", "UP046", "UP047"]
```

### `TypeAliasType` — no fix offered

```py
from typing import Any, TypeAliasType, TypeVar

T = TypeVar("T", **{"default": Any})
AnyList = TypeAliasType("AnyList", list[T], type_params=(T,))
```

### `TypeAlias` — no fix offered

```py
from typing import Any, TypeAlias, TypeVar

T = TypeVar("T", **{"default": Any})
Alias: TypeAlias = list[T]
```

### Mixed `TypeAlias` — only non-unpacked TypeVar converted

```py
from typing import Any, TypeAlias, TypeVar

T = TypeVar("T", **{"default": Any})
U = TypeVar("U")
MixedAlias: TypeAlias = tuple[T, U]
```

### Generic function — no fix offered

```py
from typing import Any, TypeVar

T = TypeVar("T", **{"default": Any})
U = TypeVar("U")

def f(first: T, second: U) -> tuple[T, U]:
return first, second
```
31 changes: 30 additions & 1 deletion crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ struct TypeVarReferenceVisitor<'a> {
/// Tracks whether any non-TypeVars have been seen to avoid replacing generic parameters when an
/// unknown `TypeVar` is encountered.
any_skipped: bool,
has_unpacked_kwargs: bool,
}

/// Recursively collects the names of type variable references present in an expression.
Expand Down Expand Up @@ -267,7 +268,9 @@ impl<'a> Visitor<'a> for TypeVarReferenceVisitor<'a> {

match expr {
Expr::Name(name) if name.ctx.is_load() => {
if let Some(var) = expr_name_to_type_var(self.semantic, name) {
if type_var_has_unpacked_kwargs(self.semantic, name) {
self.has_unpacked_kwargs = true;
} else if let Some(var) = expr_name_to_type_var(self.semantic, name) {
self.vars.push(var);
} else {
self.any_skipped = true;
Expand Down Expand Up @@ -358,6 +361,32 @@ pub(crate) fn expr_name_to_type_var<'a>(
None
}

/// Returns `true` if the name refers to a `TypeVar` with unpacked keyword arguments.
pub(crate) fn type_var_has_unpacked_kwargs(semantic: &SemanticModel, name: &ExprName) -> bool {
let Some(StmtAssign { value, .. }) = semantic
.lookup_symbol(name.id.as_str())
.binding_id()
.and_then(|binding_id| semantic.binding(binding_id).source)
.map(|node_id| semantic.statement(node_id))
.and_then(|stmt| stmt.as_assign_stmt())
else {
return false;
};

let Expr::Call(ExprCall {
func, arguments, ..
}) = value.as_ref()
else {
return false;
};

if !semantic.match_typing_expr(func, "TypeVar") {
return false;
}

arguments.keywords.iter().any(|kw| kw.arg.is_none())
}

/// Check if the current statement is nested within another [`StmtClassDef`] or [`StmtFunctionDef`].
fn in_nested_context(checker: &Checker) -> bool {
checker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ pub(crate) fn non_pep695_generic_class(checker: &Checker, class_def: &StmtClassD
vars: vec![],
semantic: checker.semantic(),
any_skipped: false,
has_unpacked_kwargs: false,
};
visitor.visit_expr(slice);

Expand All @@ -231,7 +232,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 {
if !visitor.any_skipped && !visitor.has_unpacked_kwargs {
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 @@ -146,20 +146,23 @@ pub(crate) fn non_pep695_generic_function(checker: &Checker, function_def: &Stmt
}

let mut type_vars = Vec::new();
let mut has_unpacked_kwargs = false;
for parameter in parameters {
if let Some(annotation) = parameter.annotation() {
let vars = {
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
any_skipped: false,
};
visitor.visit_expr(annotation);
visitor.vars
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
any_skipped: false,
has_unpacked_kwargs: false,
};
type_vars.extend(vars);
visitor.visit_expr(annotation);
has_unpacked_kwargs |= visitor.has_unpacked_kwargs;
type_vars.extend(visitor.vars);
}
}
if has_unpacked_kwargs {
return;
}

// Deduplicate type vars that appear in multiple parameter annotations
let mut seen = FxHashSet::default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
use ruff_python_ast::PythonVersion;

use super::{
DisplayTypeVars, TypeParamKind, TypeVar, TypeVarReferenceVisitor, expr_name_to_type_var,
non_default_follows_default,
DisplayTypeVars, TypeVar, TypeVarReferenceVisitor, expr_name_to_type_var,
non_default_follows_default, type_var_has_unpacked_kwargs,
};

/// ## What it does
Expand Down Expand Up @@ -168,17 +168,20 @@ pub(crate) fn non_pep695_type_alias_type(checker: &Checker, stmt: &StmtAssign) {
return;
}

// Bail out if any type parameter has unpacked keyword arguments that
// cannot be represented in PEP 695 syntax.
if type_params.iter().any(|expr| {
expr.as_name_expr()
.is_some_and(|name| type_var_has_unpacked_kwargs(checker.semantic(), name))
}) {
return;
}

let Some(vars) = type_params
.iter()
.map(|expr| {
expr.as_name_expr().map(|name| {
expr_name_to_type_var(checker.semantic(), name).unwrap_or(TypeVar {
name: &name.id,
restriction: None,
kind: TypeParamKind::TypeVar,
default: None,
})
})
expr.as_name_expr()
.and_then(|name| expr_name_to_type_var(checker.semantic(), name))
})
.collect::<Option<Vec<_>>>()
else {
Expand Down Expand Up @@ -223,15 +226,17 @@ pub(crate) fn non_pep695_type_alias(checker: &Checker, stmt: &StmtAnnAssign) {
return;
};

let vars = {
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
any_skipped: false,
};
visitor.visit_expr(value);
visitor.vars
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
any_skipped: false,
has_unpacked_kwargs: false,
};
visitor.visit_expr(value);
if visitor.has_unpacked_kwargs {
return;
}
let vars = visitor.vars;

// Type variables must be unique; filter while preserving order.
let vars = vars
Expand Down Expand Up @@ -317,4 +322,4 @@ fn create_diagnostic(
stmt.range(),
)
.set_fix(Fix::applicable_edit(edit, applicability));
}
}