Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pyupgrade] lint TypeAliasType in UP040 #11530

Merged
merged 2 commits into from
May 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
34 changes: 34 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,37 @@ 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
265 changes: 191 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,23 @@
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 +38,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 +191,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 +249,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 +298,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
Loading