implement #217

Feuermagier · Aug 24, 2023 · 4d29763 · 4d29763
1 parent 9c35106
commit 4d29763
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 7 deletions.
diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java
@@ -24,10 +24,14 @@
 import de.firemage.autograder.treeg.ast.RegExCharacter;
 import de.firemage.autograder.treeg.ast.RegExNode;
 import spoon.processing.AbstractProcessor;
+import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtLiteral;
-import spoon.reflect.code.CtStatement;
+import spoon.reflect.code.CtTypeAccess;
+import spoon.reflect.code.CtVariableAccess;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtVariable;
+import spoon.reflect.reference.CtExecutableReference;
+import spoon.reflect.visitor.filter.VariableAccessFilter;
 
 import java.util.List;
 import java.util.Map;
@@ -49,13 +53,43 @@ private static boolean looksLikeRegex(String value) {
         return REGEX_HINTS.stream().filter(value::contains).count() >= MIN_REGEX_HINTS;
     }
 
+    private static boolean isRegexInvocation(CtInvocation<?> ctInvocation) {
+        CtExecutableReference<?> ctExecutable = ctInvocation.getExecutable();
+
+        return ctInvocation.getTarget() instanceof CtTypeAccess<?> ctTypeAccess
+            && SpoonUtil.isTypeEqualTo(ctTypeAccess.getAccessedType(), java.util.regex.Pattern.class)
+            && List.of("matches", "compile").contains(ctExecutable.getSimpleName())
+            || SpoonUtil.isTypeEqualTo(ctInvocation.getTarget().getType(), java.lang.String.class)
+            && (
+                SpoonUtil.isSignatureEqualTo(ctExecutable, boolean.class, "matches", String.class)
+                || SpoonUtil.isSignatureEqualTo(ctExecutable, String.class, "replaceAll", String.class, String.class)
+                || SpoonUtil.isSignatureEqualTo(ctExecutable, String.class, "replaceFirst", String.class, String.class)
+                || SpoonUtil.isSignatureEqualTo(ctExecutable, String[].class, "split", String.class)
+                || SpoonUtil.isSignatureEqualTo(ctExecutable, String[].class, "split", String.class, int.class)
+            );
+    }
+
+    private static boolean isInAllowedContext(CtLiteral<?> ctLiteral) {
+        CtElement parent = ctLiteral.getParent();
+        if (parent instanceof CtVariable<?> ctVariable
+            && SpoonUtil.isEffectivelyFinal(ctVariable.getReference())) {
+            List<CtVariableAccess<?>> invocations = parent.getFactory().getModel().getElements(new VariableAccessFilter<>(ctVariable.getReference()));
+
+            return !invocations.isEmpty() &&
+                invocations
+                    .stream()
+                    .allMatch(ctVariableAccess -> ctVariableAccess.getParent() instanceof CtInvocation<?> ctInvocation && isRegexInvocation(ctInvocation));
+        }
+
+        return parent instanceof CtInvocation<?> ctInvocation && isRegexInvocation(ctInvocation);
+    }
 
     @Override
     protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
         staticAnalysis.processWith(new AbstractProcessor<CtLiteral<String>>() {
             @Override
             public void process(CtLiteral<String> literal) {
-                if (!SpoonUtil.isString(literal.getType())) {
+                if (!SpoonUtil.isString(literal.getType()) || !isInAllowedContext(literal)) {
                     return;
                 }
 

diff --git a/...ader-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java b/...ader-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/code/Test.java
@@ -1,3 +1,7 @@
+package de.firemage.autograder.core.check_tests.Regex.code;
+
+import java.util.regex.Pattern;
+
 public class Test {
     private String noRegex = "Should we do this? I guess we shouldn't! f*ck you!";
     private String regex1 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
@@ -19,4 +23,43 @@ public class Test {
 
     private static final String FORMAT_STRING_1 = "coordinate (%s, %s) is invalid!";
     private static final String FORMAT_STRING_2 = "coordinate (%s, %s)\n is invalid?\n";
+
+    private void foo() {
+        Pattern pattern = Pattern.compile(regex1);
+        pattern = Pattern.compile(regex2);
+        pattern = Pattern.compile(regex3);
+        pattern = Pattern.compile(regex4);
+        pattern = Pattern.compile(regex5);
+        pattern = Pattern.compile(simpleRegex1);
+        pattern = Pattern.compile(simpleRegex2);
+        pattern = Pattern.compile(simpleRegex3);
+        pattern = Pattern.compile(invalidRegex);
+        pattern = Pattern.compile(COMPLICATED_REGEX_1);
+        pattern = Pattern.compile(COMPLICATED_REGEX_2);
+        pattern = Pattern.compile(FORMAT_STRING_1);
+        pattern = Pattern.compile(FORMAT_STRING_2);
+    }
+}
+
+// test that context of regex is considered
+class RegexContext {
+    private static final String DEFINITELY_REGEX_1 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String DEFINITELY_REGEX_2 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String DEFINITELY_REGEX_3 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String DEFINITELY_REGEX_4 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String DEFINITELY_REGEX_5 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String DEFINITELY_REGEX_6 = "(foo)* [bar]+ x? x?"; /*# not ok #*/
+    private static final String UNUSED_REGEX = "(foo)* [bar]+ x? x?"; /*# ok #*/
+    private static final String NOT_USED_AS_REGEX = "(foo)* [bar]+ x? x?"; /*# ok #*/
+
+    void foo() {
+        boolean matches = Pattern.matches(DEFINITELY_REGEX_1, "foo bar x");
+        matches = "foo bar x".matches(DEFINITELY_REGEX_2);
+        String f = "foo bar x".replaceAll(DEFINITELY_REGEX_3, "foo bar x");
+        f = "foo bar x".replaceFirst(DEFINITELY_REGEX_4, "foo bar x");
+        String[] parts = "foo bar x".split(DEFINITELY_REGEX_5);
+        parts = "foo bar x".split(DEFINITELY_REGEX_6, -1);
+
+        System.out.println(NOT_USED_AS_REGEX);
+    }
 }
diff --git a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/Regex/config.txt
@@ -1,7 +1,14 @@
 complexity.RegexCheck
 Complex regex
-Test.java:3
-Test.java:4
-Test.java:5
-Test.java:6
-Test.java:7
+Test.java:7
+Test.java:8
+Test.java:9
+Test.java:10
+Test.java:11
+
+Test.java:46
+Test.java:47
+Test.java:48
+Test.java:49
+Test.java:50
+Test.java:51