SONARPY-907 S5361 and initial version of the RegexSource and CharacterParser classes (#958)

* Initial implementations for AbstractRegexCheck

* Rule implementation

* Add license headers

* Fix code smells

* Add tests for the characterParser

* Add tests for the abstractRegexCheck

* Add ITS expected results

* Add test

Author: karim-ouerghemmi-sonarsource

its/ruling/src/test/resources/expected/python-S5361.json

@@ -0,0 +1,8 @@
+{
+'project:tensorflow/python/debug/cli/command_parser.py':[
+264,
+],
+'project:tensorflow/python/keras/layers/einsum_dense.py':[
+212,
+],
+}

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.16.0.719</sonar-analyzer-commons.version>
+    <sonar-analyzer-commons.version>1.21.0.804</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>
@@ -131,6 +131,11 @@
         <artifactId>sonar-analyzer-test-commons</artifactId>
         <version>${sonar-analyzer-commons.version}</version>
       </dependency>
+      <dependency>
+        <groupId>org.sonarsource.analyzer-commons</groupId>
+        <artifactId>sonar-regex-parsing</artifactId>
+        <version>${sonar-analyzer-commons.version}</version>
+      </dependency>
       <dependency>
         <groupId>org.codehaus.staxmate</groupId>
         <artifactId>staxmate</artifactId>

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

@@ -49,6 +49,7 @@
 import org.sonar.python.checks.hotspots.StrongCryptographicKeysCheck;
 import org.sonar.python.checks.hotspots.UnsafeHttpMethodsCheck;
 import org.sonar.python.checks.hotspots.UnverifiedHostnameCheck;
+import org.sonar.python.checks.regex.StringReplaceCheck;
 
 public final class CheckList {
 
@@ -199,6 +200,7 @@ public static Iterable<Class> getChecks() {
       StringFormatCorrectnessCheck.class,
       StringFormatMisuseCheck.class,
       StringLiteralDuplicationCheck.class,
+      StringReplaceCheck.class,
       StrongCryptographicKeysCheck.class,
       TempFileCreationCheck.class,
       TooManyLinesInFileCheck.class,

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

@@ -0,0 +1,88 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.symbols.Symbol;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+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.RegexContext;
+import org.sonar.python.tree.TreeUtils;
+import org.sonarsource.analyzer.commons.regex.RegexParseResult;
+
+public abstract class AbstractRegexCheck extends PythonSubscriptionCheck {
+
+  private static final Set<String> REGEX_FUNCTIONS = new HashSet<>(Collections.singletonList("re.sub"));
+  protected RegexContext regexContext;
+
+  protected Set<String> lookedUpFunctionNames() {
+    return REGEX_FUNCTIONS;
+  }
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::checkCall);
+  }
+
+  public abstract void checkRegex(RegexParseResult regexParseResult, CallExpression regexFunctionCall);
+
+  private void checkCall(SubscriptionContext ctx) {
+    regexContext = (RegexContext) ctx;
+    CallExpression callExpression = (CallExpression) ctx.syntaxNode();
+    Symbol calleeSymbol = callExpression.calleeSymbol();
+    if (calleeSymbol == null || calleeSymbol.fullyQualifiedName() == null) {
+      return;
+    }
+    if (lookedUpFunctionNames().contains(calleeSymbol.fullyQualifiedName())) {
+      patternArgStringLiteral(callExpression)
+        .flatMap(this::regexForStringLiteral)
+        .ifPresent(parseResult -> checkRegex(parseResult, callExpression));
+    }
+  }
+
+  private Optional<RegexParseResult> regexForStringLiteral(StringLiteral literal) {
+    // TODO: for now we only handle strings with an "r" prefix. This will be extended.
+    if (literal.stringElements().size() == 1 && "r".equalsIgnoreCase(literal.stringElements().get(0).prefix())) {
+      return Optional.of(regexContext.regexForStringElement(literal.stringElements().get(0)));
+    }
+    return Optional.empty();
+  }
+
+  private static Optional<StringLiteral> patternArgStringLiteral(CallExpression regexFunctionCall) {
+    RegularArgument patternArgument = TreeUtils.nthArgumentOrKeyword(0, "pattern", regexFunctionCall.arguments());
+    if (patternArgument == null) {
+      return Optional.empty();
+    }
+    Expression patternArgumentExpression = patternArgument.expression();
+    if (patternArgumentExpression.is(Tree.Kind.STRING_LITERAL)) {
+      return Optional.of((StringLiteral) patternArgumentExpression);
+    }
+    return Optional.empty();
+  }
+
+}

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

@@ -0,0 +1,56 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+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.ast.RegexTree;
+import org.sonarsource.analyzer.commons.regex.ast.SequenceTree;
+
+@Rule(key = "S5361")
+public class StringReplaceCheck extends AbstractRegexCheck {
+
+  private static final String MESSAGE = "Replace this \"re.sub()\" call by a \"str.replace()\" function call.";
+
+  @Override
+  protected Set<String> lookedUpFunctionNames() {
+    return new HashSet<>(Collections.singletonList("re.sub"));
+  }
+
+  @Override
+  public void checkRegex(RegexParseResult regexParseResult, CallExpression callExpression) {
+    RegexTree regex = regexParseResult.getResult();
+    if (!regexParseResult.hasSyntaxErrors() && isPlainString(regex)) {
+      regexContext.addIssue(callExpression.callee(), MESSAGE);
+    }
+  }
+
+  private static boolean isPlainString(RegexTree regex) {
+    return regex.is(RegexTree.Kind.CHARACTER)
+      || (regex.is(RegexTree.Kind.SEQUENCE)
+      && !((SequenceTree) regex).getItems().isEmpty()
+      && ((SequenceTree) regex).getItems().stream().allMatch(item -> item.is(RegexTree.Kind.CHARACTER)));
+  }
+
+}

python-checks/src/main/java/org/sonar/python/checks/regex/package-info.java

@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+@ParametersAreNonnullByDefault
+package org.sonar.python.checks.regex;
+
+import javax.annotation.ParametersAreNonnullByDefault;

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

@@ -0,0 +1,25 @@
+<p>An <code>re.sub</code> call always performs an evaluation of the first argument as a regular expression, even if no regular expression features
+were used. This has a significant performance cost and therefore should be used with care.</p>
+<p>When <code>re.sub</code> is used, the first argument should be a real regular expression. If it’s not the case, <code>str.replace</code> does
+exactly the same thing as <code>re.sub</code> without the performance drawback of the regex.</p>
+<p>This rule raises an issue for each <code>re.sub</code> used with a simple string as first argument which doesn’t contains special regex character
+or pattern.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+input = "Bob is a Bird... Bob is a Plane... Bob is Superman!"
+changed = re.sub("Bob is", "It's", input) # Noncompliant
+changed = re.sub("\.\.\.", ";", changed) # Noncompliant
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+input = "Bob is a Bird... Bob is a Plane... Bob is Superman!"
+changed = str.replace("Bob is", "It's", input)
+changed = str.replace("...", ";", changed)
+</pre>
+<p>Or, with a regex:</p>
+<pre>
+input = "Bob is a Bird... Bob is a Plane... Bob is Superman!"
+changed = re.sub(r"\w*\sis", "It's", input)
+changed = re.sub(r"\.{3}", ";", changed)
+</pre>
+

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

@@ -0,0 +1,18 @@
+{
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "2min"
+  },
+  "tags": [
+    "regex",
+    "performance"
+  ],
+  "defaultSeverity": "Critical",
+  "ruleSpecification": "RSPEC-5361",
+  "sqKey": "S5361",
+  "scope": "All",
+  "quickfix": "unknown",
+  "title": "`str.replace` should be preferred to `re.sub`"
+}

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

@@ -93,6 +93,7 @@
     "S5122",
     "S5247",
     "S5332",
+    "S5361",
     "S5443",
     "S5445",
     "S5527",

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

@@ -0,0 +1,80 @@
+/*
+ * 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 java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.junit.Test;
+import org.sonar.plugins.python.api.PythonCheck;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.PythonVisitorContext;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.python.SubscriptionVisitor;
+import org.sonar.python.TestPythonVisitorRunner;
+import org.sonarsource.analyzer.commons.regex.RegexParseResult;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class AbstractRegexCheckTest {
+
+  private static final File FILE = new File("src/test/resources/checks/regex/abstractRegexCheck.py");
+
+  private static PythonVisitorContext fileContext(File file) {
+    return TestPythonVisitorRunner.createContext(file);
+  }
+  
+  private static List<PythonCheck.PreciseIssue> scanFileForIssues(PythonVisitorContext fileContext, List<PythonSubscriptionCheck> checks) {
+    checks.forEach(c -> c.scanFile(fileContext));
+    SubscriptionVisitor.analyze(checks, fileContext);
+    return fileContext.getIssues();
+  }
+
+  @Test
+  public void test_regex_is_visited() {
+    Check check = new Check();
+    List<PythonCheck.PreciseIssue> issues = scanFileForIssues(fileContext(FILE), Collections.singletonList(check));
+    assertThat(check.receivedRegexParseResults).hasSize(1);
+    assertThat(issues).hasSize(1);
+  }
+
+  @Test
+  public void test_regex_parse_result_is_retrieved_from_cache_in_context() {
+    PythonVisitorContext fileContext = fileContext(FILE);
+
+    Check checkOne = new Check();
+    Check checkTwo = new Check();
+    scanFileForIssues(fileContext, Arrays.asList(checkOne, checkTwo));
+
+    assertThat(checkOne.receivedRegexParseResults.get(0)).isSameAs(checkTwo.receivedRegexParseResults.get(0));
+  }
+
+  private static class Check extends AbstractRegexCheck {
+    private final List<RegexParseResult> receivedRegexParseResults = new ArrayList<>();
+
+    public void checkRegex(RegexParseResult regexParseResult, CallExpression regexFunctionCall) {
+      receivedRegexParseResults.add(regexParseResult);
+      regexContext.addIssue(regexFunctionCall, "MESSAGE");
+    }
+  }
+
+}