typed abstractproblemtype

Feuermagier · Jul 13, 2024 · 25818e5 · 25818e5
1 parent 2331753
commit 25818e5
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 26 deletions.
diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/AbstractProblemType.java b/autograder-api/src/main/java/de/firemage/autograder/api/AbstractProblemType.java
@@ -0,0 +1,4 @@
+package de.firemage.autograder.api;
+
+public interface AbstractProblemType {
+}
diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/CheckConfiguration.java b/autograder-api/src/main/java/de/firemage/autograder/api/CheckConfiguration.java
@@ -1,14 +1,16 @@
 package de.firemage.autograder.api;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
+import de.firemage.autograder.api.loader.ImplementationBinder;
 
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
 
-public record CheckConfiguration(List<String> problemsToReport, List<String> excludedClasses) {
+public record CheckConfiguration(List<? extends AbstractProblemType> problemsToReport, List<String> excludedClasses) {
     public static CheckConfiguration empty() {
         return new CheckConfiguration(List.of(), List.of());
     }
@@ -21,12 +23,19 @@ public static CheckConfiguration fromConfigString(String configString) throws IO
         if (!configString.contains("problemsToReport") && configString.startsWith("[")) {
             configString = "problemsToReport: " + configString;
         }
-        var config =  new ObjectMapper(new YAMLFactory()).readValue(configString, CheckConfiguration.class);
+
+        // Tell Jackson how to instantiate AbstractProblemType
+        var coreModule = new SimpleModule("autograder-core");
+        coreModule.addAbstractTypeMapping(AbstractProblemType.class, new ImplementationBinder<>(AbstractProblemType.class).findImplementation());
+
+        var mapper = new ObjectMapper(new YAMLFactory());
+        mapper.registerModule(coreModule);
+        var config =  mapper.readValue(configString, CheckConfiguration.class);
         config.validate();
         return config;
     }
 
-    public static CheckConfiguration fromProblemTypes(List<String> problemsToReport) {
+    public static CheckConfiguration fromProblemTypes(List<? extends AbstractProblemType> problemsToReport) {
         return new CheckConfiguration(problemsToReport, List.of());
     }
 

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
@@ -1,6 +1,7 @@
 package de.firemage.autograder.api.loader;
 
 import de.firemage.autograder.api.AbstractLinter;
+import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.AbstractTempLocation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -70,6 +71,13 @@ public static AbstractTempLocation instantiateTempLocation() {
                 .instantiate();
     }
 
+    public static AbstractProblemType convertProblemType(String problemType) {
+        return new ImplementationBinder<>(AbstractProblemType.class)
+                .param(String.class, problemType)
+                .classLoader(autograderClassLoader)
+                .callStatic("fromString", AbstractProblemType.class);
+    }
+
     private static String getAutograderVersionTag() throws IOException {
         URLConnection connection = new URL(AUTOGRADER_RELEASE_PATH).openConnection();
         connection.connect();

diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/loader/ImplementationBinder.java b/autograder-api/src/main/java/de/firemage/autograder/api/loader/ImplementationBinder.java
@@ -39,6 +39,27 @@ public ImplementationBinder<T> classLoader(ClassLoader classLoader) {
     }
 
     public T instantiate() {
+        var implementation = findImplementation();
+
+        try {
+            return implementation.getConstructor(parameterTypes.toArray(new Class[0])).newInstance(arguments.toArray());
+        } catch (ReflectiveOperationException e) {
+            throw new IllegalStateException("Failed to instantiate " + implementation.getName() + " with a constructor with parameters " + parameterTypes, e);
+        }
+    }
+
+    public <R> R callStatic(String methodName, Class<R> returnType) {
+        var implementation = findImplementation();
+
+        try {
+            var method = implementation.getMethod(methodName, parameterTypes.toArray(new Class[0]));
+            return returnType.cast(method.invoke(null, arguments.toArray()));
+        } catch (ReflectiveOperationException e) {
+            throw new IllegalStateException("Failed to call static method " + methodName + " on " + implementation.getName(), e);
+        }
+    }
+
+    public Class<T> findImplementation() {
         if (this.classLoader == null) {
             this.classLoader = this.getClass().getClassLoader();
         }
@@ -66,12 +87,7 @@ public T instantiate() {
 
         @SuppressWarnings("unchecked")
         var implementation = (Class<T>) reflectionCache.get(superType.getName());
-
-        try {
-            return implementation.getConstructor(parameterTypes.toArray(new Class[0])).newInstance(arguments.toArray());
-        } catch (ReflectiveOperationException e) {
-            throw new IllegalStateException("Failed to instantiate " + implementation.getName() + " with a constructor with parameters " + parameterTypes, e);
-        }
+        return implementation;
     }
 
     public static void invalidateCache() {

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
@@ -1,5 +1,6 @@
 package de.firemage.autograder.core;
 
+import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.CheckConfiguration;
 import de.firemage.autograder.api.AbstractLinter;
 import de.firemage.autograder.api.LinterException;
@@ -84,8 +85,7 @@ public List<Problem> checkFile(
         CheckConfiguration checkConfiguration,
         Consumer<Translatable> statusConsumer
     ) throws LinterException, IOException {
-        var typedProblems = ProblemType.fromStrings(checkConfiguration.problemsToReport());
-        var checks = this.findChecksForProblemTypes(typedProblems);
+        var checks = this.findChecksForProblemTypes(checkConfiguration.problemsToReport());
         return this.checkFile(file, checkConfiguration, checks, statusConsumer);
     }
 
@@ -220,7 +220,7 @@ public String translateMessage(Translatable message) {
             .getTypesAnnotatedWith(ExecutableCheck.class)
     );
 
-    public List<Check> findChecksForProblemTypes(Collection<ProblemType> problems) {
+    public List<Check> findChecksForProblemTypes(Collection<? extends AbstractProblemType> problems) {
         return CHECKS
             .stream()
             .filter(check -> isRequiredCheck(check.getAnnotation(ExecutableCheck.class), problems))
@@ -257,7 +257,7 @@ public List<? extends CodeLinter<?>> findCodeLinter() {
     }
 
 
-    private boolean isRequiredCheck(ExecutableCheck check, Collection<ProblemType> problems) {
+    private boolean isRequiredCheck(ExecutableCheck check, Collection<? extends AbstractProblemType> problems) {
         return check.enabled() && problems.stream().anyMatch(p -> List.of(check.reportedProblems()).contains(p));
     }
 

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
@@ -1,11 +1,12 @@
 package de.firemage.autograder.core;
 
+import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.HasFalsePositives;
 
 import java.util.Collection;
 import java.util.List;
 
-public enum ProblemType {
+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.
      * <br/>
@@ -1055,11 +1056,12 @@ public enum ProblemType {
     @HasFalsePositives
     REDUNDANT_UNINITIALIZED_VARIABLE;
 
+    /**
+     * <strong>Used via reflection, so don't remove, even if your IDE shows no usages!</strong>
+     * @param name
+     * @return
+     */
     public static ProblemType fromString(String name) {
         return ProblemType.valueOf(name);
     }
-
-    public static List<ProblemType> fromStrings(Collection<String> names) {
-        return names.stream().map(ProblemType::fromString).toList();
-    }
 }
diff --git a/autograder-core/src/test/java/de/firemage/autograder/core/TestSampleConfig.java b/autograder-core/src/test/java/de/firemage/autograder/core/TestSampleConfig.java
@@ -1,5 +1,6 @@
 package de.firemage.autograder.core;
 
+import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.CheckConfiguration;
 import de.firemage.autograder.api.LinterConfigurationException;
 import org.junit.jupiter.api.Test;
@@ -15,11 +16,11 @@
 import static org.junit.jupiter.api.Assertions.fail;
 
 class TestSampleConfig {
-    private static <T extends Comparable<? super T>> void assertProblemTypes(List<T> expected, List<T> actual) {
-        Collection<T> actualSet = new TreeSet<>(actual);
+    private static void assertProblemTypes(List<? extends AbstractProblemType> expected, List<? extends AbstractProblemType> actual) {
+        var actualSet = new TreeSet<>(actual);
 
-        Collection<T> difference = new ArrayList<>();
-        for (T value : expected) {
+        Collection<AbstractProblemType> difference = new ArrayList<>();
+        for (AbstractProblemType value : expected) {
             if (!actualSet.remove(value)) {
                 difference.add(value);
             }
@@ -38,7 +39,7 @@ private static <T extends Comparable<? super T>> void assertProblemTypes(List<T>
     void hasAllProblemTypes() throws IOException, LinterConfigurationException {
         // the `System.getProperty("user.dir")` is the path to the autograder-core directory
         Path path = Path.of(System.getProperty("user.dir"), "..", "sample_config.yaml");
-        List<ProblemType> present = ProblemType.fromStrings(CheckConfiguration.fromConfigFile(path).problemsToReport());
+        var present = CheckConfiguration.fromConfigFile(path).problemsToReport();
 
         assertProblemTypes(Arrays.asList(ProblemType.values()), present);
     }

diff --git a/autograder-core/src/test/java/de/firemage/autograder/core/check/AbstractCheckTest.java b/autograder-core/src/test/java/de/firemage/autograder/core/check/AbstractCheckTest.java
@@ -1,6 +1,7 @@
 package de.firemage.autograder.core.check;
 
 import de.firemage.autograder.api.AbstractLinter;
+import de.firemage.autograder.api.AbstractProblemType;
 import de.firemage.autograder.api.CheckConfiguration;
 import de.firemage.autograder.api.LinterException;
 import de.firemage.autograder.core.Linter;
@@ -41,13 +42,12 @@ private AbstractCheckTest(TempLocation tempLocation, int limit) {
 
     protected List<Problem> check(
         SourceInfo sourceInfo,
-        List<ProblemType> problemTypes
+        List<? extends AbstractProblemType> problemTypes
     ) throws LinterException, IOException {
-        var problemTypeStrings = problemTypes.stream().map(ProblemType::toString).toList();
         return this.linter.checkFile(
             UploadedFile.build(sourceInfo, this.tempLocation, status -> {
             }, null),
-            CheckConfiguration.fromProblemTypes(problemTypeStrings),
+            CheckConfiguration.fromProblemTypes(problemTypes),
             status -> {
             }
         );