From 6153116289fd5a824ce31742df298dae1182c6ae Mon Sep 17 00:00:00 2001 From: Luro02 <24826124+Luro02@users.noreply.github.com> Date: Tue, 17 Sep 2024 18:05:23 +0200 Subject: [PATCH] allow variable declarations in loop should be for between counter and loop #539 --- .../core/check/general/LoopShouldBeFor.java | 9 +++-- .../check/general/TestLoopShouldBeFor.java | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/general/LoopShouldBeFor.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/general/LoopShouldBeFor.java index b5db18e3..8855d7cd 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/general/LoopShouldBeFor.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/general/LoopShouldBeFor.java @@ -1,6 +1,5 @@ package de.firemage.autograder.core.check.general; -import de.firemage.autograder.core.CodeModel; import de.firemage.autograder.core.LocalizedMessage; import de.firemage.autograder.core.ProblemType; import de.firemage.autograder.core.check.ExecutableCheck; @@ -63,7 +62,7 @@ public String toString() { } } - private static LoopSuggestion getCounter(CtLoop ctLoop, CodeModel model) { + private static LoopSuggestion getCounter(CtLoop ctLoop) { List statements = StatementUtil.getEffectiveStatements(ctLoop.getBody()); if (statements.isEmpty()) { @@ -72,6 +71,10 @@ private static LoopSuggestion getCounter(CtLoop ctLoop, CodeModel model) { CtStatement previous = StatementUtil.getPreviousStatement(ctLoop).orElse(null); + while (previous instanceof CtLocalVariable ctLocalVariable && !TypeUtil.isPrimitiveNumeric(ctLocalVariable.getType())) { + previous = StatementUtil.getPreviousStatement(previous).orElse(null); + } + if (!(previous instanceof CtLocalVariable ctLocalVariable) || !TypeUtil.isPrimitiveNumeric(ctLocalVariable.getType())) { return null; } @@ -203,7 +206,7 @@ public void process(CtLoop ctLoop) { return; } - LoopSuggestion forLoop = getCounter(ctLoop, staticAnalysis.getCodeModel()); + LoopSuggestion forLoop = getCounter(ctLoop); if (forLoop != null) { addLocalProblem( diff --git a/autograder-core/src/test/java/de/firemage/autograder/core/check/general/TestLoopShouldBeFor.java b/autograder-core/src/test/java/de/firemage/autograder/core/check/general/TestLoopShouldBeFor.java index 7473a2da..95a8753e 100644 --- a/autograder-core/src/test/java/de/firemage/autograder/core/check/general/TestLoopShouldBeFor.java +++ b/autograder-core/src/test/java/de/firemage/autograder/core/check/general/TestLoopShouldBeFor.java @@ -257,6 +257,43 @@ void test(String[] array) { problems.assertExhausted(); } + @Test + void testCounterDeclaredWithOtherVariablesBeforeLoop() throws IOException, LinterException { + ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString( + JavaVersion.JAVA_17, + "Test", + """ + class Test { + String test(String sentence) { + int c = 0; + boolean spaceBefore = true; + String word = ""; + while (c < sentence.length()) { + if (Character.isWhitespace(sentence.charAt(c))) { + spaceBefore = true; + } else if (spaceBefore) { + word += sentence.charAt(c); + spaceBefore = false; + } + c++; + } + return word; + } + } + """ + ), PROBLEM_TYPES); + + assertEqualsFor( + problems.next(), + "int c = 0", + "c < sentence.length()", + "c++", + "{%n if (Character.isWhitespace(sentence.charAt(c))) {%n spaceBefore = true;%n } else if (spaceBefore) {%n word += sentence.charAt(c);%n spaceBefore = false;%n }%n}".formatted() + ); + + problems.assertExhausted(); + } + @Test void testMissingUpdate() throws IOException, LinterException { ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(