Skip to content

Commit

Permalink
[pyupgrade] lint TypeAliasType in UP040
Browse files Browse the repository at this point in the history
  • Loading branch information
alex-700 committed May 24, 2024
1 parent 52c946a commit a05665a
Show file tree
Hide file tree
Showing 4 changed files with 325 additions and 74 deletions.
28 changes: 28 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,31 @@ class Foo:
# type alias.
T = typing.TypeVar["T"]
Decorator: TypeAlias = typing.Callable[[T], T]


from typing import TypeVar, Annotated, TypeAliasType

from annotated_types import Gt, SupportGt


# https://github.com/astral-sh/ruff/issues/11422
T = TypeVar('T')
PositiveList = TypeAliasType('PositiveList', list[Annotated[T, Gt(0)]], type_params=(T,))

# Bound
T = TypeVar('T', bound=SupportGt)
PositiveList = TypeAliasType('PositiveList', list[Annotated[T, Gt(0)]], type_params=(T,))

# Multiple bounds
T1 = TypeVar('T1', bound=SupportGt)
T2 = TypeVar('T2')
T3 = TypeVar('T3')
Tuple3 = TypeAliasType('Tuple3', tuple[T1, T2, T3], type_params=(T1, T2, T3))

# No type_params
PositiveInt = TypeAliasType('PositiveInt', Annotated[int, Gt(0)])
PositiveInt = TypeAliasType('PositiveInt', Annotated[int, Gt(0)], type_params=())

# OK: Other name
T = TypeVar('T', bound=SupportGt)
PositiveList = TypeAliasType('PositiveList2', list[Annotated[T, Gt(0)]], type_params=(T,))
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ListReverseCopy) {
refurb::rules::list_assign_reversed(checker, assign);
}
if checker.enabled(Rule::NonPEP695TypeAlias) {
pyupgrade::rules::non_pep695_type_alias_type(checker, assign);
}
}
Stmt::AnnAssign(
assign_stmt @ ast::StmtAnnAssign {
Expand Down
264 changes: 190 additions & 74 deletions crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
use itertools::Itertools;

use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{
self as ast,
visitor::{self, Visitor},
Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAnnAssign, StmtAssign,
Expr, ExprCall, ExprName, ExprSubscript, Identifier, Keyword, Stmt, StmtAnnAssign, StmtAssign,
StmtTypeAlias, TypeParam, TypeParamTypeVar,
};
use ruff_python_codegen::Generator;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion;

/// ## What it does
/// Checks for use of `TypeAlias` annotation for declaring type aliases.
/// Checks for use of `TypeAlias` annotation or `TypeAliasType` assignment for declaring type aliases.
///
/// ## Why is this bad?
/// The `type` keyword was introduced in Python 3.12 by [PEP 695] for defining
Expand All @@ -36,33 +37,113 @@ use crate::settings::types::PythonVersion;
/// ## Example
/// ```python
/// ListOfInt: TypeAlias = list[int]
/// PositiveInt = TypeAliasType('PositiveInt', Annotated[int, Gt(0)])
/// ```
///
/// Use instead:
/// ```python
/// type ListOfInt = list[int]
/// type PositiveInt = Annotated[int, Gt(0)]
/// ```
///
/// [PEP 695]: https://peps.python.org/pep-0695/
#[violation]
pub struct NonPEP695TypeAlias {
name: String,
type_alias_kind: TypeAliasKind,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum TypeAliasKind {
TypeAlias,
TypeAliasType,
}

impl Violation for NonPEP695TypeAlias {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always;

#[derive_message_formats]
fn message(&self) -> String {
let NonPEP695TypeAlias { name } = self;
format!("Type alias `{name}` uses `TypeAlias` annotation instead of the `type` keyword")
let NonPEP695TypeAlias {
name,
type_alias_kind,
} = self;
let type_alias_method = match type_alias_kind {
TypeAliasKind::TypeAlias => "`TypeAlias` annotation",
TypeAliasKind::TypeAliasType => "`TypeAliasType` assignment",
};
format!("Type alias `{name}` uses {type_alias_method} instead of the `type` keyword")
}

fn fix_title(&self) -> Option<String> {
Some("Use the `type` keyword".to_string())
}
}

/// UP040
pub(crate) fn non_pep695_type_alias_type(checker: &mut Checker, stmt: &StmtAssign) {
if checker.settings.target_version < PythonVersion::Py312 {
return;
}

let StmtAssign { targets, value, .. } = stmt;
let Expr::Call(ExprCall {
func, arguments, ..
}) = value.as_ref()
else {
return;
};
let [Expr::Name(target_name)] = targets.as_slice() else {
return;
};
let [Expr::StringLiteral(name), value] = arguments.args.as_ref() else {
return;
};
if name.value.to_str() != target_name.id {
return;
}
let type_params = match arguments.keywords.as_ref() {
[] => &[],
[Keyword {
arg: Some(name),
value: Expr::Tuple(type_params),
..
}] if name.as_str() == "type_params" => type_params.elts.as_slice(),
_ => return,
};

if !checker
.semantic()
.match_typing_expr(func.as_ref(), "TypeAliasType")
{
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,
restriction: None,
})
})
})
.collect()
else {
return;
};
checker.diagnostics.push(get_diagnostic(
checker.generator(),
stmt.range(),
target_name.id.clone(),
Box::new(value.clone()),
vars,
Applicability::Safe,
TypeAliasKind::TypeAliasType,
));
}

/// UP040
pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign) {
let StmtAnnAssign {
Expand Down Expand Up @@ -109,6 +190,32 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign)
.unique_by(|TypeVar { name, .. }| name.id.as_str())
.collect::<Vec<_>>();

checker.diagnostics.push(get_diagnostic(
checker.generator(),
stmt.range(),
name.clone(),
value.clone(),
vars,
// 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
if checker.source_type.is_stub() {
Applicability::Safe
} else {
Applicability::Unsafe
},
TypeAliasKind::TypeAlias,
));
}

fn get_diagnostic(
generator: Generator,
stmt_range: TextRange,
name: String,
value: Box<Expr>,
vars: Vec<TypeVar>,
applicability: Applicability,
type_alias_kind: TypeAliasKind,
) -> Diagnostic {
let type_params = if vars.is_empty() {
None
} else {
Expand Down Expand Up @@ -141,27 +248,29 @@ 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(),
name: target.clone(),
type_params,
value: value.clone(),
})),
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);
Diagnostic::new(
NonPEP695TypeAlias {
name: name.clone(),
type_alias_kind,
},
stmt_range,
)
.with_fix(Fix::applicable_edit(
Edit::range_replacement(
generator.stmt(&Stmt::from(StmtTypeAlias {
range: TextRange::default(),
name: Box::new(Expr::Name(ExprName {
range: TextRange::default(),
id: name,
ctx: ast::ExprContext::Load,
})),
type_params,
value,
})),
stmt_range,
),
applicability,
))
}

#[derive(Debug)]
Expand All @@ -188,57 +297,64 @@ impl<'a> Visitor<'a> for TypeVarReferenceVisitor<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
match expr {
Expr::Name(name) if name.ctx.is_load() => {
let Some(Stmt::Assign(StmtAssign { value, .. })) = self
.semantic
.lookup_symbol(name.id.as_str())
.and_then(|binding_id| {
self.semantic
.binding(binding_id)
.source
.map(|node_id| self.semantic.statement(node_id))
})
else {
return;
self.vars.extend(expr_name_to_type_var(self.semantic, name));
}
_ => visitor::walk_expr(self, expr),
}
}
}

fn expr_name_to_type_var<'a>(
semantic: &'a SemanticModel,
name: &'a ExprName,
) -> Option<TypeVar<'a>> {
let Some(Stmt::Assign(StmtAssign { value, .. })) = semantic
.lookup_symbol(name.id.as_str())
.and_then(|binding_id| {
semantic
.binding(binding_id)
.source
.map(|node_id| semantic.statement(node_id))
})
else {
return None;
};

match value.as_ref() {
Expr::Subscript(ExprSubscript {
value: ref subscript_value,
..
}) => {
if semantic.match_typing_expr(subscript_value, "TypeVar") {
return Some(TypeVar {
name,
restriction: None,
});
}
}
Expr::Call(ExprCall {
func, arguments, ..
}) => {
if semantic.match_typing_expr(func, "TypeVar")
&& arguments
.args
.first()
.is_some_and(Expr::is_string_literal_expr)
{
let restriction = if let Some(bound) = arguments.find_keyword("bound") {
Some(TypeVarRestriction::Bound(&bound.value))
} else if arguments.args.len() > 1 {
Some(TypeVarRestriction::Constraint(
arguments.args.iter().skip(1).collect(),
))
} else {
None
};

match value.as_ref() {
Expr::Subscript(ExprSubscript {
value: ref subscript_value,
..
}) => {
if self.semantic.match_typing_expr(subscript_value, "TypeVar") {
self.vars.push(TypeVar {
name,
restriction: None,
});
}
}
Expr::Call(ExprCall {
func, arguments, ..
}) => {
if self.semantic.match_typing_expr(func, "TypeVar")
&& arguments
.args
.first()
.is_some_and(Expr::is_string_literal_expr)
{
let restriction = if let Some(bound) = arguments.find_keyword("bound") {
Some(TypeVarRestriction::Bound(&bound.value))
} else if arguments.args.len() > 1 {
Some(TypeVarRestriction::Constraint(
arguments.args.iter().skip(1).collect(),
))
} else {
None
};

self.vars.push(TypeVar { name, restriction });
}
}
_ => {}
}
return Some(TypeVar { name, restriction });
}
_ => visitor::walk_expr(self, expr),
}
_ => {}
}
None
}
Loading

0 comments on commit a05665a

Please sign in to comment.