SONARPY-3951 Rule S8521 Dictionary membership tests should not explicitly call ".keys()" (#993)

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

Author: 3 people

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

@@ -0,0 +1,62 @@
+/*
+ * 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.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.InExpression;
+import org.sonar.plugins.python.api.tree.QualifiedExpression;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+
+@Rule(key = "S8521")
+public class DictKeysMembershipTestCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Remove this unnecessary \"keys()\" call.";
+  private static final String KEYS_METHOD_NAME = "keys";
+  private static final TypeMatcher DICT_OR_SUBCLASS_KEYS_MATCHER =
+    TypeMatchers.isFunctionOwnerSatisfying(TypeMatchers.isOrExtendsType("builtins.dict"));
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.IN, DictKeysMembershipTestCheck::checkInExpression);
+  }
+
+  private static void checkInExpression(SubscriptionContext ctx) {
+    InExpression inExpression = (InExpression) ctx.syntaxNode();
+    if (!(inExpression.rightOperand() instanceof CallExpression callExpression)) {
+      return;
+    }
+    if (!callExpression.arguments().isEmpty()) {
+      return;
+    }
+    if (!isKeysCall(callExpression)) {
+      return;
+    }
+    if (DICT_OR_SUBCLASS_KEYS_MATCHER.isTrueFor(callExpression.callee(), ctx)) {
+      ctx.addIssue(callExpression, MESSAGE);
+    }
+  }
+
+  private static boolean isKeysCall(CallExpression callExpression) {
+    return callExpression.callee() instanceof QualifiedExpression qualifiedExpression
+      && KEYS_METHOD_NAME.equals(qualifiedExpression.name().name());
+  }
+}

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

@@ -187,6 +187,7 @@ public Stream<Class<?>> getChecks() {
       DedicatedAssertionCheck.class,
       DefaultFactoryArgumentCheck.class,
       DeprecatedNumpyTypesCheck.class,
+      DictKeysMembershipTestCheck.class,
       DictionaryDuplicateKeyCheck.class,
       DictionaryStaticKeyCheck.class,
       DirectTypeComparisonCheck.class,

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

@@ -0,0 +1,35 @@
+<p>This rule raises an issue when <code>.keys()</code> is called explicitly on a dictionary for membership testing.</p>
+<h2>Why is this an issue?</h2>
+<p>In Python, dictionaries support direct membership testing without needing to explicitly call the <code>.keys()</code> method. When you write
+<code>key in dict.keys()</code>, you are being unnecessarily verbose.</p>
+<p>The expression <code>key in dict</code> is the idiomatic Python way to check for key membership. It is:</p>
+<ul>
+  <li><em>more readable</em>: the intent is clearer and the code is shorter</li>
+  <li><em>slightly more efficient</em>: it avoids creating an intermediate dictionary view object</li>
+  <li><em>more Pythonic</em>: it follows Python's principle of having one obvious way to do it</li>
+</ul>
+<p>The <code>.keys()</code> method exists primarily for cases where you need to perform set operations on dictionary keys or when you need an explicit
+view object. For simple membership testing, it is redundant because dictionaries are designed to support the <code>in</code> operator directly.</p>
+<h2>How to fix it</h2>
+<p>Remove the <code>.keys()</code> call and use direct membership testing on the dictionary.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+my_dict = {'a': 1, 'b': 2, 'c': 3}
+
+if 'a' in my_dict.keys():  # Noncompliant
+    print('Found')
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+my_dict = {'a': 1, 'b': 2, 'c': 3}
+
+if 'a' in my_dict:
+    print('Found')
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#mapping-types-dict">Mapping Types — dict</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#dictionary-view-objects">Dictionary view objects</a></li>
+</ul>

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

@@ -0,0 +1,15 @@
+{
+  "title": "Dictionary membership tests should not explicitly call \".keys()\"",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "2min"
+  },
+  "tags": [],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-8521",
+  "sqKey": "S8521",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

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

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

@@ -0,0 +1,40 @@
+def noncompliant_basic():
+    my_dict = {'a': 1, 'b': 2}
+    if 'a' in my_dict.keys():  # Noncompliant {{Remove this unnecessary "keys()" call.}}
+#             ^^^^^^^^^^^^^^
+        print('Found')
+
+
+def noncompliant_not_in():
+    my_dict = {'a': 1, 'b': 2}
+    if 'a' not in my_dict.keys():  # Noncompliant {{Remove this unnecessary "keys()" call.}}
+#                 ^^^^^^^^^^^^^^
+        print('Not found')
+
+
+def compliant_direct_membership():
+    my_dict = {'a': 1, 'b': 2}
+    if 'a' in my_dict:
+        print('Found')
+
+
+def compliant_keys_with_argument():
+    my_dict = {'a': 1}
+    if 'a' in my_dict.keys('unexpected'):
+        print('Not flagged')
+
+
+def compliant_values_call():
+    my_dict = {'a': 1, 'b': 2}
+    if 1 in my_dict.values():
+        print('Found value')
+
+
+def compliant_custom_class():
+    class CustomMapping:
+        def keys(self):
+            return ['x', 'y']
+
+    obj = CustomMapping()
+    if 'x' in obj.keys():
+        print('Found')