Skip to content

Commit

Permalink
[pylint] Implement Pylint typevar-name-mismatch (C0132) (#5501)
Browse files Browse the repository at this point in the history
## Summary

Implement Pylint `typevar-name-mismatch` (`C0132`) as
`type-param-name-mismatch` (`PLC0132`). Includes documentation. Related
to #970.

The Pylint implementation checks only `TypeVar`, but this PR checks
`TypeVarTuple`, `ParamSpec`, and `NewType` as well. This seems to better
represent the Pylint rule's [intended
behaviour](pylint-dev/pylint#5224).

Full disclosure: I am not a fan of the translated name and think it
should probably be different.

## Test Plan

`cargo test`
  • Loading branch information
tjkuson committed Jul 4, 2023
1 parent c395e44 commit 0e67757
Show file tree
Hide file tree
Showing 8 changed files with 308 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from typing import TypeVar, ParamSpec, NewType, TypeVarTuple

# Errors.

X = TypeVar("T")
X = TypeVar(name="T")

Y = ParamSpec("T")
Y = ParamSpec(name="T")

Z = NewType("T", int)
Z = NewType(name="T", tp=int)

Ws = TypeVarTuple("Ts")
Ws = TypeVarTuple(name="Ts")

# Non-errors.

T = TypeVar("T")
T = TypeVar(name="T")

T = ParamSpec("T")
T = ParamSpec(name="T")

T = NewType("T", int)
T = NewType(name="T", tp=int)

Ts = TypeVarTuple("Ts")
Ts = TypeVarTuple(name="Ts")

# Errors, but not covered by this rule.

# Non-string literal name.
T = TypeVar(some_str)
T = TypeVar(name=some_str)
T = TypeVar(1)
T = TypeVar(name=1)
T = ParamSpec(some_str)
T = ParamSpec(name=some_str)
T = ParamSpec(1)
T = ParamSpec(name=1)
T = NewType(some_str, int)
T = NewType(name=some_str, tp=int)
T = NewType(1, int)
T = NewType(name=1, tp=int)
Ts = TypeVarTuple(some_str)
Ts = TypeVarTuple(name=some_str)
Ts = TypeVarTuple(1)
Ts = TypeVarTuple(name=1)

# No names provided.
T = TypeVar()
T = ParamSpec()
T = NewType()
T = NewType(tp=int)
Ts = TypeVarTuple()
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,9 @@ where
self.diagnostics.push(diagnostic);
}
}
if self.settings.rules.enabled(Rule::TypeParamNameMismatch) {
pylint::rules::type_param_name_mismatch(self, value, targets);
}
if self.is_stub {
if self.any_enabled(&[
Rule::UnprefixedTypeParam,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pyflakes, "901") => (RuleGroup::Unspecified, rules::pyflakes::rules::RaiseNotImplemented),

// pylint
(Pylint, "C0132") => (RuleGroup::Unspecified, rules::pylint::rules::TypeParamNameMismatch),
(Pylint, "C0205") => (RuleGroup::Unspecified, rules::pylint::rules::SingleStringSlots),
(Pylint, "C0414") => (RuleGroup::Unspecified, rules::pylint::rules::UselessImportAlias),
(Pylint, "C1901") => (RuleGroup::Nursery, rules::pylint::rules::CompareToEmptyString),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ mod tests {
Path::new("too_many_return_statements.py")
)]
#[test_case(Rule::TooManyStatements, Path::new("too_many_statements.py"))]
#[test_case(Rule::TypeParamNameMismatch, Path::new("type_param_name_mismatch.py"))]
#[test_case(
Rule::UnexpectedSpecialMethodSignature,
Path::new("unexpected_special_method_signature.py")
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) use too_many_arguments::*;
pub(crate) use too_many_branches::*;
pub(crate) use too_many_return_statements::*;
pub(crate) use too_many_statements::*;
pub(crate) use type_param_name_mismatch::*;
pub(crate) use unexpected_special_method_signature::*;
pub(crate) use unnecessary_direct_lambda_call::*;
pub(crate) use useless_else_on_loop::*;
Expand Down Expand Up @@ -84,6 +85,7 @@ mod too_many_arguments;
mod too_many_branches;
mod too_many_return_statements;
mod too_many_statements;
mod type_param_name_mismatch;
mod unexpected_special_method_signature;
mod unnecessary_direct_lambda_call;
mod useless_else_on_loop;
Expand Down
166 changes: 166 additions & 0 deletions crates/ruff/src/rules/pylint/rules/type_param_name_mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
use std::fmt;

use rustpython_parser::ast::{self, Constant, Expr, Ranged};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `TypeVar`, `TypeVarTuple`, `ParamSpec`, and `NewType`
/// definitions in which the name of the type parameter does not match the name
/// of the variable to which it is assigned.
///
/// ## Why is this bad?
/// When defining a `TypeVar` or a related type parameter, Python allows you to
/// provide a name for the type parameter. According to [PEP 484], the name
/// provided to the `TypeVar` constructor must be equal to the name of the
/// variable to which it is assigned.
///
/// ## Example
/// ```python
/// from typing import TypeVar
///
/// T = TypeVar("U")
/// ```
///
/// Use instead:
/// ```python
/// from typing import TypeVar
///
/// T = TypeVar("T")
/// ```
///
/// ## References
/// - [Python documentation: `typing` — Support for type hints](https://docs.python.org/3/library/typing.html)
/// - [PEP 484 – Type Hints: Generics](https://peps.python.org/pep-0484/#generics)
///
/// [PEP 484]:https://peps.python.org/pep-0484/#generics
#[violation]
pub struct TypeParamNameMismatch {
kind: VarKind,
var_name: String,
param_name: String,
}

impl Violation for TypeParamNameMismatch {
#[derive_message_formats]
fn message(&self) -> String {
let TypeParamNameMismatch {
kind,
var_name,
param_name,
} = self;
format!("`{kind}` name `{param_name}` does not match assigned variable name `{var_name}`")
}
}

/// PLC0132
pub(crate) fn type_param_name_mismatch(checker: &mut Checker, value: &Expr, targets: &[Expr]) {
let [target] = targets else {
return;
};

let Expr::Name(ast::ExprName { id: var_name, .. }) = &target else {
return;
};

let Some(param_name) = param_name(value) else {
return;
};

if var_name == param_name {
return;
}

let Expr::Call(ast::ExprCall { func, .. }) = value else {
return;
};

let Some(kind) = checker
.semantic()
.resolve_call_path(func)
.and_then(|call_path| {
if checker
.semantic()
.match_typing_call_path(&call_path, "ParamSpec")
{
Some(VarKind::ParamSpec)
} else if checker
.semantic()
.match_typing_call_path(&call_path, "TypeVar")
{
Some(VarKind::TypeVar)
} else if checker
.semantic()
.match_typing_call_path(&call_path, "TypeVarTuple")
{
Some(VarKind::TypeVarTuple)
} else if checker
.semantic()
.match_typing_call_path(&call_path, "NewType")
{
Some(VarKind::NewType)
} else {
None
}
})
else {
return;
};

checker.diagnostics.push(Diagnostic::new(
TypeParamNameMismatch {
kind,
var_name: var_name.to_string(),
param_name: param_name.to_string(),
},
value.range(),
));
}

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum VarKind {
TypeVar,
ParamSpec,
TypeVarTuple,
NewType,
}

impl fmt::Display for VarKind {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
VarKind::TypeVar => fmt.write_str("TypeVar"),
VarKind::ParamSpec => fmt.write_str("ParamSpec"),
VarKind::TypeVarTuple => fmt.write_str("TypeVarTuple"),
VarKind::NewType => fmt.write_str("NewType"),
}
}
}

/// Returns the value of the `name` parameter to, e.g., a `TypeVar` constructor.
fn param_name(value: &Expr) -> Option<&str> {
// Handle both `TypeVar("T")` and `TypeVar(name="T")`.
let call = value.as_call_expr()?;
let name_param = call
.keywords
.iter()
.find(|keyword| {
keyword
.arg
.as_ref()
.map_or(false, |keyword| keyword.as_str() == "name")
})
.map(|keyword| &keyword.value)
.or_else(|| call.args.get(0))?;
if let Expr::Constant(ast::ExprConstant {
value: Constant::Str(name),
..
}) = &name_param
{
Some(name)
} else {
None
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
---
type_param_name_mismatch.py:5:5: PLC0132 `TypeVar` name `T` does not match assigned variable name `X`
|
3 | # Errors.
4 |
5 | X = TypeVar("T")
| ^^^^^^^^^^^^ PLC0132
6 | X = TypeVar(name="T")
|

type_param_name_mismatch.py:6:5: PLC0132 `TypeVar` name `T` does not match assigned variable name `X`
|
5 | X = TypeVar("T")
6 | X = TypeVar(name="T")
| ^^^^^^^^^^^^^^^^^ PLC0132
7 |
8 | Y = ParamSpec("T")
|

type_param_name_mismatch.py:8:5: PLC0132 `ParamSpec` name `T` does not match assigned variable name `Y`
|
6 | X = TypeVar(name="T")
7 |
8 | Y = ParamSpec("T")
| ^^^^^^^^^^^^^^ PLC0132
9 | Y = ParamSpec(name="T")
|

type_param_name_mismatch.py:9:5: PLC0132 `ParamSpec` name `T` does not match assigned variable name `Y`
|
8 | Y = ParamSpec("T")
9 | Y = ParamSpec(name="T")
| ^^^^^^^^^^^^^^^^^^^ PLC0132
10 |
11 | Z = NewType("T", int)
|

type_param_name_mismatch.py:11:5: PLC0132 `NewType` name `T` does not match assigned variable name `Z`
|
9 | Y = ParamSpec(name="T")
10 |
11 | Z = NewType("T", int)
| ^^^^^^^^^^^^^^^^^ PLC0132
12 | Z = NewType(name="T", tp=int)
|

type_param_name_mismatch.py:12:5: PLC0132 `NewType` name `T` does not match assigned variable name `Z`
|
11 | Z = NewType("T", int)
12 | Z = NewType(name="T", tp=int)
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0132
13 |
14 | Ws = TypeVarTuple("Ts")
|

type_param_name_mismatch.py:14:6: PLC0132 `TypeVarTuple` name `Ts` does not match assigned variable name `Ws`
|
12 | Z = NewType(name="T", tp=int)
13 |
14 | Ws = TypeVarTuple("Ts")
| ^^^^^^^^^^^^^^^^^^ PLC0132
15 | Ws = TypeVarTuple(name="Ts")
|

type_param_name_mismatch.py:15:6: PLC0132 `TypeVarTuple` name `Ts` does not match assigned variable name `Ws`
|
14 | Ws = TypeVarTuple("Ts")
15 | Ws = TypeVarTuple(name="Ts")
| ^^^^^^^^^^^^^^^^^^^^^^^ PLC0132
16 |
17 | # Non-errors.
|


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0e67757

Please sign in to comment.