Skip to content

Commit

Permalink
[pylint] Impement self-assigning-variable (W0127) (#6015)
Browse files Browse the repository at this point in the history
## Summary

Implements Pylint rule [`self-assigning-variable`
(`W0127`)](https://pylint.pycqa.org/en/latest/user_guide/messages/warning/self-assigning-variable.html)
as `self-assigning-variable` (`PLW0127`). Includes documentation.
Related to #970.

## Test Plan

`cargo test`
  • Loading branch information
tjkuson authored Jul 24, 2023
1 parent 574c0e0 commit 727153c
Show file tree
Hide file tree
Showing 8 changed files with 488 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
foo = 1
bar = 2
baz = 3

# Errors.
foo = foo
bar = bar
foo, bar = foo, bar
bar, foo = bar, foo
(foo, bar) = (foo, bar)
(bar, foo) = (bar, foo)
foo, (bar, baz) = foo, (bar, baz)
bar, (foo, baz) = bar, (foo, baz)
(foo, bar), baz = (foo, bar), baz
(foo, (bar, baz)) = (foo, (bar, baz))
foo, bar = foo, 1
bar, foo = bar, 1
(foo, bar) = (foo, 1)
(bar, foo) = (bar, 1)
foo, (bar, baz) = foo, (bar, 1)
bar, (foo, baz) = bar, (foo, 1)
(foo, bar), baz = (foo, bar), 1
(foo, (bar, baz)) = (foo, (bar, 1))
foo: int = foo
bar: int = bar

# Non-errors.
foo = bar
bar = foo
foo, bar = bar, foo
foo, bar = bar, foo
(foo, bar) = (bar, foo)
foo, bar = bar, 1
bar, foo = foo, 1
foo: int = bar
bar: int = 1


class Foo:
foo = foo
bar = bar
12 changes: 10 additions & 2 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,11 @@ where
self.diagnostics.push(diagnostic);
}
}
if self.settings.rules.enabled(Rule::SelfAssigningVariable) {
if let [target] = targets.as_slice() {
pylint::rules::self_assigning_variable(self, target, value);
}
}
if self.settings.rules.enabled(Rule::TypeParamNameMismatch) {
pylint::rules::type_param_name_mismatch(self, value, targets);
}
Expand Down Expand Up @@ -1635,8 +1640,8 @@ where
annotation,
..
}) => {
if self.enabled(Rule::LambdaAssignment) {
if let Some(value) = value {
if let Some(value) = value {
if self.enabled(Rule::LambdaAssignment) {
pycodestyle::rules::lambda_assignment(
self,
target,
Expand All @@ -1645,6 +1650,9 @@ where
stmt,
);
}
if self.enabled(Rule::SelfAssigningVariable) {
pylint::rules::self_assigning_variable(self, target, value);
}
}
if self.enabled(Rule::UnintentionalTypeAnnotation) {
flake8_bugbear::rules::unintentional_type_annotation(
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 @@ -214,6 +214,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R2004") => (RuleGroup::Unspecified, rules::pylint::rules::MagicValueComparison),
(Pylint, "R5501") => (RuleGroup::Unspecified, rules::pylint::rules::CollapsibleElseIf),
(Pylint, "W0120") => (RuleGroup::Unspecified, rules::pylint::rules::UselessElseOnLoop),
(Pylint, "W0127") => (RuleGroup::Unspecified, rules::pylint::rules::SelfAssigningVariable),
(Pylint, "W0129") => (RuleGroup::Unspecified, rules::pylint::rules::AssertOnStringLiteral),
(Pylint, "W0131") => (RuleGroup::Unspecified, rules::pylint::rules::NamedExprWithoutContext),
(Pylint, "W0406") => (RuleGroup::Unspecified, rules::pylint::rules::ImportSelf),
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 @@ -116,6 +116,7 @@ mod tests {
Rule::RepeatedEqualityComparisonTarget,
Path::new("repeated_equality_comparison_target.py")
)]
#[test_case(Rule::SelfAssigningVariable, Path::new("self_assigning_variable.py"))]
#[test_case(
Rule::SubprocessPopenPreexecFn,
Path::new("subprocess_popen_preexec_fn.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 @@ -32,6 +32,7 @@ pub(crate) use redefined_loop_name::*;
pub(crate) use repeated_equality_comparison_target::*;
pub(crate) use repeated_isinstance_calls::*;
pub(crate) use return_in_init::*;
pub(crate) use self_assigning_variable::*;
pub(crate) use single_string_slots::*;
pub(crate) use subprocess_popen_preexec_fn::*;
pub(crate) use sys_exit_alias::*;
Expand Down Expand Up @@ -84,6 +85,7 @@ mod redefined_loop_name;
mod repeated_equality_comparison_target;
mod repeated_isinstance_calls;
mod return_in_init;
mod self_assigning_variable;
mod single_string_slots;
mod subprocess_popen_preexec_fn;
mod sys_exit_alias;
Expand Down
70 changes: 70 additions & 0 deletions crates/ruff/src/rules/pylint/rules/self_assigning_variable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use rustpython_parser::ast::{self, Expr, Ranged};

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

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

/// ## What it does
/// Checks for self-assignment of variables.
///
/// ## Why is this bad?
/// Self-assignment of variables is redundant and likely a mistake.
///
/// ## Example
/// ```python
/// country = "Poland"
/// country = country
/// ```
///
/// Use instead:
/// ```python
/// country = "Poland"
/// ```
#[violation]
pub struct SelfAssigningVariable {
name: String,
}

impl Violation for SelfAssigningVariable {
#[derive_message_formats]
fn message(&self) -> String {
let SelfAssigningVariable { name } = self;
format!("Self-assignment of variable `{name}`")
}
}

/// PLW0127
pub(crate) fn self_assigning_variable(checker: &mut Checker, target: &Expr, value: &Expr) {
fn inner(left: &Expr, right: &Expr, diagnostics: &mut Vec<Diagnostic>) {
match (left, right) {
(
Expr::Tuple(ast::ExprTuple { elts: lhs_elts, .. }),
Expr::Tuple(ast::ExprTuple { elts: rhs_elts, .. }),
) if lhs_elts.len() == rhs_elts.len() => lhs_elts
.iter()
.zip(rhs_elts.iter())
.for_each(|(lhs, rhs)| inner(lhs, rhs, diagnostics)),
(
Expr::Name(ast::ExprName { id: lhs_name, .. }),
Expr::Name(ast::ExprName { id: rhs_name, .. }),
) if lhs_name == rhs_name => {
diagnostics.push(Diagnostic::new(
SelfAssigningVariable {
name: lhs_name.to_string(),
},
left.range(),
));
}
_ => {}
}
}

// Assignments in class bodies are attributes (e.g., `x = x` assigns `x` to `self.x`, and thus
// is not a self-assignment).
if checker.semantic().scope().kind.is_class() {
return;
}

inner(target, value, &mut checker.diagnostics);
}
Loading

0 comments on commit 727153c

Please sign in to comment.