Improve PIE794 autofix behavior (#1794)
We now: (1) trigger PIE794 for objects without bases (not sure why this was omitted before); and (2) remove the entire line, rather than leaving behind trailing whitespace. Resolves #1787.
This commit is contained in:
7
resources/test/fixtures/flake8_pie/PIE794.py
vendored
7
resources/test/fixtures/flake8_pie/PIE794.py
vendored
@@ -31,3 +31,10 @@ class User(BaseModel):
|
||||
@buzz.setter
|
||||
def buzz(self, value: str | int) -> None:
|
||||
...
|
||||
|
||||
|
||||
class User:
|
||||
bar: str = StringField()
|
||||
foo: bool = BooleanField()
|
||||
# ...
|
||||
bar = StringField() # PIE794
|
||||
|
||||
@@ -658,7 +658,7 @@ where
|
||||
}
|
||||
|
||||
if self.settings.enabled.contains(&RuleCode::PIE794) {
|
||||
flake8_pie::rules::dupe_class_field_definitions(self, bases, body);
|
||||
flake8_pie::rules::dupe_class_field_definitions(self, stmt, body);
|
||||
}
|
||||
|
||||
self.check_builtin_shadowing(name, stmt, false);
|
||||
|
||||
@@ -2,7 +2,7 @@ use log::error;
|
||||
use rustc_hash::FxHashSet;
|
||||
use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
|
||||
|
||||
use crate::ast::types::Range;
|
||||
use crate::ast::types::{Range, RefEquality};
|
||||
use crate::autofix::helpers::delete_stmt;
|
||||
use crate::autofix::Fix;
|
||||
use crate::checkers::ast::Checker;
|
||||
@@ -48,12 +48,14 @@ pub fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) {
|
||||
}
|
||||
|
||||
/// PIE794
|
||||
pub fn dupe_class_field_definitions(checker: &mut Checker, bases: &[Expr], body: &[Stmt]) {
|
||||
if bases.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut seen_targets = FxHashSet::default();
|
||||
pub fn dupe_class_field_definitions<'a, 'b>(
|
||||
checker: &mut Checker<'a>,
|
||||
parent: &'b Stmt,
|
||||
body: &'b [Stmt],
|
||||
) where
|
||||
'b: 'a,
|
||||
{
|
||||
let mut seen_targets: FxHashSet<&str> = FxHashSet::default();
|
||||
for stmt in body {
|
||||
// Extract the property name from the assignment statement.
|
||||
let target = match &stmt.node {
|
||||
@@ -77,17 +79,29 @@ pub fn dupe_class_field_definitions(checker: &mut Checker, bases: &[Expr], body:
|
||||
_ => continue,
|
||||
};
|
||||
|
||||
if seen_targets.contains(target) {
|
||||
if !seen_targets.insert(target) {
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
violations::DupeClassFieldDefinitions(target.to_string()),
|
||||
Range::from_located(stmt),
|
||||
);
|
||||
if checker.patch(&RuleCode::PIE794) {
|
||||
diagnostic.amend(Fix::deletion(stmt.location, stmt.end_location.unwrap()));
|
||||
let deleted: Vec<&Stmt> = checker
|
||||
.deletions
|
||||
.iter()
|
||||
.map(std::convert::Into::into)
|
||||
.collect();
|
||||
let locator = checker.locator;
|
||||
match delete_stmt(stmt, Some(parent), &deleted, locator) {
|
||||
Ok(fix) => {
|
||||
checker.deletions.insert(RefEquality(stmt));
|
||||
diagnostic.amend(fix);
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Failed to remove duplicate class definition: {}", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
} else {
|
||||
seen_targets.insert(target);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
source: src/flake8_pie/mod.rs
|
||||
expression: checks
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
DupeClassFieldDefinitions: name
|
||||
@@ -14,10 +14,10 @@ expression: checks
|
||||
content: ""
|
||||
location:
|
||||
row: 4
|
||||
column: 4
|
||||
column: 0
|
||||
end_location:
|
||||
row: 4
|
||||
column: 24
|
||||
row: 5
|
||||
column: 0
|
||||
parent: ~
|
||||
- kind:
|
||||
DupeClassFieldDefinitions: name
|
||||
@@ -31,10 +31,10 @@ expression: checks
|
||||
content: ""
|
||||
location:
|
||||
row: 13
|
||||
column: 4
|
||||
column: 0
|
||||
end_location:
|
||||
row: 13
|
||||
column: 24
|
||||
row: 14
|
||||
column: 0
|
||||
parent: ~
|
||||
- kind:
|
||||
DupeClassFieldDefinitions: bar
|
||||
@@ -48,9 +48,26 @@ expression: checks
|
||||
content: ""
|
||||
location:
|
||||
row: 23
|
||||
column: 4
|
||||
column: 0
|
||||
end_location:
|
||||
row: 23
|
||||
column: 23
|
||||
row: 24
|
||||
column: 0
|
||||
parent: ~
|
||||
- kind:
|
||||
DupeClassFieldDefinitions: bar
|
||||
location:
|
||||
row: 40
|
||||
column: 4
|
||||
end_location:
|
||||
row: 40
|
||||
column: 23
|
||||
fix:
|
||||
content: ""
|
||||
location:
|
||||
row: 40
|
||||
column: 0
|
||||
end_location:
|
||||
row: 41
|
||||
column: 0
|
||||
parent: ~
|
||||
|
||||
|
||||
Reference in New Issue
Block a user