SONARPY-910 Extend AbstractRegexCheck to consider all relevant methods from re package (#959)

* SONARPY-910 Extend AbstractRegexCheck to consider all relevant methods from re package

* Further test simplification and improvement

Author: karim-ouerghemmi-sonarsource

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

@@ -19,7 +19,7 @@
  */
 package org.sonar.python.checks.regex;
 
-import java.util.Collections;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
@@ -37,7 +37,8 @@
 
 public abstract class AbstractRegexCheck extends PythonSubscriptionCheck {
 
-  private static final Set<String> REGEX_FUNCTIONS = new HashSet<>(Collections.singletonList("re.sub"));
+  private static final Set<String> REGEX_FUNCTIONS = new HashSet<>(Arrays.asList("re.sub", "re.subn", "re.compile", "re.search", "re.match",
+    "re.fullmatch", "re.split", "re.findall", "re.finditer"));
   protected RegexContext regexContext;
 
   protected Set<String> lookedUpFunctionNames() {

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

@@ -22,15 +22,13 @@
 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.sonar.python.checks.utils.PythonCheckVerifier;
 import org.sonarsource.analyzer.commons.regex.RegexParseResult;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -39,33 +37,23 @@ 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);
+    PythonCheckVerifier.verify(FILE.getAbsolutePath(), check);
+    assertThat(check.receivedRegexParseResults).hasSize(10);
   }
 
   @Test
   public void test_regex_parse_result_is_retrieved_from_cache_in_context() {
-    PythonVisitorContext fileContext = fileContext(FILE);
-
+    PythonVisitorContext fileContext = TestPythonVisitorRunner.createContext(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));
+    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));
+    }
   }
 
   private static class Check extends AbstractRegexCheck {

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

@@ -1,6 +1,16 @@
 import re
 
-re.sub(r'.*', "x", "a")
+re.sub(r'.*', "x", "a") # Noncompliant {{MESSAGE}}
+re.sub(repl='x', pattern=r'.*', string='yyyy') # Noncompliant
+re.subn(r'.*', "x", "a") # Noncompliant
+re.compile(r'.*') # Noncompliant
+re.search(r'.*', "foo") # Noncompliant
+re.match(r'.*', "foo") # Noncompliant
+re.fullmatch(r'.*', "foo") # Noncompliant
+re.split(r'.*', "foo") # Noncompliant
+re.findall(r'.*', "foo") # Noncompliant
+re.finditer(r'.*', "foo") # Noncompliant
+
 re.sub('.*', "x", "a") # We only look at raw strings for now
 re.sub(r'.*' r'.*', "x", "a") # We do not look at concats for now
 re.sub() # Required arguments not provided

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

@@ -36,6 +36,7 @@ def compliant():
     changed = re.sub(r"x+", "It's", input)
     changed = re.sub(r"[\\]", "It's", input)
     changed = re.sub(r"", "It's", input)
+    changed = re.match(r"Bob is", input)
 
 def coverage(input):
     re.match(r"Bob is", input)