Skip to content

Commit

Permalink
[pylint] - implement redeclared-assigned-name (W0128) (#9268)
Browse files Browse the repository at this point in the history
## Summary

Implements
[`W0128`/`redeclared-assigned-name`](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/redeclared-assigned-name.html)

See: #970 

## Test Plan

`cargo test`
  • Loading branch information
diceroll123 authored Mar 15, 2024
1 parent 7e652e8 commit 740c08b
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FIRST, FIRST = (1, 2) # PLW0128
FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128

FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK
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 @@ -1389,6 +1389,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => {
if checker.enabled(Rule::RedeclaredAssignedName) {
pylint::rules::redeclared_assigned_name(checker, targets);
}
if checker.enabled(Rule::LambdaAssignment) {
if let [target] = &targets[..] {
pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt);
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 @@ -294,6 +294,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0108") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryLambda),
(Pylint, "W0120") => (RuleGroup::Stable, rules::pylint::rules::UselessElseOnLoop),
(Pylint, "W0127") => (RuleGroup::Stable, rules::pylint::rules::SelfAssigningVariable),
(Pylint, "W0128") => (RuleGroup::Preview, rules::pylint::rules::RedeclaredAssignedName),
(Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral),
(Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext),
(Pylint, "W0133") => (RuleGroup::Preview, rules::pylint::rules::UselessExceptionStatement),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ mod tests {
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))]
#[test_case(Rule::NonSlotAssignment, Path::new("non_slot_assignment.py"))]
#[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))]
#[test_case(Rule::RedeclaredAssignedName, Path::new("redeclared_assigned_name.py"))]
#[test_case(
Rule::RedefinedArgumentFromLocal,
Path::new("redefined_argument_from_local.py")
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub(crate) use non_slot_assignment::*;
pub(crate) use nonlocal_without_binding::*;
pub(crate) use potential_index_error::*;
pub(crate) use property_with_parameters::*;
pub(crate) use redeclared_assigned_name::*;
pub(crate) use redefined_argument_from_local::*;
pub(crate) use redefined_loop_name::*;
pub(crate) use repeated_equality_comparison::*;
Expand Down Expand Up @@ -136,6 +137,7 @@ mod non_slot_assignment;
mod nonlocal_without_binding;
mod potential_index_error;
mod property_with_parameters;
mod redeclared_assigned_name;
mod redefined_argument_from_local;
mod redefined_loop_name;
mod repeated_equality_comparison;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use ruff_python_ast::{self as ast, Expr};

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

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

/// ## What it does
/// Checks for declared assignments to the same variable multiple times
/// in the same assignment.
///
/// ## Why is this bad?
/// Assigning a variable multiple times in the same assignment is redundant,
/// as the final assignment to the variable is what the value will be.
///
/// ## Example
/// ```python
/// a, b, a = (1, 2, 3)
/// print(a) # 3
/// ```
///
/// Use instead:
/// ```python
/// # this is assuming you want to assign 3 to `a`
/// _, b, a = (1, 2, 3)
/// print(a) # 3
/// ```
///
#[violation]
pub struct RedeclaredAssignedName {
name: String,
}

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

/// PLW0128
pub(crate) fn redeclared_assigned_name(checker: &mut Checker, targets: &Vec<Expr>) {
let mut names: Vec<String> = Vec::new();

for target in targets {
check_expr(checker, target, &mut names);
}
}

fn check_expr(checker: &mut Checker, expr: &Expr, names: &mut Vec<String>) {
match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
for target in elts {
check_expr(checker, target, names);
}
}
Expr::Name(ast::ExprName { id, .. }) => {
if checker.settings.dummy_variable_rgx.is_match(id) {
// Ignore dummy variable assignments
return;
}
if names.contains(id) {
checker.diagnostics.push(Diagnostic::new(
RedeclaredAssignedName {
name: id.to_string(),
},
expr.range(),
));
}
names.push(id.to_string());
}
_ => {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
redeclared_assigned_name.py:1:8: PLW0128 Redeclared variable `FIRST` in assignment
|
1 | FIRST, FIRST = (1, 2) # PLW0128
| ^^^^^ PLW0128
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
|

redeclared_assigned_name.py:2:9: PLW0128 Redeclared variable `FIRST` in assignment
|
1 | FIRST, FIRST = (1, 2) # PLW0128
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
| ^^^^^ PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128
|

redeclared_assigned_name.py:3:9: PLW0128 Redeclared variable `FIRST` in assignment
|
1 | FIRST, FIRST = (1, 2) # PLW0128
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
| ^^^^^ PLW0128
4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128
|

redeclared_assigned_name.py:3:32: PLW0128 Redeclared variable `FIRST` in assignment
|
1 | FIRST, FIRST = (1, 2) # PLW0128
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
| ^^^^^ PLW0128
4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128
|

redeclared_assigned_name.py:4:23: PLW0128 Redeclared variable `FIRST` in assignment
|
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128
| ^^^^^ PLW0128
5 |
6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK
|

redeclared_assigned_name.py:4:30: PLW0128 Redeclared variable `SECOND` in assignment
|
2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128
3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128
4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128
| ^^^^^^ PLW0128
5 |
6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK
|


1 change: 1 addition & 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 740c08b

Please sign in to comment.