Merge pull request #236 from Feuermagier/test-framework

New Test Framework

Test_Framework.md

@@ -0,0 +1,47 @@
+# The Test Framework
+
+## Syntax
+The `core` module contains a framework for testing in checks in `src/test/java/de/firemage/autograder/core/framework`.
+It allows you to create tests simply by writing valid Java code and inserting so-called *meta comments* that specify e.g. where the Autograder is expected to find problems.
+Meta comments are valid Java comments with additional '#' characters: Either block comments denoted by `/*#` and `#*/` or line comments denoted by `//#`.
+The framework will parse them and remove the comments before passing the code to the Autograder, so that they do not impact the actual source code to check.
+In particular, meta block comments can be used inside of standard Java strings and comments.
+
+The content of a meta comment consists of up to three parts, separated by semicolons:
+`<tag> ; <optional comment> ;  <optional expected problem type>`
+Whitespace around the parts is ignored.
+The first part must be a tag from the following table:
+
+| tag            | description                                                                   |
+|----------------|-------------------------------------------------------------------------------|
+| <empty string> | No problem expected here; use for comments                                    |
+| ok             | No problem expected here; used for documenting why this is the case           |
+| not ok         | Expect a problem here                                                         |
+| false positive | Expect a problem here, but warn about the existence of a false positive       |
+| false negative | Don't expect a problem here, but warn about the existence of a false positive |
+
+The second part of the meta comment is optional and can be used to provide a comment.
+The third part of the meta comment is also optional and may only be present if the second (comment) part is present (which may be empty).
+It should contain the name of the problem type to expect here.
+If specified, a problem of this type is expected at this line, as long as the tag indicated that a problem is expected here.
+
+## Examples
+```
+    //# not ok
+    
+    //# not ok; This is a comment
+    
+    //# not ok; auto-generated javadoc; JAVADOC_STUB_DESCRIPTION
+    
+    //# not ok;; JAVADOC_STUB_DESCRIPTION
+    
+    /*# not ok #*/
+    
+    /*#not ok#*/
+    
+    /*# false positive; see github issue #*/
+    
+    /*# ok; This is a comment*/
+    
+    /*# ;This is only a comment #*/
+```

autograder-core/src/main/java/de/firemage/autograder/core/check/complexity/RegexCheck.java

@@ -39,7 +39,9 @@ public class RegexCheck extends IntegratedCheck {
     private static final int MIN_REGEX_HINTS = 2;
 
     private static boolean hasComment(CtElement ctElement) {
-        return !ctElement.getComments().isEmpty()
+        return (!ctElement.getComments().isEmpty()
+            // test-framework comments start with //#, which should be ignored
+            && ctElement.getComments().stream().anyMatch(ctComment -> !ctComment.getContent().startsWith("#")))
             || ctElement.getParent() instanceof CtVariable<?> ctVariable && hasComment(ctVariable);
     }
 

autograder-core/src/main/java/de/firemage/autograder/core/check/general/ImportTypes.java

@@ -5,6 +5,8 @@
 import de.firemage.autograder.core.ProblemType;
 import de.firemage.autograder.core.check.ExecutableCheck;
 import de.firemage.autograder.core.dynamic.DynamicAnalysis;
+import de.firemage.autograder.core.file.CompilationUnit;
+import de.firemage.autograder.core.file.SourceInfo;
 import de.firemage.autograder.core.integrated.IntegratedCheck;
 import de.firemage.autograder.core.integrated.SpoonUtil;
 import de.firemage.autograder.core.integrated.StaticAnalysis;
@@ -14,13 +16,15 @@
 import spoon.reflect.cu.SourcePosition;
 import spoon.reflect.cu.SourcePositionHolder;
 import spoon.reflect.cu.position.DeclarationSourcePosition;
+import spoon.reflect.declaration.CtCompilationUnit;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.ParentNotInitializedException;
 import spoon.reflect.reference.CtArrayTypeReference;
-import spoon.reflect.reference.CtExecutableReference;
 import spoon.reflect.reference.CtTypeReference;
 
+import java.io.IOException;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -49,19 +53,57 @@ private static String getCodeSnippet(SourcePositionHolder ctElement) {
             return ctElement.getOriginalSourceFragment().getSourceCode();
         } catch (SpoonException e) {
             // Invalid start/end interval. It overlaps multiple fragments.
-            SourcePosition position = ctElement.getPosition();
+            SourcePosition sourcePosition = ctElement.getPosition();
+            Position position = Position.of(sourcePosition);
+
+            return position.substring(sourcePosition.getCompilationUnit().getOriginalSourceCode());
+        }
+    }
+
+    private record Position(int start, int end) {
+        private Position {
+            if (start > end) {
+                throw new IllegalArgumentException(
+                    "start %d of code position must be smaller than the end %d".formatted(start, end)
+                );
+            }
+        }
+
+        public static Position of(SourcePosition position) {
             int start = position.getSourceStart();
             int end = position.getSourceEnd();
             if (position instanceof DeclarationSourcePosition declarationSourcePosition) {
                 start = declarationSourcePosition.getDeclarationStart();
                 end = declarationSourcePosition.getDeclarationEnd();
             }
 
-            return position.getCompilationUnit().getOriginalSourceCode().substring(start, end + 1);
+            return new Position(start, end + 1);
+        }
+
+        public String substring(String string) {
+            return string.substring(this.start, this.end);
+        }
+    }
+
+    private static Optional<String> getCodeSnippetFromSourceInfo(SourcePosition sourcePosition, SourceInfo sourceInfo) {
+        CtCompilationUnit ctCompilationUnit = sourcePosition.getCompilationUnit();
+
+        if (ctCompilationUnit.getFile() == null) {
+            return Optional.empty();
+        }
+
+        CompilationUnit compilationUnit = sourceInfo.getCompilationUnit(ctCompilationUnit.getFile().toPath());
+
+        Position position = Position.of(sourcePosition);
+
+        try {
+            return Optional.of(position.substring(compilationUnit.readString()));
+        } catch (IOException e) {
+            throw new IllegalArgumentException("Could not read compilation unit", e);
         }
     }
 
-    private static SourcePosition resolveArraySourcePosition(CtArrayTypeReference<?> ctArrayTypeReference) {
+    private SourcePosition resolveArraySourcePosition(CtArrayTypeReference<?> ctArrayTypeReference) {
         CtElement ctElement = ctArrayTypeReference;
 
         while (ctElement.isParentInitialized() && !ctElement.getPosition().isValidPosition()) {
@@ -87,6 +129,11 @@ private static SourcePosition resolveArraySourcePosition(CtArrayTypeReference<?>
 
         String code = getCodeSnippet(ctElement);
 
+        // only happens when the ctElement is from a virtual file
+        if (code == null) {
+            code = getCodeSnippetFromSourceInfo(position, this.getRoot()).orElseThrow();
+        }
+
         // NOTE: this only matches the "normal" array syntax: String[] value and not String value[]
         // the latter example is especially bad when it comes to things like methods, where you can do:
         // String myMethod()[]

autograder-core/src/main/java/de/firemage/autograder/core/file/SourcePath.java

@@ -87,6 +87,10 @@ SourcePath resolve(SourcePath other) {
         return SourcePath.of(result);
     }
 
+    public Path toPath() {
+        return Path.of(this.toString());
+    }
+
     @Override
     public boolean equals(Object other) {
         if (!(other instanceof SourcePath otherPath)) {

autograder-core/src/main/java/de/firemage/autograder/core/file/StringSourceInfo.java

@@ -6,10 +6,16 @@
 import spoon.support.compiler.VirtualFile;
 import spoon.support.compiler.VirtualFolder;
 
+import javax.lang.model.element.Modifier;
+import javax.lang.model.element.NestingKind;
 import javax.tools.JavaFileObject;
-import javax.tools.SimpleJavaFileObject;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
 import java.io.Serializable;
+import java.io.StringReader;
+import java.io.Writer;
 import java.net.URI;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -81,35 +87,68 @@ public JavaVersion getVersion() {
         return this.version;
     }
 
-    // TODO: this class is not serializable...
-    private static final class VirtualFileObject extends SimpleJavaFileObject implements Serializable, CompilationUnit {
+    private static final class VirtualFileObject implements JavaFileObject, Serializable, CompilationUnit {
         private final ClassPath classPath;
         private final String code;
 
         private VirtualFileObject(ClassPath classPath, String code) {
-            super(virtualUri(classPath), Kind.SOURCE);
             this.classPath = classPath;
             this.code = code;
         }
 
         private static URI virtualUri(ClassPath classPath) {
             return URI.create("string:///%s/%s%s".formatted(
-                VIRTUAL_FOLDER,
-                classPath.toString().replace('.', '/'),
-                JavaFileObject.Kind.SOURCE.extension
+                    VIRTUAL_FOLDER,
+                    classPath.toString().replace('.', '/'),
+                    JavaFileObject.Kind.SOURCE.extension
             ));
         }
 
+        @Override
+        public URI toUri() {
+            return virtualUri(this.classPath);
+        }
+
         @Override
         public String getName() {
             return this.path().toString();
         }
 
+        @Override
+        public InputStream openInputStream() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public OutputStream openOutputStream() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Reader openReader(boolean ignoreEncodingErrors) {
+            return new StringReader(this.code);
+        }
+
         @Override
         public CharSequence getCharContent(boolean ignoreEncodingErrors) {
             return this.code;
         }
 
+        @Override
+        public Writer openWriter() throws IOException {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public long getLastModified() {
+            return 0;
+        }
+
+        @Override
+        public boolean delete() {
+            return false;
+        }
+
         public String getCode() {
             return this.code;
         }
@@ -129,6 +168,29 @@ public Charset charset() {
             // virtual files are always UTF-8
             return StandardCharsets.UTF_8;
         }
+
+        @Override
+        public Kind getKind() {
+            return Kind.SOURCE;
+        }
+
+        @Override
+        public boolean isNameCompatible(String simpleName, Kind kind) {
+            String baseName = simpleName + kind.extension;
+            return kind.equals(getKind())
+                    && (baseName.equals(toUri().getPath())
+                    || toUri().getPath().endsWith("/" + baseName));
+        }
+
+        @Override
+        public NestingKind getNestingKind() {
+            return null;
+        }
+
+        @Override
+        public Modifier getAccessLevel() {
+            return null;
+        }
     }
 
     private record ClassPath(List<String> path, String name) implements Serializable {
@@ -138,8 +200,8 @@ private static ClassPath fromString(String string) {
             }
             List<String> parts = Arrays.asList(string.split("\\.", -1));
             return new ClassPath(
-                new ArrayList<>(parts.subList(0, parts.size() - 1)),
-                parts.get(parts.size() - 1)
+                    new ArrayList<>(parts.subList(0, parts.size() - 1)),
+                    parts.get(parts.size() - 1)
             );
         }
 

autograder-core/src/main/java/de/firemage/autograder/core/integrated/SpoonUtil.java

@@ -63,6 +63,7 @@
 import java.util.stream.Stream;
 
 public final class SpoonUtil {
+
     private SpoonUtil() {
 
     }

autograder-core/src/main/java/de/firemage/autograder/core/pmd/PMDLinter.java

@@ -52,7 +52,7 @@ public List<Problem> lint(UploadedFile file, List<PMDCheck> checks, ClassLoader
             FileCollector collector = pmd.files();
             for (CompilationUnit compilationUnit : file.getSource().compilationUnits()) {
                 collector.addSourceFile(
-                    FileId.fromPathLikeString(compilationUnit.path().toString()),
+                    FileId.fromPathLikeString(file.getSource().path().resolve(compilationUnit.path().toPath()).toString()),
                     compilationUnit.readString()
                 );
             }

autograder-core/src/test/java/de/firemage/autograder/core/framework/ExpectedProblem.java

@@ -0,0 +1,18 @@
+package de.firemage.autograder.core.framework;
+
+import de.firemage.autograder.core.ProblemType;
+import de.firemage.autograder.core.file.SourcePath;
+
+public record ExpectedProblem(SourcePath file, int line, ProblemType problemType, String comment) {
+    public String format() {
+        return "%s@%s:%d".formatted(
+                this.problemType != null ? this.problemType : "?",
+                this.file,
+                this.line
+        );
+    }
+
+    public String toString() {
+        return this.format();
+    }
+}

autograder-core/src/test/java/de/firemage/autograder/core/framework/ReportedProblem.java

@@ -0,0 +1,6 @@
+package de.firemage.autograder.core.framework;
+
+import de.firemage.autograder.core.Problem;
+
+public record ReportedProblem(Problem problem, String translatedMessage) {
+}

autograder-core/src/test/java/de/firemage/autograder/core/framework/TestConfig.java

@@ -0,0 +1,44 @@
+package de.firemage.autograder.core.framework;
+
+import de.firemage.autograder.core.check.Check;
+
+import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+public record TestConfig(List<String> lines) {
+    public static TestConfig fromPath(Path path) {
+        try {
+            List<String> lines = Files.readAllLines(path.resolve("config.txt"));
+
+            if (lines.size() < 2) {
+                throw new IllegalArgumentException("Config file must contain at least two lines");
+            }
+
+            return new TestConfig(lines);
+        } catch (IOException ex) {
+            throw new RuntimeException(ex);
+        }
+    }
+
+    public String checkPath() {
+        return this.lines.get(0);
+    }
+
+    public String description() {
+        return this.lines.get(1);
+    }
+
+    public String qualifiedName() {
+        return "de.firemage.autograder.core.check." + this.checkPath();
+    }
+
+    public Check check() throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, InstantiationException, IllegalAccessException {
+        return (Check) Class.forName(this.qualifiedName())
+                .getDeclaredConstructor()
+                .newInstance();
+    }
+}