SONARPY-981 Rule S6328: Replacement strings should reference existing regular expression groups (#1112)

Author: marco-bearzi-sonarsource

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

@@ -91,6 +91,7 @@ static List<String> bugRuleKeys() {
       "S5996",
       "S6002",
       "S6323",
+      "S6328",
       "S905",
       "S930"
     );

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

@@ -55,6 +55,7 @@
 import org.sonar.python.checks.regex.EmptyAlternativeCheck;
 import org.sonar.python.checks.regex.EmptyStringRepetitionCheck;
 import org.sonar.python.checks.regex.GraphemeClustersInClassesCheck;
+import org.sonar.python.checks.regex.GroupReplacementCheck;
 import org.sonar.python.checks.regex.ImpossibleBoundariesCheck;
 import org.sonar.python.checks.regex.InvalidRegexCheck;
 import org.sonar.python.checks.regex.MultipleWhitespaceCheck;
@@ -151,6 +152,7 @@ public static Iterable<Class> getChecks() {
       FunctionUsingLoopVariableCheck.class,
       GenericExceptionRaisedCheck.class,
       GraphemeClustersInClassesCheck.class,
+      GroupReplacementCheck.class,
       HardCodedCredentialsCheck.class,
       HardcodedIPCheck.class,
       HashingDataCheck.class,

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

@@ -0,0 +1,97 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2022 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 java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.tree.CallExpression;
+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.sonarsource.analyzer.commons.regex.RegexParseResult;
+import org.sonarsource.analyzer.commons.regex.ast.CapturingGroupTree;
+import org.sonarsource.analyzer.commons.regex.ast.RegexBaseVisitor;
+
+import static org.sonar.python.tree.TreeUtils.nthArgumentOrKeyword;
+
+@Rule(key = "S6328")
+public class GroupReplacementCheck extends AbstractRegexCheck {
+
+  private static final String MESSAGE = "Referencing non-existing group%s: %s.";
+  private static final Pattern REFERENCE_PATTERN = Pattern.compile("\\\\(\\d+)|\\\\g<(\\d+)>");
+
+  @Override
+  protected Map<String, Integer> lookedUpFunctions() {
+    return Collections.singletonMap("re.sub", 4);
+  }
+
+  @Override
+  public void checkRegex(RegexParseResult regexParseResult, CallExpression regexFunctionCall) {
+    GroupFinder groupFinder = new GroupFinder();
+    groupFinder.visit(regexParseResult);
+    checkReplacement(regexFunctionCall, groupFinder.groups);
+  }
+
+  private void checkReplacement(CallExpression tree, Set<CapturingGroupTree> groups) {
+    RegularArgument regArg = nthArgumentOrKeyword(1, "replacement", tree.arguments());
+    if (regArg == null) return;
+
+    if (regArg.expression().is(Tree.Kind.STRING_LITERAL)) {
+      StringLiteral expression = (StringLiteral) regArg.expression();
+      List<Integer> references = collectReferences(expression.trimmedQuotesValue());
+      references.removeIf(reference -> groups.stream().anyMatch(group -> group.getGroupNumber() == reference));
+      if (!references.isEmpty()) {
+        List<String> stringReferences = references.stream().map(String::valueOf).collect(Collectors.toList());
+        regexContext.addIssue(expression, String.format(MESSAGE, references.size() == 1 ? "" : "s", String.join(", ", stringReferences)));
+      }
+    }
+  }
+
+  private static List<Integer> collectReferences(String replacement) {
+    Matcher match = REFERENCE_PATTERN.matcher(replacement);
+    List<Integer> references = new ArrayList<>();
+    while (match.find()) {
+      // extract reference number out of one of the possible 2 groups of the regex
+      Optional.ofNullable(match.group(1)).map(Integer::valueOf).filter(ref -> ref != 0).ifPresent(references::add);
+      Optional.ofNullable(match.group(2)).map(Integer::valueOf).filter(ref -> ref != 0).ifPresent(references::add);
+    }
+    return references;
+  }
+
+  static class GroupFinder extends RegexBaseVisitor {
+
+    private final Set<CapturingGroupTree> groups = new HashSet<>();
+
+    @Override
+    public void visitCapturingGroup(CapturingGroupTree group) {
+      groups.add(group);
+      super.visitCapturingGroup(group);
+    }
+  }
+}

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

@@ -0,0 +1,17 @@
+<p>The regex function <code>re.sub</code> can be used to perform a search and replace based on regular expression matches. The <code>repl</code>
+parameter can contain references to capturing groups used in the <code>pattern</code> parameter. This can be achieved with <code>\n</code> to
+reference the n’th group.</p>
+<p>When referencing a nonexistent group an error will be thrown for Python &lt; 3.5 or replaced by an empty string for Python &gt;= 3.5.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+re.sub(r"(a)(b)(c)", r"\1, \9, \3", "abc") # Noncompliant - result is an re.error: invalid group reference
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+re.sub(r"(a)(b)(c)", r"\1, \2, \3", "abc")
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://docs.python.org/3.10/library/re.html#re.sub">re.sub</a> - Python Documentation </li>
+</ul>
+

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

@@ -0,0 +1,17 @@
+{
+  "title": "Replacement strings should reference existing regular expression groups",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "10min"
+  },
+  "tags": [
+    "regex"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6328",
+  "sqKey": "S6328",
+  "scope": "Main",
+  "quickfix": "unknown"
+}

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

@@ -151,10 +151,11 @@
     "S5996",
     "S6002",
     "S6019",
+    "S6035",
     "S6323",
     "S6326",
+    "S6328",
     "S6331",
-    "S6035",
     "S6395",
     "S6396",
     "S6397"

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

@@ -0,0 +1,32 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2022 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 GroupReplacementCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/regex/groupReplacementCheck.py", new GroupReplacementCheck());
+  }
+
+}

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

@@ -0,0 +1,37 @@
+import re
+
+
+def non_compliant():
+    input = "Bob is a Bird... Bob is a Plane... Bob is Superman!"
+
+    re.sub(r"(a)(b)(c)", r"\1, \9, \3", "abc")  # Noncompliant
+    #                    ^^^^^^^^^^^^^
+    re.sub("(a)", r"\2", input)  # Noncompliant {{Referencing non-existing group: 2.}}
+    #             ^^^^^
+    re.sub("(a)", r'\g<2>', input)  # Noncompliant
+    re.sub("a", r"\g<1>", input)  # Noncompliant
+    re.sub("(?!a)", r"\g<1>", input)  # Noncompliant
+    re.sub("(a)", r'\2', input)  # Noncompliant
+    re.sub("(a)", r"\g<2> \2", input)  # Noncompliant
+    re.sub("(a)", r"\3 \g<2>", input)  # Noncompliant {{Referencing non-existing groups: 3, 2.}}
+    re.sub("(a)", r"\2 \1", input)  # Noncompliant
+    re.sub("(s)", r"\12 \1", input)  # Noncompliant
+
+
+def compliant():
+    input = "Bob is a Bird... Bob is a Plane... Bob is Superman!"
+    re.sub(r'\sAND\s', ' & ', 'Baked Beans And Spam', flags=re.IGNORECASE)
+
+    re.sub("(a)", r"\0c", input)
+    re.sub("(a)", r"\g<0>c", input)
+    re.sub("(a)", r'\1', input)
+    re.sub("(a)", r"\g<1>ttt", input)
+    re.sub("(a)(b)", r"\g<1>aaa \2bbb", input)
+    re.sub("(a(n))", r"\1a \g<2>", input)
+
+    # coverage
+    re.match("up", input)
+    pattern = r"a"
+    re.sub("(ab)", pattern, input)
+    re.sub("(a)", r"[\?]", input)
+    re.sub("(A)")