problem -> abstractproblem, problemimpl -> problem

Feuermagier · Jul 13, 2024 · 6dab6fc · 6dab6fc
1 parent db1f3a7
commit 6dab6fc
Show file tree

Hide file tree

Showing 98 changed files with 266 additions and 264 deletions.
diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/AbstractLinter.java b/autograder-api/src/main/java/de/firemage/autograder/api/AbstractLinter.java
@@ -9,7 +9,7 @@
 import java.util.function.Consumer;
 
 public interface AbstractLinter {
-    List<? extends Problem> checkFile(Path file, JavaVersion version, CheckConfiguration checkConfiguration, Consumer<Translatable> statusConsumer) throws LinterException, IOException;
+    List<? extends AbstractProblem> checkFile(Path file, JavaVersion version, CheckConfiguration checkConfiguration, Consumer<Translatable> statusConsumer) throws LinterException, IOException;
     String translateMessage(Translatable translatable);
     FluentBundle getFluentBundle();
 

diff --git a/...a/de/firemage/autograder/api/Problem.java → ...emage/autograder/api/AbstractProblem.java b/...a/de/firemage/autograder/api/Problem.java → ...emage/autograder/api/AbstractProblem.java
@@ -1,6 +1,6 @@
 package de.firemage.autograder.api;
 
-public interface Problem {
+public interface AbstractProblem {
     String getCheckName();
     Translatable getLinterName();
     Translatable getExplanation();

diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/loader/AutograderLoader.java b/autograder-api/src/main/java/de/firemage/autograder/api/loader/AutograderLoader.java
@@ -65,11 +65,6 @@ public static AbstractTempLocation instantiateTempLocation(Path path) {
     }
 
     public static AbstractTempLocation instantiateTempLocation() {
-        try {
-            Class.forName("de.firemage.autograder.core.file.TempLocationImpl", true, autograderClassLoader);
-        } catch (ClassNotFoundException e) {
-            throw new RuntimeException(e);
-        }
         return new ImplementationBinder<>(AbstractTempLocation.class)
                 .classLoader(autograderClassLoader)
                 .instantiate();

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
@@ -9,7 +9,7 @@
 import de.firemage.autograder.api.AbstractLinter;
 import de.firemage.autograder.api.LinterConfigurationException;
 import de.firemage.autograder.api.LinterException;
-import de.firemage.autograder.api.Problem;
+import de.firemage.autograder.api.AbstractProblem;
 import de.firemage.autograder.api.AbstractTempLocation;
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.api.loader.AutograderLoader;
@@ -238,7 +238,7 @@ public Integer call() {
         return 0;
     }
 
-    private void printProblems(List<? extends Problem> problems, AbstractLinter linter) {
+    private void printProblems(List<? extends AbstractProblem> problems, AbstractLinter linter) {
         if (problems.isEmpty()) {
             CmdUtil.println("No problems found - good job!");
         } else {
@@ -247,7 +247,7 @@ private void printProblems(List<? extends Problem> problems, AbstractLinter lint
         }
     }
 
-    private void printProblemsAsJson(Collection<? extends Problem> problems, AbstractLinter linter) {
+    private void printProblemsAsJson(Collection<? extends AbstractProblem> problems, AbstractLinter linter) {
         try {
             ObjectMapper mapper = new ObjectMapper();
             String jsonOutput = mapper.writeValueAsString(problems.stream().map(problem -> {
@@ -266,7 +266,7 @@ private void printProblemsAsJson(Collection<? extends Problem> problems, Abstrac
         }
     }
 
-    private String formatProblem(Problem problem, AbstractLinter linter) {
+    private String formatProblem(AbstractProblem problem, AbstractLinter linter) {
         return String.format("%s %s (Source: %s)",
                 problem.getDisplayLocation(),
                 linter.translateMessage(problem.getExplanation()),

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/CodeLinter.java b/autograder-core/src/main/java/de/firemage/autograder/core/CodeLinter.java
@@ -23,7 +23,7 @@ public interface CodeLinter<T extends Check> {
      * @return a list of problems found in the submission
      * @throws IOException if an I/O error occurs
      */
-    List<ProblemImpl> lint(
+    List<Problem> lint(
         UploadedFile submission,
         AbstractTempLocation tempLocation,
         ClassLoader classLoader,

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/Linter.java b/autograder-core/src/main/java/de/firemage/autograder/core/Linter.java
@@ -3,7 +3,7 @@
 import de.firemage.autograder.api.CheckConfiguration;
 import de.firemage.autograder.api.AbstractLinter;
 import de.firemage.autograder.api.LinterException;
-import de.firemage.autograder.api.Problem;
+import de.firemage.autograder.api.AbstractProblem;
 import de.firemage.autograder.api.ProblemType;
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.check.Check;
@@ -65,7 +65,7 @@ public Linter(
             throw new IllegalStateException(e);
         }
 
-        this.tempLocation = builder.getTempLocation() != null ? builder.getTempLocation() : new TempLocation();
+        this.tempLocation = builder.getTempLocation() != null ? builder.getTempLocation() : TempLocation.random();
         this.threads = builder.getThreads();
         this.classLoader = builder.getClassLoader();
         this.maxProblemsPerCheck = builder.getMaxProblemsPerCheck();
@@ -76,13 +76,13 @@ public FluentBundle getFluentBundle() {
     }
 
     @Override
-    public List<ProblemImpl> checkFile(Path file, JavaVersion version, CheckConfiguration checkConfiguration, Consumer<Translatable> statusConsumer) throws LinterException, IOException {
+    public List<Problem> checkFile(Path file, JavaVersion version, CheckConfiguration checkConfiguration, Consumer<Translatable> statusConsumer) throws LinterException, IOException {
         try (var uploadedFile = UploadedFile.build(file, version, this.tempLocation, statusConsumer, this.classLoader)) {
             return this.checkFile(uploadedFile, checkConfiguration, statusConsumer);
         }
     }
 
-    public List<ProblemImpl> checkFile(
+    public List<Problem> checkFile(
         UploadedFile file,
         CheckConfiguration checkConfiguration,
         Consumer<Translatable> statusConsumer
@@ -101,7 +101,7 @@ private static <T> List<T> castUnsafe(Iterable<?> list, Class<? extends T> clazz
         return result;
     }
 
-    public List<ProblemImpl> checkFile(
+    public List<Problem> checkFile(
         UploadedFile file,
         CheckConfiguration checkConfiguration,
         Iterable<? extends Check> checks,
@@ -178,20 +178,20 @@ public List<ProblemImpl> checkFile(
         return this.mergeProblems(unreducedProblems);
     }
 
-    private List<ProblemImpl> mergeProblems(Collection<? extends ProblemImpl> unreducedProblems) {
+    private List<Problem> mergeProblems(Collection<? extends Problem> unreducedProblems) {
         // -1 means no limit (useful for unit tests, where one wants to see all problems)
         if (this.maxProblemsPerCheck == -1) {
             return new ArrayList<>(unreducedProblems);
         }
 
         // first group all problems by the check that created them
-        Map<Check, List<ProblemImpl>> problems = unreducedProblems.stream()
-            .collect(Collectors.groupingBy(ProblemImpl::getCheck, LinkedHashMap::new, Collectors.toList()));
+        Map<Check, List<Problem>> problems = unreducedProblems.stream()
+            .collect(Collectors.groupingBy(Problem::getCheck, LinkedHashMap::new, Collectors.toList()));
 
-        List<ProblemImpl> result = new ArrayList<>();
-        for (Map.Entry<Check, List<ProblemImpl>> entry : problems.entrySet()) {
+        List<Problem> result = new ArrayList<>();
+        for (Map.Entry<Check, List<Problem>> entry : problems.entrySet()) {
             Check check = entry.getKey();
-            List<ProblemImpl> problemsForCheck = entry.getValue();
+            List<Problem> problemsForCheck = entry.getValue();
 
             int targetNumberOfProblems = Math.min(
                 this.maxProblemsPerCheck,
@@ -202,8 +202,8 @@ private List<ProblemImpl> mergeProblems(Collection<? extends ProblemImpl> unredu
             if (problemsForCheck.size() > targetNumberOfProblems) {
                 // further partition the problems by their ProblemType
                 // (one does not want to merge different types of problems):
-                Map<ProblemType, List<ProblemImpl>> problemsByType = problemsForCheck.stream()
-                    .collect(Collectors.groupingBy(Problem::getProblemType, LinkedHashMap::new, Collectors.toList()));
+                Map<ProblemType, List<Problem>> problemsByType = problemsForCheck.stream()
+                    .collect(Collectors.groupingBy(AbstractProblem::getProblemType, LinkedHashMap::new, Collectors.toList()));
 
                 problemsForCheck = problemsByType.values()
                     .stream()

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/MultiInCodeProblem.java b/autograder-core/src/main/java/de/firemage/autograder/core/MultiInCodeProblem.java
@@ -12,11 +12,11 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-public class MultiInCodeProblem extends ProblemImpl {
+public class MultiInCodeProblem extends Problem {
 
     public MultiInCodeProblem(
-        ProblemImpl firstProblem,
-        Collection<? extends ProblemImpl> otherProblems
+        Problem firstProblem,
+        Collection<? extends Problem> otherProblems
     ) {
         super(
             firstProblem.getCheck(),
@@ -26,7 +26,7 @@ public MultiInCodeProblem(
         );
     }
 
-    private static Translatable makeExplanation(ProblemImpl first, Collection<? extends ProblemImpl> problems) {
+    private static Translatable makeExplanation(Problem first, Collection<? extends Problem> problems) {
         return bundle -> {
             String message = first.getExplanation().format(bundle);
             if (!message.endsWith(".")) {
@@ -39,7 +39,7 @@ private static Translatable makeExplanation(ProblemImpl first, Collection<? exte
                     "message", message,
                     "locations", displayLocations(
                         first.getPosition().file(),
-                        problems.stream().map(ProblemImpl::getPosition)
+                        problems.stream().map(Problem::getPosition)
                     )
                 )
             ).format(bundle);

diff --git a/...firemage/autograder/core/ProblemImpl.java → .../de/firemage/autograder/core/Problem.java b/...firemage/autograder/core/ProblemImpl.java → .../de/firemage/autograder/core/Problem.java
@@ -1,14 +1,14 @@
 package de.firemage.autograder.core;
 
-import de.firemage.autograder.api.Problem;
+import de.firemage.autograder.api.AbstractProblem;
 import de.firemage.autograder.api.ProblemType;
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.check.Check;
 
 /**
- * Contains the default implementation of most {@link Problem} methods.
+ * Contains the default implementation of most {@link AbstractProblem} methods.
  */
-public abstract class ProblemImpl implements Problem {
+public abstract class Problem implements AbstractProblem {
 
     private final Check check;
 
@@ -18,7 +18,7 @@ public abstract class ProblemImpl implements Problem {
 
     private final ProblemType problemType;
 
-    protected ProblemImpl(Check check, CodePosition position, Translatable explanation, ProblemType problemType) {
+    protected Problem(Check check, CodePosition position, Translatable explanation, ProblemType problemType) {
         this.check = check;
         this.position = position;
         this.explanation = explanation;

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/check/Check.java b/autograder-core/src/main/java/de/firemage/autograder/core/check/Check.java
@@ -2,7 +2,7 @@
 
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.MultiInCodeProblem;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -11,14 +11,14 @@
 public interface Check {
     Translatable getLinter();
 
-    default List<ProblemImpl> merge(List<ProblemImpl> problems, int limit) {
+    default List<Problem> merge(List<Problem> problems, int limit) {
         // use a dumb algorithm: keep the first limit - 1 problems, and merge the rest into a single problem
         if (problems.size() <= limit) {
             return problems;
         }
 
-        List<ProblemImpl> result = new ArrayList<>(problems.subList(0, limit - 1));
-        List<ProblemImpl> toMerge = problems.subList(limit - 1, problems.size());
+        List<Problem> result = new ArrayList<>(problems.subList(0, limit - 1));
+        List<Problem> toMerge = problems.subList(limit - 1, problems.size());
 
         result.add(new MultiInCodeProblem(toMerge.get(0), toMerge.subList(1, toMerge.size())));
 

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/file/TempLocation.java b/autograder-core/src/main/java/de/firemage/autograder/core/file/TempLocation.java
@@ -14,6 +14,14 @@ public record TempLocation(File tempLocation) implements AbstractTempLocation {
     private static final Random RANDOM = new Random();
     private static final String TEMPORARY_DIR_FORMAT = "%s%d";
 
+    public static TempLocation random() {
+        return new TempLocation();
+    }
+
+    public static TempLocation of(Path path) {
+        return new TempLocation(path);
+    }
+
     public TempLocation(Path path) {
         this(path.toFile());
     }
@@ -22,6 +30,9 @@ public TempLocation(String first, String... other) {
        this(Path.of(first, other).toFile());
     }
 
+    /**
+     * Creates a new random temporary location (same as {@link TempLocation#random()}).
+     */
     public TempLocation() {
         this(tryCreateTempDirectory());
     }

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedAnalysis.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedAnalysis.java
@@ -4,7 +4,7 @@
 import de.firemage.autograder.core.CodeLinter;
 import de.firemage.autograder.core.LinterStatus;
 import de.firemage.autograder.api.AbstractTempLocation;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 import de.firemage.autograder.core.file.UploadedFile;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -63,7 +63,7 @@ public Class<IntegratedCheck> supportedCheckType() {
     }
 
     @Override
-    public List<ProblemImpl> lint(
+    public List<Problem> lint(
         UploadedFile submission,
         AbstractTempLocation tempLocation,
         ClassLoader classLoader,
@@ -77,7 +77,7 @@ public List<ProblemImpl> lint(
 
         statusConsumer.accept(LinterStatus.RUNNING_INTEGRATED_CHECKS.getMessage());
 
-        List<ProblemImpl> result = new ArrayList<>();
+        List<Problem> result = new ArrayList<>();
         for (IntegratedCheck check : checks) {
             long beforeTime = System.nanoTime();
             result.addAll(check.run(

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedCheck.java b/autograder-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedCheck.java
@@ -1,7 +1,7 @@
 package de.firemage.autograder.core.integrated;
 
 import de.firemage.autograder.core.CodePosition;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 import de.firemage.autograder.core.LocalizedMessage;
 import de.firemage.autograder.api.ProblemType;
 import de.firemage.autograder.api.Translatable;
@@ -13,7 +13,7 @@
 import java.util.List;
 
 public abstract class IntegratedCheck implements Check {
-    private final List<ProblemImpl> problems = new ArrayList<>();
+    private final List<Problem> problems = new ArrayList<>();
     private SourceInfo sourceInfo;
 
     protected IntegratedCheck() {}
@@ -23,10 +23,10 @@ protected void addLocalProblem(CtElement element, Translatable explanation, Prob
     }
 
     protected void addLocalProblem(CodePosition position, Translatable explanation, ProblemType problemType) {
-        this.problems.add(new ProblemImpl(this, position, explanation, problemType) {});
+        this.problems.add(new Problem(this, position, explanation, problemType) {});
     }
 
-    public List<ProblemImpl> run(StaticAnalysis staticAnalysis, SourceInfo sourceInfo) {
+    public List<Problem> run(StaticAnalysis staticAnalysis, SourceInfo sourceInfo) {
         this.problems.clear();
         this.sourceInfo = sourceInfo;
         this.check(staticAnalysis);

diff --git a/...er-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedInCodeProblem.java b/...er-core/src/main/java/de/firemage/autograder/core/integrated/IntegratedInCodeProblem.java
@@ -1,14 +1,14 @@
 package de.firemage.autograder.core.integrated;
 
 import de.firemage.autograder.core.CodePosition;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 import de.firemage.autograder.api.ProblemType;
 import de.firemage.autograder.api.Translatable;
 import de.firemage.autograder.core.check.Check;
 import de.firemage.autograder.core.file.SourceInfo;
 import spoon.reflect.declaration.CtElement;
 
-public class IntegratedInCodeProblem extends ProblemImpl {
+public class IntegratedInCodeProblem extends Problem {
     private final CtElement element;
 
     public IntegratedInCodeProblem(Check check, CtElement element, Translatable explanation,

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/parallel/AnalysisResult.java b/autograder-core/src/main/java/de/firemage/autograder/core/parallel/AnalysisResult.java
@@ -1,13 +1,12 @@
 package de.firemage.autograder.core.parallel;
 
-import de.firemage.autograder.api.Problem;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 
 import java.util.List;
 import java.util.Objects;
 
-public record AnalysisResult(List<ProblemImpl> problems, Exception thrownException) {
-    public static AnalysisResult forSuccess(List<ProblemImpl> problems) {
+public record AnalysisResult(List<Problem> problems, Exception thrownException) {
+    public static AnalysisResult forSuccess(List<Problem> problems) {
         Objects.requireNonNull(problems);
         return new AnalysisResult(problems, null);
     }

diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/parallel/AnalysisScheduler.java b/autograder-core/src/main/java/de/firemage/autograder/core/parallel/AnalysisScheduler.java
@@ -1,7 +1,6 @@
 package de.firemage.autograder.core.parallel;
 
-import de.firemage.autograder.api.Problem;
-import de.firemage.autograder.core.ProblemImpl;
+import de.firemage.autograder.core.Problem;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -82,7 +81,7 @@ public AnalysisResult collectProblems() {
     private AnalysisResult collectProblemsFromThreads() {
         this.completionAllowed = true;
 
-        List<ProblemImpl> allProblems = new ArrayList<>();
+        List<Problem> allProblems = new ArrayList<>();
         for (var thread : this.analysisThreads) {
             try {
                 var result = thread.join();
@@ -101,16 +100,16 @@ private AnalysisResult collectProblemsFromThreads() {
     }
 
     private AnalysisResult executeChecksSingleThreaded() {
-        List<ProblemImpl> allProblems = new ArrayList<>();
+        List<Problem> allProblems = new ArrayList<>();
 
         var reporter = new ProblemReporter() {
             @Override
-            public void reportProblem(ProblemImpl problem) {
+            public void reportProblem(Problem problem) {
                 allProblems.add(problem);
             }
 
             @Override
-            public void reportProblems(Collection<ProblemImpl> problems) {
+            public void reportProblems(Collection<Problem> problems) {
                 allProblems.addAll(problems);
             }
         };