From 4c1663d25444decf2a6b6316e875ec7c08c66af5 Mon Sep 17 00:00:00 2001 From: Luro02 <24826124+Luro02@users.noreply.github.com> Date: Mon, 8 Apr 2024 20:20:53 +0200 Subject: [PATCH 1/2] ignore overridden methods for empty block check --- .../autograder/core/check/unnecessary/EmptyBlockCheck.java | 7 +++++++ .../core/check_tests/EmptyBlockCheck/code/Test.java | 2 +- .../autograder/core/check_tests/EmptyBlockCheck/config.txt | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/unnecessary/EmptyBlockCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/unnecessary/EmptyBlockCheck.java index 41757b64..a1163e81 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/unnecessary/EmptyBlockCheck.java +++ b/autograder-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/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/code/Test.java b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/code/Test.java index 8ae5312d..3c124669 100644 --- a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/code/Test.java +++ b/autograder-core/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/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/config.txt b/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/config.txt index 4c11a9a2..f41d48b6 100644 --- a/autograder-core/src/test/resources/de/firemage/autograder/core/check_tests/EmptyBlockCheck/config.txt +++ b/autograder-core/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 From d29b8e4bd98602f01aea5a6d2464029b621453ed Mon Sep 17 00:00:00 2001 From: Luro02 <24826124+Luro02@users.noreply.github.com> Date: Tue, 9 Apr 2024 13:50:09 +0200 Subject: [PATCH 2/2] improve performance --- .../check/complexity/RedundantVariable.java | 2 +- .../autograder/core/integrated/SpoonUtil.java | 38 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantVariable.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantVariable.java index 243d1318..1765b3ae 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantVariable.java +++ b/autograder-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/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 index 312ab4ea..76411e43 100644 --- 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))); }