use the new UsesFinder in AvoidRecompilingRegex and RegexCheck …

…to improve performance
Feuermagier · Jun 1, 2024 · 623aab0 · 623aab0
1 parent 11e6aaf
commit 623aab0
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 26 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
@@ -6,6 +6,7 @@
 import de.firemage.autograder.core.integrated.IntegratedCheck;
 import de.firemage.autograder.core.integrated.SpoonUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
+import de.firemage.autograder.core.integrated.UsesFinder;
 import de.firemage.autograder.treeg.InvalidRegExSyntaxException;
 import de.firemage.autograder.treeg.RegExParser;
 import de.firemage.autograder.treeg.RegularExpression;
@@ -26,11 +27,9 @@
 import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtLiteral;
 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;
@@ -77,12 +76,10 @@ private static boolean isInAllowedContext(CtLiteral<?> ctLiteral) {
         CtElement parent = ctLiteral.getParent();
         if (parent instanceof CtVariable<?> ctVariable
             && SpoonUtil.isEffectivelyFinal(ctVariable)) {
-            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));
+            // Check if the variable is only used in a regex invocation (e.g. Pattern.compile)
+            return UsesFinder.variableUses(ctVariable)
+                .hasAnyAndAllMatch(ctVariableAccess -> ctVariableAccess.getParent() instanceof CtInvocation<?> ctInvocation
+                    && isRegexInvocation(ctInvocation));
         }
 
         return parent instanceof CtInvocation<?> ctInvocation && isRegexInvocation(ctInvocation);

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/UnusedImport.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/UnusedImport.java
@@ -1,6 +1,5 @@
 package de.firemage.autograder.core.check.complexity;
 
-import de.firemage.autograder.core.CodeModel;
 import de.firemage.autograder.core.LocalizedMessage;
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
@@ -126,7 +125,7 @@ private boolean isInSamePackage(CtElement ctElement, CtCompilationUnit ctCompila
         return Objects.equals(declaredPackage, ctCompilationUnit.getDeclaredPackage());
     }
 
-    private void checkImport(CtImport ctImport, CtCompilationUnit ctCompilationUnit, Collection<? super CtElement> importedElements, CodeModel model) {
+    private void checkImport(CtImport ctImport, CtCompilationUnit ctCompilationUnit, Collection<? super CtElement> importedElements) {
         // check if the import is from the java.lang package, which is redundant
 
         // inner class imports might not be redundant, therefore, they are skipped here
@@ -220,7 +219,7 @@ protected void check(StaticAnalysis staticAnalysis) {
                     continue;
                 }
 
-                this.checkImport(ctImport, ctCompilationUnit, importedElements, staticAnalysis.getCodeModel());
+                this.checkImport(ctImport, ctCompilationUnit, importedElements);
             }
         });
     }

diff --git a/...r-core/src/main/java/de/firemage/autograder/core/check/general/AvoidRecompilingRegex.java b/...r-core/src/main/java/de/firemage/autograder/core/check/general/AvoidRecompilingRegex.java
@@ -6,14 +6,13 @@
 import de.firemage.autograder.core.integrated.IntegratedCheck;
 import de.firemage.autograder.core.integrated.SpoonUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
+import de.firemage.autograder.core.integrated.UsesFinder;
 import spoon.processing.AbstractProcessor;
 import spoon.reflect.code.CtExpression;
 import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtLiteral;
 import spoon.reflect.code.CtTypeAccess;
 import spoon.reflect.declaration.CtField;
-import spoon.reflect.reference.CtReference;
-import spoon.reflect.visitor.filter.DirectReferenceFilter;
 
 import java.util.List;
 import java.util.Map;
@@ -45,19 +44,9 @@ public void process(CtField<String> ctField) {
                     return;
                 }
 
-                List<CtReference> references = staticAnalysis.getModel()
-                    .getElements(new DirectReferenceFilter<>(ctField.getReference()));
-
-                boolean isPattern = false;
-
-                for (CtReference ctReference : references) {
-                    CtInvocation<?> ctInvocation = ctReference.getParent(CtInvocation.class);
-                    if (ctInvocation == null || !isPatternInvocation(ctInvocation)) {
-                        return;
-                    }
-
-                    isPattern = true;
-                }
+                boolean isPattern = UsesFinder.variableUses(ctField)
+                    .hasAnyAndAllMatch(ctVariableAccess -> ctVariableAccess.getParent() instanceof CtInvocation<?> ctInvocation
+                        && isPatternInvocation(ctInvocation));
 
                 if (isPattern) {
                     addLocalProblem(

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/CtElementStream.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/CtElementStream.java
@@ -168,6 +168,20 @@ public boolean hasAnyMatch(Predicate<? super T> predicate) {
         return new CtElementStream<>(baseStream.filter(predicate)).hasAny();
     }
 
+    /**
+     * Checks whether all elements in the stream match the given predicate AND the stream is not empty.
+     *
+     * @param predicate the predicate to test the elements against
+     * @return true if all elements match the predicate and the stream is not empty
+     */
+    public boolean hasAnyAndAllMatch(Predicate<? super T> predicate) {
+        boolean[] isEmpty = { true };
+        return this.allMatch(element -> {
+            isEmpty[0] = false;
+            return predicate.test(element);
+        }) && !isEmpty[0];
+    }
+
     /**
      * Checks whether the stream contains no elements.
      * @return