diff --git a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java index c70f4fb..b99116a 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java @@ -8,17 +8,30 @@ import net.neoforged.jst.api.Replacements; import net.neoforged.jst.api.SourceTransformer; import net.neoforged.jst.api.TransformContext; +import net.neoforged.problems.Problem; +import net.neoforged.problems.ProblemGroup; +import net.neoforged.problems.ProblemId; +import net.neoforged.problems.ProblemLocation; +import net.neoforged.problems.ProblemReporter; +import net.neoforged.problems.ProblemSeverity; import picocli.CommandLine; -import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; public class AccessTransformersTransformer implements SourceTransformer { + private static final ProblemGroup PROBLEM_GROUP = ProblemGroup.create("access-transformer", "Access Transformers"); + static final ProblemId INVALID_AT = ProblemId.create("invalid-at", "Invalid", PROBLEM_GROUP); + private static final ProblemId MISSING_TARGET = ProblemId.create("missing-target", "Missing Target", PROBLEM_GROUP); + + private static final Pattern LINE_PATTERN = Pattern.compile("\\bline\\s+(\\d+)"); + private static final Pattern ORIGIN_PATTERN = Pattern.compile("(.*):(\\d+)$"); + @CommandLine.Option(names = "--access-transformer", required = true) public List atFiles; @@ -28,19 +41,36 @@ public class AccessTransformersTransformer implements SourceTransformer { private AccessTransformerFiles ats; private Map pendingATs; private Logger logger; + private ProblemReporter problemReporter; private volatile boolean errored; @Override public void beforeRun(TransformContext context) { ats = new AccessTransformerFiles(); logger = context.logger(); + problemReporter = context.problemReporter(); for (Path path : atFiles) { try { ats.loadFromPath(path); - } catch (IOException ex) { - logger.error("Failed to parse access transformer file %s: %s", path, ex.getMessage()); - throw new UncheckedIOException(ex); + } catch (Exception e) { + logger.error("Failed to parse access transformer file %s: %s", path, e.getMessage()); + + if (e.getMessage() != null) { + var m = LINE_PATTERN.matcher(e.getMessage()); + if (m.matches()) { + // The AT parser internally uses 0-based line numbering, but the problem reporter uses 1-based + int line = 1 + Integer.parseUnsignedInt(m.group(1)); + problemReporter.report(INVALID_AT, ProblemSeverity.ERROR, ProblemLocation.ofLocationInFile(path, line), e.getMessage()); + } else { + problemReporter.report(INVALID_AT, ProblemSeverity.ERROR, ProblemLocation.ofFile(path), e.getMessage()); + } + } + + if (e instanceof RuntimeException re) { + throw re; + } + throw new RuntimeException(e); } } @@ -55,6 +85,12 @@ public boolean afterRun(TransformContext context) { // so we don't log the ClassTarget as that will cause duplication if (target instanceof Target.ClassTarget && target.className().contains("$")) return; logger.error("Access transformer %s, targeting %s did not apply as its target doesn't exist", transformation, target); + + var problem = Problem.builder(MISSING_TARGET) + .severity(ProblemSeverity.ERROR) + .contextualLabel("The target " + target + " does not exist.") + .build(); + reportProblem(problemReporter, transformation, problem); }); errored = true; } @@ -62,9 +98,26 @@ public boolean afterRun(TransformContext context) { return !(errored && validation == AccessTransformerValidation.ERROR); } + static void reportProblem(ProblemReporter problemReporter, Transformation transformation, Problem problem) { + // Report a problem for each origin of the transform + for (String origin : transformation.origins()) { + var m = ORIGIN_PATTERN.matcher(origin); + ProblemLocation problemLocation; + if (!m.matches()) { + problemLocation = ProblemLocation.ofFile(Paths.get(origin)); + } else { + var file = Path.of(m.group(1)); + var line = Integer.parseUnsignedInt(m.group(2)); + problemLocation = ProblemLocation.ofLocationInFile(file, line); + } + + problemReporter.report(Problem.builder(problem).location(problemLocation).build()); + } + } + @Override public void visitFile(PsiFile psiFile, Replacements replacements) { - var visitor = new ApplyATsVisitor(ats, replacements, pendingATs, logger); + var visitor = new ApplyATsVisitor(ats, replacements, pendingATs, logger, problemReporter); visitor.visitFile(psiFile); if (visitor.errored) { errored = true; diff --git a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java index 2cfce18..3fc6748 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java @@ -14,17 +14,20 @@ import com.intellij.psi.PsiRecursiveElementVisitor; import com.intellij.psi.PsiWhiteSpace; import com.intellij.psi.util.ClassUtil; -import com.intellij.psi.util.PsiClassUtil; import net.neoforged.accesstransformer.parser.AccessTransformerFiles; import net.neoforged.accesstransformer.parser.Target; import net.neoforged.accesstransformer.parser.Transformation; import net.neoforged.jst.api.Logger; import net.neoforged.jst.api.PsiHelper; import net.neoforged.jst.api.Replacements; +import net.neoforged.problems.Problem; +import net.neoforged.problems.ProblemReporter; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumMap; import java.util.Locale; import java.util.Map; @@ -47,13 +50,15 @@ class ApplyATsVisitor extends PsiRecursiveElementVisitor { private final Replacements replacements; private final Map pendingATs; private final Logger logger; + private final ProblemReporter problemReporter; boolean errored = false; - public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements, Map pendingATs, Logger logger) { + public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements, Map pendingATs, Logger logger, ProblemReporter problemReporter) { this.ats = ats; this.replacements = replacements; this.logger = logger; this.pendingATs = pendingATs; + this.problemReporter = problemReporter; } @Override @@ -119,7 +124,7 @@ public void visitElement(@NotNull PsiElement element) { private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiClass containingClass) { if (at == null) return; if (!at.isValid()) { - error("Found invalid access transformer: %s. Final state: conflicting", at); + error(at, "Found invalid access transformer. Final state: conflicting"); return; } @@ -147,7 +152,7 @@ public String toString() { // If we're modifying a non-static interface method we can only make it public, meaning it must be defined as default if (containingClass.isInterface() && owner instanceof PsiMethod && !modifiers.hasModifierProperty(PsiModifier.STATIC)) { if (targetAcc != Transformation.Modifier.PUBLIC) { - error("Access transformer %s targeting %s attempted to make a non-static interface method %s. They can only be made public.", at, targetInfo, targetAcc); + error(at, "Access transformer targeting %s attempted to make a non-static interface method %s. They can only be made public.", targetInfo, targetAcc); } else { for (var kw : modifiers.getChildren()) { if (kw instanceof PsiKeyword && kw.getText().equals(PsiKeyword.PRIVATE)) { // Strip private, replace it with default @@ -158,7 +163,7 @@ public String toString() { } } else if (containingClass.isEnum() && owner instanceof PsiMethod mtd && mtd.isConstructor() && at.modifier().ordinal() < Transformation.Modifier.DEFAULT.ordinal()) { // Enum constructors can at best be package-private, any other attempt must be prevented - error("Access transformer %s targeting %s attempted to make an enum constructor %s", at, targetInfo, at.modifier()); + error(at, "Access transformer targeting %s attempted to make an enum constructor %s", targetInfo, at.modifier()); } else if (targetAcc.ordinal() < detectModifier(modifiers, null).ordinal()) { // PUBLIC (0) < PROTECTED (1) < DEFAULT (2) < PRIVATE (3) modify(targetAcc, modifiers, Arrays.stream(modifiers.getChildren()) .filter(el -> el instanceof PsiKeyword) @@ -176,7 +181,7 @@ public String toString() { .findFirst() .ifPresent(replacements::remove); } else if (finalState == Transformation.FinalState.MAKEFINAL && !modifiers.hasModifierProperty(PsiModifier.FINAL)) { - error("Access transformer %s attempted to make %s final. Was non-final", at, targetInfo); + error(at, "Access transformer attempted to make %s final. Was non-final", targetInfo); } } @@ -250,11 +255,11 @@ private void checkImplicitConstructor(PsiClass psiClass, String className, @Null var implicitAT = pendingATs.remove(new Target.MethodTarget(className, "", desc)); if (implicitAT != null && implicitAT.modifier() != detectModifier(psiClass.getModifierList(), classAt)) { - error("Access transformer %s targeting the implicit constructor of %s is not valid, as a record's constructor must have the same access level as the record class. Please AT the record too: \"%s\"", implicitAT, className, + error(implicitAT, "Access transformer targeting the implicit constructor of %s is not valid, as a record's constructor must have the same access level as the record class. Please AT the record too: \"%s\"", className, implicitAT.modifier().toString().toLowerCase(Locale.ROOT) + " " + className); pendingATs.remove(new Target.MethodTarget(className, "", desc)); } else if (classAt != null && detectModifier(psiClass.getModifierList(), null).ordinal() > classAt.modifier().ordinal() && implicitAT == null) { - error("Access transformer %s targeting record class %s attempts to widen its access without widening the constructor's access. You must AT the constructor too: \"%s\"", classAt, className, + error(classAt, "Access transformer targeting record class %s attempts to widen its access without widening the constructor's access. You must AT the constructor too: \"%s\"", className, classAt.modifier().toString().toLowerCase(Locale.ROOT) + " " + className + " " + desc); pendingATs.remove(new Target.MethodTarget(className, "", desc)); } @@ -294,8 +299,16 @@ private void injectConstructor(PsiClass psiClass, String className, Transformati " ".repeat(indent) + modifierString + psiClass.getName() + "() {}"); } - private void error(String message, Object... args) { - logger.error(message, args); + private void error(Transformation transformation, String message, Object... args) { + var problem = Problem.builder(AccessTransformersTransformer.INVALID_AT) + .contextualLabel(String.format(Locale.ROOT, message, args)) + .build(); + AccessTransformersTransformer.reportProblem(problemReporter, transformation, problem); + + var formatArgs = new ArrayList<>(); + Collections.addAll(formatArgs, args); + formatArgs.add(transformation); + logger.error(message + " at %s", formatArgs.toArray()); errored = true; } diff --git a/api/build.gradle b/api/build.gradle index cc03fd5..24814ba 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -19,6 +19,7 @@ java { dependencies { api "com.jetbrains.intellij.java:java-psi-impl:$intellij_version" api "info.picocli:picocli:$picocli_version" + api "net.neoforged.installertools:problems-api:$problems_api_version" compileOnly "org.jetbrains:annotations:$jetbrains_annotations_version" } diff --git a/api/src/main/java/net/neoforged/jst/api/TransformContext.java b/api/src/main/java/net/neoforged/jst/api/TransformContext.java index 890e0fa..ebaa184 100644 --- a/api/src/main/java/net/neoforged/jst/api/TransformContext.java +++ b/api/src/main/java/net/neoforged/jst/api/TransformContext.java @@ -1,4 +1,9 @@ package net.neoforged.jst.api; -public record TransformContext(IntelliJEnvironment environment, FileSource source, FileSink sink, Logger logger) { +import net.neoforged.problems.ProblemReporter; + +public record TransformContext(IntelliJEnvironment environment, FileSource source, FileSink sink, Logger logger, ProblemReporter problemReporter) { + public TransformContext(IntelliJEnvironment environment, FileSource source, FileSink sink, Logger logger) { + this(environment, source, sink, logger, ProblemReporter.NOOP); + } } diff --git a/cli/src/main/java/net/neoforged/jst/cli/Main.java b/cli/src/main/java/net/neoforged/jst/cli/Main.java index 613d2e1..1b2cbbf 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/Main.java +++ b/cli/src/main/java/net/neoforged/jst/cli/Main.java @@ -5,6 +5,9 @@ import net.neoforged.jst.api.SourceTransformerPlugin; import net.neoforged.jst.cli.io.FileSinks; import net.neoforged.jst.cli.io.FileSources; +import net.neoforged.problems.FileProblemReporter; +import net.neoforged.problems.ProblemReporter; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.VisibleForTesting; import picocli.CommandLine; @@ -12,6 +15,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.ServiceLoader; import java.util.concurrent.Callable; @@ -41,9 +45,12 @@ public class Main implements Callable { @CommandLine.Option(names = "--max-queue-depth", description = "When both input and output support ordering (archives), the transformer will try to maintain that order. To still process items in parallel, a queue is used. Larger queue depths lead to higher memory usage.") int maxQueueDepth = 100; - @CommandLine.Command(name = "--debug", description = "Print additional debugging information") + @CommandLine.Option(names = "--debug", description = "Print additional debugging information") boolean debug = false; + @CommandLine.Option(names = "--problems-report", description = "Write problems to this report file.") + Path problemsReport; + private final HashSet enabledTransformers = new HashSet<>(); public static void main(String[] args) { @@ -68,7 +75,8 @@ public static int innerMain(String... args) { public Integer call() throws Exception { var logger = debug ? new Logger(System.out, System.err) : new Logger(null, System.err); try (var source = FileSources.create(inputPath, inputFormat); - var processor = new SourceFileProcessor(logger)) { + var problemReporter = createProblemReporter(problemsReport); + var processor = new SourceFileProcessor(logger, Objects.requireNonNullElse(problemReporter, ProblemReporter.NOOP))) { if (librariesList != null) { processor.addLibrariesList(librariesList); @@ -96,6 +104,15 @@ public Integer call() throws Exception { return 0; } + @Nullable + private FileProblemReporter createProblemReporter(Path problemsReport) { + if (problemsReport == null) { + return null; + } else { + return new FileProblemReporter(problemsReport); + } + } + private void setupPluginCliOptions(List plugins, CommandLine.Model.CommandSpec spec) { for (var plugin : plugins) { var transformer = plugin.createTransformer(); diff --git a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java index 5e7dd06..0f624e2 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -12,6 +12,7 @@ import net.neoforged.jst.api.TransformContext; import net.neoforged.jst.cli.intellij.ClasspathSetup; import net.neoforged.jst.cli.intellij.IntelliJEnvironmentImpl; +import net.neoforged.problems.ProblemReporter; import java.io.IOException; import java.io.UncheckedIOException; @@ -32,11 +33,13 @@ class SourceFileProcessor implements AutoCloseable { private final IntelliJEnvironmentImpl ijEnv; private int maxQueueDepth = 50; private final Logger logger; + private final ProblemReporter problemReporter; private final List ignoredPrefixes = new ArrayList<>(); - public SourceFileProcessor(Logger logger) throws IOException { + public SourceFileProcessor(Logger logger, ProblemReporter problemReporter) throws IOException { this.logger = logger; + this.problemReporter = problemReporter; ijEnv = new IntelliJEnvironmentImpl(logger); ijEnv.addCurrentJdkToClassPath(); } @@ -46,7 +49,7 @@ public boolean process(FileSource source, FileSink sink, List throw new IllegalStateException("Cannot have an input with possibly more than one file when the output is a single file."); } - var context = new TransformContext(ijEnv, source, sink, logger); + var context = new TransformContext(ijEnv, source, sink, logger, problemReporter); var sourceRoot = source.createSourceRoot(VirtualFileManager.getInstance()); ijEnv.addSourceRoot(sourceRoot); diff --git a/gradle.properties b/gradle.properties index 6e777aa..91dfa49 100644 --- a/gradle.properties +++ b/gradle.properties @@ -5,3 +5,4 @@ picocli_version=4.7.6 junit_version=5.10.3 assertj_version=3.26.0 gson_version=2.10.1 +problems_api_version=3.0.3 diff --git a/settings.gradle b/settings.gradle index 7ff6427..cf699a4 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,3 +1,5 @@ +import org.gradle.api.initialization.resolve.RepositoriesMode + pluginManagement { repositories { gradlePluginPortal() @@ -13,6 +15,7 @@ plugins { } dependencyResolutionManagement { + repositoriesMode = RepositoriesMode.FAIL_ON_PROJECT_REPOS repositories { mavenCentral() maven { diff --git a/tests/data/accesstransformer/classes/expected_report.json b/tests/data/accesstransformer/classes/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/classes/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/fields/expected_report.json b/tests/data/accesstransformer/fields/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/fields/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/folder_classpath_entry/expected_report.json b/tests/data/accesstransformer/folder_classpath_entry/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/folder_classpath_entry/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/illegal/expected.log b/tests/data/accesstransformer/illegal/expected.log index 0f3cbf8..3eb24fb 100644 --- a/tests/data/accesstransformer/illegal/expected.log +++ b/tests/data/accesstransformer/illegal/expected.log @@ -1 +1 @@ -Access transformer PUBLIC LEAVE {atpath}:1 targeting constructor of AnEnum attempted to make an enum constructor PUBLIC +Access transformer targeting constructor of AnEnum attempted to make an enum constructor PUBLIC at PUBLIC LEAVE {atpath}:1 diff --git a/tests/data/accesstransformer/illegal/expected_report.json b/tests/data/accesstransformer/illegal/expected_report.json new file mode 100644 index 0000000..1916dd4 --- /dev/null +++ b/tests/data/accesstransformer/illegal/expected_report.json @@ -0,0 +1,18 @@ +[ + { + "problemId": { + "id": "invalid-at", + "displayName": "Invalid", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "WARNING", + "location": { + "file": "accesstransformer.cfg", + "line": 1 + }, + "contextualLabel": "Access transformer targeting constructor of AnEnum attempted to make an enum constructor PUBLIC" + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/implicit_constructors/expected.log b/tests/data/accesstransformer/implicit_constructors/expected.log index 6d7d4c6..d055143 100644 --- a/tests/data/accesstransformer/implicit_constructors/expected.log +++ b/tests/data/accesstransformer/implicit_constructors/expected.log @@ -1 +1 @@ -Access transformer PUBLIC LEAVE {atpath}:9 targeting record class PrivateRecord attempts to widen its access without widening the constructor's access. You must AT the constructor too: "public PrivateRecord (I)V" +Access transformer targeting record class PrivateRecord attempts to widen its access without widening the constructor's access. You must AT the constructor too: "public PrivateRecord (I)V" at PUBLIC LEAVE {atpath}:9 diff --git a/tests/data/accesstransformer/implicit_constructors/expected_report.json b/tests/data/accesstransformer/implicit_constructors/expected_report.json new file mode 100644 index 0000000..667b753 --- /dev/null +++ b/tests/data/accesstransformer/implicit_constructors/expected_report.json @@ -0,0 +1,18 @@ +[ + { + "problemId": { + "id": "invalid-at", + "displayName": "Invalid", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "WARNING", + "location": { + "file": "accesstransformer.cfg", + "line": 9 + }, + "contextualLabel": "Access transformer targeting record class PrivateRecord attempts to widen its access without widening the constructor\u0027s access. You must AT the constructor too: \"public PrivateRecord \u003cinit\u003e(I)V\"" + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/inner_classes/expected_report.json b/tests/data/accesstransformer/inner_classes/expected_report.json new file mode 100644 index 0000000..e8a1703 --- /dev/null +++ b/tests/data/accesstransformer/inner_classes/expected_report.json @@ -0,0 +1,18 @@ +[ + { + "problemId": { + "id": "missing-target", + "displayName": "Missing Target", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "ERROR", + "location": { + "file": "accesstransformer.cfg", + "line": 5 + }, + "contextualLabel": "The target com.example.RootClass INNERCLASS InnerDoesntExist does not exist." + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/inner_members/expected_report.json b/tests/data/accesstransformer/inner_members/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/inner_members/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/interfaces/expected.log b/tests/data/accesstransformer/interfaces/expected.log index d2ad1dd..cee4108 100644 --- a/tests/data/accesstransformer/interfaces/expected.log +++ b/tests/data/accesstransformer/interfaces/expected.log @@ -1 +1 @@ -Access transformer PROTECTED LEAVE {atpath}:2 targeting callThingy2 of If1 attempted to make a non-static interface method PROTECTED. They can only be made public. +Access transformer targeting callThingy2 of If1 attempted to make a non-static interface method PROTECTED. They can only be made public. at PROTECTED LEAVE {atpath}:2 diff --git a/tests/data/accesstransformer/interfaces/expected_report.json b/tests/data/accesstransformer/interfaces/expected_report.json new file mode 100644 index 0000000..81cace4 --- /dev/null +++ b/tests/data/accesstransformer/interfaces/expected_report.json @@ -0,0 +1,18 @@ +[ + { + "problemId": { + "id": "invalid-at", + "displayName": "Invalid", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "WARNING", + "location": { + "file": "accesstransformer.cfg", + "line": 2 + }, + "contextualLabel": "Access transformer targeting callThingy2 of If1 attempted to make a non-static interface method PROTECTED. They can only be made public." + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/methods/expected_report.json b/tests/data/accesstransformer/methods/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/methods/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/missing_target/expected_report.json b/tests/data/accesstransformer/missing_target/expected_report.json new file mode 100644 index 0000000..8a0af0e --- /dev/null +++ b/tests/data/accesstransformer/missing_target/expected_report.json @@ -0,0 +1,50 @@ +[ + { + "problemId": { + "id": "missing-target", + "displayName": "Missing Target", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "ERROR", + "location": { + "file": "accesstransformer.cfg", + "line": 3 + }, + "contextualLabel": "The target ExistingClass FIELD notAField does not exist." + }, + { + "problemId": { + "id": "missing-target", + "displayName": "Missing Target", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "ERROR", + "location": { + "file": "accesstransformer.cfg", + "line": 5 + }, + "contextualLabel": "The target DoesntExist CLASS does not exist." + }, + { + "problemId": { + "id": "missing-target", + "displayName": "Missing Target", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "ERROR", + "location": { + "file": "accesstransformer.cfg", + "line": 4 + }, + "contextualLabel": "The target ExistingClass METHOD notAMethod()V does not exist." + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/no_modifiers/expected_report.json b/tests/data/accesstransformer/no_modifiers/expected_report.json new file mode 100644 index 0000000..150bc30 --- /dev/null +++ b/tests/data/accesstransformer/no_modifiers/expected_report.json @@ -0,0 +1,18 @@ +[ + { + "problemId": { + "id": "invalid-at", + "displayName": "Invalid", + "group": { + "id": "access-transformer", + "displayName": "Access Transformers" + } + }, + "severity": "WARNING", + "location": { + "file": "accesstransformer.cfg", + "line": 1 + }, + "contextualLabel": "Access transformer targeting record class C1 attempts to widen its access without widening the constructor\u0027s access. You must AT the constructor too: \"public C1 \u003cinit\u003e()V\"" + } +] \ No newline at end of file diff --git a/tests/data/accesstransformer/wildcard/expected_report.json b/tests/data/accesstransformer/wildcard/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/wildcard/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/wildcard_and_explicit/expected_report.json b/tests/data/accesstransformer/wildcard_and_explicit/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/wildcard_and_explicit/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java index 365b3c0..e7de356 100644 --- a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java +++ b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java @@ -1,7 +1,14 @@ package net.neoforged.jst.tests; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.TypeAdapter; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; import com.intellij.util.ArrayUtil; import net.neoforged.jst.cli.Main; +import net.neoforged.problems.FileProblemReporter; +import net.neoforged.problems.Problem; import org.assertj.core.util.CanIgnoreReturnValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -365,10 +372,10 @@ protected final void runATTest(String testDirName, final String... extraArgs) th testDirName = "accesstransformer/" + testDirName; var atPath = testDataRoot.resolve(testDirName).resolve("accesstransformer.cfg"); runTest(testDirName, txt -> txt.replace(atPath.toAbsolutePath().toString(), "{atpath}"), ArrayUtil.mergeArrays( - new String[]{ - "--enable-accesstransformers", "--access-transformer", atPath.toString() - }, - extraArgs + new String[]{ + "--enable-accesstransformers", "--access-transformer", atPath.toString() + }, + extraArgs )); } @@ -395,10 +402,19 @@ protected final void runTest(String testDirName, UnaryOperator consoleMa var librariesFile = tempDir.resolve("libraries.txt"); Files.write(librariesFile, List.of("-e=" + junitJarPath)); + var reportFile = tempDir.resolve("report.json"); + var expectedReport = testDir.resolve("expected_report.json"); + final List arguments = new ArrayList<>(Arrays.asList( "--max-queue-depth=1", "--libraries-list", librariesFile.toString())); + + if (Files.exists(expectedReport)) { + arguments.add("--problems-report"); + arguments.add(reportFile.toString()); + } + arguments.addAll(Arrays.asList(args)); arguments.add(inputFile.toString()); arguments.add(outputFile.toString()); @@ -408,10 +424,44 @@ protected final void runTest(String testDirName, UnaryOperator consoleMa var expectedLog = testDir.resolve("expected.log"); if (Files.exists(expectedLog)) { - assertThat(expectedLog).content().isEqualToNormalizingNewlines(consoleOut); + var expectedLogContent = Files.readString(expectedLog); + assertThat(consoleOut).isEqualToNormalizingNewlines(expectedLogContent); + } + + if (Files.exists(expectedReport)) { + var expectedRecords = FileProblemReporter.loadRecords(expectedReport); + var actualRecords = FileProblemReporter.loadRecords(reportFile); + + // Relativize the paths to make them comparable to the reference data. + actualRecords = actualRecords.stream().map(record -> { + if (record.location() == null) { + return record; + } + return Problem.builder(record) + .location(record.location().withFile(testDir.relativize(record.location().file()))) + .build(); + } + ).toList(); + + assertEquals(problemsToJson(expectedRecords), problemsToJson(actualRecords)); } } + private String problemsToJson(List problems) { + return new GsonBuilder() + .setPrettyPrinting() + .registerTypeHierarchyAdapter(Path.class, new TypeAdapter() { + public void write(JsonWriter out, Path value) throws IOException { + out.value(value.toString().replace('\\', '/')); + } + + public Path read(JsonReader in) throws IOException { + return Paths.get(in.nextString()); + } + }) + .create().toJson(problems); + } + protected final void assertZipEqualsDir(Path zip, Path expectedDir) throws IOException { try (var zipFile = new ZipFile(zip.toFile())) { var it = zipFile.entries().asIterator();