SONARPY-3939 Rule S8512 Class fields should not be defined multiple times (#977)

GitOrigin-RevId: 11a73bd19351302bd185b8ec1b9d3589129c9752

Author: sonar-nigel[bot]

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

@@ -0,0 +1,94 @@
+/*
+ * 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 java.util.Optional;
+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.AnnotatedAssignment;
+import org.sonar.plugins.python.api.tree.AssignmentStatement;
+import org.sonar.plugins.python.api.tree.ClassDef;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.ExpressionList;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Statement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.Tree.Kind;
+
+@Rule(key = "S8512")
+public class ClassFieldDefinedMultipleTimesCheck extends PythonSubscriptionCheck {
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Kind.CLASSDEF, ctx -> checkClass(ctx, (ClassDef) ctx.syntaxNode()));
+  }
+
+  private static void checkClass(SubscriptionContext ctx, ClassDef classDef) {
+    Map<String, List<Tree>> fieldDefinitions = new LinkedHashMap<>();
+
+    for (Statement stmt : classDef.body().statements()) {
+      collectFieldDefinition(stmt, fieldDefinitions);
+    }
+
+    reportDuplicateDefinitions(ctx, fieldDefinitions);
+  }
+
+  private static void collectFieldDefinition(Statement stmt, Map<String, List<Tree>> fieldDefinitions) {
+    if (stmt.is(Kind.ASSIGNMENT_STMT)) {
+      getAssignmentName((AssignmentStatement) stmt)
+        .ifPresent(name -> fieldDefinitions.computeIfAbsent(name.name(), k -> new ArrayList<>()).add(name));
+    } else if (stmt.is(Kind.ANNOTATED_ASSIGNMENT)) {
+      getAnnotatedAssignmentName((AnnotatedAssignment) stmt)
+        .ifPresent(name -> fieldDefinitions.computeIfAbsent(name.name(), k -> new ArrayList<>()).add(name));
+    }
+  }
+
+  private static Optional<Name> getAssignmentName(AssignmentStatement assignment) {
+    List<ExpressionList> lhsList = assignment.lhsExpressions();
+    if (lhsList.size() != 1) {
+      return Optional.empty();
+    }
+    List<Expression> expressions = lhsList.get(0).expressions();
+    if (expressions.size() == 1 && expressions.get(0).is(Kind.NAME)) {
+      return Optional.of((Name) expressions.get(0));
+    }
+    return Optional.empty();
+  }
+
+  private static Optional<Name> getAnnotatedAssignmentName(AnnotatedAssignment annotated) {
+    if (annotated.variable().is(Kind.NAME) && annotated.assignedValue() != null) {
+      return Optional.of((Name) annotated.variable());
+    }
+    return Optional.empty();
+  }
+
+  private static void reportDuplicateDefinitions(SubscriptionContext ctx, Map<String, List<Tree>> fieldDefinitions) {
+    fieldDefinitions.forEach((name, definitions) -> {
+      for (int i = 0; i < definitions.size() - 1; i++) {
+        Tree current = definitions.get(i);
+        Tree next = definitions.get(i + 1);
+        String message = String.format("Remove this assignment; \"%s\" is assigned again on line %d.", name, next.firstToken().line());
+        ctx.addIssue(current, message).secondary(next, "Reassignment.");
+      }
+    });
+  }
+}

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

@@ -159,6 +159,7 @@ public Stream<Class<?>> getChecks() {
       ChildRouterBeforeParentRegistrationCheck.class,
       CipherBlockChainingCheck.class,
       ClassComplexityCheck.class,
+      ClassFieldDefinedMultipleTimesCheck.class,
       ClassMethodFirstArgumentNameCheck.class,
       ClassNameCheck.class,
       ClearTextProtocolsCheck.class,

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

@@ -0,0 +1,32 @@
+<h2>Why is this an issue?</h2>
+<p>When a class field is defined multiple times in the same class body, only the last assignment takes effect. All previous assignments are dead code
+that serve no purpose.</p>
+<p>This pattern typically occurs due to copy-paste errors, refactoring mistakes, merge conflicts, or misunderstanding of how class attributes work.</p>
+<h3>What is the potential impact?</h3>
+<ul>
+  <li>Developers may not immediately realize which value is actually used.</li>
+  <li>Dead code must be read, understood, and maintained even though it has no effect.</li>
+  <li>The duplicate definition might indicate that two different fields were intended, suggesting a naming error.</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Remove all but the last assignment to the class field.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+class MyClass:
+    x = 1
+    y = 2
+    x = 3  # Noncompliant: "x" is defined twice
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+class MyClass:
+    x = 3
+    y = 2
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/tutorial/classes.html">Classes</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/reference/datamodel.html#the-standard-type-hierarchy">The standard type hierarchy</a></li>
+</ul>

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

@@ -0,0 +1,23 @@
+{
+  "title": "Class fields should not be defined multiple times",
+  "type": "CODE_SMELL",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CLEAR"
+  },
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "suspicious"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8512",
+  "sqKey": "S8512",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

@@ -0,0 +1,29 @@
+/*
+ * 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 ClassFieldDefinedMultipleTimesCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/classFieldDefinedMultipleTimes.py", new ClassFieldDefinedMultipleTimesCheck());
+  }
+
+}