Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] - implement redeclared-assigned-name (W0128) #9268

Merged
merged 5 commits into from Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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
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
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
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
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
@@ -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.
///
zanieb marked this conversation as resolved.
Show resolved Hide resolved
/// ## 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());
}
_ => {}
}
}
@@ -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.