SONARPY-886 Rule S5855 Regex alternatives should not be redundant (#962)

* SONARPY-886 Rule S5855 Regex alternatives should not be redundant

* Add code coverage

* Remove POSIX class test cases

* Use new analyzer-commons version to do not support POSIX for python

Author: nils-werner-sonarsource

pom.xml

@@ -91,7 +91,7 @@
     <mockito.version>3.9.0</mockito.version>
     <sonar.version>8.9.0.43852</sonar.version>
     <sonar.orchestrator.version>3.35.1.2719</sonar.orchestrator.version>
-    <sonar-analyzer-commons.version>1.21.0.804</sonar-analyzer-commons.version>
+    <sonar-analyzer-commons.version>1.21.0.805</sonar-analyzer-commons.version>
     <sonarlint-core.version>6.0.0.32513</sonarlint-core.version>
     <sslr.version>1.23</sslr.version>
     <protobuf.version>3.17.3</protobuf.version>

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

@@ -50,6 +50,7 @@
 import org.sonar.python.checks.hotspots.UnsafeHttpMethodsCheck;
 import org.sonar.python.checks.hotspots.UnverifiedHostnameCheck;
 import org.sonar.python.checks.regex.EmptyStringRepetitionCheck;
+import org.sonar.python.checks.regex.RedundantRegexAlternativesCheck;
 import org.sonar.python.checks.regex.StringReplaceCheck;
 
 public final class CheckList {
@@ -184,6 +185,7 @@ public static Iterable<Class> getChecks() {
       PubliclyWritableDirectoriesCheck.class,
       RaiseOutsideExceptCheck.class,
       RedundantJumpCheck.class,
+      RedundantRegexAlternativesCheck.class,
       RegexCheck.class,
       ReturnAndYieldInOneFunctionCheck.class,
       ReturnYieldOutsideFunctionCheck.class,

python-checks/src/main/java/org/sonar/python/checks/regex/AbstractRegexCheck.java

@@ -33,6 +33,7 @@
 import org.sonar.plugins.python.api.tree.RegularArgument;
 import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.python.regex.PythonRegexIssueLocation;
 import org.sonar.python.regex.RegexContext;
 import org.sonar.python.tree.TreeUtils;
 import org.sonarsource.analyzer.commons.regex.RegexIssueLocation;
@@ -95,8 +96,8 @@ private static Optional<StringLiteral> patternArgStringLiteral(CallExpression re
 
   public void addIssue(RegexSyntaxElement regexTree, String message, @Nullable Integer cost, List<RegexIssueLocation> secondaries) {
     if (reportedRegexTrees.add(regexTree)) {
-      regexContext.addIssue(regexTree, message);
-      // TODO: Add secondary location to the issue SONARPY-886
+      PreciseIssue issue = regexContext.addIssue(regexTree, message);
+      secondaries.stream().map(PythonRegexIssueLocation::preciseLocation).forEach(issue::secondary);
       // TODO: Add cost to the issue SONARPY-893
     }
   }

python-checks/src/main/java/org/sonar/python/checks/regex/RedundantRegexAlternativesCheck.java

@@ -0,0 +1,34 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2021 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.regex;
+
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonarsource.analyzer.commons.regex.RegexParseResult;
+import org.sonarsource.analyzer.commons.regex.finders.RedundantRegexAlternativesFinder;
+
+@Rule(key = "S5855")
+public class RedundantRegexAlternativesCheck extends AbstractRegexCheck{
+
+  @Override
+  public void checkRegex(RegexParseResult regexParseResult, CallExpression regexFunctionCall) {
+    new RedundantRegexAlternativesFinder(this::addIssue).visit(regexParseResult);
+  }
+}

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

@@ -0,0 +1,15 @@
+<p>If an alternative in a regular expression only matches things that are already matched by another alternative, that alternative is redundant and
+serves no purpose.</p>
+<p>In the best case this means that the offending subpattern is merely redundant and should be removed. In the worst case it’s a sign that this regex
+does not match what it was intended to match and should be reworked.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+r"[ab]|a"   # The "|a" is redundant because "[ab]" already matches "a"
+r".*|a"     # .* matches everything, so any other alternative is redundant
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+r"[ab]"
+r".*"
+</pre>
+

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

@@ -0,0 +1,17 @@
+{
+  "title": "Regex alternatives should not be redundant",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "regex"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-5855",
+  "sqKey": "S5855",
+  "scope": "All",
+  "quickfix": "unknown"
+}

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

@@ -134,6 +134,7 @@
     "S5807",
     "S5828",
     "S5842",
+    "S5855",
     "S5864",
     "S5886",
     "S5890"

python-checks/src/test/java/org/sonar/python/checks/regex/RedundantRegexAlternativesCheckTest.java

@@ -0,0 +1,31 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2021 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.regex;
+
+import org.junit.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class RedundantRegexAlternativesCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/regex/redundantRegexAlternativesCheckTest.py", new RedundantRegexAlternativesCheck());
+  }
+}

python-checks/src/test/resources/checks/regex/redundantRegexAlternativesCheckTest.py

@@ -0,0 +1,25 @@
+import re
+
+
+def non_compliant(input):
+    re.match(r"a|[ab]", input)  # Noncompliant {{Remove or rework this redundant alternative.}}
+    #          ^ ^^^^< {{Alternative to keep}}
+    re.match(r".|a", input)  # Noncompliant
+    re.match(r"a|.", input)  # Noncompliant
+    re.match(r"(.)|(a)", input)  # Noncompliant
+    re.match(r"a|b|bc?", input)  # Noncompliant
+    re.match(r"a|b|bc*", input)  # Noncompliant
+    re.match(r"a|b|bb*", input)  # Noncompliant
+    re.match(r"a|b|a|b|a|b|a|b", input)  # Noncompliant 2
+    re.match(r"[1-2]|[1-4]|[1-8]|[1-3]", input)  # Noncompliant
+    re.match(r"1|[1-2]", input)  # Noncompliant
+
+
+def compliant(input):
+    re.match(r"(a)|(.)", input)  # Compliant
+    re.match(r"a|(.)", input)  # Compliant
+    re.match(r"(a)|.", input)  # Compliant
+    re.match(r"a|b|bc+", input)  # Compliant
+    re.match(r"|a", input)  # Compliant
+    re.match(r"[ab]", input)  # Compliant
+    re.match(r".*", input)  # Compliant

python-frontend/src/main/java/org/sonar/python/regex/PythonRegexIssueLocation.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Objects;
 import org.sonar.plugins.python.api.IssueLocation;
+import org.sonarsource.analyzer.commons.regex.RegexIssueLocation;
 import org.sonarsource.analyzer.commons.regex.ast.IndexRange;
 import org.sonarsource.analyzer.commons.regex.ast.RegexSyntaxElement;
 
@@ -32,6 +33,10 @@ private PythonRegexIssueLocation() {
 
   }
 
+  public static IssueLocation preciseLocation(RegexIssueLocation regexIssueLocation) {
+    return preciseLocation(regexIssueLocation.syntaxElements(), regexIssueLocation.message());
+  }
+
   public static IssueLocation preciseLocation(RegexSyntaxElement syntaxElement, String message) {
     return preciseLocation(Collections.singletonList(syntaxElement), message);
   }