SonarSource · guillaume-dequenne · May 19, 2020 · May 18, 2020 · May 19, 2020
diff --git a/python-checks/src/main/java/org/sonar/python/checks/AbstractDuplicateKeyCheck.java b/python-checks/src/main/java/org/sonar/python/checks/AbstractDuplicateKeyCheck.java
@@ -0,0 +1,111 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.NumericLiteral;
+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.plugins.python.api.tree.Tuple;
+
+public abstract class AbstractDuplicateKeyCheck extends PythonSubscriptionCheck {
+
+  // Avoid performance issues for big dictionary/set literals
+  static final int SIZE_THRESHOLD = 100;
+
+  boolean isSameKey(Expression key, Expression comparedKey) {
+    if (key.is(Tree.Kind.TUPLE) && comparedKey.is(Tree.Kind.TUPLE)) {
+      return areEquivalentTuples((Tuple) key, (Tuple) comparedKey);
+    }
+    if (key.is(Tree.Kind.STRING_LITERAL) && comparedKey.is(Tree.Kind.STRING_LITERAL)) {
+      return areEquivalentStringLiterals((StringLiteral) key, (StringLiteral) comparedKey);
+    }
+    if (isANumber(key) && isANumber(comparedKey)) {
+      return areEquivalentNumbers(key, comparedKey);
+    }
+    return !key.is(Tree.Kind.CALL_EXPR) && CheckUtils.areEquivalent(key, comparedKey);
+  }
+
+  private boolean areEquivalentTuples(Tuple key, Tuple comparedKey) {
+    List<Expression> first = key.elements();
+    List<Expression> second = comparedKey.elements();
+    if (first.size() != second.size()) {
+      return false;
+    }
+    for (int i = 0; i < first.size(); i++) {
+      if (!isSameKey(first.get(i), second.get(i))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  private boolean areEquivalentNumbers(Tree key, Tree comparedKey) {
+    // BigDecimal#compareTo is required as equals() returns true only with identical scales
+    return toBigDecimal(key).compareTo(toBigDecimal(comparedKey)) == 0;
+  }
+
+  private BigDecimal toBigDecimal(Tree numberTree) {
+    if (numberTree.is(Tree.Kind.NUMERIC_LITERAL)) {
+      return parseAsBigDecimal(((NumericLiteral) numberTree).valueAsString());
+    }
+    return ((Name) numberTree).name().equals("True") ? BigDecimal.ONE : BigDecimal.ZERO;
+  }
+
+  private static boolean areEquivalentStringLiterals(StringLiteral key, StringLiteral comparedKey) {
+    if (key.stringElements().stream().anyMatch(StringElement::isInterpolated) || comparedKey.stringElements().stream().anyMatch(StringElement::isInterpolated)) {
+      return false;
+    }
+    if (key.trimmedQuotesValue().equals(comparedKey.trimmedQuotesValue())) {
+      String keyWithPrefixes = key.stringElements().stream()
+        .map(s -> s.prefix().toLowerCase(Locale.ENGLISH) + s.trimmedQuotesValue()).collect(Collectors.joining());
+      String comparedKeyWithPrefixes = comparedKey.stringElements().stream()
+        .map(s -> s.prefix().toLowerCase(Locale.ENGLISH) + s.trimmedQuotesValue()).collect(Collectors.joining());
+      return keyWithPrefixes.equals(comparedKeyWithPrefixes);
+    }
+    return false;
+  }
+
+  public BigDecimal parseAsBigDecimal(String numberLiteralValue) {
+    String numberValue = numberLiteralValue.replace("_", "");
+    if (numberValue.startsWith("0b") || numberValue.startsWith("0B")) {
+      return new BigDecimal(new BigInteger(numberValue.substring(2), 2));
+    }
+    if (numberValue.startsWith("0o") || numberValue.startsWith("0O")) {
+      return new BigDecimal(new BigInteger(numberValue.substring(2), 8));
+    }
+    if (numberValue.startsWith("0x") || numberValue.startsWith("0X")) {
+      return new BigDecimal(new BigInteger(numberValue.substring(2), 16));
+    }
+    return new BigDecimal(numberValue);
+  }
+
+  private static boolean isANumber(Tree tree) {
+    return tree.is(Tree.Kind.NUMERIC_LITERAL) || (tree.is(Tree.Kind.NAME) && (((Name) tree).name().equals("True") || ((Name) tree).name().equals("False")));
+  }
+}
diff --git a/python-checks/src/main/java/org/sonar/python/checks/CheckList.java b/python-checks/src/main/java/org/sonar/python/checks/CheckList.java
@@ -170,6 +170,7 @@ public static Iterable<Class> getChecks() {
       SecureCookieCheck.class,
       SecureModeEncryptionAlgorithmsCheck.class,
       SelfAssignmentCheck.class,
+      SetDuplicateKeyCheck.class,
       SillyEqualityCheck.class,
       SillyIdentityCheck.class,
       SpecialMethodParamListCheck.class,

diff --git a/python-checks/src/main/java/org/sonar/python/checks/DictionaryDuplicateKeyCheck.java b/python-checks/src/main/java/org/sonar/python/checks/DictionaryDuplicateKeyCheck.java
@@ -19,36 +19,26 @@
  */
 package org.sonar.python.checks;
 
-import java.math.BigDecimal;
-import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Locale;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.sonar.check.Rule;
-import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.tree.DictionaryLiteral;
 import org.sonar.plugins.python.api.tree.DictionaryLiteralElement;
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.KeyValuePair;
-import org.sonar.plugins.python.api.tree.Name;
-import org.sonar.plugins.python.api.tree.NumericLiteral;
-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.plugins.python.api.tree.Tuple;
 
 @Rule(key = "S5780")
-public class DictionaryDuplicateKeyCheck extends PythonSubscriptionCheck {
+public class DictionaryDuplicateKeyCheck extends AbstractDuplicateKeyCheck {
 
   @Override
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Tree.Kind.DICTIONARY_LITERAL, ctx -> {
       DictionaryLiteral dictionaryLiteral = (DictionaryLiteral) ctx.syntaxNode();
       Set<Integer> issueIndexes = new HashSet<>();
-      if (dictionaryLiteral.elements().size() > 100) {
+      if (dictionaryLiteral.elements().size() > SIZE_THRESHOLD) {
         return;
       }
       for (int i = 0; i < dictionaryLiteral.elements().size(); i++) {
@@ -80,75 +70,4 @@ private List<Tree> findIdenticalKeys(int startIndex, List<DictionaryLiteralEleme
     }
     return duplicates;
   }
-
-  private boolean isSameKey(Expression key, Expression comparedKey) {
-    if (key.is(Tree.Kind.TUPLE) && comparedKey.is(Tree.Kind.TUPLE)) {
-      return areEquivalentTuples((Tuple) key, (Tuple) comparedKey);
-    }
-    if (key.is(Tree.Kind.STRING_LITERAL) && comparedKey.is(Tree.Kind.STRING_LITERAL)) {
-      return areEquivalentStringLiterals((StringLiteral) key, (StringLiteral) comparedKey);
-    }
-    if (isANumber(key) && isANumber(comparedKey)) {
-      return areEquivalentNumbers(key, comparedKey);
-    }
-    return !key.is(Tree.Kind.CALL_EXPR) && CheckUtils.areEquivalent(key, comparedKey);
-  }
-
-  private boolean areEquivalentTuples(Tuple key, Tuple comparedKey) {
-    List<Expression> first = key.elements();
-    List<Expression> second = comparedKey.elements();
-    if (first.size() != second.size()) {
-      return false;
-    }
-    for (int i = 0; i < first.size(); i++) {
-      if (!isSameKey(first.get(i), second.get(i))) {
-        return false;
-      }
-    }
-    return true;
-  }
-
-  private boolean areEquivalentNumbers(Tree key, Tree comparedKey) {
-    // BigDecimal#compareTo is required as equals() returns true only with identical scales
-    return toBigDecimal(key).compareTo(toBigDecimal(comparedKey)) == 0;
-  }
-
-  private BigDecimal toBigDecimal(Tree numberTree) {
-    if (numberTree.is(Tree.Kind.NUMERIC_LITERAL)) {
-      return parseAsBigDecimal(((NumericLiteral) numberTree).valueAsString());
-    }
-    return ((Name) numberTree).name().equals("True") ? BigDecimal.ONE : BigDecimal.ZERO;
-  }
-
-  private static boolean areEquivalentStringLiterals(StringLiteral key, StringLiteral comparedKey) {
-    if (key.stringElements().stream().anyMatch(StringElement::isInterpolated) || comparedKey.stringElements().stream().anyMatch(StringElement::isInterpolated)) {
-      return false;
-    }
-    if (key.trimmedQuotesValue().equals(comparedKey.trimmedQuotesValue())) {
-      String keyWithPrefixes = key.stringElements().stream()
-        .map(s -> s.prefix().toLowerCase(Locale.ENGLISH) + s.trimmedQuotesValue()).collect(Collectors.joining());
-      String comparedKeyWithPrefixes = comparedKey.stringElements().stream()
-        .map(s -> s.prefix().toLowerCase(Locale.ENGLISH) + s.trimmedQuotesValue()).collect(Collectors.joining());
-      return keyWithPrefixes.equals(comparedKeyWithPrefixes);
-    }
-    return false;
-  }
-
-  public BigDecimal parseAsBigDecimal(String numberLiteralValue) {
-    String numberValue = numberLiteralValue.replace("_", "");
-    if (numberValue.startsWith("0b") || numberValue.startsWith("0B")) {
-      return new BigDecimal(new BigInteger(numberValue.substring(2), 2));
-    }
-    if (numberValue.startsWith("0o") || numberValue.startsWith("0O")) {
-      return new BigDecimal(new BigInteger(numberValue.substring(2), 8));
-    }
-    if (numberValue.startsWith("0x") || numberValue.startsWith("0X")) {
-      return new BigDecimal(new BigInteger(numberValue.substring(2), 16));
-    }
-    return new BigDecimal(numberValue);
-  }
-
-  private static boolean isANumber(Tree tree) {
-    return tree.is(Tree.Kind.NUMERIC_LITERAL) || (tree.is(Tree.Kind.NAME) && (((Name) tree).name().equals("True") || ((Name) tree).name().equals("False")));
-  }
 }
diff --git a/python-checks/src/main/java/org/sonar/python/checks/SetDuplicateKeyCheck.java b/python-checks/src/main/java/org/sonar/python/checks/SetDuplicateKeyCheck.java
@@ -0,0 +1,68 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.SetLiteral;
+import org.sonar.plugins.python.api.tree.Tree;
+
+@Rule(key = "S5781")
+public class SetDuplicateKeyCheck extends AbstractDuplicateKeyCheck {
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.SET_LITERAL, ctx -> {
+      SetLiteral setLiteral = (SetLiteral) ctx.syntaxNode();
+      Set<Integer> issueIndexes = new HashSet<>();
+      if (setLiteral.elements().size() > SIZE_THRESHOLD) {
+        return;
+      }
+      for (int i = 0; i < setLiteral.elements().size(); i++) {
+        if (issueIndexes.contains(i)) {
+          continue;
+        }
+        Expression key = setLiteral.elements().get(i);
+        List<Tree> duplicateKeys = findIdenticalKeys(i, setLiteral.elements(), issueIndexes);
+        if (!duplicateKeys.isEmpty()) {
+          PreciseIssue issue = ctx.addIssue(key, "Change or remove duplicates of this key.");
+          duplicateKeys.forEach(d -> issue.secondary(d, "Duplicate key"));
+        }
+      }
+    });
+  }
+
+  private List<Tree> findIdenticalKeys(int startIndex, List<Expression> elements, Set<Integer> issueIndexes) {
+    Expression key = elements.get(startIndex);
+    List<Tree> duplicates = new ArrayList<>();
+    for (int i = startIndex + 1; i < elements.size(); i++) {
+      Expression comparedKey = elements.get(i);
+      if (isSameKey(key, comparedKey)) {
+        issueIndexes.add(i);
+        duplicates.add(comparedKey);
+      }
+    }
+    return duplicates;
+  }
+}
diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5781.html b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5781.html
@@ -0,0 +1,22 @@
+<p>A set cannot have two identical values. When a value is repeated in a set literal, only the last occurence will remain. Thus duplicate values
+should be either modified or removed.</p>
+<p>This rule raises an issue when the same value is used multiple times as a value in a set literal.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+{"one", "two", "one"}  # Noncompliant
+
+def func(a1, a2, a3):
+    {a1, a2, a1}  # Noncompliant.
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+{"one", "two", "three"}
+
+def func(a1, a2, a3):
+    {a1, a2, a3}
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://docs.python.org/3/reference/expressions.html#set-displays">Python documentation - Set displays</a> </li>
+</ul>
+
diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5781.json b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S5781.json
@@ -0,0 +1,12 @@
+{
+  "title": "Expressions creating sets should not have duplicate values",
+  "type": "BUG",
+  "status": "ready",
+  "tags": [
+
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-5781",
+  "sqKey": "S5781",
+  "scope": "All"
+}
diff --git a/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json b/python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json
@@ -112,6 +112,7 @@
     "S5747",
     "S5754",
     "S5780",
+    "S5781",
     "S5795",
     "S5796",
     "S5797"

diff --git a/python-checks/src/test/java/org/sonar/python/checks/SetDuplicateKeyCheckTest.java b/python-checks/src/test/java/org/sonar/python/checks/SetDuplicateKeyCheckTest.java
@@ -0,0 +1,31 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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 SetDuplicateKeyCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/setDuplicateKey.py", new SetDuplicateKeyCheck());
+  }
+}