improve redundant variable check Feuermagier#635

Luro02 · Dec 9, 2024 · c2d06de · c2d06de
1 parent 1f1a0a8
commit c2d06de
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 31 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
@@ -38,7 +38,7 @@ private boolean isComplexExpression(CtExpression<?> ctExpression) {
         return ctExpression instanceof CtSwitchExpression<?,?> || ctExpression.toString().length() > MAX_EXPRESSION_SIZE;
     }
 
-    private void checkVariableRead(CtStatement ctStatement, CtVariableRead<?> ctVariableRead, CodeModel model) {
+    private void checkVariableRead(CtStatement ctStatement, CtVariableRead<?> ctVariableRead) {
         if (// the variable must be a local variable
             !(ctVariableRead.getVariable().getDeclaration() instanceof CtLocalVariable<?> ctLocalVariable)
             // it should not have any annotations (e.g. @SuppressWarnings("unchecked"))
@@ -82,19 +82,19 @@ private void checkVariableRead(CtStatement ctStatement, CtVariableRead<?> ctVari
     protected void check(StaticAnalysis staticAnalysis) {
         staticAnalysis.getModel().getRootPackage().accept(new CtScanner() {
             @Override
-            public <T> void visitCtInvocation(CtInvocation<T> ctInvocation) {
-                if (!ctInvocation.getPosition().isValidPosition()
-                    || ctInvocation.isImplicit()
-                    // only check invocations with a single variable
-                    || ctInvocation.getArguments().size() != 1
-                    || !(ctInvocation.getArguments().get(0) instanceof CtVariableRead<?> ctVariableRead)) {
-                    super.visitCtInvocation(ctInvocation);
+            public <T> void visitCtLocalVariable(CtLocalVariable<T> ctLocalVariable) {
+                if (!ctLocalVariable.getPosition().isValidPosition()
+                    || ctLocalVariable.isImplicit()
+                    // only check local variables with a default expression
+                    || ctLocalVariable.getDefaultExpression() == null
+                    || !(ctLocalVariable.getDefaultExpression() instanceof CtVariableRead<?> ctVariableRead)) {
+                    super.visitCtLocalVariable(ctLocalVariable);
                     return;
                 }
 
-                checkVariableRead(ctInvocation, ctVariableRead, staticAnalysis.getCodeModel());
+                checkVariableRead(ctLocalVariable, ctVariableRead);
 
-                super.visitCtInvocation(ctInvocation);
+                super.visitCtLocalVariable(ctLocalVariable);
             }
 
             @Override
@@ -107,7 +107,7 @@ public <T> void visitCtReturn(CtReturn<T> ctReturn) {
                     return;
                 }
 
-                checkVariableRead(ctReturn, ctVariableRead, staticAnalysis.getCodeModel());
+                checkVariableRead(ctReturn, ctVariableRead);
 
                 super.visitCtReturn(ctReturn);
             }

diff --git a/...ore/src/test/java/de/firemage/autograder/core/check/complexity/TestRedundantVariable.java b/...ore/src/test/java/de/firemage/autograder/core/check/complexity/TestRedundantVariable.java
@@ -355,26 +355,6 @@ public String get() {
         problems.assertExhausted();
     }
 
-    @Test
-    void testRedundantPrintln() throws IOException, LinterException {
-        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
-            JavaVersion.JAVA_17,
-            "Test",
-            """
-                public class Test {
-                    public static void main(String[] args) {
-                        int a = 5;
-                        System.out.println(a);
-                    }
-                }
-                """
-        ), PROBLEM_TYPES);
-
-        assertEqualsRedundant(problems.next(), "a", "System.out.println(5)");
-
-        problems.assertExhausted();
-    }
-
     @Test
     void testRedundantSwitchExpression() throws IOException, LinterException {
         ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(