Skip to content

Commit

Permalink
[pylint] Add fix for duplicate-bases (PLE0241) (#12105)
Browse files Browse the repository at this point in the history
## Summary

This adds a fix for the `duplicate-bases` rule that removes the
duplicate base from the class definition.

## Test Plan

`cargo nextest run duplicate_bases`, `cargo insta review`.
  • Loading branch information
Peiffap authored Jun 29, 2024
1 parent da78de0 commit d107968
Show file tree
Hide file tree
Showing 4 changed files with 225 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,55 @@ class A:
...


class B(A, A):
class B:
...


# Duplicate base class is last.
class F1(A, A):
...


class F2(A, A,):
...


class F3(
A,
A
):
...


class F4(
A,
A,
):
...


# Duplicate base class is not last.
class G1(A, A, B):
...


class G2(A, A, B,):
...


class G3(
A,
A,
B
):
...


class G4(
A,
A,
B,
):
...


Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,11 @@ pub(crate) enum Parentheses {
}

/// Generic function to remove arguments or keyword arguments in function
/// calls and class definitions. (For classes `args` should be considered
/// `bases`)
/// calls and class definitions. (For classes, `args` should be considered
/// `bases`.)
///
/// Supports the removal of parentheses when this is the only (kw)arg left.
/// For this behavior, set `remove_parentheses` to `true`.
/// For this behavior, set `parentheses` to `Parentheses::Remove`.
pub(crate) fn remove_argument<T: Ranged>(
argument: &T,
arguments: &Arguments,
Expand Down
26 changes: 22 additions & 4 deletions crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use ruff_python_ast::{self as ast, Arguments, Expr};
use rustc_hash::{FxBuildHasher, FxHashSet};

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

use crate::checkers::ast::Checker;
use crate::fix::edits::{remove_argument, Parentheses};

/// ## What it does
/// Checks for duplicate base classes in class definitions.
Expand Down Expand Up @@ -42,30 +43,47 @@ pub struct DuplicateBases {
}

impl Violation for DuplicateBases {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let DuplicateBases { base, class } = self;
format!("Duplicate base `{base}` for class `{class}`")
}

fn fix_title(&self) -> Option<String> {
Some("Remove duplicate base".to_string())
}
}

/// PLE0241
pub(crate) fn duplicate_bases(checker: &mut Checker, name: &str, arguments: Option<&Arguments>) {
let Some(Arguments { args: bases, .. }) = arguments else {
let Some(arguments) = arguments else {
return;
};
let bases = &arguments.args;

let mut seen: FxHashSet<&str> = FxHashSet::with_capacity_and_hasher(bases.len(), FxBuildHasher);
for base in bases.iter() {
if let Expr::Name(ast::ExprName { id, .. }) = base {
if !seen.insert(id) {
checker.diagnostics.push(Diagnostic::new(
let mut diagnostic = Diagnostic::new(
DuplicateBases {
base: id.to_string(),
class: name.to_string(),
},
base.range(),
));
);
diagnostic.try_set_fix(|| {
remove_argument(
base,
arguments,
Parentheses::Remove,
checker.locator().contents(),
)
.map(Fix::safe_edit)
});
checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,156 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
duplicate_bases.py:8:12: PLE0241 Duplicate base `A` for class `B`
|
8 | class B(A, A):
| ^ PLE0241
9 | ...
|
duplicate_bases.py:13:13: PLE0241 [*] Duplicate base `A` for class `F1`
|
12 | # Duplicate base class is last.
13 | class F1(A, A):
| ^ PLE0241
14 | ...
|
= help: Remove duplicate base

Safe fix
10 10 |
11 11 |
12 12 | # Duplicate base class is last.
13 |-class F1(A, A):
13 |+class F1(A):
14 14 | ...
15 15 |
16 16 |

duplicate_bases.py:17:13: PLE0241 [*] Duplicate base `A` for class `F2`
|
17 | class F2(A, A,):
| ^ PLE0241
18 | ...
|
= help: Remove duplicate base

Safe fix
14 14 | ...
15 15 |
16 16 |
17 |-class F2(A, A,):
17 |+class F2(A,):
18 18 | ...
19 19 |
20 20 |

duplicate_bases.py:23:5: PLE0241 [*] Duplicate base `A` for class `F3`
|
21 | class F3(
22 | A,
23 | A
| ^ PLE0241
24 | ):
25 | ...
|
= help: Remove duplicate base

Safe fix
19 19 |
20 20 |
21 21 | class F3(
22 |- A,
23 22 | A
24 23 | ):
25 24 | ...

duplicate_bases.py:30:5: PLE0241 [*] Duplicate base `A` for class `F4`
|
28 | class F4(
29 | A,
30 | A,
| ^ PLE0241
31 | ):
32 | ...
|
= help: Remove duplicate base

Safe fix
27 27 |
28 28 | class F4(
29 29 | A,
30 |- A,
31 30 | ):
32 31 | ...
33 32 |

duplicate_bases.py:36:13: PLE0241 [*] Duplicate base `A` for class `G1`
|
35 | # Duplicate base class is not last.
36 | class G1(A, A, B):
| ^ PLE0241
37 | ...
|
= help: Remove duplicate base

Safe fix
33 33 |
34 34 |
35 35 | # Duplicate base class is not last.
36 |-class G1(A, A, B):
36 |+class G1(A, B):
37 37 | ...
38 38 |
39 39 |

duplicate_bases.py:40:13: PLE0241 [*] Duplicate base `A` for class `G2`
|
40 | class G2(A, A, B,):
| ^ PLE0241
41 | ...
|
= help: Remove duplicate base

Safe fix
37 37 | ...
38 38 |
39 39 |
40 |-class G2(A, A, B,):
40 |+class G2(A, B,):
41 41 | ...
42 42 |
43 43 |

duplicate_bases.py:46:5: PLE0241 [*] Duplicate base `A` for class `G3`
|
44 | class G3(
45 | A,
46 | A,
| ^ PLE0241
47 | B
48 | ):
|
= help: Remove duplicate base

Safe fix
43 43 |
44 44 | class G3(
45 45 | A,
46 |- A,
47 46 | B
48 47 | ):
49 48 | ...

duplicate_bases.py:54:5: PLE0241 [*] Duplicate base `A` for class `G4`
|
52 | class G4(
53 | A,
54 | A,
| ^ PLE0241
55 | B,
56 | ):
|
= help: Remove duplicate base

Safe fix
51 51 |
52 52 | class G4(
53 53 | A,
54 |- A,
55 54 | B,
56 55 | ):
57 56 | ...

0 comments on commit d107968

Please sign in to comment.