SONARPY-1404 Rule S5642: "in" and "not in" operators should be used on objects supporting them (#1522)

Author: anton-haubner-sonarsource

its/ruling/src/test/java/org/sonar/python/it/RulingHelper.java

@@ -122,6 +122,7 @@ static List<String> bugRuleKeys() {
       "S5549",
       "S5607",
       "S5632",
+      "S5642",
       "S5644",
       "S5707",
       "S5708",

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

@@ -247,6 +247,7 @@ public static Iterable<Class> getChecks() {
       LoopExecutingAtMostOnceCheck.class,
       MandatoryFunctionParameterTypeHintCheck.class,
       MandatoryFunctionReturnTypeHintCheck.class,
+      MembershipTestSupportCheck.class,
       MethodNameCheck.class,
       MethodShouldBeStaticCheck.class,
       MissingDocstringCheck.class,

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

@@ -0,0 +1,169 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 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 GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import java.util.List;
+import java.util.stream.Collectors;
+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.symbols.AmbiguousSymbol;
+import org.sonar.plugins.python.api.symbols.Symbol;
+import org.sonar.plugins.python.api.symbols.Usage;
+import org.sonar.plugins.python.api.tree.AssignmentStatement;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.InExpression;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.InferredType;
+import org.sonar.python.tree.TreeUtils;
+import org.sonar.python.types.InferredTypes;
+
+@Rule(key = "S5642")
+public class MembershipTestSupportCheck extends PythonSubscriptionCheck {
+  private static final String PRIMARY_MESSAGE = "Change the type of %s";
+  private static final String PRIMARY_MESSAGE_MULTILINE = "Change the type for the target expression of `in`";
+  private static final String KNOWN_TYPE_MESSAGE = "; type %s does not support membership protocol.";
+  private static final String UNKNOWN_TYPE_MESSAGE = "; the type does not support the membership protocol.";
+  private static final String SECONDARY_MESSAGE = "The result value of this expression does not support the membership protocol.";
+
+  // The ordering of membership protocol methods matters here (for extreme edge cases).
+  //
+  // For instance, a class that has __contains__ set to None but which defines __iter__ does not fulfill the membership protocol
+  // A class that defines __contains__ and has __iter__ set to None does fulfill the membership protocol
+  //
+  // Hence, we check for these special methods in the same order as the python interpreter does at runtime.
+  private static final List<String> MEMBERSHIP_PROTOCOL_ENABLING_METHODS = List.of("__contains__", "__iter__", "__getitem__");
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.IN, ctx -> checkInExpression(ctx, (InExpression) ctx.syntaxNode()));
+  }
+
+  private static void checkInExpression(SubscriptionContext ctx, InExpression inExpression) {
+    Expression rhs = inExpression.rightOperand();
+    InferredType rhsType = rhs.type();
+
+    if (canSupportMembershipProtocol(rhsType)) {
+      return;
+    }
+
+    addIssueOnInAndNotIn(ctx, inExpression, genPrimaryMessage(rhs, rhsType))
+      .secondary(inExpression.rightOperand(), SECONDARY_MESSAGE);
+  }
+
+  private static String genPrimaryMessage(Expression rhs, InferredType rhsType) {
+    // Try to render rhs into the message, but only if it is not multiline to avoid generating messages that are too long.
+    String inTarget = TreeUtils.treeToString(rhs, false);
+    String message;
+    if (inTarget == null) {
+      message = PRIMARY_MESSAGE_MULTILINE;
+    } else {
+      message = String.format(PRIMARY_MESSAGE, inTarget);
+    }
+
+    var typeName = InferredTypes.typeName(rhsType);
+    if (typeName != null) {
+      message += String.format(KNOWN_TYPE_MESSAGE, typeName);
+    } else {
+      message += UNKNOWN_TYPE_MESSAGE;
+    }
+
+    return message;
+  }
+
+  private static PreciseIssue addIssueOnInAndNotIn(SubscriptionContext ctx, InExpression inExpression, String message) {
+    var notToken = inExpression.notToken();
+    if (notToken == null) {
+      return ctx.addIssue(inExpression.operator(), message);
+    }
+
+    return ctx.addIssue(notToken, inExpression.operator(), message);
+  }
+
+  private static boolean canSupportMembershipProtocol(InferredType type) {
+    for (var methodName : MEMBERSHIP_PROTOCOL_ENABLING_METHODS) {
+      switch (canMemberBeMethod(type, methodName)) {
+        case NOT_A_METHOD:
+          return false;
+        case METHOD:
+        case UNKNOWN:
+          return true;
+        default:
+      }
+    }
+
+    // all membership protocol methods are guaranteed not to be present
+    return false;
+  }
+
+  private enum MemberType {
+    METHOD,
+    NOT_A_METHOD,
+    NOT_PRESENT,
+    UNKNOWN
+  }
+
+  private static boolean canBeMethodSymbol(Symbol symbol) {
+    if (symbol.is(Symbol.Kind.FUNCTION)) {
+      return true;
+    }
+
+    // To avoid FPs, we accept OTHER unless we can show that it is a non-callable.
+    //
+    // This handles cases like __contains__ = other() or __contains__ = None.
+    // Although it is unclear, whether such edge cases really appear in the wild
+    if (symbol.is(Symbol.Kind.OTHER)) {
+      var bindingUsages = symbol.usages().stream().filter(Usage::isBindingUsage).limit(2).collect(Collectors.toUnmodifiableList());
+      if (bindingUsages.size() == 1) {
+        var bindingUsage = bindingUsages.get(0);
+        var assignment = TreeUtils.firstAncestorOfKind(bindingUsage.tree(), Tree.Kind.ASSIGNMENT_STMT);
+
+        return assignment == null || ((AssignmentStatement) assignment).assignedValue().type().canHaveMember("__call__");
+      }
+
+      return true;
+    }
+
+    if (symbol.is(Symbol.Kind.AMBIGUOUS)) {
+      return ((AmbiguousSymbol) symbol).alternatives().stream().anyMatch(MembershipTestSupportCheck::canBeMethodSymbol);
+    }
+
+    return false;
+  }
+
+  private static MemberType canMemberBeMethod(InferredType type, String methodName) {
+    var maybeMember = type.resolveMember(methodName);
+    if (maybeMember.isPresent()) {
+      var symbol = maybeMember.get();
+
+      if (canBeMethodSymbol(symbol)) {
+        return MemberType.METHOD;
+      }
+
+      return MemberType.NOT_A_METHOD;
+    }
+
+    if (!type.canHaveMember(methodName)) {
+      return MemberType.NOT_PRESENT;
+    }
+
+    return MemberType.UNKNOWN;
+  }
+}

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

@@ -0,0 +1,73 @@
+<p>This rule raises an issue when operators <code>in</code> and <code>not in</code> are called with a right operand not supporting membership
+protocol.</p>
+<h2>Why is this an issue?</h2>
+<p>Operators <code>in</code> and <code>not in</code>, also called <a
+href="https://docs.python.org/3/reference/expressions.html#membership-test-operations">"membership test operators"</a>, require that the right operand
+supports the membership protocol.</p>
+<p>In order to support the membership protocol, a user-defined class should implement at least one of the following methods:
+<code>__contains__</code>, <code>__iter__</code>, <code>__getitem__</code>.</p>
+<p>If none of these methods is implemented, a <code>TypeError</code> will be raised when performing a membership test.</p>
+<h2>How to fix it</h2>
+<h2>Code examples</h2>
+<h3>Noncompliant code example</h3>
+<pre>
+myint = 42
+
+if 42 in myint:  # Noncompliant: integers don't support membership protocol
+    ...
+
+class A:
+    def __init__(self, values):
+        self._values = values
+
+if "mystring" in A(["mystring"]):  # Noncompliant: class A doesn't support membership protocol
+    ...
+</pre>
+<h3>Compliant solution</h3>
+<pre>
+mylist = [42]
+
+if 42 in mylist:
+    ...
+
+class MyContains:
+    def __init__(self, values):
+        self._values = values
+
+    def __contains__(self, value):
+        return value in self._values
+
+if "mystring" in MyContains(["mystring"]):
+    ...
+
+# OR
+
+class MyIterable:
+    def __init__(self, values):
+        self._values = values
+
+    def __iter__(self):
+        return iter(self._values)
+
+if "mystring" in MyIterable(["mystring"]):
+    ...
+
+# OR
+
+class MyGetItem:
+    def __init__(self, values):
+        self._values = values
+
+    def __getitem__(self, key):
+        return self._values[key]
+
+if "mystring" in MyGetItem(["mystring"]):
+    ...
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Python Documentation - <a href="https://docs.python.org/3/reference/expressions.html#membership-test-operations">Membership test operations</a>
+  </li>
+</ul>
+

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

@@ -0,0 +1,15 @@
+{
+  "title": "\"in\" and \"not in\" operators should be used on objects supporting them",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "20min"
+  },
+  "tags": [],
+  "defaultSeverity": "Blocker",
+  "ruleSpecification": "RSPEC-5642",
+  "sqKey": "S5642",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

@@ -110,6 +110,7 @@
     "S5603",
     "S5607",
     "S5632",
+    "S5642",
     "S5644",
     "S5655",
     "S5659",

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

@@ -0,0 +1,31 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2023 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 GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks;
+
+import org.junit.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class MembershipTestSupportCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/membershipTestSupportCheck.py", new MembershipTestSupportCheck());
+  }
+}