Skip to content

Commit

Permalink
Respect duplicates when rewriting type aliases (#9905)
Browse files Browse the repository at this point in the history
## Summary

If a generic appears multiple times on the right-hand side, we should
only include it once on the left-hand side when rewriting.

Closes #9904.
  • Loading branch information
charliermarsh authored Feb 9, 2024
1 parent 12a91f4 commit 52ebfc9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 10 deletions.
5 changes: 5 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,8 @@ class Foo:
# OK
x: TypeAlias
x: int = 1

# Ensure that "T" appears only once in the type parameters for the modernized
# type alias.
T = typing.TypeVar["T"]
Decorator: TypeAlias = typing.Callable[[T], T]
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use itertools::Itertools;

use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{
Expand Down Expand Up @@ -92,20 +94,27 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign)

// TODO(zanie): We should check for generic type variables used in the value and define them
// as type params instead
let mut diagnostic = Diagnostic::new(NonPEP695TypeAlias { name: name.clone() }, stmt.range());
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
let vars = {
let mut visitor = TypeVarReferenceVisitor {
vars: vec![],
semantic: checker.semantic(),
};
visitor.visit_expr(value);
visitor.vars
};
visitor.visit_expr(value);

let type_params = if visitor.vars.is_empty() {
// Type variables must be unique; filter while preserving order.
let vars = vars
.into_iter()
.unique_by(|TypeVar { name, .. }| name.id.as_str())
.collect::<Vec<_>>();

let type_params = if vars.is_empty() {
None
} else {
Some(ast::TypeParams {
range: TextRange::default(),
type_params: visitor
.vars
type_params: vars
.into_iter()
.map(|TypeVar { name, restriction }| {
TypeParam::TypeVar(TypeParamTypeVar {
Expand All @@ -128,6 +137,8 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign)
})
};

let mut diagnostic = Diagnostic::new(NonPEP695TypeAlias { name: name.clone() }, stmt.range());

let edit = Edit::range_replacement(
checker.generator().stmt(&Stmt::from(StmtTypeAlias {
range: TextRange::default(),
Expand All @@ -137,16 +148,15 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign)
})),
stmt.range(),
);

// The fix is only safe in a type stub because new-style aliases have different runtime behavior
// See https://github.com/astral-sh/ruff/issues/6434
let fix = if checker.source_type.is_stub() {
Fix::safe_edit(edit)
} else {
Fix::unsafe_edit(edit)
};

diagnostic.set_fix(fix);

checker.diagnostics.push(diagnostic);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,4 +230,20 @@ UP040.py:44:1: UP040 [*] Type alias `x` uses `TypeAlias` annotation instead of t
46 46 | # OK
47 47 | x: TypeAlias
UP040.py:53:1: UP040 [*] Type alias `Decorator` uses `TypeAlias` annotation instead of the `type` keyword
|
51 | # type alias.
52 | T = typing.TypeVar["T"]
53 | Decorator: TypeAlias = typing.Callable[[T], T]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP040
|
= help: Use the `type` keyword
ℹ Unsafe fix
50 50 | # Ensure that "T" appears only once in the type parameters for the modernized
51 51 | # type alias.
52 52 | T = typing.TypeVar["T"]
53 |-Decorator: TypeAlias = typing.Callable[[T], T]
53 |+type Decorator[T] = typing.Callable[[T], T]

0 comments on commit 52ebfc9

Please sign in to comment.