Merge pull request #566 from Luro02/main

Implement things
Feuermagier · Jul 27, 2024 · 6161577 · 6161577
2 parents 224ba4a + 2434d0e
commit 6161577
Show file tree

Hide file tree

Showing 128 changed files with 2,947 additions and 2,495 deletions.
diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/ArtemisUtil.java b/autograder-api/src/main/java/de/firemage/autograder/api/ArtemisUtil.java
diff --git a/autograder-cmd/src/main/java/de/firemage/autograder/cmd/Application.java b/autograder-cmd/src/main/java/de/firemage/autograder/cmd/Application.java
@@ -2,7 +2,6 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import de.firemage.autograder.api.ArtemisUtil;
 import de.firemage.autograder.api.CheckConfiguration;
 import de.firemage.autograder.api.AbstractCodePosition;
 import de.firemage.autograder.api.JavaVersion;
@@ -14,6 +13,7 @@
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.api.loader.AutograderLoader;
 import de.firemage.autograder.cmd.output.Annotation;
+import de.firemage.autograder.core.integrated.CoreUtil;
 import de.firemage.autograder.span.Formatter;
 import de.firemage.autograder.span.Highlight;
 import de.firemage.autograder.span.Position;
@@ -58,10 +58,6 @@ public class Application implements Callable<Integer> {
     @Option(names = {"-j", "--java", "--java-version"}, defaultValue = "17", description = "Set the Java version.")
     private String javaVersion;
 
-    @Option(names = {
-            "--artemis"}, description = "Assume that the given root folder is the workspace root of the grading tool.")
-    private boolean artemisFolders;
-
     @Option(names = {
             "--output-json"}, description = "Output the found problems in JSON format instead of more readable plain text")
     private boolean outputJson;
@@ -81,6 +77,9 @@ public class Application implements Callable<Integer> {
     @Option(names = {"--max-problems"}, description = "The maximum number of problems to report per check", defaultValue = "10")
     private int maxProblemsPerCheck;
 
+    @Option(names = {"--debug"}, description = "Enables debug mode, note that this slows down execution", defaultValue = "false")
+    private boolean isInDebugMode;
+
     @Spec
     private CommandSpec spec;
 
@@ -186,13 +185,21 @@ public Integer call() {
             throw new ParameterException(this.spec.commandLine(), "Unknown java version '" + javaVersion + "'");
         }
 
-        if (this.artemisFolders) {
-            try {
-                this.file = ArtemisUtil.resolveCodePathEclipseGradingTool(this.file);
-            } catch (IOException e) {
-                e.printStackTrace();
-                return IO_EXIT_CODE;
-            }
+        // Depending on the structure of the project, the code might be in a subdirectory.
+        // By default, we support explicitly specifying the folder to the first package (./src/main/java)
+        //
+        // Here we check if the project has a folder `src/<here the first package>` or `assignment/src/<here the first package>`
+        // and if so, we assume that the code is in that folder.
+        if (Files.exists(this.file.resolve("src"))) {
+            this.file = this.file.resolve("src");
+        }
+
+        if (Files.exists(this.file.resolve("assignment/src"))) {
+            this.file = this.file.resolve("assignment/src");
+        }
+
+        if (this.isInDebugMode) {
+            CoreUtil.setDebugMode();
         }
 
         if (!outputJson) {

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/CodeModel.java b/autograder-core/src/main/java/de/firemage/autograder/core/CodeModel.java
@@ -2,8 +2,8 @@
 
 import de.firemage.autograder.core.file.SourceInfo;
 import de.firemage.autograder.core.integrated.MethodHierarchy;
+import de.firemage.autograder.core.integrated.MethodUtil;
 import de.firemage.autograder.core.integrated.ModelBuildException;
-import de.firemage.autograder.core.integrated.SpoonUtil;
 import de.firemage.autograder.core.integrated.UsesFinder;
 import spoon.Launcher;
 import spoon.compiler.Environment;
@@ -98,7 +98,7 @@ public CtMethod<Void> findMain() {
             this.mainMethod = this.getModel()
                 .getElements(new NamedElementFilter<>(CtMethod.class, "main"))
                 .stream()
-                .filter(SpoonUtil::isMainMethod)
+                .filter(MethodUtil::isMainMethod)
                 .findFirst()
                 .map(ctMethod -> (CtMethod<Void>) ctMethod);
         }

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java b/autograder-core/src/main/java/de/firemage/autograder/core/ProblemType.java
@@ -3,9 +3,6 @@
 import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.HasFalsePositives;
 
-import java.util.Collection;
-import java.util.List;
-
 public enum ProblemType implements AbstractProblemType {
     /**
      * If the code is split into multiple packages, all input must happen in one package. Otherwise, one class must do all input.
@@ -342,7 +339,7 @@ public enum ProblemType implements AbstractProblemType {
      * Reports code where the Collection#addAll method could be used.
      */
     @HasFalsePositives
-    COLLECTION_ADD_ALL,
+    SEQUENTIAL_ADD_ALL,
 
     /**
      * Reports cases where Pattern#compile is not in a static final field.
@@ -675,16 +672,17 @@ public enum ProblemType implements AbstractProblemType {
     COMMON_REIMPLEMENTATION_HYPOT,
 
     /**
-     * Reports code where the Collection#addAll method could be used.
+     * Reports loops where a method is repeatedly called like Collection#add,
+     * which could be replaced with a single call to Collection#addAll.
      */
     @HasFalsePositives
-    COMMON_REIMPLEMENTATION_ADD_ALL,
+    FOR_LOOP_CAN_BE_INVOCATION,
 
     /**
      * Reports code where Enum#values() could be used.
      */
     @HasFalsePositives
-    COMMON_REIMPLEMENTATION_ADD_ENUM_VALUES,
+    USE_ENUM_VALUES,
 
     /**
      * Reports code where Arrays#fill could be used.

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/AvoidStringConcat.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/AvoidStringConcat.java
@@ -5,7 +5,7 @@
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
 import de.firemage.autograder.core.integrated.IntegratedCheck;
-import de.firemage.autograder.core.integrated.SpoonUtil;
+import de.firemage.autograder.core.integrated.MethodUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
 import spoon.processing.AbstractProcessor;
 import spoon.reflect.code.CtInvocation;
@@ -26,7 +26,7 @@ public void process(CtInvocation<?> ctInvocation) {
 
                 if (ctInvocation.getTarget() == null
                     || ctInvocation.getTarget().getType() == null
-                    || !SpoonUtil.isSignatureEqualTo(ctInvocation.getExecutable(), String.class, "concat", String.class)) {
+                    || !MethodUtil.isSignatureEqualTo(ctInvocation.getExecutable(), String.class, "concat", String.class)) {
                     return;
                 }
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/api/CharRange.java
@@ -4,9 +4,11 @@
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
 import de.firemage.autograder.core.integrated.CtRange;
+import de.firemage.autograder.core.integrated.ExpressionUtil;
+import de.firemage.autograder.core.integrated.FactoryUtil;
 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.integrated.TypeUtil;
 import org.apache.commons.lang3.Range;
 import spoon.processing.AbstractProcessor;
 import spoon.reflect.code.BinaryOperatorKind;
@@ -39,35 +41,35 @@ default CtExpression<R> suggest(CtExpression<T> ctExpression) {
 
     private static final Map<Range<Character>, Suggester<Character, Boolean>> MAPPING = Map.of(
         Range.of('a', 'z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
-            SpoonUtil.createStaticInvocation(
+            FactoryUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
-                SpoonUtil.castExpression(int.class, ctExpression)
+                ExpressionUtil.castExpression(int.class, ctExpression)
             ),
-            SpoonUtil.createStaticInvocation(
+            FactoryUtil.createStaticInvocation(
                 targetType,
                 "isLowerCase",
-                SpoonUtil.castExpression(char.class, ctExpression)
+                ExpressionUtil.castExpression(char.class, ctExpression)
             ),
             BinaryOperatorKind.AND
         ),
         Range.of('A', 'Z'), (factory, ctExpression, targetType) -> factory.createBinaryOperator(
-            SpoonUtil.createStaticInvocation(
+            FactoryUtil.createStaticInvocation(
                 targetType,
                 "isAlphabetic",
-                SpoonUtil.castExpression(int.class, ctExpression)
+                ExpressionUtil.castExpression(int.class, ctExpression)
             ),
-            SpoonUtil.createStaticInvocation(
+            FactoryUtil.createStaticInvocation(
                 targetType,
                 "isUpperCase",
-                SpoonUtil.castExpression(char.class, ctExpression)
+                ExpressionUtil.castExpression(char.class, ctExpression)
             ),
             BinaryOperatorKind.AND
         ),
-        Range.of('0', '9'), (factory, ctExpression, targetType) -> SpoonUtil.createStaticInvocation(
+        Range.of('0', '9'), (factory, ctExpression, targetType) -> FactoryUtil.createStaticInvocation(
             targetType,
             "isDigit",
-            SpoonUtil.castExpression(char.class, ctExpression)
+            ExpressionUtil.castExpression(char.class, ctExpression)
         )
     );
 
@@ -87,15 +89,15 @@ protected void check(StaticAnalysis staticAnalysis) {
             public void process(CtBinaryOperator<Boolean> ctBinaryOperator) {
                 if (ctBinaryOperator.isImplicit()
                     || !ctBinaryOperator.getPosition().isValidPosition()
-                    || !SpoonUtil.isTypeEqualTo(ctBinaryOperator.getType(), java.lang.Boolean.class, boolean.class)) {
+                    || !TypeUtil.isTypeEqualTo(ctBinaryOperator.getType(), java.lang.Boolean.class, boolean.class)) {
                     return;
                 }
 
                 boolean isNegated;
                 CtBinaryOperator<Boolean> operator = ctBinaryOperator;
                 if (ctBinaryOperator.getKind() == BinaryOperatorKind.OR) {
                     isNegated = true;
-                    operator = (CtBinaryOperator<Boolean>) SpoonUtil.negate(ctBinaryOperator);
+                    operator = (CtBinaryOperator<Boolean>) ExpressionUtil.negate(ctBinaryOperator);
                 } else {
                     isNegated = false;
                     if (ctBinaryOperator.getKind() != BinaryOperatorKind.AND) {
@@ -132,7 +134,7 @@ public void process(CtBinaryOperator<Boolean> ctBinaryOperator) {
 
                 makeSuggestion(leftRange.ctExpression(), intersection).ifPresent(suggestion -> {
                     if (isNegated) {
-                        suggestion = SpoonUtil.negate(suggestion);
+                        suggestion = ExpressionUtil.negate(suggestion);
                     }
 
                     addLocalProblem(

diff --git a/...der-core/src/main/java/de/firemage/autograder/core/check/api/CheckIterableDuplicates.java b/...der-core/src/main/java/de/firemage/autograder/core/check/api/CheckIterableDuplicates.java
@@ -4,8 +4,9 @@
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
 import de.firemage.autograder.core.integrated.IntegratedCheck;
-import de.firemage.autograder.core.integrated.SpoonUtil;
+import de.firemage.autograder.core.integrated.StatementUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
+import de.firemage.autograder.core.integrated.TypeUtil;
 import spoon.processing.AbstractProcessor;
 import spoon.reflect.code.CtAssignment;
 import spoon.reflect.code.CtBreak;
@@ -53,7 +54,7 @@ public void process(CtForEach ctForEach) {
                     return;
                 }
 
-                List<CtStatement> statements = SpoonUtil.getEffectiveStatements(ctForEach.getBody());
+                List<CtStatement> statements = StatementUtil.getEffectiveStatements(ctForEach.getBody());
                 if (statements.size() != 1 || !(statements.get(0) instanceof CtIf ctIf)) {
                     return;
                 }
@@ -63,7 +64,7 @@ public void process(CtForEach ctForEach) {
                     return;
                 }
 
-                List<CtStatement> ifStatements = SpoonUtil.getEffectiveStatements(ctIf.getThenStatement());
+                List<CtStatement> ifStatements = StatementUtil.getEffectiveStatements(ctIf.getThenStatement());
                 if (ifStatements.isEmpty()) {
                     return;
                 }
@@ -92,12 +93,12 @@ public void process(CtForEach ctForEach) {
                 if (!(ctIf.getCondition() instanceof CtUnaryOperator<Boolean> ctUnaryOperator
                     && ctUnaryOperator.getKind() == UnaryOperatorKind.NOT
                     && ctUnaryOperator.getOperand() instanceof CtInvocation<?> ctInvocation
-                    && SpoonUtil.isTypeEqualTo(ctInvocation.getExecutable().getType(), boolean.class)
+                    && TypeUtil.isTypeEqualTo(ctInvocation.getExecutable().getType(), boolean.class)
                     && ctInvocation.getExecutable().getSimpleName().equals("add")
                     && ctInvocation.getArguments().size() == 1
                     && ctInvocation.getArguments().get(0) instanceof CtVariableRead<?> ctVariableRead
                     && ctVariableRead.getVariable().equals(ctForEach.getVariable().getReference())
-                    && SpoonUtil.isSubtypeOf(ctInvocation.getTarget().getType(), java.util.Set.class)))
+                    && TypeUtil.isSubtypeOf(ctInvocation.getTarget().getType(), java.util.Set.class)))
                 {
                     return;
                 }