SONARPY-2946 Rule S7506: Dictionary comprehensions shouldn't use a static key (#264)

GitOrigin-RevId: 77773b5151b63293a1148464a8be7de664ef2590

Author: thomas-serre-sonarsource

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

@@ -0,0 +1,61 @@
+/*
+ * 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 java.util.function.Predicate;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.tree.DictCompExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.StringElement;
+import org.sonar.plugins.python.api.tree.StringLiteral;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S7506")
+public class DictionaryStaticKeyCheck extends PythonSubscriptionCheck {
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.DICT_COMPREHENSION, ctx -> {
+      DictCompExpression dictCompExpression = (DictCompExpression) ctx.syntaxNode();
+      Expression keyExpression = dictCompExpression.keyExpression();
+      if (isLiteralDictKey(keyExpression) || isAssignedFromLiteralDictKey(keyExpression)) {
+        ctx.addIssue(keyExpression, "Don't use a static key in a dictionary comprehension.");
+      }
+    });
+  }
+
+  private static boolean isLiteralDictKey(Expression expression) {
+    if (expression instanceof StringLiteral stringLiteral) {
+      return !isFString(stringLiteral);
+    }
+    return false;
+  }
+
+  private static boolean isAssignedFromLiteralDictKey(Expression expression) {
+    return Expressions.ifNameGetSingleAssignedNonNameValue(expression)
+      .map(TreeUtils.toInstanceOfMapper(StringLiteral.class))
+      .filter(Predicate.not(DictionaryStaticKeyCheck::isFString))
+      .isPresent();
+  }
+
+  private static boolean isFString(StringLiteral stringLiteral) {
+    return stringLiteral.stringElements().stream().anyMatch(StringElement::isInterpolated);
+  }
+}

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

@@ -159,6 +159,7 @@ public Stream<Class<?>> getChecks() {
       DedicatedAssertionCheck.class,
       DeprecatedNumpyTypesCheck.class,
       DictionaryDuplicateKeyCheck.class,
+      DictionaryStaticKeyCheck.class,
       DirectTypeComparisonCheck.class,
       DisabledEFSEncryptionCheck.class,
       DisabledESDomainEncryptionCheck.class,

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

@@ -0,0 +1,44 @@
+<p>This rule raises an issue when a dictionary comprehension uses a static key.</p>
+<h2>Why is this an issue?</h2>
+<p>Dictionary comprehensions are a concise way to create dictionaries, typically by dynamically generating key-value pairs during iteration. When the
+key part of a dictionary comprehension is static (e.g., a string literal like <code>"key"</code> or a variable defined outside the comprehension that
+isn’t updated during the comprehension), each iteration of the comprehension will attempt to assign a value to this <strong>same single
+key</strong>.</p>
+<p>The consequence is that the dictionary will only contain one entry for that static key, and its value will be the one computed during the
+<strong>last</strong> iteration of the comprehension. This behavior is often a misunderstanding of how dictionary comprehensions work or a logical
+error, as the intention is usually to create multiple distinct key-value pairs.</p>
+<p>Consider this example:</p>
+<pre>
+data = ["apple", "banana", "cherry"]
+
+# Each iteration overwrites the value associated with "fruit_type"
+result_dict = {"fruit_type": value.capitalize() for value in data}
+# After the first iteration: {"fruit_type": "Apple"}
+# After the second iteration: {"fruit_type": "Banana"}
+# Final result: {"fruit_type": "Cherry"}
+</pre>
+<p>In the code above, the loop iterates three times, but because <code>"fruit_type"</code> is always the same key, the final dictionary
+<code>result_dict</code> will only be <code>{'fruit_type': 'CHERRY'}</code>. All previous assignments for this key are overwritten. This is usually
+not what the developer intends when using a comprehension over <code>data</code>.</p>
+<p>If the goal was to have multiple distinct keys, the key expression in the comprehension must vary with each iteration.</p>
+<h2>How to fix it</h2>
+<p>To fix this issue ensure that the key expression within the dictionary comprehension is dynamic, meaning it changes with each iteration, typically
+by using the iteration variable(s). This ensures that unique keys are generated, leading to a dictionary with multiple entries as usually
+intended.</p>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+data = ["some", "Data"]
+
+output_dict = {"key": value.upper() for value in data} # Noncompliant: "key" is not modify for each iteration
+</pre>
+<pre data-diff-id="1" data-diff-type="compliant">
+data = ["some", "Data"]
+
+output_dict = {value: value.upper() for value in data} # Compliant
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#dictionaries">Dictionaries</a> </li>
+  <li> Python Documentation - <a href="https://docs.python.org/3/reference/expressions.html#dictionary-displays">Dictionary displays</a> </li>
+</ul>
+

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

@@ -0,0 +1,21 @@
+{
+  "title": "Dictionary comprehension should not use a static key",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Critical",
+  "ruleSpecification": "RSPEC-7506",
+  "sqKey": "S7506",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "HIGH"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -263,6 +263,7 @@
     "S7498",
     "S7499",
     "S7501",
+    "S7506",
     "S7510",
     "S7511"
   ]

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

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

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

@@ -0,0 +1,26 @@
+import another_module
+
+data = ["some", "Data", "example"]
+my_dict = {"key": value.upper() for value in data} # Noncompliant {{Don't use a static key in a dictionary comprehension.}}
+#          ^^^^^
+
+key_literal = "key"
+my_dict = {key_literal: value.upper() for value in data} # Noncompliant {{Don't use a static key in a dictionary comprehension.}}
+#          ^^^^^^^^^^^
+
+another_key_literal = key_literal
+my_dict = {another_key_literal: value.upper() for value in data} # Noncompliant {{Don't use a static key in a dictionary comprehension.}}
+#          ^^^^^^^^^^^^^^^^^^^
+
+f_string_key = f"{value}_str"
+another_f_string_key = f_string_key
+my_dict = {another_f_string_key: value.upper() for value in data} # Compliant
+
+my_dict = {value: value.upper() for value in data} # Compliant
+
+key_from_another_module = another_module.key
+my_dict = {key_from_another_module: value.upper() for value in data} # Compliant
+
+my_dict = {f"{value}_str": value.upper() for value in data} # Compliant
+
+my_dict = {f"{key_literal}_str": value.upper() for value in data} # FN : interpolated string is constant