Skip to content

Commit

Permalink
Omit unrolled augmented assignments in PIE794
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 12, 2023
1 parent 776eb87 commit b566bdd
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 4 deletions.
12 changes: 12 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE794.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,15 @@ class User:
foo: bool = BooleanField()
# ...
bar = StringField() # PIE794


class Person:
name = "Foo"
name = name + " Bar"
name = "Bar" # PIE794


class Person:
name: str = "Foo"
name: str = name + " Bar"
name: str = "Bar" # PIE794
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc_hash::FxHashSet;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::{AlwaysFixableViolation, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::any_over_expr;
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -55,14 +56,14 @@ pub(crate) fn duplicate_class_field_definition(checker: &mut Checker, body: &[St
// Extract the property name from the assignment statement.
let target = match stmt {
Stmt::Assign(ast::StmtAssign { targets, .. }) => {
if let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() {
if let [Expr::Name(id)] = targets.as_slice() {
id
} else {
continue;
}
}
Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => {
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
if let Expr::Name(id) = target.as_ref() {
id
} else {
continue;
Expand All @@ -71,10 +72,31 @@ pub(crate) fn duplicate_class_field_definition(checker: &mut Checker, body: &[St
_ => continue,
};

if !seen_targets.insert(target) {
// If this is an unrolled augmented assignment (e.g., `x = x + 1`), skip it.
match stmt {
Stmt::Assign(ast::StmtAssign { value, .. }) => {
if any_over_expr(value.as_ref(), &|expr| {
expr.as_name_expr().is_some_and(|name| name.id == target.id)
}) {
continue;
}
}
Stmt::AnnAssign(ast::StmtAnnAssign {
value: Some(value), ..
}) => {
if any_over_expr(value.as_ref(), &|expr| {
expr.as_name_expr().is_some_and(|name| name.id == target.id)
}) {
continue;
}
}
_ => continue,
}

if !seen_targets.insert(target.id.as_str()) {
let mut diagnostic = Diagnostic::new(
DuplicateClassFieldDefinition {
name: target.to_string(),
name: target.id.to_string(),
},
stmt.range(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,41 @@ PIE794.py:40:5: PIE794 [*] Class field `bar` is defined multiple times
38 38 | foo: bool = BooleanField()
39 39 | # ...
40 |- bar = StringField() # PIE794
41 40 |
42 41 |
43 42 | class Person:

PIE794.py:46:5: PIE794 [*] Class field `name` is defined multiple times
|
44 | name = "Foo"
45 | name = name + " Bar"
46 | name = "Bar" # PIE794
| ^^^^^^^^^^^^ PIE794
|
= help: Remove duplicate field definition for `name`

Unsafe fix
43 43 | class Person:
44 44 | name = "Foo"
45 45 | name = name + " Bar"
46 |- name = "Bar" # PIE794
47 46 |
48 47 |
49 48 | class Person:

PIE794.py:52:5: PIE794 [*] Class field `name` is defined multiple times
|
50 | name: str = "Foo"
51 | name: str = name + " Bar"
52 | name: str = "Bar" # PIE794
| ^^^^^^^^^^^^^^^^^ PIE794
|
= help: Remove duplicate field definition for `name`

Unsafe fix
49 49 | class Person:
50 50 | name: str = "Foo"
51 51 | name: str = name + " Bar"
52 |- name: str = "Bar" # PIE794


0 comments on commit b566bdd

Please sign in to comment.