detect reimplementation of Collections.nCopies #389

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

@@ -10,6 +10,7 @@ public enum ProblemType {
     AVOID_LABELS,
     SIMPLIFY_ARRAYS_FILL,
     AVOID_SHADOWING,
+    COLLECTIONS_N_COPIES,
     DO_NOT_USE_SYSTEM_EXIT,
     SCANNER_MUST_BE_CLOSED,
     EQUALS_HASHCODE_COMPARABLE_CONTRACT,

autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionsNCopies.java

@@ -0,0 +1,72 @@
+package de.firemage.autograder.core.check.api;
+
+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.ForLoopRange;
+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.CtExpression;
+import spoon.reflect.code.CtFor;
+import spoon.reflect.code.CtInvocation;
+import spoon.reflect.code.CtStatement;
+import spoon.reflect.code.CtVariableRead;
+import spoon.reflect.visitor.filter.VariableAccessFilter;
+
+import java.util.List;
+import java.util.Map;
+
+@ExecutableCheck(reportedProblems = { ProblemType.COLLECTIONS_N_COPIES })
+public class CollectionsNCopies extends IntegratedCheck {
+    @Override
+    protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
+        staticAnalysis.processWith(new AbstractProcessor<CtFor>() {
+            @Override
+            public void process(CtFor ctFor) {
+                if (ctFor.isImplicit() || !ctFor.getPosition().isValidPosition()) {
+                    return;
+                }
+
+                ForLoopRange forLoopRange = ForLoopRange.fromCtFor(ctFor).orElse(null);
+
+                List<CtStatement> statements = SpoonUtil.getEffectiveStatements(ctFor.getBody());
+
+                if (statements.size() != 1
+                    || forLoopRange == null
+                    || !(statements.get(0) instanceof CtInvocation<?> ctInvocation)
+                    || !(ctInvocation.getExecutable().getSimpleName().equals("add"))
+                    || ctInvocation.getArguments().size() != 1
+                    || !(ctInvocation.getTarget() instanceof CtVariableRead<?> ctVariableRead)
+                    || !SpoonUtil.isSubtypeOf(ctVariableRead.getType(), java.util.Collection.class)) {
+                    return;
+                }
+
+                // return if the for loop uses the loop variable (would not be a simple repetition)
+                if (!ctFor.getBody().getElements(new VariableAccessFilter<>(forLoopRange.loopVariable())).isEmpty()) {
+                    return;
+                }
+
+                CtExpression<?> rhs = ctInvocation.getArguments().get(0);
+                if (!SpoonUtil.isImmutable(rhs.getType())) {
+                    return;
+                }
+
+                addLocalProblem(
+                    ctFor,
+                    new LocalizedMessage(
+                        "common-reimplementation",
+                        Map.of("suggestion", "%s.addAll(Collections.nCopies(%s, %s))".formatted(
+                            ctVariableRead.prettyprint(),
+                            forLoopRange.length().prettyprint(),
+                            rhs.prettyprint()
+                        ))
+                    ),
+                    ProblemType.COLLECTIONS_N_COPIES
+                );
+            }
+        });
+    }
+}

autograder-core/src/test/java/de/firemage/autograder/core/check/api/TestCollectionsNCopies.java

@@ -0,0 +1,78 @@
+package de.firemage.autograder.core.check.api;
+
+import de.firemage.autograder.core.LinterException;
+import de.firemage.autograder.core.LocalizedMessage;
+import de.firemage.autograder.core.Problem;
+import de.firemage.autograder.core.ProblemType;
+import de.firemage.autograder.core.check.AbstractCheckTest;
+import de.firemage.autograder.core.compiler.JavaVersion;
+import de.firemage.autograder.core.file.StringSourceInfo;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class TestCollectionsNCopies extends AbstractCheckTest {
+    private static final String LOCALIZED_MESSAGE_KEY = "common-reimplementation";
+    private static final List<ProblemType> PROBLEM_TYPES = List.of(ProblemType.COLLECTIONS_N_COPIES);
+
+    private void assertReimplementation(Problem problem, String suggestion) {
+        assertEquals(
+            this.linter.translateMessage(
+                new LocalizedMessage(
+                    LOCALIZED_MESSAGE_KEY,
+                    Map.of("suggestion", suggestion)
+                )),
+            this.linter.translateMessage(problem.getExplanation())
+        );
+    }
+
+    @Test
+    void testListAdd() throws LinterException, IOException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+                import java.util.List;
+
+                public class Test {
+                    public void foo(List<String> list) {
+                        for (int i = 0; i < 10; i++) {
+                            list.add(" ");
+                        }
+                    }
+                }
+                """
+        ), PROBLEM_TYPES);
+
+        assertReimplementation(problems.next(), "list.addAll(Collections.nCopies(10, \" \"))");
+
+        problems.assertExhausted();
+    }
+
+    @Test
+    void testCollectionAdd() throws LinterException, IOException {
+        ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
+            JavaVersion.JAVA_17,
+            "Test",
+            """
+                import java.util.Collection;
+
+                public class Test {
+                    public void foo(Collection<Integer> collection, int n) {
+                        for (int i = 1; i < n; i++) {
+                            collection.add(1);
+                        }
+                    }
+                }
+                """
+        ), PROBLEM_TYPES);
+
+        assertReimplementation(problems.next(), "collection.addAll(Collections.nCopies(n - 1, 1))");
+
+        problems.assertExhausted();
+    }
+}

sample_config.yaml

@@ -152,3 +152,4 @@
 - IMPLEMENT_COMPARABLE
 - SIMPLIFY_ARRAYS_FILL
 - COMMON_REIMPLEMENTATION_ITERABLE_DUPLICATES
+- COLLECTIONS_N_COPIES