SONARPY-923 Analyze regex in variables whose values we can infer (#978)

* Check regex pattern in variables

* Make regex cache depend on flagSet

* Add unit test for regex cache in frontend package

* Remove regex cache tests from checks package

* Small test refactoring

Author: karim-ouerghemmi-sonarsource

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

@@ -35,11 +35,13 @@
 import org.sonar.plugins.python.api.tree.BinaryExpression;
 import org.sonar.plugins.python.api.tree.CallExpression;
 import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.QualifiedExpression;
 import org.sonar.plugins.python.api.tree.RegularArgument;
 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.python.checks.Expressions;
 import org.sonar.python.regex.PythonRegexIssueLocation;
 import org.sonar.python.regex.RegexContext;
 import org.sonar.python.tree.TreeUtils;
@@ -129,9 +131,13 @@ private static Optional<StringLiteral> patternArgStringLiteral(CallExpression re
     if (patternArgument == null) {
       return Optional.empty();
     }
-    Expression patternArgumentExpression = patternArgument.expression();
-    if (patternArgumentExpression.is(Tree.Kind.STRING_LITERAL)) {
-      return Optional.of((StringLiteral) patternArgumentExpression);
+    Expression patternValueExpression = patternArgument.expression();
+    if (patternValueExpression.is(Tree.Kind.NAME)) {
+      patternValueExpression = Expressions.singleAssignedValue((Name) patternValueExpression);
+    }
+
+    if (patternValueExpression != null && patternValueExpression.is(Tree.Kind.STRING_LITERAL)) {
+      return Optional.of((StringLiteral) patternValueExpression);
     }
     return Optional.empty();
   }

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

@@ -21,7 +21,6 @@
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.regex.Pattern;
@@ -58,20 +57,8 @@ public void checkRegex(RegexParseResult regexParseResult, CallExpression regexFu
 
     PythonVisitorContext fileContext = TestPythonVisitorRunner.createContext(FILE);
     SubscriptionVisitor.analyze(Collections.singletonList(check), fileContext);
-    assertThat(check.reportedRegexTrees).hasSize(11);
-    assertThat(fileContext.getIssues()).hasSize(11);
-  }
-
-  @Test
-  public void test_regex_parse_result_is_retrieved_from_cache_in_context() {
-    PythonVisitorContext fileContext = TestPythonVisitorRunner.createContext(FILE);
-    Check checkOne = new Check();
-    Check checkTwo = new Check();
-    SubscriptionVisitor.analyze(Arrays.asList(checkOne, checkTwo), fileContext);
-    assertThat(checkOne.receivedRegexParseResults).hasSameSizeAs(checkTwo.receivedRegexParseResults);
-    for (int i = 0; i < checkOne.receivedRegexParseResults.size(); i++) {
-      assertThat(checkOne.receivedRegexParseResults.get(i)).isSameAs(checkTwo.receivedRegexParseResults.get(i));
-    }
+    assertThat(check.reportedRegexTrees).hasSize(12);
+    assertThat(fileContext.getIssues()).hasSize(12);
   }
 
   @Test
@@ -81,7 +68,6 @@ public void test_flags() {
   }
 
   private static class Check extends AbstractRegexCheck {
-    private final List<RegexParseResult> receivedRegexParseResults = new ArrayList<>();
     private boolean printFlags;
 
     private Check() {
@@ -94,7 +80,6 @@ private Check(boolean printFlags) {
 
     @Override
     public void checkRegex(RegexParseResult regexParseResult, CallExpression regexFunctionCall) {
-      receivedRegexParseResults.add(regexParseResult);
       addIssue(regexParseResult.getResult(), printFlags ? flagsToString(regexParseResult) : "MESSAGE", null, Collections.emptyList());
     }
 

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

@@ -11,6 +11,11 @@
 re.findall(r'.*', "foo") # Noncompliant
 re.finditer('.*', "foo") # Noncompliant
 
+some_pattern = '.*' # Noncompliant
+re.match(some_pattern, 'some string')
+re.match(some_unknown_pattern, 'some string')
+re.match(get_pattern(), 'some string')
+
 re.match('.*\N{GREEK SMALL LETTER FINAL SIGMA}', 'foo') # We do ignore not raw strings containing \N escape sequences
 re.match(r'.*\N{GREEK SMALL LETTER FINAL SIGMA}', 'foo') # Noncompliant
 

python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java

@@ -58,7 +58,7 @@ public class SubscriptionVisitor {
   private final EnumMap<Kind, List<SubscriptionContextImpl>> consumers = new EnumMap<>(Kind.class);
   private final PythonVisitorContext pythonVisitorContext;
   private Tree currentElement;
-  private final HashMap<StringElement, RegexParseResult> regexCache = new HashMap<>();
+  private final HashMap<String, RegexParseResult> regexCache = new HashMap<>();
 
   public static void analyze(Collection<PythonSubscriptionCheck> checks, PythonVisitorContext pythonVisitorContext) {
     SubscriptionVisitor subscriptionVisitor = new SubscriptionVisitor(checks, pythonVisitorContext);
@@ -169,8 +169,8 @@ public File workingDirectory() {
     }
 
     public RegexParseResult regexForStringElement(StringElement stringElement, FlagSet flagSet) {
-      return regexCache.computeIfAbsent(stringElement,
-        s -> new RegexParser(new PythonAnalyzerRegexSource(s), flagSet).parse());
+      return regexCache.computeIfAbsent(stringElement.hashCode() + "-" + flagSet.getMask(),
+        s -> new RegexParser(new PythonAnalyzerRegexSource(stringElement), flagSet).parse());
     }
   }
 }

python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java

@@ -0,0 +1,61 @@
+/*
+ * 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;
+
+import java.util.Collections;
+import java.util.regex.Pattern;
+import org.junit.Test;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.PythonVisitorContext;
+import org.sonar.plugins.python.api.tree.FileInput;
+import org.sonar.plugins.python.api.tree.StringElement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.python.regex.RegexContext;
+import org.sonarsource.analyzer.commons.regex.RegexParseResult;
+import org.sonarsource.analyzer.commons.regex.ast.FlagSet;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SubscriptionVisitorTest {
+
+  @Test
+  public void test_regex_cache() {
+    PythonSubscriptionCheck check = new PythonSubscriptionCheck() {
+      @Override
+      public void initialize(Context context) {
+        context.registerSyntaxNodeConsumer(Tree.Kind.STRING_ELEMENT, ctx -> {
+          StringElement stringElement = (StringElement)ctx.syntaxNode();
+          RegexContext regexCtx = (RegexContext) ctx;
+          RegexParseResult resultWithNoFlags = regexCtx.regexForStringElement(stringElement, new FlagSet());
+          RegexParseResult resultWithFlags = regexCtx.regexForStringElement(stringElement, new FlagSet(Pattern.MULTILINE));
+
+          assertThat(resultWithNoFlags).isNotSameAs(resultWithFlags);
+          // When we retrieve them again, it will be the same instance retrieved from the cache.
+          assertThat(resultWithNoFlags).isSameAs(regexCtx.regexForStringElement(stringElement, new FlagSet()));
+          assertThat(resultWithFlags).isSameAs(regexCtx.regexForStringElement(stringElement, new FlagSet(Pattern.MULTILINE)));
+        });
+      }
+    };
+
+    FileInput fileInput = PythonTestUtils.parse("'.*'");
+    PythonVisitorContext context = new PythonVisitorContext(fileInput, PythonTestUtils.pythonFile("file"), null, null);
+    SubscriptionVisitor.analyze(Collections.singleton(check), context);
+  }
+}