SONARPY-2996 Rule S7517: Use dict.items() to iterate over a dictionary (#306)

GitOrigin-RevId: 9da82d50ff31eb00406c9a5b564f0a42459b6082

Author: maksim-grebeniuk-sonarsource

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

@@ -0,0 +1,66 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * 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.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.ComprehensionFor;
+import org.sonar.plugins.python.api.tree.ForStatement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.Tuple;
+import org.sonar.plugins.python.api.types.v2.TriBool;
+import org.sonar.python.types.v2.TypeCheckBuilder;
+
+@Rule(key = "S7517")
+public class LoopOverDictKeyValuesCheck extends PythonSubscriptionCheck {
+  public static final String DICT_FQN = "dict";
+  public static final String MESSAGE = "Use items to iterate over key-value pairs";
+  private TypeCheckBuilder dictTypeCheck;
+
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initChecks);
+    context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::checkForStatement);
+    context.registerSyntaxNodeConsumer(Tree.Kind.COMP_FOR, this::checkComprehensionFor);
+  }
+
+  private void initChecks(SubscriptionContext ctx) {
+    dictTypeCheck = ctx.typeChecker().typeCheckBuilder().isInstanceOf(DICT_FQN);
+  }
+
+  private void checkForStatement(SubscriptionContext ctx) {
+    var forStatement = (ForStatement) ctx.syntaxNode();
+    var expressions = forStatement.expressions();
+    var testExpressions = forStatement.testExpressions();
+    if (expressions.size() == 2
+        && testExpressions.size() == 1
+        && dictTypeCheck.check(testExpressions.get(0).typeV2()) == TriBool.TRUE) {
+      ctx.addIssue(testExpressions.get(0), MESSAGE);
+    }
+  }
+
+  private void checkComprehensionFor(SubscriptionContext ctx) {
+    var comprehensionFor = (ComprehensionFor) ctx.syntaxNode();
+    if (comprehensionFor.loopExpression() instanceof Tuple tuple
+        && tuple.elements().size() == 2
+        && dictTypeCheck.check(comprehensionFor.iterable().typeV2()) == TriBool.TRUE) {
+      ctx.addIssue(comprehensionFor.iterable(), MESSAGE);
+    }
+  }
+}

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

@@ -268,6 +268,7 @@ public Stream<Class<?>> getChecks() {
       LoggersConfigurationCheck.class,
       LongIntegerWithLowercaseSuffixUsageCheck.class,
       LoopExecutingAtMostOnceCheck.class,
+      LoopOverDictKeyValuesCheck.class,
       MandatoryFunctionParameterTypeHintCheck.class,
       MandatoryFunctionReturnTypeHintCheck.class,
       MembershipTestSupportCheck.class,

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

@@ -0,0 +1,38 @@
+<p>This rule raises an issue when a dictionary is iterated over without explicitly calling the <code>.items()</code> method.</p>
+<h2>Why is this an issue?</h2>
+<p>When iterating directly over a dictionary e.g., <code>for k, v in some_dict:</code> or <code>{k: v for k, v in some_dict}</code>, Python iterates
+over the dictionary’s keys by default. If the intention is to access both the key and the value, omitting <code>.items()</code> leads to unexpected
+behavior. In such cases, the <code>k</code> variable would receive the key, and the <code>v</code> variable would attempt to unpack the key itself,
+which can lead to errors or subtle bugs if the key is iterable, like a <code>string</code>. For example, if a key is a <code>string</code> like
+<code>"hi"</code>, <code>k</code> would be <code>'h'</code> and <code>v</code> would be <code>'i'</code>.</p>
+<h2>How to fix it</h2>
+<p>To fix this, simply append <code>.items()</code> to your dictionary when iterating.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+some_dict = { "k1": "v1", "k2": "v2"}
+
+{k: v for k, v in some_dict} # Noncompliant: `v` will not receive the value, but the first character of the key
+</pre>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+some_dict = { "k1": "v1", "k2": "v2"}
+
+for k, v in some_dict: # Noncompliant: `v` will not receive the value, but the first character of the key
+  ...
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+some_dict = { "k1": "v1", "k2": "v2"}
+{k: v for k, v in some_dict.items()}
+</pre>
+<pre data-diff-id="2" data-diff-type="compliant">
+some_dict = { "k1": "v1", "k2": "v2"}
+for k, v in some_dict.items():
+  ...
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#looping-techniques">Looping Techniques</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Iteration over a dictionary key value pairs should be done with the items() method call",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "1min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-7517",
+  "sqKey": "S7517",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -280,6 +280,7 @@
     "S7508",
     "S7510",
     "S7511",
-    "S7512"
+    "S7512",
+    "S7517"
   ]
 }

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

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * 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 LoopOverDictKeyValuesCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/loopOverDictKeyValues.py", new LoopOverDictKeyValuesCheck());
+  }
+
+}

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

@@ -0,0 +1,29 @@
+
+def case1():
+  some_dict = { "hi": "hello"}
+  for k, v in some_dict: # Noncompliant {{Use items to iterate over key-value pairs}}
+    #         ^^^^^^^^^
+    ...
+  {k: v for k, v in some_dict} # Noncompliant {{Use items to iterate over key-value pairs}}
+  #                 ^^^^^^^^^
+
+def case2():
+  some_dict = { "hi": "hello"}
+  for k, v in some_dict.items():
+    ...
+  {k: v for k, v in some_dict.items()}
+
+def case3():
+  not_a_dict = ["hi", "hello"]
+  for k, v in not_a_dict:
+    ...
+  {k: v for k, v in not_a_dict}
+
+def case4():
+  some_dict = { "hi": "hello"}
+  for k in some_dict:
+    ...
+  for k, v in some_dict, something_else:
+    ...
+  {k: 1 for k in some_dict}
+  {k: 1 for k, v, i in not_a_dict}