From d1079680bb29f6b797b5df15327195300f635f3c Mon Sep 17 00:00:00 2001
From: Gilles Peiffer <gilles.peiffer.yt@gmail.com>
Date: Sat, 29 Jun 2024 19:48:24 +0200
Subject: [PATCH] [`pylint`] Add fix for `duplicate-bases` (`PLE0241`) (#12105)

## 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`.
---
 .../test/fixtures/pylint/duplicate_bases.py   |  50 +++++-
 crates/ruff_linter/src/fix/edits.rs           |   6 +-
 .../src/rules/pylint/rules/duplicate_bases.rs |  26 ++-
 ...nt__tests__PLE0241_duplicate_bases.py.snap | 157 +++++++++++++++++-
 4 files changed, 225 insertions(+), 14 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/duplicate_bases.py b/crates/ruff_linter/resources/test/fixtures/pylint/duplicate_bases.py
index 491421ccf5eb2..e59b561ec70ac 100644
--- a/crates/ruff_linter/resources/test/fixtures/pylint/duplicate_bases.py
+++ b/crates/ruff_linter/resources/test/fixtures/pylint/duplicate_bases.py
@@ -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,
+):
     ...
 
 
diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs
index 0901a9f694a2f..6de9c25420ded 100644
--- a/crates/ruff_linter/src/fix/edits.rs
+++ b/crates/ruff_linter/src/fix/edits.rs
@@ -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,
diff --git a/crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs b/crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs
index a601e86900bd7..890ca16cdc53d 100644
--- a/crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs
+++ b/crates/ruff_linter/src/rules/pylint/rules/duplicate_bases.rs
@@ -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.
@@ -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);
             }
         }
     }
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0241_duplicate_bases.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0241_duplicate_bases.py.snap
index ecacbd9b02217..f939250ceb6f3 100644
--- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0241_duplicate_bases.py.snap
+++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0241_duplicate_bases.py.snap
@@ -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 |     ...