From ceaef23cb993140c12dc593c74d4c752e7205fe9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 9 Feb 2024 06:02:41 -0800 Subject: [PATCH] Respect duplicates when rewriting type aliases (#9905) ## 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 https://github.com/astral-sh/ruff/issues/9904. --- .../test/fixtures/pyupgrade/UP040.py | 5 ++++ .../pyupgrade/rules/use_pep695_type_alias.rs | 30 ++++++++++++------- ...er__rules__pyupgrade__tests__UP040.py.snap | 16 ++++++++++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py index 175303a201120..0368a34800db2 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py @@ -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] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs index 547e40f7d87f1..29f9039d2109a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs @@ -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::{ @@ -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::>(); + + 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 { @@ -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(), @@ -137,7 +148,6 @@ 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() { @@ -145,8 +155,8 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign) } else { Fix::unsafe_edit(edit) }; - diagnostic.set_fix(fix); + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP040.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP040.py.snap index 4fffff62a2d7f..e7692cb305451 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP040.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP040.py.snap @@ -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] +