Skip to content

Commit

Permalink
[ruff] Classes with mixed type variable style (RUF060)
Browse files Browse the repository at this point in the history
  • Loading branch information
InSyncWithFoo committed Jan 31, 2025
1 parent fab86de commit 79763f4
Show file tree
Hide file tree
Showing 11 changed files with 795 additions and 32 deletions.
87 changes: 87 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF060.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from typing import Generic, ParamSpec, TypeVar, TypeVarTuple, Unpack


_A = TypeVar('_A')
_B = TypeVar('_B', bound=int)
_C = TypeVar('_C', str, bytes)
_D = TypeVar('_D', default=int)
_E = TypeVar('_E', bound=int, default=int)
_F = TypeVar('_F', str, bytes, default=str)

_As = TypeVarTuple('_As')
_Bs = TypeVarTuple('_Bs', bound=tuple[int, str])
_Cs = TypeVarTuple('_Cs', default=tuple[int, str])


_P1 = ParamSpec('_P1')
_P2 = ParamSpec('_P2', bound=[bytes, bool])
_P3 = ParamSpec('_P3', default=[int, str])


### Errors

class C[T](Generic[_A]): ...
class C[T](Generic[_B], str): ...
class C[T](int, Generic[_C]): ...
class C[T](bytes, Generic[_D], bool): ... # TODO: Type parameter defaults
class C[T](Generic[_E], list[_E]): ... # TODO: Type parameter defaults
class C[T](list[_F], Generic[_F]): ... # TODO: Type parameter defaults

class C[*Ts](Generic[*_As]): ...
class C[*Ts](Generic[Unpack[_As]]): ...
class C[*Ts](Generic[Unpack[_Bs]], tuple[*Bs]): ...
class C[*Ts](Callable[[*_Cs], tuple[*Ts]], Generic[_Cs]): ... # TODO: Type parameter defaults


class C[**P](Generic[_P1]): ...
class C[**P](Generic[_P2]): ...
class C[**P](Generic[_P3]): ... # TODO: Type parameter defaults


class C[T](Generic[T, _A]): ...


# See `is_existing_param_of_same_class`.
# `expr_name_to_type_var` doesn't handle named expressions,
# only simple assignments, so there is no fix.
class C[T: (_Z := TypeVar('_Z'))](Generic[_Z]): ...


class C(Generic[_B]):
class D[T](Generic[_B, T]): ...


class C[T]:
class D[U](Generic[T, U]): ...


# In a single run, only the first is reported.
# Others will be reported/fixed in following iterations.
class C[T](Generic[_C], Generic[_D]): ...
class C[T, _C: (str, bytes)](Generic[_D]): ... # TODO: Type parameter defaults


class C[
T # Comment
](Generic[_E]): ... # TODO: Type parameter defaults


class C[T](Generic[Generic[_F]]): ...
class C[T](Generic[Unpack[_A]]): ...
class C[T](Generic[Unpack[_P1]]): ...
class C[T](Generic[Unpack[Unpack[_P2]]]): ...
class C[T](Generic[Unpack[*_As]]): ...
class C[T](Generic[Unpack[_As, _Bs]]): ...


class C[T](Generic[_A, _A]): ...
class C[T](Generic[_A, Unpack[_As]]): ...
class C[T](Generic[*_As, _A]): ...


### No errors

class C(Generic[_A]): ...
class C[_A]: ...
class C[_A](list[_A]): ...
class C[_A](list[Generic[_A]]): ...
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 @@ -563,6 +563,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::NonPEP695GenericClass) {
pyupgrade::rules::non_pep695_generic_class(checker, class_def);
}
if checker.enabled(Rule::ClassWithMixedTypeVars) {
ruff::rules::class_with_mixed_type_vars(checker, class_def);
}
}
Stmt::Import(ast::StmtImport { names, range: _ }) => {
if checker.enabled(Rule::MultipleImportsOnOneLine) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "056") => (RuleGroup::Preview, rules::ruff::rules::FalsyDictGetFallback),
(Ruff, "057") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRound),
(Ruff, "058") => (RuleGroup::Preview, rules::ruff::rules::StarmapZip),
(Ruff, "060") => (RuleGroup::Preview, rules::ruff::rules::ClassWithMixedTypeVars),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ mod native_literals;
mod open_alias;
mod os_error_alias;
mod outdated_version_block;
mod pep695;
pub(crate) mod pep695;
mod printf_string_formatting;
mod quoted_annotation;
mod redundant_open_modes;
Expand Down
69 changes: 56 additions & 13 deletions crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ruff_python_ast::{
self as ast,
name::Name,
visitor::{self, Visitor},
Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAssign, TypeParam,
Arguments, Expr, ExprCall, ExprName, ExprSubscript, Identifier, Stmt, StmtAssign, TypeParam,
TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple,
};
use ruff_python_semantic::SemanticModel;
Expand All @@ -26,7 +26,7 @@ mod non_pep695_generic_function;
mod non_pep695_type_alias;

#[derive(Debug)]
enum TypeVarRestriction<'a> {
pub(crate) enum TypeVarRestriction<'a> {
/// A type variable with a bound, e.g., `TypeVar("T", bound=int)`.
Bound(&'a Expr),
/// A type variable with constraints, e.g., `TypeVar("T", int, str)`.
Expand All @@ -37,25 +37,25 @@ enum TypeVarRestriction<'a> {
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
enum TypeParamKind {
pub(crate) enum TypeParamKind {
TypeVar,
TypeVarTuple,
ParamSpec,
}

#[derive(Debug)]
struct TypeVar<'a> {
name: &'a str,
restriction: Option<TypeVarRestriction<'a>>,
kind: TypeParamKind,
default: Option<&'a Expr>,
pub(crate) struct TypeVar<'a> {
pub(crate) name: &'a str,
pub(crate) restriction: Option<TypeVarRestriction<'a>>,
pub(crate) kind: TypeParamKind,
pub(crate) default: Option<&'a Expr>,
}

/// Wrapper for formatting a sequence of [`TypeVar`]s for use as a generic type parameter (e.g. `[T,
/// *Ts, **P]`). See [`DisplayTypeVar`] for further details.
struct DisplayTypeVars<'a> {
type_vars: &'a [TypeVar<'a>],
source: &'a str,
pub(crate) struct DisplayTypeVars<'a> {
pub(crate) type_vars: &'a [TypeVar<'a>],
pub(crate) source: &'a str,
}

impl Display for DisplayTypeVars<'_> {
Expand All @@ -79,7 +79,7 @@ impl Display for DisplayTypeVars<'_> {

/// Used for displaying `type_var`. `source` is the whole file, which will be sliced to recover the
/// `TypeVarRestriction` values for generic bounds and constraints.
struct DisplayTypeVar<'a> {
pub(crate) struct DisplayTypeVar<'a> {
type_var: &'a TypeVar<'a>,
source: &'a str,
}
Expand Down Expand Up @@ -190,6 +190,34 @@ impl<'a> From<&'a TypeVar<'a>> for TypeParam {
}
}

impl<'a> From<&'a TypeParam> for TypeVar<'a> {
fn from(param: &'a TypeParam) -> Self {
let (kind, restriction) = match param {
TypeParam::TypeVarTuple(_) => (TypeParamKind::TypeVarTuple, None),
TypeParam::ParamSpec(_) => (TypeParamKind::ParamSpec, None),

TypeParam::TypeVar(param) => {
let restriction = match param.bound.as_deref() {
None => None,
Some(Expr::Tuple(constraints)) => Some(TypeVarRestriction::Constraint(
constraints.elts.iter().collect::<Vec<_>>(),
)),
Some(bound) => Some(TypeVarRestriction::Bound(bound)),
};

(TypeParamKind::TypeVar, restriction)
}
};

Self {
name: param.name(),
kind,
restriction,
default: param.default(),
}
}
}

struct TypeVarReferenceVisitor<'a> {
vars: Vec<TypeVar<'a>>,
semantic: &'a SemanticModel<'a>,
Expand Down Expand Up @@ -240,7 +268,7 @@ impl<'a> Visitor<'a> for TypeVarReferenceVisitor<'a> {
}
}

fn expr_name_to_type_var<'a>(
pub(crate) fn expr_name_to_type_var<'a>(
semantic: &'a SemanticModel,
name: &'a ExprName,
) -> Option<TypeVar<'a>> {
Expand Down Expand Up @@ -347,3 +375,18 @@ fn check_type_vars(vars: Vec<TypeVar<'_>>) -> Option<Vec<TypeVar<'_>>> {
== vars.len())
.then_some(vars)
}

/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if
/// any), along with its index in the class's bases tuple.
pub(crate) fn find_generic<'a>(
class_bases: &'a Arguments,
semantic: &SemanticModel,
) -> Option<(usize, &'a ExprSubscript)> {
class_bases.args.iter().enumerate().find_map(|(idx, expr)| {
expr.as_subscript_expr().and_then(|sub_expr| {
semantic
.match_typing_expr(&sub_expr.value, "Generic")
.then_some((idx, sub_expr))
})
})
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{Arguments, ExprSubscript, StmtClassDef};
use ruff_python_semantic::SemanticModel;
use ruff_python_ast::{ExprSubscript, StmtClassDef};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::fix::edits::{remove_argument, Parentheses};
use crate::settings::types::PythonVersion;

use super::{check_type_vars, in_nested_context, DisplayTypeVars, TypeVarReferenceVisitor};
use super::{
check_type_vars, find_generic, in_nested_context, DisplayTypeVars, TypeVarReferenceVisitor,
};

/// ## What it does
///
Expand Down Expand Up @@ -203,18 +204,3 @@ pub(crate) fn non_pep695_generic_class(checker: &mut Checker, class_def: &StmtCl

checker.diagnostics.push(diagnostic);
}

/// Search `class_bases` for a `typing.Generic` base class. Returns the `Generic` expression (if
/// any), along with its index in the class's bases tuple.
fn find_generic<'a>(
class_bases: &'a Arguments,
semantic: &SemanticModel,
) -> Option<(usize, &'a ExprSubscript)> {
class_bases.args.iter().enumerate().find_map(|(idx, expr)| {
expr.as_subscript_expr().and_then(|sub_expr| {
semantic
.match_typing_expr(&sub_expr.value, "Generic")
.then_some((idx, sub_expr))
})
})
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ mod tests {
#[test_case(Rule::DataclassEnum, Path::new("RUF049.py"))]
#[test_case(Rule::StarmapZip, Path::new("RUF058_0.py"))]
#[test_case(Rule::StarmapZip, Path::new("RUF058_1.py"))]
#[test_case(Rule::ClassWithMixedTypeVars, Path::new("RUF060.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
Loading

0 comments on commit 79763f4

Please sign in to comment.