SONARPY-3941 Rule S8509 Classes should not inherit from the same base class multiple times (#979)

Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com>
Co-authored-by: joke1196 <david.kunzmann@sonarsource.com>
GitOrigin-RevId: a0640cc32e6b22f696b79fbd4b6cf710354ef909

Author: 3 people

python-checks/src/main/java/org/sonar/python/checks/DuplicateBaseClassCheck.java

@@ -0,0 +1,87 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.CheckForNull;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.ArgList;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.ClassDef;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.FullyQualifiedNameHelper;
+
+@Rule(key = "S8509")
+public class DuplicateBaseClassCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Remove this duplicate base class.";
+  private static final String SECONDARY_MESSAGE = "Already listed here.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, ctx -> checkClassDef((ClassDef) ctx.syntaxNode(), ctx));
+  }
+
+  private static void checkClassDef(ClassDef classDef, SubscriptionContext ctx) {
+    ArgList args = classDef.args();
+    if (args == null) {
+      return;
+    }
+    var duplicates = gatherDuplicates(args);
+    raiseOnDuplicates(duplicates, ctx);
+  }
+
+  private static Map<String, List<RegularArgument>> gatherDuplicates(ArgList args) {
+    Map<String, List<RegularArgument>> groups = new LinkedHashMap<>();
+    for (Argument argument : args.arguments()) {
+      if (argument instanceof RegularArgument regularArgument) {
+        if (regularArgument.keywordArgument() != null) {
+          continue;
+        }
+        String key = expressionKey(regularArgument.expression());
+        if (key != null) {
+          groups.computeIfAbsent(key, k -> new ArrayList<>()).add(regularArgument);
+        }
+      }
+    }
+    return groups;
+  }
+
+  private static void raiseOnDuplicates(Map<String, List<RegularArgument>> duplicates, SubscriptionContext ctx) {
+    for (List<RegularArgument> group : duplicates.values()) {
+      if (group.size() > 1) {
+        RegularArgument first = group.get(0);
+        PreciseIssue issue = ctx.addIssue(first.expression(), MESSAGE);
+        for (int i = 1; i < group.size(); i++) {
+          issue.secondary(group.get(i).expression(), SECONDARY_MESSAGE);
+        }
+      }
+    }
+  }
+
+  @CheckForNull
+  private static String expressionKey(Expression expression) {
+    return FullyQualifiedNameHelper.getFullyQualifiedName(expression.typeV2()).orElse(null);
+  }
+}

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

@@ -200,6 +200,7 @@ public Stream<Class<?>> getChecks() {
       DjangoRenderContextCheck.class,
       DoublePrefixOperatorCheck.class,
       DuplicateArgumentCheck.class,
+      DuplicateBaseClassCheck.class,
       DuplicatedMethodFieldNamesCheck.class,
       DuplicatedMethodImplementationCheck.class,
       DuplicatesInCharacterClassCheck.class,

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8509.html

@@ -0,0 +1,25 @@
+<p>This rule raises an issue when the same class appears more than once in the inheritance list of a class definition.</p>
+<h2>Why is this an issue?</h2>
+<p>In Python, a class definition can inherit from one or more base classes. When the same base class appears multiple times in the inheritance list,
+it serves no purpose and is always a programming error. Python's Method Resolution Order (MRO) ensures that each class appears only once in the
+inheritance chain, even if it is listed multiple times.</p>
+<p>This typically happens due to copy-paste errors, misunderstanding of inheritance, or refactoring mistakes.</p>
+<h2>How to fix it</h2>
+<p>Remove the duplicate base class from the inheritance list, keeping only one occurrence.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+class MyWidget(QWidget, Serializable, QWidget):  # Noncompliant
+    pass
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+class MyWidget(QWidget, Serializable):
+    pass
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/tutorial/classes.html#multiple-inheritance">Multiple Inheritance</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/glossary.html#term-method-resolution-order">Method Resolution Order</a></li>
+</ul>

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8509.json

@@ -0,0 +1,24 @@
+{
+  "title": "Classes should not inherit from the same base class multiple times",
+  "type": "CODE_SMELL",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CLEAR"
+  },
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "confusing",
+    "suspicious"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8509",
+  "sqKey": "S8509",
+  "scope": "All",
+  "quickfix": "unknown"
+}

python-checks/src/test/java/org/sonar/python/checks/DuplicateBaseClassCheckTest.java

@@ -0,0 +1,33 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class DuplicateBaseClassCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/duplicateBaseClass.py", new DuplicateBaseClassCheck());
+  }
+
+  @Test
+  void test_no_issue() {
+    PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/duplicateBaseClassCompliant.py", new DuplicateBaseClassCheck());
+  }
+}

python-checks/src/test/resources/checks/duplicateBaseClass.py

@@ -0,0 +1,99 @@
+from os import PathLike
+import my_class
+from my_class import A, B, C, X, mixin
+import a
+import mod
+# Examples with issues:
+
+# Duplicate base with a resolved fully-qualified symbol (exercises the FQN path in expressionKey)
+class ResolvedSymbolDuplicate(PathLike, PathLike): # Noncompliant {{Remove this duplicate base class.}}
+#                             ^^^^^^^^  ^^^^^^^^< {{Already listed here.}}
+    pass
+
+class SimpleEndDuplicate(A, B, A): # Noncompliant {{Remove this duplicate base class.}}
+#                        ^     ^< {{Already listed here.}}
+    pass
+
+class MidAndEndDuplicate(A, B, my_class.B): # Noncompliant {{Remove this duplicate base class.}}
+#                           ^  ^^^^^^^^^^< {{Already listed here.}}
+    pass
+
+class QualifiedDuplicate(mod.Base, Other, mod.Base): # Noncompliant {{Remove this duplicate base class.}}
+#                        ^^^^^^^^         ^^^^^^^^< {{Already listed here.}}
+    pass
+
+# One issue with two secondaries (2nd and 3rd are duplicates of 1st)
+class TripleDuplicate(A, A, A): # Noncompliant {{Remove this duplicate base class.}}
+#                     ^  ^< {{Already listed here.}}
+#                           ^@-1< {{Already listed here.}}
+    pass
+
+# metaclass keyword argument is skipped, duplicate among positional bases still flagged
+class WithMetaclassDuplicate(A, B, A, metaclass=Meta): # Noncompliant
+    pass
+
+# Two separate groups of duplicates (2 issues: one for A, one for B)
+class TwoPairsDuplicates(A, B, A, B): # Noncompliant 2
+    pass
+
+class ManyBasesDuplicate(A, B, C, D, B): # Noncompliant
+    pass
+
+class DeepQualifiedDuplicate(a.b.C, D, a.b.C): # Noncompliant
+    pass
+
+# One issue with two secondaries (2nd and 3rd are duplicates of 1st)
+class ThreeTimeDuplicate(mixin, mixin, mixin): # Noncompliant
+#                        ^^^^^  ^^^^^< ^^^^^<
+    pass
+
+# Star-unpacking is skipped; duplicate among remaining RegularArguments is still detected
+class StarAndDuplicate(*extra, A, A): # Noncompliant
+    pass
+
+# Name with a symbol but no fully-qualified name (parameter symbol has no FQN)
+def make_class(other):
+    class Inner(other, other): # FN
+        pass
+
+# Compliant examples:
+
+class NoBaseClass:
+    pass
+
+class SingleBase(A):
+    pass
+
+class MultipleUnique(A, B, C):
+    pass
+
+class MultipleUniqueQualified(mod.A, mod.B, mod.C):
+    pass
+
+class OnlyMetaclass(metaclass=Meta):
+    pass
+
+class UniqueBasesWithMetaclass(A, B, metaclass=Meta):
+    pass
+
+# Subscript expressions yield a null key, so duplicates are not detected
+class GenericSubscript(Generic[T]):
+    pass
+
+class TwoIdenticalSubscripts(Generic[T], Generic[T]):
+    pass
+
+class SubscriptAndName(Generic[T], Base):
+    pass
+
+# Call expressions yield a null key, so duplicates are not detected
+class TwoCallBases(make_base(), make_base()):
+    pass
+
+# Qualified name whose qualifier is a call result — null key, no issue
+class CallQualifiedBase(obj.method().Attr, obj.method().Attr):
+    pass
+
+# Deep qualified expression where an intermediate qualifier is a call result — null key via recursive path
+class DeepCallQualifiedBase(a.b().c.D, a.b().c.D):
+    pass

python-checks/src/test/resources/checks/duplicateBaseClassCompliant.py

@@ -0,0 +1,32 @@
+class NoBaseClass:
+    pass
+
+class SingleBase(A):
+    pass
+
+class MultipleUnique(A, B, C):
+    pass
+
+class MultipleUniqueQualified(mod.A, mod.B, mod.C):
+    pass
+
+class OnlyMetaclass(metaclass=Meta):
+    pass
+
+class UniqueBasesWithMetaclass(A, B, metaclass=Meta):
+    pass
+
+class GenericSubscript(Generic[T]):
+    pass
+
+class TwoIdenticalSubscripts(Generic[T], Generic[T]):
+    pass
+
+class SubscriptAndName(Generic[T], Base):
+    pass
+
+class TwoCallBases(make_base(), make_base()):
+    pass
+
+class CallQualifiedBase(obj.method().Attr, obj.method().Attr):
+    pass