Merge pull request #491 from Luro02/main

Implement some things
Feuermagier · Apr 17, 2024 · 3448a3e · 3448a3e
2 parents dd5176a + dbb33f2
commit 3448a3e
Show file tree

Hide file tree

Showing 51 changed files with 1,643 additions and 97 deletions.
diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java b/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java
@@ -79,6 +79,8 @@ public enum ProblemType {
     DOUBLE_BRACE_INITIALIZATION,
     INSTANCE_FIELD_CAN_BE_LOCAL,
     FOR_CAN_BE_FOREACH,
+    LOOP_SHOULD_BE_DO_WHILE,
+    LOOP_SHOULD_BE_FOR,
     OVERRIDE_ANNOTATION_MISSING,
     SYSTEM_SPECIFIC_LINE_BREAK,
     BOOLEAN_GETTER_NOT_CALLED_IS,

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/AvoidStringConcat.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/AvoidStringConcat.java
@@ -46,8 +46,8 @@ public void process(CtInvocation<?> ctInvocation) {
                         "common-reimplementation",
                         Map.of(
                             "suggestion", "%s + %s".formatted(
-                                ctInvocation.getTarget().prettyprint(),
-                                ctInvocation.getArguments().get(0).prettyprint()
+                                ctInvocation.getTarget(),
+                                ctInvocation.getArguments().get(0)
                             )
                         )
                     ),

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java
@@ -39,7 +39,7 @@ default CtExpression<R> suggest(CtExpression<T> ctExpression) {
     }
 
     private static final Map<Range<Character>, Suggester<Character, Boolean>> MAPPING = Map.of(
-        Range.between('a', 'z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
+        Range.of('a', 'z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
@@ -52,7 +52,7 @@ default CtExpression<R> suggest(CtExpression<T> ctExpression) {
             ),
             BinaryOperatorKind.AND
         ),
-        Range.between('A', 'Z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
+        Range.of('A', 'Z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
             SpoonUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
@@ -65,7 +65,7 @@ default CtExpression<R> suggest(CtExpression<T> ctExpression) {
             ),
             BinaryOperatorKind.AND
         ),
-        Range.between('0', '9'), (factory, ctExpression, targetType) -> SpoonUtil.createStaticInvocation(
+        Range.of('0', '9'), (factory, ctExpression, targetType) -> SpoonUtil.createStaticInvocation(
             targetType,
             "isDigit",
             SpoonUtil.castExpression(char.class, ctExpression)
@@ -140,7 +140,7 @@ public void process(CtBinaryOperator<Boolean> ctBinaryOperator) {
                         ctBinaryOperator,
                         new LocalizedMessage(
                             "common-reimplementation",
-                            Map.of("suggestion", suggestion.prettyprint())
+                            Map.of("suggestion", suggestion)
                         ),
                         ProblemType.CHAR_RANGE
                     );

diff --git a/...der-core/src/main/java/de/firemage/autograder/core/check/api/CheckIterableDuplicates.java b/...der-core/src/main/java/de/firemage/autograder/core/check/api/CheckIterableDuplicates.java
@@ -30,12 +30,12 @@ public class CheckIterableDuplicates extends IntegratedCheck {
     private static String buildSuggestion(CtExpression<?> ctExpression, boolean isNegated) {
         CtTypeReference<?> type = ctExpression.getType();
 
-        String leftSide = ctExpression.prettyprint();
+        String leftSide = ctExpression.toString();
         String rightSide = "%s.size()".formatted(leftSide);
 
         if (type.isArray()) {
             leftSide = "Arrays.asList(%s)".formatted(leftSide);
-            rightSide = "%s.length".formatted(ctExpression.prettyprint());
+            rightSide = "%s.length".formatted(ctExpression);
         }
 
         if (isNegated) {

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionAddAll.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionAddAll.java
@@ -199,7 +199,7 @@ private void checkAddAll(CtForEach ctFor) {
                 return;
             }
 
-            String addAllArg = ctFor.getExpression().prettyprint();
+            String addAllArg = ctFor.getExpression().toString();
             if (ctFor.getExpression().getType().isArray()) {
                 addAllArg = "Arrays.asList(%s)".formatted(addAllArg);
             }
@@ -211,7 +211,7 @@ private void checkAddAll(CtForEach ctFor) {
                     "common-reimplementation",
                     Map.of(
                         "suggestion", "%s.addAll(%s)".formatted(
-                            ctInvocation.getTarget().prettyprint(),
+                            ctInvocation.getTarget(),
                             addAllArg
                         )
                     )

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionsNCopies.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionsNCopies.java
@@ -59,9 +59,9 @@ public void process(CtFor ctFor) {
                     new LocalizedMessage(
                         "common-reimplementation",
                         Map.of("suggestion", "%s.addAll(Collections.nCopies(%s, %s))".formatted(
-                            ctVariableRead.prettyprint(),
-                            forLoopRange.length().prettyprint(),
-                            rhs.prettyprint()
+                            ctVariableRead,
+                            forLoopRange.length(),
+                            rhs
                         ))
                     ),
                     ProblemType.COLLECTIONS_N_COPIES

diff --git a/...ore/src/main/java/de/firemage/autograder/core/check/api/IsEmptyReimplementationCheck.java b/...ore/src/main/java/de/firemage/autograder/core/check/api/IsEmptyReimplementationCheck.java
@@ -135,19 +135,19 @@ private void checkIsEmptyReimplementation(CtExpression<?> target, CtBinaryOperat
             // f() <= 0    : isEmpty
             case EQ, LE -> {
                 if (isZero) {
-                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.prettyprint(), suggestion.prettyprint(), problemType);
+                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.toString(), suggestion.toString(), problemType);
                 }
             }
             // f() != 0    : !isEmpty
             case NE -> {
                 if (isZero) {
-                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.prettyprint(), SpoonUtil.negate(suggestion).prettyprint(), problemType);
+                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.toString(), SpoonUtil.negate(suggestion).toString(), problemType);
                 }
             }
             // f() >= 1    : !isEmpty
             case GE -> {
                 if (isOne) {
-                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.prettyprint(), SpoonUtil.negate(suggestion).prettyprint(), problemType);
+                    this.reportProblem(ctBinaryOperator, ctBinaryOperator.toString(), SpoonUtil.negate(suggestion).toString(), problemType);
                 }
             }
         }
@@ -156,12 +156,12 @@ private void checkIsEmptyReimplementation(CtExpression<?> target, CtBinaryOperat
     private void checkEqualsCall(CtExpression<?> target, CtInvocation<?> ctInvocation, ProblemType problemType) {
         CtExpression<?> argument = ctInvocation.getArguments().get(0);
         if (SpoonUtil.isStringLiteral(SpoonUtil.resolveConstant(argument), "")) {
-            this.reportProblem(ctInvocation, ctInvocation.prettyprint(), buildIsEmptySuggestion(target).prettyprint(), problemType);
+            this.reportProblem(ctInvocation, ctInvocation.toString(), buildIsEmptySuggestion(target).toString(), problemType);
         }
 
         // detect "".equals(s)
         if (SpoonUtil.isStringLiteral(SpoonUtil.resolveConstant(target), "")) {
-            this.reportProblem(ctInvocation, ctInvocation.prettyprint(), buildIsEmptySuggestion(argument).prettyprint(), problemType);
+            this.reportProblem(ctInvocation, ctInvocation.toString(), buildIsEmptySuggestion(argument).toString(), problemType);
         }
     }
 

diff --git a/...grader-core/src/main/java/de/firemage/autograder/core/check/api/MathReimplementation.java b/...grader-core/src/main/java/de/firemage/autograder/core/check/api/MathReimplementation.java
@@ -88,7 +88,7 @@ private boolean checkHypot(CtExpression<?> ctExpression) {
                 ctExpression,
                 new LocalizedMessage(
                     "common-reimplementation",
-                    Map.of("suggestion", "Math.hypot(%s, %s)".formatted(left.get().prettyprint(), right.get().prettyprint()))
+                    Map.of("suggestion", "Math.hypot(%s, %s)".formatted(left.get(), right.get()))
                 ),
                 ProblemType.COMMON_REIMPLEMENTATION_HYPOT
             );
@@ -109,7 +109,7 @@ private void checkSqrt(CtExpression<?> ctExpression) {
                 ctExpression,
                 new LocalizedMessage(
                     "common-reimplementation",
-                    Map.of("suggestion", "Math.sqrt(%s)".formatted(ctInvocation.getArguments().get(0).prettyprint()))
+                    Map.of("suggestion", "Math.sqrt(%s)".formatted(ctInvocation.getArguments().get(0)))
                 ),
                 ProblemType.COMMON_REIMPLEMENTATION_SQRT
             );
@@ -191,9 +191,9 @@ private void checkMaxMin(CtIf ctIf) {
                     "common-reimplementation",
                     Map.of(
                         "suggestion", "%s = Math.max(%s, %s)".formatted(
-                            ctVariableWrite.prettyprint(),
-                            elseValue.prettyprint(),
-                            condition.getRightHandOperand().prettyprint()
+                            ctVariableWrite,
+                            elseValue,
+                            condition.getRightHandOperand()
                         )
                     )
                 ),
@@ -214,9 +214,9 @@ private void checkMaxMin(CtIf ctIf) {
                     "common-reimplementation",
                     Map.of(
                         "suggestion", "%s = Math.min(%s, %s)".formatted(
-                            ctVariableWrite.prettyprint(),
-                            elseValue.prettyprint(),
-                            condition.getRightHandOperand().prettyprint()
+                            ctVariableWrite,
+                            elseValue,
+                            condition.getRightHandOperand()
                         )
                     )
                 ),

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/StringRepeat.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/StringRepeat.java
@@ -56,15 +56,15 @@ private void checkStringRepeat(CtFor ctFor) {
                     Map.of(
                         // string.repeat(count)
                         "suggestion", "%s += %s".formatted(
-                            lhs.prettyprint(),
+                            lhs,
                             rhs.getFactory().createInvocation(
                                 rhs.clone(),
                                 rhs.getFactory().Type().get(java.lang.String.class)
                                     .getMethod("repeat", rhs.getFactory().createCtTypeReference(int.class))
                                     .getReference()
                                     .clone(),
                                 forLoopRange.length().clone()
-                            ).prettyprint())
+                            ))
                     )
                 ),
                 ProblemType.COMMON_REIMPLEMENTATION_STRING_REPEAT

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseArrayCopy.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseArrayCopy.java
@@ -47,11 +47,11 @@ private void checkArrayCopy(CtFor ctFor) {
                     Map.of(
                         // System.arraycopy(src, srcPos, dest, destPos, length)
                         "suggestion", "System.arraycopy(%s, %s, %s, %s, %s)".formatted(
-                            rhs.getTarget().prettyprint(),
-                            forLoopRange.start().prettyprint(),
-                            lhs.getTarget().prettyprint(),
-                            forLoopRange.start().prettyprint(),
-                            forLoopRange.length().prettyprint()
+                            rhs.getTarget(),
+                            forLoopRange.start(),
+                            lhs.getTarget(),
+                            forLoopRange.start(),
+                            forLoopRange.length()
                         )
                     )
                 ),

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseArraysFill.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseArraysFill.java
@@ -58,19 +58,19 @@ private void checkArraysFill(CtFor ctFor) {
         }
 
         String suggestion = "Arrays.fill(%s, %s, %s, %s)".formatted(
-            ctArrayWrite.getTarget().prettyprint(),
-            forLoopRange.start().prettyprint(),
-            forLoopRange.end().prettyprint(),
-            ctAssignment.getAssignment().prettyprint()
+            ctArrayWrite.getTarget(),
+            forLoopRange.start(),
+            forLoopRange.end(),
+            ctAssignment.getAssignment()
         );
         if (forLoopRange.start() instanceof CtLiteral<Integer> ctLiteral
             && ctLiteral.getValue() == 0
             && forLoopRange.end() instanceof CtFieldAccess<Integer> fieldAccess
             && ctArrayWrite.getTarget().equals(fieldAccess.getTarget())
             && fieldAccess.getVariable().getSimpleName().equals("length")) {
             suggestion = "Arrays.fill(%s, %s)".formatted(
-                ctArrayWrite.getTarget().prettyprint(),
-                ctAssignment.getAssignment().prettyprint()
+                ctArrayWrite.getTarget(),
+                ctAssignment.getAssignment()
             );
         }
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseFormatString.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseFormatString.java
@@ -114,7 +114,7 @@ private String buildFormattedString(Iterable<? extends CtExpression<?>> ctExpres
             }
 
             formatString.append(this.getFormatPlaceholder(ctExpression.getType()));
-            args.add(ctExpression.prettyprint());
+            args.add(ctExpression.toString());
         }
 
         if (args.isEmpty() && formatString.length() >= MAXIMUM_STRING_LENGTH_IN_LINE) {
@@ -257,7 +257,7 @@ private void checkCtInvocation(CtInvocation<?> ctInvocation) {
 
         Collections.reverse(formatArgs);
 
-        String target = invocationExpression.prettyprint();
+        String target = invocationExpression.toString();
         this.checkArgs(ctInvocation, formatArgs, suggestion -> "%s.append(%s)".formatted(target, suggestion));
     }
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseModuloOperator.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseModuloOperator.java
@@ -80,8 +80,8 @@ private void checkModulo(CtIf ctIf) {
                     "common-reimplementation",
                     Map.of(
                         "suggestion", "%s %%= %s".formatted(
-                            assignedVariable.prettyprint(),
-                            right.prettyprint()
+                            assignedVariable,
+                            right
                         )
                     )
                 ),

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseSubList.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseSubList.java
@@ -58,10 +58,10 @@ private void checkSubList(CtFor ctFor) {
                 "common-reimplementation",
                 Map.of(
                     "suggestion", "for (%s value : %s.subList(%s, %s)) { ... }".formatted(
-                        listElementType.unbox().prettyprint(),
+                        listElementType.unbox(),
                         ctListVariable.getSimpleName(),
-                        forLoopRange.start().prettyprint(),
-                        forLoopRange.end().prettyprint()
+                        forLoopRange.start(),
+                        forLoopRange.end()
                     )
                 )
             ),

diff --git a/...rader-core/src/main/java/de/firemage/autograder/core/check/complexity/ChainedIfCheck.java b/...rader-core/src/main/java/de/firemage/autograder/core/check/complexity/ChainedIfCheck.java
@@ -42,7 +42,7 @@ public void process(CtIf ctIf) {
                                     ctIf.getCondition(),
                                     nestedIf.getCondition(),
                                     spoon.reflect.code.BinaryOperatorKind.AND
-                                ).prettyprint()
+                                )
                             )
                         ),
                         ProblemType.MERGE_NESTED_IF

diff --git a/...ore/src/main/java/de/firemage/autograder/core/check/complexity/RedundantBooleanEqual.java b/...ore/src/main/java/de/firemage/autograder/core/check/complexity/RedundantBooleanEqual.java
@@ -38,7 +38,7 @@ private void reportProblem(CtBinaryOperator<?> ctBinaryOperator, boolean literal
             new LocalizedMessage(
                 "redundant-boolean-equal",
                 Map.of(
-                    "suggestion", suggestion.prettyprint()
+                    "suggestion", suggestion
                 )
             ),
             ProblemType.REDUNDANT_BOOLEAN_EQUAL

diff --git a/...grader-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantElse.java b/...grader-core/src/main/java/de/firemage/autograder/core/check/complexity/RedundantElse.java
@@ -84,8 +84,8 @@ private void checkCtIf(CtIf ctIf) {
             new LocalizedMessage(
                 "redundant-else",
                 Map.of(
-                    "expected", "if (a) { ... %s; }%s elseCode;".formatted(effect.ctStatement().prettyprint(), elseIf),
-                    "given", "if (a) { ... %s; }%s else { elseCode; }".formatted(effect.ctStatement().prettyprint(), elseIf)
+                    "expected", "if (a) { ... %s; }%s elseCode;".formatted(effect.ctStatement(), elseIf),
+                    "given", "if (a) { ... %s; }%s else { elseCode; }".formatted(effect.ctStatement(), elseIf)
                 )
             ),
             ProblemType.REDUNDANT_ELSE

diff --git a/...rc/main/java/de/firemage/autograder/core/check/complexity/RedundantIfForBooleanCheck.java b/...rc/main/java/de/firemage/autograder/core/check/complexity/RedundantIfForBooleanCheck.java
@@ -30,13 +30,13 @@ public class RedundantIfForBooleanCheck extends IntegratedCheck {
     private String makeSuggestion(CtExpression<?> ctExpression, Effect thenEffect, Effect elseEffect) {
         if (thenEffect instanceof AssignmentEffect thenAssignment && elseEffect instanceof AssignmentEffect) {
             return "%s = %s".formatted(
-                thenAssignment.target().prettyprint(),
-                ctExpression.prettyprint()
+                thenAssignment.target(),
+                ctExpression
             );
         }
 
         // otherwise it is a return statement
-        return "return %s".formatted(ctExpression.prettyprint());
+        return "return %s".formatted(ctExpression);
     }
 
     private void checkIfElse(CtExpression<?> condition, CtStatement thenStmt, CtStatement elseStmt) {

diff --git a/...re/src/main/java/de/firemage/autograder/core/check/complexity/RedundantNegationCheck.java b/...re/src/main/java/de/firemage/autograder/core/check/complexity/RedundantNegationCheck.java
@@ -58,7 +58,7 @@ public void process(CtExpression<?> ctExpression) {
                     ctExpression,
                     new LocalizedMessage(
                         "common-reimplementation",
-                        Map.of("suggestion", negated.prettyprint())
+                        Map.of("suggestion", negated)
                     ),
                     ProblemType.REDUNDANT_NEGATION
                 );

diff --git a/...ain/java/de/firemage/autograder/core/check/complexity/RedundantUninitializedVariable.java b/...ain/java/de/firemage/autograder/core/check/complexity/RedundantUninitializedVariable.java
@@ -29,7 +29,7 @@ private static String makeSuggestion(CtLocalVariable<?> ctLocalVariable, CtExpre
 
         return "%s%s %s = %s".formatted(
             modifier,
-            ctLocalVariable.getType().prettyprint(),
+            ctLocalVariable.getType(),
             ctLocalVariable.getSimpleName(),
             ctValue
         );
@@ -66,7 +66,7 @@ public void process(CtLocalVariable<?> ctLocalVariable) {
                     ctLocalVariable,
                     new LocalizedMessage("redundant-uninitialized-variable", Map.of(
                         "variable", ctLocalVariable.getSimpleName(),
-                        "value", ctValue.prettyprint(),
+                        "value", ctValue,
                         "suggestion", makeSuggestion(ctLocalVariable, ctValue)
                     )),
                     ProblemType.REDUNDANT_UNINITIALIZED_VARIABLE

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
@@ -69,7 +69,7 @@ private void checkVariableRead(CtStatement ctStatement, CtVariableRead<?> ctVari
                     "redundant-variable",
                     Map.of(
                         "name", ctLocalVariable.getSimpleName(),
-                        "suggestion", ctStatement.prettyprint().replace(ctLocalVariable.getSimpleName(), ctLocalVariable.getDefaultExpression().prettyprint())
+                        "suggestion", ctStatement.toString().replace(ctLocalVariable.getSimpleName(), ctLocalVariable.getDefaultExpression().toString())
                     )
                 ),
                 ProblemType.REDUNDANT_VARIABLE

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
@@ -69,7 +69,7 @@ private void reportProblem(CtImport ctImport) {
             ctImport,
             new LocalizedMessage(
                 "unused-import",
-                Map.of("import", ctImport.prettyprint())
+                Map.of("import", ctImport)
             ),
             ProblemType.UNUSED_IMPORT
         );

diff --git a/...src/main/java/de/firemage/autograder/core/check/complexity/WrapperInstantiationCheck.java b/...src/main/java/de/firemage/autograder/core/check/complexity/WrapperInstantiationCheck.java
@@ -40,7 +40,7 @@ public void process(CtConstructorCall<?> ctConstructorCall) {
                 if (isPrimitiveWrapper(boxedType) && ctConstructorCall.getArguments().size() == 1) {
                     // the primitive wrapper constructors should all have exactly one argument
                     CtExpression<?> value = ctConstructorCall.getArguments().get(0);
-                    String suggestion = "%s".formatted(value.prettyprint());
+                    String suggestion = "%s".formatted(value);
 
                     // check if the argument is not the unboxed type
                     // for example Integer(String)
@@ -54,7 +54,7 @@ public void process(CtConstructorCall<?> ctConstructorCall) {
                             "suggest-replacement",
                             Map.of(
                                 "suggestion", suggestion,
-                                "original", ctConstructorCall.prettyprint()
+                                "original", ctConstructorCall
                             )
                         ),
                         ProblemType.PRIMITIVE_WRAPPER_INSTANTIATION