Skip to content

Commit

Permalink
merge CollectionAddAll and UseAddAll
Browse files Browse the repository at this point in the history
  • Loading branch information
@Luro02
Luro02 committed Apr 4, 2024
1 parent 6daf58f commit 0a9c202
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 209 deletions.
71 changes: 67 additions & 4 deletions autograder-core/src/main/java/de/firemage/autograder/core/check/api/CollectionAddAll.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
import de.firemage.autograder.core.integrated.SpoonUtil;
import de.firemage.autograder.core.integrated.StaticAnalysis;
import de.firemage.autograder.core.check.api.UseEnumValues.CtEnumFieldRead;
import spoon.processing.AbstractProcessor;
import spoon.reflect.code.CtBlock;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtForEach;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtStatement;
import spoon.reflect.code.CtVariableAccess;
import spoon.reflect.code.CtVariableRead;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtEnum;
import spoon.reflect.declaration.CtEnumValue;
Expand All @@ -23,6 +24,7 @@
import spoon.reflect.reference.CtTypeParameterReference;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.reference.CtVariableReference;
import spoon.reflect.visitor.CtScanner;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -31,7 +33,11 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

@ExecutableCheck(reportedProblems = { ProblemType.COLLECTION_ADD_ALL, ProblemType.COMMON_REIMPLEMENTATION_ADD_ENUM_VALUES })
@ExecutableCheck(reportedProblems = {
ProblemType.COLLECTION_ADD_ALL,
ProblemType.COMMON_REIMPLEMENTATION_ADD_ENUM_VALUES,
ProblemType.COMMON_REIMPLEMENTATION_ADD_ALL
})
public class CollectionAddAll extends IntegratedCheck {
private record AddInvocation(
CtVariableReference<?> collection,
Expand Down Expand Up @@ -165,16 +171,73 @@ private void checkAddAll(CtBlock<?> ctBlock) {
}
}

private void checkAddAll(CtForEach ctFor) {
List<CtStatement> statements = SpoonUtil.getEffectiveStatements(ctFor.getBody());
if (statements.size() != 1) {
return;
}

if (statements.get(0) instanceof CtInvocation<?> ctInvocation
&& SpoonUtil.isSubtypeOf(ctInvocation.getTarget().getType(), java.util.Collection.class)
&& SpoonUtil.isSignatureEqualTo(ctInvocation.getExecutable(), boolean.class, "add", Object.class)
&& ctInvocation.getExecutable().getParameters().size() == 1
// ensure that the add argument simply accesses the for variable:
// for (T t : array) {
// collection.add(t);
// }
&& ctInvocation.getArguments().get(0) instanceof CtVariableRead<?> ctVariableRead
&& ctVariableRead.getVariable().equals(ctFor.getVariable().getReference())) {

// allow explicit casting
if (!ctInvocation.getArguments().get(0).getTypeCasts().isEmpty()) {
return;
}

String addAllArg = ctFor.getExpression().prettyprint();
if (ctFor.getExpression().getType().isArray()) {
addAllArg = "Arrays.asList(%s)".formatted(addAllArg);
}


this.addLocalProblem(
ctFor,
new LocalizedMessage(
"common-reimplementation",
Map.of(
"suggestion", "%s.addAll(%s)".formatted(
ctInvocation.getTarget().prettyprint(),
addAllArg
)
)
),
ProblemType.COMMON_REIMPLEMENTATION_ADD_ALL
);
}
}

@Override
protected void check(StaticAnalysis staticAnalysis, DynamicAnalysis dynamicAnalysis) {
staticAnalysis.processWith(new AbstractProcessor<CtBlock<?>>() {
staticAnalysis.getModel().getRootPackage().accept(new CtScanner() {
@Override
public void process(CtBlock<?> ctBlock) {
public <T> void visitCtBlock(CtBlock<T> ctBlock) {
if (ctBlock.isImplicit() || !ctBlock.getPosition().isValidPosition()) {
super.visitCtBlock(ctBlock);
return;
}

checkAddAll(ctBlock);
super.visitCtBlock(ctBlock);
}

@Override
public void visitCtForEach(CtForEach ctFor) {
if (ctFor.isImplicit() || !ctFor.getPosition().isValidPosition()) {
super.visitCtForEach(ctFor);
return;
}

checkAddAll(ctFor);
super.visitCtForEach(ctFor);
}
});
}
Expand Down
80 changes: 0 additions & 80 deletions autograder-core/src/main/java/de/firemage/autograder/core/check/api/UseAddAll.java
View file

This file was deleted.

109 changes: 97 additions & 12 deletions ...core/check/api/TestCollectionsAddAll.java → .../core/check/api/TestCollectionAddAll.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

class TestCollectionsAddAll extends AbstractCheckTest {
private static final String LOCALIZED_MESSAGE_KEY = "common-reimplementation";
private static final List<ProblemType> PROBLEM_TYPES = List.of(ProblemType.COLLECTION_ADD_ALL, ProblemType.COMMON_REIMPLEMENTATION_ADD_ENUM_VALUES);
class TestCollectionAddAll extends AbstractCheckTest {
private static final List<ProblemType> PROBLEM_TYPES = List.of(
ProblemType.COLLECTION_ADD_ALL,
ProblemType.COMMON_REIMPLEMENTATION_ADD_ENUM_VALUES,
ProblemType.COMMON_REIMPLEMENTATION_ADD_ALL
);

private void assertReimplementation(Problem problem, String suggestion) {
private void assertEqualsReimplementation(Problem problem, String suggestion) {
assertEquals(
this.linter.translateMessage(
new LocalizedMessage(
LOCALIZED_MESSAGE_KEY,
Map.of("suggestion", suggestion)
"common-reimplementation",
Map.of(
"suggestion", suggestion
)
)),
this.linter.translateMessage(problem.getExplanation())
);
Expand All @@ -48,7 +53,7 @@ public void foo(List<String> list) {
"""
), PROBLEM_TYPES);

assertReimplementation(problems.next(), "list.addAll(List.of(\" \", \"a\", \"b\"))");
assertEqualsReimplementation(problems.next(), "list.addAll(List.of(\" \", \"a\", \"b\"))");

problems.assertExhausted();
}
Expand Down Expand Up @@ -81,7 +86,7 @@ public static void main(String[] args) {
"""
), PROBLEM_TYPES);

assertReimplementation(problems.next(), "fruits.addAll(Arrays.asList(Fruit.values()))");
assertEqualsReimplementation(problems.next(), "fruits.addAll(Arrays.asList(Fruit.values()))");
problems.assertExhausted();
}

Expand Down Expand Up @@ -164,10 +169,90 @@ private static List<GodCard> getAvailableCardsDuplicateEnd() {
"""
), PROBLEM_TYPES);

assertReimplementation(problems.next(), "availableCards.addAll(Arrays.asList(GodCard.values()))");
assertReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.HERMES, GodCard.DEMETER, GodCard.ATLAS, GodCard.APOLLO, GodCard.ATHENA, GodCard.ARTEMIS))");
assertReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.APOLLO, GodCard.ARTEMIS, GodCard.ATHENA, GodCard.ATLAS, GodCard.ATLAS, GodCard.DEMETER, GodCard.HERMES))");
assertReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.APOLLO, GodCard.ARTEMIS, GodCard.ATHENA, GodCard.ATLAS, GodCard.DEMETER, GodCard.HERMES, GodCard.HERMES))");
assertEqualsReimplementation(problems.next(), "availableCards.addAll(Arrays.asList(GodCard.values()))");
assertEqualsReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.HERMES, GodCard.DEMETER, GodCard.ATLAS, GodCard.APOLLO, GodCard.ATHENA, GodCard.ARTEMIS))");
assertEqualsReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.APOLLO, GodCard.ARTEMIS, GodCard.ATHENA, GodCard.ATLAS, GodCard.ATLAS, GodCard.DEMETER, GodCard.HERMES))");
assertEqualsReimplementation(problems.next(), "availableCards.addAll(List.of(GodCard.APOLLO, GodCard.ARTEMIS, GodCard.ATHENA, GodCard.ATLAS, GodCard.DEMETER, GodCard.HERMES, GodCard.HERMES))");
problems.assertExhausted();
}

@Test
void testAddAllArray() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
JavaVersion.JAVA_17,
"Main",
"""
import java.util.Collection;
import java.util.ArrayList;
public class Main {
public static <T> Collection<T> toCollection(T[] array) {
Collection<T> result = new ArrayList<>();
for (T element : array) {
result.add(element);
}
return result;
}
}
"""
), PROBLEM_TYPES);

assertEqualsReimplementation(problems.next(), "result.addAll(Arrays.asList(array))");
problems.assertExhausted();
}

@Test
void testAddAllCollection() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
JavaVersion.JAVA_17,
"Main",
"""
import java.util.Collection;
import java.util.ArrayList;
public class Main {
public static <T> Collection<T> toCollection(Iterable<T> input) {
Collection<T> result = new ArrayList<>();
for (T element : input) {
result.add(element);
}
return result;
}
}
"""
), PROBLEM_TYPES);

assertEqualsReimplementation(problems.next(), "result.addAll(input)");
problems.assertExhausted();
}

@Test
void testAddAllCast() throws LinterException, IOException {
ProblemIterator problems = this.checkIterator(StringSourceInfo.fromSourceString(
JavaVersion.JAVA_17,
"Main",
"""
import java.util.Collection;
import java.util.ArrayList;
public class Main {
public static <T, U> Collection<U> toCollection(Iterable<T> input) {
Collection<U> result = new ArrayList<>();
for (T element : input) {
result.add((U) element);
}
return result;
}
}
"""
), PROBLEM_TYPES);

problems.assertExhausted();
}
}
Loading

0 comments on commit 0a9c202

Please sign in to comment.