[pylint] Implement self-cls-assignment (W0642) (#9267)

## Summary

This PR implements [`W0642`/`self-cls-assignment`](https://pylint.readthedocs.io/en/stable/user_guide/messages/warning/self-cls-assignment.html)

See: #970 

## Test Plan

Add test cases and verified the updated snapshots.

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
This commit is contained in:
Steve C 2024-04-15 05:06:01 -04:00 committed by GitHub
parent 670d66f54c
commit c2210359e7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 366 additions and 0 deletions

View file

@ -0,0 +1,43 @@
class Fruit:
@classmethod
def list_fruits(cls) -> None:
cls = "apple" # PLW0642
cls: Fruit = "apple" # PLW0642
cls += "orange" # PLW0642
*cls = "banana" # PLW0642
cls, blah = "apple", "orange" # PLW0642
blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
blah, [cls, blah2] = "apple", ("orange", "banana") # PLW0642
@classmethod
def add_fruits(cls, fruits, /) -> None:
cls = fruits # PLW0642
def print_color(self) -> None:
self = "red" # PLW0642
self: Self = "red" # PLW0642
self += "blue" # PLW0642
*self = "blue" # PLW0642
self, blah = "red", "blue" # PLW0642
blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642
blah, [self, blah2] = "apple", ("orange", "banana") # PLW0642
def print_color(self, color, /) -> None:
self = color
def ok(self) -> None:
cls = None # OK because the rule looks for the name in the signature
@classmethod
def ok(cls) -> None:
self = None
@staticmethod
def list_fruits_static(self, cls) -> None:
self = "apple" # Ok
cls = "banana" # Ok
def list_fruits(self, cls) -> None:
self = "apple" # Ok
cls = "banana" # Ok

View file

@ -1055,6 +1055,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
} }
} }
Stmt::AugAssign(aug_assign @ ast::StmtAugAssign { target, .. }) => { Stmt::AugAssign(aug_assign @ ast::StmtAugAssign { target, .. }) => {
if checker.enabled(Rule::SelfOrClsAssignment) {
pylint::rules::self_or_cls_assignment(checker, target);
}
if checker.enabled(Rule::GlobalStatement) { if checker.enabled(Rule::GlobalStatement) {
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
pylint::rules::global_statement(checker, id); pylint::rules::global_statement(checker, id);
@ -1410,6 +1413,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
} }
} }
Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => {
if checker.enabled(Rule::SelfOrClsAssignment) {
for target in targets {
pylint::rules::self_or_cls_assignment(checker, target);
}
}
if checker.enabled(Rule::RedeclaredAssignedName) { if checker.enabled(Rule::RedeclaredAssignedName) {
pylint::rules::redeclared_assigned_name(checker, targets); pylint::rules::redeclared_assigned_name(checker, targets);
} }
@ -1547,6 +1555,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
); );
} }
} }
if checker.enabled(Rule::SelfOrClsAssignment) {
pylint::rules::self_or_cls_assignment(checker, target);
}
if checker.enabled(Rule::SelfAssigningVariable) { if checker.enabled(Rule::SelfAssigningVariable) {
pylint::rules::self_annotated_assignment(checker, assign_stmt); pylint::rules::self_annotated_assignment(checker, assign_stmt);
} }

View file

@ -311,6 +311,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned), (Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned),
(Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement), (Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement),
(Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel), (Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel),
(Pylint, "W0642") => (RuleGroup::Preview, rules::pylint::rules::SelfOrClsAssignment),
(Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException), (Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException),
(Pylint, "W1501") => (RuleGroup::Preview, rules::pylint::rules::BadOpenMode), (Pylint, "W1501") => (RuleGroup::Preview, rules::pylint::rules::BadOpenMode),
(Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault), (Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault),

View file

@ -180,6 +180,7 @@ mod tests {
#[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))] #[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
#[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))] #[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))]
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))] #[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
#[test_case(Rule::SelfOrClsAssignment, Path::new("self_or_cls_assignment.py"))]
#[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))] #[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))]
#[test_case(Rule::DictIterMissingItems, Path::new("dict_iter_missing_items.py"))] #[test_case(Rule::DictIterMissingItems, Path::new("dict_iter_missing_items.py"))]
#[test_case( #[test_case(

View file

@ -61,6 +61,7 @@ pub(crate) use repeated_isinstance_calls::*;
pub(crate) use repeated_keyword_argument::*; pub(crate) use repeated_keyword_argument::*;
pub(crate) use return_in_init::*; pub(crate) use return_in_init::*;
pub(crate) use self_assigning_variable::*; pub(crate) use self_assigning_variable::*;
pub(crate) use self_or_cls_assignment::*;
pub(crate) use single_string_slots::*; pub(crate) use single_string_slots::*;
pub(crate) use singledispatch_method::*; pub(crate) use singledispatch_method::*;
pub(crate) use singledispatchmethod_function::*; pub(crate) use singledispatchmethod_function::*;
@ -158,6 +159,7 @@ mod repeated_isinstance_calls;
mod repeated_keyword_argument; mod repeated_keyword_argument;
mod return_in_init; mod return_in_init;
mod self_assigning_variable; mod self_assigning_variable;
mod self_or_cls_assignment;
mod single_string_slots; mod single_string_slots;
mod singledispatch_method; mod singledispatch_method;
mod singledispatchmethod_function; mod singledispatchmethod_function;

View file

@ -0,0 +1,148 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, ParameterWithDefault};
use ruff_python_semantic::analyze::function_type::{self as function_type, FunctionType};
use ruff_python_semantic::ScopeKind;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for assignment of `self` and `cls` in instance and class methods respectively.
///
/// ## Why is this bad?
/// The identifiers `self` and `cls` are conventional in Python for the first argument of instance
/// methods and class methods, respectively.
///
/// ## Example
///
/// ```python
/// class Versions:
/// def add(self, version):
/// self = version
///
/// @classmethod
/// def from_list(cls, versions):
/// cls = versions
/// ```
///
/// Use instead:
/// ```python
/// class Versions:
/// def add(self, version):
/// self.versions.append(version)
///
/// @classmethod
/// def from_list(cls, versions):
/// return cls(versions)
/// ```
#[violation]
pub struct SelfOrClsAssignment {
method_type: MethodType,
}
impl Violation for SelfOrClsAssignment {
#[derive_message_formats]
fn message(&self) -> String {
let SelfOrClsAssignment { method_type } = self;
format!(
"Invalid assignment to `{}` argument in {method_type} method",
method_type.arg_name(),
)
}
}
/// PLW0127
pub(crate) fn self_or_cls_assignment(checker: &mut Checker, target: &Expr) {
let ScopeKind::Function(ast::StmtFunctionDef {
name,
decorator_list,
parameters,
..
}) = checker.semantic().current_scope().kind
else {
return;
};
let Some(parent) = &checker
.semantic()
.first_non_type_parent_scope(checker.semantic().current_scope())
else {
return;
};
let Some(ParameterWithDefault {
parameter: self_or_cls,
..
}) = parameters
.posonlyargs
.first()
.or_else(|| parameters.args.first())
else {
return;
};
let function_type = function_type::classify(
name,
decorator_list,
parent,
checker.semantic(),
&checker.settings.pep8_naming.classmethod_decorators,
&checker.settings.pep8_naming.staticmethod_decorators,
);
let method_type = match (function_type, self_or_cls.name.as_str()) {
(FunctionType::Method { .. }, "self") => MethodType::Instance,
(FunctionType::ClassMethod { .. }, "cls") => MethodType::Class,
_ => return,
};
check_expr(checker, target, method_type);
}
fn check_expr(checker: &mut Checker, target: &Expr, method_type: MethodType) {
match target {
Expr::Name(_) => {
if let Expr::Name(ast::ExprName { id, .. }) = target {
if id.as_str() == method_type.arg_name() {
checker.diagnostics.push(Diagnostic::new(
SelfOrClsAssignment { method_type },
target.range(),
));
}
}
}
Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) => {
for element in elts {
check_expr(checker, element, method_type);
}
}
Expr::Starred(ast::ExprStarred { value, .. }) => check_expr(checker, value, method_type),
_ => {}
}
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum MethodType {
Instance,
Class,
}
impl MethodType {
fn arg_name(self) -> &'static str {
match self {
MethodType::Instance => "self",
MethodType::Class => "cls",
}
}
}
impl std::fmt::Display for MethodType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
MethodType::Instance => f.write_str("instance"),
MethodType::Class => f.write_str("class"),
}
}
}

View file

@ -0,0 +1,158 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
self_or_cls_assignment.py:4:9: PLW0642 Invalid assignment to `cls` argument in class method
|
2 | @classmethod
3 | def list_fruits(cls) -> None:
4 | cls = "apple" # PLW0642
| ^^^ PLW0642
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
|
self_or_cls_assignment.py:5:9: PLW0642 Invalid assignment to `cls` argument in class method
|
3 | def list_fruits(cls) -> None:
4 | cls = "apple" # PLW0642
5 | cls: Fruit = "apple" # PLW0642
| ^^^ PLW0642
6 | cls += "orange" # PLW0642
7 | *cls = "banana" # PLW0642
|
self_or_cls_assignment.py:6:9: PLW0642 Invalid assignment to `cls` argument in class method
|
4 | cls = "apple" # PLW0642
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
| ^^^ PLW0642
7 | *cls = "banana" # PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
|
self_or_cls_assignment.py:7:10: PLW0642 Invalid assignment to `cls` argument in class method
|
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
7 | *cls = "banana" # PLW0642
| ^^^ PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
9 | blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:8:9: PLW0642 Invalid assignment to `cls` argument in class method
|
6 | cls += "orange" # PLW0642
7 | *cls = "banana" # PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
| ^^^ PLW0642
9 | blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
10 | blah, [cls, blah2] = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:9:16: PLW0642 Invalid assignment to `cls` argument in class method
|
7 | *cls = "banana" # PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
9 | blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
| ^^^ PLW0642
10 | blah, [cls, blah2] = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:10:16: PLW0642 Invalid assignment to `cls` argument in class method
|
8 | cls, blah = "apple", "orange" # PLW0642
9 | blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
10 | blah, [cls, blah2] = "apple", ("orange", "banana") # PLW0642
| ^^^ PLW0642
11 |
12 | @classmethod
|
self_or_cls_assignment.py:14:9: PLW0642 Invalid assignment to `cls` argument in class method
|
12 | @classmethod
13 | def add_fruits(cls, fruits, /) -> None:
14 | cls = fruits # PLW0642
| ^^^ PLW0642
15 |
16 | def print_color(self) -> None:
|
self_or_cls_assignment.py:17:9: PLW0642 Invalid assignment to `self` argument in instance method
|
16 | def print_color(self) -> None:
17 | self = "red" # PLW0642
| ^^^^ PLW0642
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
|
self_or_cls_assignment.py:18:9: PLW0642 Invalid assignment to `self` argument in instance method
|
16 | def print_color(self) -> None:
17 | self = "red" # PLW0642
18 | self: Self = "red" # PLW0642
| ^^^^ PLW0642
19 | self += "blue" # PLW0642
20 | *self = "blue" # PLW0642
|
self_or_cls_assignment.py:19:9: PLW0642 Invalid assignment to `self` argument in instance method
|
17 | self = "red" # PLW0642
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
| ^^^^ PLW0642
20 | *self = "blue" # PLW0642
21 | self, blah = "red", "blue" # PLW0642
|
self_or_cls_assignment.py:20:10: PLW0642 Invalid assignment to `self` argument in instance method
|
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
20 | *self = "blue" # PLW0642
| ^^^^ PLW0642
21 | self, blah = "red", "blue" # PLW0642
22 | blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:21:9: PLW0642 Invalid assignment to `self` argument in instance method
|
19 | self += "blue" # PLW0642
20 | *self = "blue" # PLW0642
21 | self, blah = "red", "blue" # PLW0642
| ^^^^ PLW0642
22 | blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642
23 | blah, [self, blah2] = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:22:16: PLW0642 Invalid assignment to `self` argument in instance method
|
20 | *self = "blue" # PLW0642
21 | self, blah = "red", "blue" # PLW0642
22 | blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642
| ^^^^ PLW0642
23 | blah, [self, blah2] = "apple", ("orange", "banana") # PLW0642
|
self_or_cls_assignment.py:23:16: PLW0642 Invalid assignment to `self` argument in instance method
|
21 | self, blah = "red", "blue" # PLW0642
22 | blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642
23 | blah, [self, blah2] = "apple", ("orange", "banana") # PLW0642
| ^^^^ PLW0642
24 |
25 | def print_color(self, color, /) -> None:
|
self_or_cls_assignment.py:26:9: PLW0642 Invalid assignment to `self` argument in instance method
|
25 | def print_color(self, color, /) -> None:
26 | self = color
| ^^^^ PLW0642
27 |
28 | def ok(self) -> None:
|

2
ruff.schema.json generated
View file

@ -3408,6 +3408,8 @@
"PLW0602", "PLW0602",
"PLW0603", "PLW0603",
"PLW0604", "PLW0604",
"PLW064",
"PLW0642",
"PLW07", "PLW07",
"PLW071", "PLW071",
"PLW0711", "PLW0711",