From c2d06ded1120d2293dbd58b3899eef48860054c7 Mon Sep 17 00:00:00 2001
From: Luro02 <24826124+Luro02@users.noreply.github.com>
Date: Mon, 9 Dec 2024 11:55:10 +0100
Subject: [PATCH] improve redundant variable check #635

---
 .../check/complexity/RedundantVariable.java   | 22 +++++++++----------
 .../complexity/TestRedundantVariable.java     | 20 -----------------
 2 files changed, 11 insertions(+), 31 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 a3654380..dc1aa9ad 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
@@ -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/autograder-core/src/test/java/de/firemage/autograder/core/check/complexity/TestRedundantVariable.java b/autograder-core/src/test/java/de/firemage/autograder/core/check/complexity/TestRedundantVariable.java
index bae8155b..6c245bfd 100644
--- a/autograder-core/src/test/java/de/firemage/autograder/core/check/complexity/TestRedundantVariable.java
+++ b/autograder-core/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(