SONARPY-3926 Rule S8510 Loop variables should not be reused in nested loops (#974)

Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
GitOrigin-RevId: 09507f41abf92dc6af97af8697b92b4b9162c6c2

Author: 3 people

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

@@ -0,0 +1,117 @@
+/*
+ * 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 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.Expression;
+import org.sonar.plugins.python.api.tree.ForStatement;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.Tuple;
+
+@Rule(key = "S8510")
+public class NestedLoopVariableReuseCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Rename this loop variable; it shadows the outer loop variable \"%s\".";
+  private static final String SECONDARY_MESSAGE = "Outer loop variable.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, NestedLoopVariableReuseCheck::checkForStatement);
+  }
+
+  private static void checkForStatement(SubscriptionContext ctx) {
+    ForStatement forStatement = (ForStatement) ctx.syntaxNode();
+    List<Name> innerVarNames = extractLoopVarNames(forStatement);
+    if (innerVarNames.isEmpty()) {
+      return;
+    }
+    Map<Name, List<Name>> shadowedOuterNames = collectShadowedNames(forStatement, innerVarNames);
+    reportIssues(ctx, shadowedOuterNames);
+  }
+
+  private static Map<Name, List<Name>> collectShadowedNames(ForStatement forStatement, List<Name> innerVarNames) {
+    Map<Name, List<Name>> shadowedOuterNames = new LinkedHashMap<>();
+    for (Name innerName : innerVarNames) {
+      shadowedOuterNames.put(innerName, new ArrayList<>());
+    }
+    Tree current = forStatement.parent();
+    while (current != null) {
+      if (current.is(Tree.Kind.FUNCDEF, Tree.Kind.CLASSDEF, Tree.Kind.LAMBDA,
+        Tree.Kind.LIST_COMPREHENSION, Tree.Kind.SET_COMPREHENSION,
+        Tree.Kind.DICT_COMPREHENSION, Tree.Kind.GENERATOR_EXPR)) {
+        break;
+      }
+      if (current.is(Tree.Kind.FOR_STMT)) {
+        collectMatchingNames((ForStatement) current, innerVarNames, shadowedOuterNames);
+      }
+      current = current.parent();
+    }
+    return shadowedOuterNames;
+  }
+
+  private static void collectMatchingNames(ForStatement outerLoop, List<Name> innerVarNames, Map<Name, List<Name>> shadowedOuterNames) {
+    List<Name> outerVarNames = extractLoopVarNames(outerLoop);
+    for (Name innerName : innerVarNames) {
+      for (Name outerName : outerVarNames) {
+        if (innerName.name().equals(outerName.name())) {
+          shadowedOuterNames.get(innerName).add(outerName);
+        }
+      }
+    }
+  }
+
+  private static void reportIssues(SubscriptionContext ctx, Map<Name, List<Name>> shadowedOuterNames) {
+    for (Map.Entry<Name, List<Name>> entry : shadowedOuterNames.entrySet()) {
+      Name innerName = entry.getKey();
+      List<Name> outerNames = entry.getValue();
+      if (!outerNames.isEmpty()) {
+        var issue = ctx.addIssue(innerName, String.format(MESSAGE, innerName.name()));
+        for (Name outerName : outerNames) {
+          issue.secondary(outerName, SECONDARY_MESSAGE);
+        }
+      }
+    }
+  }
+
+  private static List<Name> extractLoopVarNames(ForStatement forStatement) {
+    List<Name> names = new ArrayList<>();
+    for (Expression expr : forStatement.expressions()) {
+      collectNames(expr, names);
+    }
+    return names;
+  }
+
+  private static void collectNames(Expression expr, List<Name> names) {
+    if (expr.is(Tree.Kind.NAME)) {
+      Name name = (Name) expr;
+      if (!name.name().startsWith("_")) {
+        names.add(name);
+      }
+    } else if (expr.is(Tree.Kind.TUPLE)) {
+      for (Expression element : ((Tuple) expr).elements()) {
+        collectNames(element, names);
+      }
+    }
+  }
+}

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

@@ -320,6 +320,7 @@ public Stream<Class<?>> getChecks() {
       NestedCollectionsCreationCheck.class,
       NestedConditionalExpressionCheck.class,
       NestedControlFlowDepthCheck.class,
+      NestedLoopVariableReuseCheck.class,
       NewStyleClassCheck.class,
       NonCallableCalledCheck.class,
       NonStandardCryptographicAlgorithmCheck.class,

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

@@ -0,0 +1,31 @@
+<p>This rule raises an issue when a nested loop uses the same variable name as an outer loop, causing the inner loop to shadow the outer loop variable.</p>
+<h2>Why is this an issue?</h2>
+<p>In Python, loop variables persist after the loop completes and can be reassigned by inner loops without any warning. Unlike languages with
+block-scoped variables, Python allows inner loops to silently overwrite outer loop variables, making the outer variable inaccessible within the
+inner loop scope.</p>
+<p>This pattern almost always indicates a programmer error. Common scenarios include copying loop patterns without changing variable names, or
+confusion about Python's scoping rules. The shadowing happens silently without any compiler or runtime warning, making these bugs difficult to spot.</p>
+<h2>How to fix it</h2>
+<p>Use different variable names for each loop. Choose descriptive names that reflect what each variable represents.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+for i in range(10):
+    print(f"Outer: {i}")
+    for i in range(5):  # Noncompliant
+        print(f"Inner: {i}")
+    # i now contains 4, not the outer loop value
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+for i in range(10):
+    print(f"Outer: {i}")
+    for j in range(5):
+        print(f"Inner: {j}")
+    # i still contains the outer loop value
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/reference/executionmodel.html#naming-and-binding">Naming and Binding</a></li>
+</ul>

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

@@ -0,0 +1,23 @@
+{
+  "title": "Loop variables should not be reused in nested loops",
+  "type": "CODE_SMELL",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CLEAR"
+  },
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "suspicious"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-8510",
+  "sqKey": "S8510",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

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