Merge pull request #478 from Luro02/main

Some minor changes
Feuermagier · Apr 9, 2024 · 428c966 · 428c966
2 parents 794243d + d29b8e4
commit 428c966
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 12 deletions.
diff --git a/...er-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantVariable.java b/...er-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantVariable.java
@@ -43,7 +43,7 @@ private void checkVariableRead(CtStatement ctStatement, CtVariableRead<?> ctVari
             // it should not have any annotations (e.g. @SuppressWarnings("unchecked"))
             || !ctLocalVariable.getAnnotations().isEmpty()
             // the variable must only be used in the return statement
-            || SpoonUtil.findUsesOf(ctLocalVariable).size() != 1) {
+            || SpoonUtil.hasAnyUses(ctLocalVariable, ctElement -> ctElement.getParent(CtStatement.class) != ctStatement)) {
             return;
         }
 

diff --git a/...der-core/src/main/java/de/firemage/autograder/core/check/unnecessary/EmptyBlockCheck.java b/...der-core/src/main/java/de/firemage/autograder/core/check/unnecessary/EmptyBlockCheck.java
@@ -11,6 +11,7 @@
 import spoon.reflect.code.CtCatch;
 import spoon.reflect.code.CtComment;
 import spoon.reflect.code.CtSwitch;
+import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.visitor.CtScanner;
 
 @ExecutableCheck(reportedProblems = {ProblemType.EMPTY_BLOCK, ProblemType.EMPTY_CATCH})
@@ -31,6 +32,12 @@ public <T> void visitCtBlock(CtBlock<T> ctBlock) {
                     return;
                 }
 
+                if (ctBlock.getParent() instanceof CtMethod<?> ctMethod
+                    && ctMethod.getBody().equals(ctBlock)
+                    && SpoonUtil.isInOverriddenMethod(ctBlock)) {
+                    return;
+                }
+
                 if (ctBlock.getParent() instanceof CtCatch ctCatch && ctCatch.getBody().equals(ctBlock)) {
                     addLocalProblem(
                         ctCatch,

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java
@@ -21,6 +21,7 @@
 import spoon.reflect.code.CtConstructorCall;
 import spoon.reflect.code.CtExecutableReferenceExpression;
 import spoon.reflect.code.CtExpression;
+import spoon.reflect.code.CtFieldWrite;
 import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtJavaDoc;
 import spoon.reflect.code.CtLambda;
@@ -38,6 +39,7 @@
 import spoon.reflect.cu.SourcePosition;
 import spoon.reflect.cu.position.CompoundSourcePosition;
 import spoon.reflect.declaration.CtCompilationUnit;
+import spoon.reflect.declaration.CtConstructor;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtExecutable;
 import spoon.reflect.declaration.CtImport;
@@ -61,6 +63,7 @@
 import spoon.reflect.reference.CtTypeParameterReference;
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.Filter;
+import spoon.reflect.visitor.chain.CtQueryable;
 import spoon.reflect.visitor.filter.CompositeFilter;
 import spoon.reflect.visitor.filter.DirectReferenceFilter;
 import spoon.reflect.visitor.filter.FilteringOperator;
@@ -856,9 +859,10 @@ public static boolean isEffectivelyFinal(CtVariable<?> ctVariable) {
             return true;
         }
 
-        List<? extends CtVariableAccess<?>> variableUses = SpoonUtil.findUsesOf(ctVariable);
-
-        return variableUses.isEmpty() || variableUses.stream().noneMatch(variableAccess -> variableAccess instanceof CtVariableWrite<?>);
+        return !SpoonUtil.hasAnyUses(
+            ctVariable,
+            ctElement -> ctElement instanceof CtVariableWrite<?>
+        );
     }
 
     public static <T> Optional<CtExpression<T>> getEffectivelyFinalExpression(CtVariable<T> ctVariable) {
@@ -1358,19 +1362,35 @@ public static <T> List<CtElement> findUsesOf(CtExecutable<T> ctExecutable) {
         return SpoonUtil.findUses(ctExecutable);
     }
 
-    public static boolean hasAnyUses(CtElement ctElement, Predicate<? super CtElement> predicate) {
-        return ctElement.getFactory().getModel()
+    private static boolean internalHasAnyUses(CtQueryable model, CtElement ctElement, Predicate<? super CtElement> predicate) {
+        // for local variables, one does not need to search the whole model
+        if (ctElement instanceof CtLocalVariable<?> ctLocalVariable && model == ctElement.getFactory().getModel()) {
+            CtBlock<?> parentBlock = ctLocalVariable.getParent(CtBlock.class);
+            if (parentBlock != null) {
+                return parentBlock
+                    .filterChildren(new CompositeFilter<>(FilteringOperator.INTERSECTION, predicate::test, new UsesFilter(ctElement)))
+                    .first(CtElement.class) != null;
+            }
+        }
+
+        return model
             .filterChildren(new CompositeFilter<>(FilteringOperator.INTERSECTION, predicate::test, new UsesFilter(ctElement)))
             .first(CtElement.class) != null;
     }
 
+    public static boolean hasAnyUses(CtElement ctElement, Predicate<? super CtElement> predicate) {
+        return internalHasAnyUses(ctElement.getFactory().getModel(), ctElement, predicate);
+    }
+
+    public static boolean hasAnyUsesIn(CtElement ctElement, CtElement toSearchIn) {
+        return hasAnyUsesIn(toSearchIn, ctElement, element -> true);
+    }
+
     public static boolean hasAnyUsesIn(CtElement ctElement, CtElement toSearchIn, Predicate<? super CtElement> predicate) {
-        return toSearchIn
-            .filterChildren(new CompositeFilter<>(FilteringOperator.INTERSECTION, predicate::test, new UsesFilter(ctElement)))
-            .first(CtElement.class) != null;
+        return internalHasAnyUses(toSearchIn, ctElement, predicate);
     }
 
-    public static List<CtElement> findUses(CtElement ctElement) {
+    private static List<CtElement> findUses(CtElement ctElement) {
         return new ArrayList<>(ctElement.getFactory().getModel().getElements(new UsesFilter(ctElement)));
     }
 

diff --git a/...src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/code/Test.java b/...src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/code/Test.java
@@ -16,7 +16,7 @@ interface A {
 
     class B implements A {
         @Override
-        public void call() {} /*# not ok #*/
+        public void call() {} /*# ok #*/
     }
 
     class C implements A {

diff --git a/...ore/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/config.txt b/...ore/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/config.txt
@@ -5,7 +5,6 @@ Test.java:6
 Test.java:9-10
 Test.java:10
 
-Test.java:19
 Test.java:47
 Test.java:50-51
 Test.java:51-52