From 34a44c5ef73eb7b2a7d56726100633991d898b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Fuch=C3=9F?= Date: Wed, 10 Jul 2024 22:24:10 +0200 Subject: [PATCH 1/2] Make Code compatible with JDK 17 --- .../core/check/api/CollectionAddAll.java | 4 +- .../check/comment/UnnecessaryComment.java | 8 +++- .../core/check/oop/LeakedCollectionCheck.java | 19 ++++---- .../core/check/oop/MethodShouldBeStatic.java | 15 +++--- .../core/check/structure/DuplicateCode.java | 9 ++-- .../autograder/core/check/utils/Option.java | 48 +++++++++++-------- .../autograder/core/integrated/SpoonUtil.java | 5 +- .../core/integrated/UsesFinder.java | 23 +++++---- .../structure/StructuralElement.java | 4 +- .../structure/StructuralEqualsVisitor.java | 5 +- pom.xml | 2 +- 11 files changed, 79 insertions(+), 63 deletions(-) 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 index 4189537d..5b4f13e0 100644 --- 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 @@ -3,10 +3,10 @@ import de.firemage.autograder.core.LocalizedMessage; import de.firemage.autograder.core.ProblemType; import de.firemage.autograder.core.check.ExecutableCheck; +import de.firemage.autograder.core.check.api.UseEnumValues.CtEnumFieldRead; import de.firemage.autograder.core.integrated.IntegratedCheck; 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.reflect.code.CtBlock; import spoon.reflect.code.CtExpression; import spoon.reflect.code.CtForEach; @@ -200,7 +200,7 @@ private void checkAddAll(CtForEach ctFor) { // handle edge case where the variable is implicitly cast in the invocation (Character in List, but char in iterable) List> actualTypeArguments = ctInvocation.getTarget().getType().getActualTypeArguments(); - if (!actualTypeArguments.isEmpty() && !ctFor.getVariable().getType().equals(actualTypeArguments.getFirst())) { + if (!actualTypeArguments.isEmpty() && !ctFor.getVariable().getType().equals(actualTypeArguments.get(0))) { return; } diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/comment/UnnecessaryComment.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/comment/UnnecessaryComment.java index 1406215f..997d82ce 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/comment/UnnecessaryComment.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/comment/UnnecessaryComment.java @@ -42,7 +42,11 @@ private void checkComments(Collection comments) { } private static boolean isStandaloneComment(CtComment ctComment) { - return ctComment.getParent() instanceof CtElement ctElement && !ctElement.getComments().contains(ctComment); + var parent = ctComment.getParent(); + if (parent == null) { + return false; + } + return !parent.getComments().contains(ctComment); } @Override @@ -71,7 +75,7 @@ public void process(CtElement element) { .map(CtComment.class::cast) .collect(Collectors.toCollection(ArrayList::new)); - followingComments.addFirst(ctComment); + followingComments.add(0, ctComment); checkComments(followingComments); return; diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/LeakedCollectionCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/LeakedCollectionCheck.java index 83789cc7..a09f9bcd 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/LeakedCollectionCheck.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/LeakedCollectionCheck.java @@ -40,6 +40,7 @@ import spoon.reflect.visitor.filter.TypeFilter; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -210,7 +211,9 @@ private static List> findPreviousAssignee(CtVariableRead ctVa boolean foundPreviousAssignment = false; CtStatement currentStatement = ctVariableRead.getParent(CtStatement.class); - for (CtStatement ctStatement : SpoonUtil.getEffectiveStatements(ctExecutable.getBody()).reversed()) { + var reversedStatements = new ArrayList<>(SpoonUtil.getEffectiveStatements(ctExecutable.getBody())); + Collections.reverse(reversedStatements); + for (CtStatement ctStatement : reversedStatements) { if (!foundPreviousAssignment) { if (ctStatement == currentStatement) { foundPreviousAssignment = true; @@ -249,7 +252,7 @@ && isParameterOf(ctVariableDeclaration, ctExecutable)) { List> previousAssignees = findPreviousAssignee(ctVariableRead); if (!previousAssignees.isEmpty()) { - return findParameterReference(previousAssignees.getFirst(), ctExecutable); + return findParameterReference(previousAssignees.get(0), ctExecutable); } return Option.some((CtParameter) ctVariableDeclaration); @@ -412,13 +415,11 @@ private void checkCtType(CtType ctType) { ctTypeMember = fixRecordAccessor(ctRecord, ctMethod); } - switch (ctTypeMember) { - case CtConstructor ctConstructor -> checkCtExecutableAssign(ctConstructor); - case CtMethod ctMethod -> { - checkCtExecutableReturn(ctMethod); - checkCtExecutableAssign(ctMethod); - } - default -> {} + if (ctTypeMember instanceof CtConstructor ctConstructor) { + checkCtExecutableAssign(ctConstructor); + } else if (ctTypeMember instanceof CtMethod ctMethod) { + checkCtExecutableReturn(ctMethod); + checkCtExecutableAssign(ctMethod); } } } diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/MethodShouldBeStatic.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/MethodShouldBeStatic.java index 45e06a89..a82fb53b 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/MethodShouldBeStatic.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/oop/MethodShouldBeStatic.java @@ -22,7 +22,6 @@ import java.util.Map; - @ExecutableCheck(reportedProblems = { ProblemType.METHOD_SHOULD_BE_STATIC, ProblemType.METHOD_SHOULD_BE_STATIC_NOT_PUBLIC}) public class MethodShouldBeStatic extends IntegratedCheck { /** @@ -48,12 +47,14 @@ private boolean isThisTypeAccess(CtTargetedExpression ctTargetedExpression @Override public boolean matches(CtElement element) { - return switch (element) { - case CtFieldAccess ctFieldAccess -> this.isThisTypeAccess(ctFieldAccess); - case CtInvocation ctInvocation -> this.isThisTypeAccess(ctInvocation); - case CtExecutableReferenceExpression ctExecutableReferenceExpression -> this.isThisTypeAccess(ctExecutableReferenceExpression); - default -> false; - }; + if (element instanceof CtFieldAccess ctFieldAccess) { + return this.isThisTypeAccess(ctFieldAccess); + } else if (element instanceof CtInvocation ctInvocation) { + return this.isThisTypeAccess(ctInvocation); + } else if (element instanceof CtExecutableReferenceExpression ctExecutableReferenceExpression) { + return this.isThisTypeAccess(ctExecutableReferenceExpression); + } + return false; } } diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/structure/DuplicateCode.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/structure/DuplicateCode.java index 5186bdd2..0891b8d4 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/structure/DuplicateCode.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/structure/DuplicateCode.java @@ -34,7 +34,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.SequencedSet; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -97,11 +96,11 @@ public void add(CtStatement ctStatement) { } public CtStatement getFirst() { - return this.statements.getFirst(); + return this.statements.get(0); } public CtStatement getLast() { - return this.statements.getLast(); + return this.statements.get(this.statements.size() - 1); } public List statements() { @@ -113,8 +112,8 @@ public Iterator iterator() { return this.statements().iterator(); } - private SequencedSet> declaredVariables() { - SequencedSet> declaredVariables = new LinkedHashSet<>(); + private Set> declaredVariables() { + Set> declaredVariables = new LinkedHashSet<>(); for (CtStatement ctStatement : this) { if (ctStatement instanceof CtVariable ctVariable) { diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java index 3ec39141..f7e8723f 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java @@ -20,22 +20,26 @@ static Option none() { } default T unwrap() { - return switch (this) { - case Some (var value) -> value; - case None ignored -> throw new IllegalStateException("Expected Some value, but got None."); - }; - } + if (this instanceof Some someValue) { + return someValue.value; + } else if (this instanceof None) { + throw new IllegalStateException("Expected Some value, but got None."); + } + throw new IllegalArgumentException(); + } default boolean isSome() { return this instanceof Some; } default Option map(Function function) { - return switch (this) { - case Some(var value) -> new Some<>(function.apply(value)); - case None ignored -> new None<>(); - }; - } + if (this instanceof Some someValue) { + return new Some<>(function.apply(someValue.value)); + } else if (this instanceof None) { + return new None<>(); + } + throw new IllegalArgumentException(); + } /** * Returns the value if it is present or null if it is not. @@ -43,18 +47,22 @@ default Option map(Function function) { * @return the value or null */ default T nullable() { - return switch (this) { - case Some(var value) -> value; - case None ignored -> null; - }; - } + if (this instanceof Some someValue) { + return someValue.value; + } else if (this instanceof None) { + return null; + } + throw new IllegalArgumentException(); + } default Stream stream() { - return switch (this) { - case Some(var value) -> Stream.of(value); - case None ignored -> Stream.empty(); - }; - } + if (this instanceof Some someValue) { + return Stream.of(someValue.value); + } else if (this instanceof None) { + return Stream.empty(); + } + throw new IllegalArgumentException(); + } @Override default Iterator iterator() { diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java index e865d505..f50444f6 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java @@ -8,7 +8,6 @@ import de.firemage.autograder.core.integrated.evaluator.fold.FoldUtils; import de.firemage.autograder.core.integrated.evaluator.fold.InlineVariableRead; import de.firemage.autograder.core.integrated.evaluator.fold.RemoveRedundantCasts; -import org.apache.commons.io.FilenameUtils; import spoon.processing.FactoryAccessor; import spoon.reflect.CtModel; import spoon.reflect.code.BinaryOperatorKind; @@ -55,6 +54,8 @@ import spoon.reflect.reference.CtTypeReference; import spoon.reflect.reference.CtVariableReference; +import org.apache.commons.io.FilenameUtils; + import java.io.File; import java.util.ArrayDeque; import java.util.ArrayList; @@ -1176,7 +1177,7 @@ private static int referenceIndexOf(List list, T element) { */ public static Optional getPreviousStatement(CtStatement ctStatement) { List previousStatements = getPreviousStatements(ctStatement); - return previousStatements.isEmpty() ? Optional.empty() : Optional.of(previousStatements.getLast()); + return previousStatements.isEmpty() ? Optional.empty() : Optional.of(previousStatements.get(previousStatements.size() - 1)); } public static List getPreviousStatements(CtStatement ctStatement) { diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/UsesFinder.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/UsesFinder.java index dad1b5bb..a484c94f 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/UsesFinder.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/UsesFinder.java @@ -40,7 +40,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.SequencedSet; +import java.util.Set; import java.util.stream.Stream; /** @@ -80,13 +80,16 @@ private static UsesFinder getFor(FactoryAccessor factoryAccessor) { */ @SuppressWarnings("rawtypes") public static CtElementStream getAllUses(CtNamedElement element) { - return switch (element) { - case CtVariable variable -> UsesFinder.variableUses(variable).asUntypedStream(); - case CtTypeParameter typeParameter -> UsesFinder.typeParameterUses(typeParameter).asUntypedStream(); - case CtExecutable executable -> UsesFinder.executableUses(executable).asUntypedStream(); - case CtType type -> UsesFinder.typeUses(type).asUntypedStream(); - default -> throw new IllegalArgumentException("Unsupported element: " + element.getClass().getName()); - }; + if (element instanceof CtVariable variable) { + return UsesFinder.variableUses(variable).asUntypedStream(); + } else if (element instanceof CtTypeParameter typeParameter) { + return UsesFinder.typeParameterUses(typeParameter).asUntypedStream(); + } else if (element instanceof CtExecutable executable) { + return UsesFinder.executableUses(executable).asUntypedStream(); + } else if (element instanceof CtType type) { + return UsesFinder.typeUses(type).asUntypedStream(); + } + throw new IllegalArgumentException("Unsupported element: " + element.getClass().getName()); } public static CtElementStream> variableUses(CtVariable variable) { @@ -161,8 +164,8 @@ private static class UsesScanner extends CtScanner { private final Map> typeParameterUses = new IdentityHashMap<>(); private final Map> executableUses = new IdentityHashMap<>(); private final Map> typeUses = new IdentityHashMap<>(); - private final Map> subtypes = new IdentityHashMap<>(); - private final Map> supertypes = new IdentityHashMap<>(); + private final Map> subtypes = new IdentityHashMap<>(); + private final Map> supertypes = new IdentityHashMap<>(); // Caches the current instanceof pattern variables, since Spoon doesn't track them yet // We are conservative: A pattern introduces a variable until the end of the current block diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralElement.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralElement.java index 00186d80..3d066483 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralElement.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralElement.java @@ -12,11 +12,11 @@ public boolean equals(Object otherObject) { if (this == otherObject) { return true; } - if (!(otherObject instanceof StructuralElement(var otherElement))) { + if (!(otherObject instanceof StructuralElement otherStructuralElement)) { return false; } - return StructuralEqualsVisitor.equals(this.element, otherElement); + return StructuralEqualsVisitor.equals(this.element, otherStructuralElement.element); } @Override diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralEqualsVisitor.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralEqualsVisitor.java index 56430aa1..23497434 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralEqualsVisitor.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/structure/StructuralEqualsVisitor.java @@ -19,7 +19,6 @@ import spoon.support.visitor.equals.EqualsVisitor; import java.util.LinkedHashSet; -import java.util.SequencedSet; import java.util.Set; import java.util.function.Predicate; import java.util.function.UnaryOperator; @@ -32,7 +31,7 @@ public final class StructuralEqualsVisitor extends EqualsVisitor { CtRole.COMMENT, CtRole.COMMENT_CONTENT, CtRole.COMMENT_TAG, CtRole.COMMENT_TYPE ); - private final SequencedSet differences; + private final Set differences; public record Difference(CtRole role, Object left, Object right) {} @@ -218,7 +217,7 @@ protected boolean fail(CtRole role, Object element, Object other) { * * @return the differences */ - public SequencedSet differences() { + public Set differences() { return new LinkedHashSet<>(this.differences); } } diff --git a/pom.xml b/pom.xml index f86b4d0f..b7cbe7a5 100644 --- a/pom.xml +++ b/pom.xml @@ -38,7 +38,7 @@ UTF-8 - 21 + 17 ${java.version} ${java.version} ${java.version} From 4247ee292c471e17fb52967d11bd1670242a37bb Mon Sep 17 00:00:00 2001 From: Florian Date: Sun, 14 Jul 2024 14:09:48 +0200 Subject: [PATCH 2/2] tabs -> spaces --- .../autograder/core/check/utils/Option.java | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java index f7e8723f..b3c20d12 100644 --- a/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java +++ b/autograder-core/src/main/java/de/firemage/autograder/core/check/utils/Option.java @@ -20,26 +20,26 @@ static Option none() { } default T unwrap() { - if (this instanceof Some someValue) { - return someValue.value; - } else if (this instanceof None) { - throw new IllegalStateException("Expected Some value, but got None."); - } - throw new IllegalArgumentException(); - } + if (this instanceof Some someValue) { + return someValue.value; + } else if (this instanceof None) { + throw new IllegalStateException("Expected Some value, but got None."); + } + throw new IllegalArgumentException(); + } default boolean isSome() { return this instanceof Some; } default Option map(Function function) { - if (this instanceof Some someValue) { - return new Some<>(function.apply(someValue.value)); - } else if (this instanceof None) { - return new None<>(); - } - throw new IllegalArgumentException(); - } + if (this instanceof Some someValue) { + return new Some<>(function.apply(someValue.value)); + } else if (this instanceof None) { + return new None<>(); + } + throw new IllegalArgumentException(); + } /** * Returns the value if it is present or null if it is not. @@ -47,22 +47,22 @@ default Option map(Function function) { * @return the value or null */ default T nullable() { - if (this instanceof Some someValue) { - return someValue.value; - } else if (this instanceof None) { - return null; - } - throw new IllegalArgumentException(); - } + if (this instanceof Some someValue) { + return someValue.value; + } else if (this instanceof None) { + return null; + } + throw new IllegalArgumentException(); + } default Stream stream() { - if (this instanceof Some someValue) { - return Stream.of(someValue.value); - } else if (this instanceof None) { - return Stream.empty(); - } - throw new IllegalArgumentException(); - } + if (this instanceof Some someValue) { + return Stream.of(someValue.value); + } else if (this instanceof None) { + return Stream.empty(); + } + throw new IllegalArgumentException(); + } @Override default Iterator iterator() {