detect loops that can be do-while loops

autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java

@@ -79,6 +79,7 @@ public enum ProblemType {
     DOUBLE_BRACE_INITIALIZATION,
     INSTANCE_FIELD_CAN_BE_LOCAL,
     FOR_CAN_BE_FOREACH,
+    LOOP_SHOULD_BE_DO_WHILE,
     OVERRIDE_ANNOTATION_MISSING,
     SYSTEM_SPECIFIC_LINE_BREAK,
     BOOLEAN_GETTER_NOT_CALLED_IS,

autograder-core/src/main/java/de/firemage/autograder/core/check/general/LoopShouldBeDoWhile.java

@@ -0,0 +1,93 @@
+package de.firemage.autograder.core.check.general;
+
+import de.firemage.autograder.core.LocalizedMessage;
+import de.firemage.autograder.core.ProblemType;
+import de.firemage.autograder.core.check.ExecutableCheck;
+import de.firemage.autograder.core.dynamic.DynamicAnalysis;
+import de.firemage.autograder.core.integrated.IntegratedCheck;
+import de.firemage.autograder.core.integrated.SpoonUtil;
+import de.firemage.autograder.core.integrated.StaticAnalysis;
+import spoon.processing.AbstractProcessor;
+import spoon.reflect.code.CtStatement;
+import spoon.reflect.code.CtWhile;
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.Map;
+
+
+@ExecutableCheck(reportedProblems = { ProblemType.LOOP_SHOULD_BE_DO_WHILE })
+public class LoopShouldBeDoWhile extends IntegratedCheck {
+    @Override
+    protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
+        staticAnalysis.processWith(new AbstractProcessor<CtWhile>() {
+            @Override
+            public void process(CtWhile ctWhile) {
+                if (ctWhile.isImplicit() || !ctWhile.getPosition().isValidPosition() || ctWhile.getBody() == null) {
+                    return;
+                }
+
+                // This check detects loops that should be do-while loops, for example:
+                //
+                // ```
+                // a;
+                // b;
+                // c;
+                // while (condition) {
+                //   a;
+                //   b;
+                //   c;
+                // }
+                // ```
+                //
+                // ```
+                // a;
+                // b;
+                // c;
+                // while (condition) {
+                //   b;
+                //   c;
+                // }
+                // ```
+                //
+                // ```
+                // a;
+                // b;
+                // c;
+                // while (condition) {
+                //   a;
+                //   b;
+                // }
+                // ```
+
+                Deque<CtStatement> loopStatements = new ArrayDeque<>(SpoonUtil.getEffectiveStatements(ctWhile.getBody()));
+                CtStatement currentStatementBeforeLoop = SpoonUtil.getPreviousStatement(ctWhile).orElse(null);
+                if (currentStatementBeforeLoop == null || loopStatements.isEmpty()) {
+                    return;
+                }
+
+                while (!loopStatements.isEmpty() && currentStatementBeforeLoop != null) {
+                    CtStatement lastStatement = loopStatements.removeLast();
+
+                    if (!lastStatement.equals(currentStatementBeforeLoop)) {
+                        return;
+                    }
+
+                    currentStatementBeforeLoop = SpoonUtil.getPreviousStatement(currentStatementBeforeLoop).orElse(null);
+                }
+
+                if (loopStatements.isEmpty()) {
+                    addLocalProblem(
+                        ctWhile.getLoopingExpression(),
+                        new LocalizedMessage(
+                            "loop-should-be-do-while",
+                            Map.of("suggestion", """
+                            %ndo %s while (%s)""".formatted(SpoonUtil.truncatedSuggestion(ctWhile.getBody()), ctWhile.getLoopingExpression()))
+                        ),
+                        ProblemType.LOOP_SHOULD_BE_DO_WHILE
+                    );
+                }
+            }
+        });
+    }
+}

autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java

@@ -85,6 +85,7 @@
 import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.Set;
+import java.util.StringJoiner;
 import java.util.function.BiPredicate;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -1394,6 +1395,16 @@ private static List<CtElement> findUses(CtElement ctElement) {
         return new ArrayList<>(ctElement.getFactory().getModel().getElements(new UsesFilter(ctElement)));
     }
 
+    private static <T> int referenceIndexOf(List<T> list, T element) {
+        for (int i = 0; i < list.size(); i++) {
+            if (list.get(i) == element) {
+                return i;
+            }
+        }
+
+        return -1;
+    }
+
     /**
      * Finds the statement that is before the given statement if possible.
      *
@@ -1403,7 +1414,7 @@ private static List<CtElement> findUses(CtElement ctElement) {
     public static Optional<CtStatement> getPreviousStatement(CtStatement ctStatement) {
         if (ctStatement.getParent() instanceof CtStatementList ctStatementList) {
             List<CtStatement> statements = ctStatementList.getStatements();
-            int index = statements.indexOf(ctStatement);
+            int index = referenceIndexOf(statements, ctStatement);
 
             if (index > 0) {
                 return Optional.of(statements.get(index - 1));
@@ -1417,7 +1428,7 @@ public static List<CtStatement> getNextStatements(CtStatement ctStatement) {
         List<CtStatement> result = new ArrayList<>();
         if (ctStatement.getParent() instanceof CtStatementList ctStatementList) {
             List<CtStatement> statements = ctStatementList.getStatements();
-            int index = statements.indexOf(ctStatement);
+            int index = referenceIndexOf(statements, ctStatement);
 
             if (index > 0) {
                 result.addAll(statements.subList(index + 1, statements.size()));
@@ -1427,6 +1438,30 @@ public static List<CtStatement> getNextStatements(CtStatement ctStatement) {
         return result;
     }
 
+    public static String truncatedSuggestion(CtElement ctElement) {
+        StringJoiner result = new StringJoiner(System.lineSeparator());
+
+        for (String line : ctElement.toString().split("\\r?\\n")) {
+            if (result.length() > 150) {
+                if (line.startsWith(" ")) {
+                    result.add("...".indent(line.length() - line.stripIndent().length()).stripTrailing());
+                } else {
+                    result.add("...");
+                }
+
+                if (result.toString().startsWith("{")) {
+                    result.add("}");
+                }
+
+                break;
+            }
+
+            result.add(line);
+        }
+
+        return result.toString();
+    }
+
     public static Optional<Effect> tryMakeEffect(CtStatement ctStatement) {
         return TerminalStatement.of(ctStatement).or(() -> AssignmentStatement.of(ctStatement));
     }

autograder-core/src/main/resources/strings.de.ftl

@@ -200,6 +200,8 @@ object-datatype = Statt dem Datentyp 'Object', sollte die Variable '{$variable}'
 
 magic-string = Der String '{$value}' sollte in eine Konstante ausgelagert werden. Siehe Wiki Artikel zu Magic Strings.
 
+loop-should-be-do-while = Diese Schleife sollte eine do-while Schleife sein, weil der Code vor der Schleife, der gleiche wie in der Schleife ist: {$suggestion}
+
 # Naming
 bool-getter-name = Für boolean getter bietet es sich an ein Verb als Präfix zu verwenden. Zum Beispiel '{$newName}' statt '{$oldName}'.
 

autograder-core/src/main/resources/strings.en.ftl

@@ -200,6 +200,7 @@ object-datatype = Instead of the datatype 'Object', the variable '{$variable}' s
 
 magic-string = The string '{$value}' should be in a constant. See the wiki article for magic strings.
 
+loop-should-be-do-while = This loop should be a do-while loop, because the code before the loop is the same as the code in the loop: {$suggestion}
 
 # Naming
 bool-getter-name = For boolean getters it is recommended to use a verb as a prefix. For example '{$newName}' instead of '{$oldName}'.