diff --git a/README.md b/README.md index 3c7d683..4874439 100644 --- a/README.md +++ b/README.md @@ -28,11 +28,16 @@ It can be invoked as a standalone executable Jar-File. Java 17 is required. ``` Usage: jst [-hV] [--in-format=] [--libraries-list=] - [--max-queue-depth=] [--out-format=] [--enable-parchment - --parchment-mappings= [--parchment-javadoc]] INPUT OUTPUT + [--max-queue-depth=] [--out-format=] + [--classpath=]... [--enable-parchment + --parchment-mappings= [--parchment-javadoc]] [--enable-accesstransformers + --access-transformer= [--access-transformer=]... + [--access-transformer-validation=]] INPUT OUTPUT INPUT Path to a single Java-file, a source-archive or a folder containing the source to transform. OUTPUT Path to where the resulting source should be placed. + --classpath= + Additional classpath entries to use. Is combined with --libraries-list. -h, --help Show this help message and exit. --in-format= Specify the format of INPUT explicitly. AUTO (the default) performs @@ -53,6 +58,14 @@ Plugin - parchment --enable-parchment Enable parchment --parchment-javadoc --parchment-mappings= + +Plugin - accesstransformers + --access-transformer= + + --access-transformer-validation= + The level of validation to use for ats + --enable-accesstransformers + Enable accesstransformers ``` ## Licenses diff --git a/accesstransformers/build.gradle b/accesstransformers/build.gradle index b3d29f9..d578bcf 100644 --- a/accesstransformers/build.gradle +++ b/accesstransformers/build.gradle @@ -4,7 +4,7 @@ plugins { dependencies { implementation project(":api") - implementation 'net.neoforged.accesstransformers:at-parser:11.0.0' + implementation 'net.neoforged.accesstransformers:at-parser:11.0.4' testImplementation platform("org.junit:junit-bom:$junit_version") testImplementation 'org.junit.jupiter:junit-jupiter' 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 0254ced..0309019 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java @@ -2,6 +2,9 @@ import com.intellij.psi.PsiFile; 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.Replacements; import net.neoforged.jst.api.SourceTransformer; import net.neoforged.jst.api.TransformContext; @@ -11,27 +14,60 @@ import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; public class AccessTransformersTransformer implements SourceTransformer { @CommandLine.Option(names = "--access-transformer", required = true) public List atFiles; + @CommandLine.Option(names = "--access-transformer-validation", description = "The level of validation to use for ats") + public AccessTransformerValidation validation = AccessTransformerValidation.LOG; + private AccessTransformerFiles ats; + private Map pendingATs; + private Logger logger; + private volatile boolean errored; + @Override public void beforeRun(TransformContext context) { ats = new AccessTransformerFiles(); - try { - for (Path path : atFiles) { + logger = context.logger(); + + 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 (IOException ex) { - throw new UncheckedIOException(ex); } + + pendingATs = new ConcurrentHashMap<>(ats.getAccessTransformers()); + } + + @Override + public boolean afterRun(TransformContext context) { + if (!pendingATs.isEmpty()) { + pendingATs.forEach((target, transformation) -> logger.error("Access transformer %s, targeting %s did not apply as its target doesn't exist", transformation, target)); + errored = true; + } + + return !(errored && validation == AccessTransformerValidation.ERROR); } @Override public void visitFile(PsiFile psiFile, Replacements replacements) { - new ApplyATsVisitor(ats, replacements).visitFile(psiFile); + var visitor = new ApplyATsVisitor(ats, replacements, pendingATs, logger); + visitor.visitFile(psiFile); + if (visitor.errored) { + errored = true; + } + } + + public enum AccessTransformerValidation { + LOG, + ERROR } } 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 34805a3..b18c847 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java @@ -1,5 +1,6 @@ package net.neoforged.jst.accesstransformers; +import com.intellij.navigation.NavigationItem; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiField; @@ -13,6 +14,7 @@ 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 org.jetbrains.annotations.NotNull; @@ -20,7 +22,6 @@ import java.util.Arrays; import java.util.EnumMap; -import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -35,12 +36,15 @@ class ApplyATsVisitor extends PsiRecursiveElementVisitor { private final AccessTransformerFiles ats; private final Replacements replacements; - private final Map atCopy; + private final Map pendingATs; + private final Logger logger; + boolean errored = false; - public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements) { + public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements, Map pendingATs, Logger logger) { this.ats = ats; this.replacements = replacements; - atCopy = new HashMap<>(ats.getAccessTransformers()); + this.logger = logger; + this.pendingATs = pendingATs; } @Override @@ -56,20 +60,25 @@ public void visitElement(@NotNull PsiElement element) { return; } - apply(atCopy.get(new Target.ClassTarget(className)), psiClass, psiClass); - var fieldWildcard = atCopy.get(new Target.WildcardFieldTarget(className)); + apply(pendingATs.remove(new Target.ClassTarget(className)), psiClass, psiClass); + + var fieldWildcard = pendingATs.remove(new Target.WildcardFieldTarget(className)); if (fieldWildcard != null) { for (PsiField field : psiClass.getFields()) { // Apply a merged state if an explicit AT for the field already exists - apply(merge(fieldWildcard, atCopy.remove(new Target.FieldTarget(className, field.getName()))), field, psiClass); + var newState = merge(fieldWildcard, pendingATs.remove(new Target.FieldTarget(className, field.getName()))); + logger.debug("Applying field wildcard AT %s to %s in %s", newState, field.getName(), className); + apply(newState, field, psiClass); } } - var methodWildcard = atCopy.get(new Target.WildcardMethodTarget(className)); + var methodWildcard = pendingATs.remove(new Target.WildcardMethodTarget(className)); if (methodWildcard != null) { for (PsiMethod method : psiClass.getMethods()) { // Apply a merged state if an explicit AT for the method already exists - apply(merge(methodWildcard, atCopy.remove(method(className, method))), method, psiClass); + var newState = merge(methodWildcard, pendingATs.remove(method(className, method))); + logger.debug("Applying method wildcard AT %s to %s in %s", newState, method.getName(), className); + apply(newState, method, psiClass); } } } @@ -77,22 +86,37 @@ public void visitElement(@NotNull PsiElement element) { final var cls = field.getContainingClass(); if (cls != null && cls.getQualifiedName() != null) { String className = ClassUtil.getJVMClassName(cls); - apply(atCopy.get(new Target.FieldTarget(className, field.getName())), field, cls); + apply(pendingATs.remove(new Target.FieldTarget(className, field.getName())), field, cls); } } else if (element instanceof PsiMethod method) { final var cls = method.getContainingClass(); if (cls != null && cls.getQualifiedName() != null) { String className = ClassUtil.getJVMClassName(cls); - apply(atCopy.get(method(className, method)), method, cls); + apply(pendingATs.remove(method(className, method)), method, cls); } } element.acceptChildren(this); } - // TODO - proper logging when an AT can't be applied private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiClass containingClass) { - if (at == null || !at.isValid()) return; + if (at == null) return; + if (!at.isValid()) { + error("Found invalid access transformer: %s. Final state: conflicting", at); + return; + } + + var targetInfo = new Object() { + @Override + public String toString() { + if (owner instanceof PsiClass cls) { + return ClassUtil.getJVMClassName(cls); + } + return ((NavigationItem) owner).getName() + " of " + ClassUtil.getJVMClassName(containingClass); + } + }; + logger.debug("Applying AT %s to %s", at, targetInfo); + var modifiers = owner.getModifierList(); var targetAcc = at.modifier(); @@ -100,7 +124,7 @@ private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiC // 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) { - System.err.println("Non-static interface methods can only be made 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); } else { for (var kw : modifiers.getChildren()) { if (kw instanceof PsiKeyword && kw.getText().equals(PsiKeyword.PRIVATE)) { // Strip private, replace it with default @@ -125,6 +149,8 @@ private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiC .filter(kw -> kw.getText().equals(PsiModifier.FINAL)) .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); } } @@ -180,6 +206,11 @@ private void modify(Transformation.Modifier targetAcc, PsiModifierList modifiers } } + private void error(String message, Object... args) { + logger.error(message, args); + errored = true; + } + private static String detectKind(PsiClass cls) { if (cls.isRecord()) { return "record"; diff --git a/api/src/main/java/net/neoforged/jst/api/Logger.java b/api/src/main/java/net/neoforged/jst/api/Logger.java new file mode 100644 index 0000000..87c10ac --- /dev/null +++ b/api/src/main/java/net/neoforged/jst/api/Logger.java @@ -0,0 +1,28 @@ +package net.neoforged.jst.api; + +import org.jetbrains.annotations.Nullable; + +import java.io.PrintStream; +import java.util.Locale; + +public class Logger { + private final PrintStream debugOut; + private final PrintStream errorOut; + + public Logger(@Nullable PrintStream debugOut, @Nullable PrintStream errorOut) { + this.debugOut = debugOut; + this.errorOut = errorOut; + } + + public void error(String message, Object... args) { + if (errorOut != null) { + errorOut.printf(Locale.ROOT, message + "\n", args); + } + } + + public void debug(String message, Object... args) { + if (debugOut != null) { + debugOut.printf(Locale.ROOT, message + "\n", args); + } + } +} diff --git a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java index 9f4835f..984beb2 100644 --- a/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java +++ b/api/src/main/java/net/neoforged/jst/api/SourceTransformer.java @@ -6,7 +6,8 @@ public interface SourceTransformer { default void beforeRun(TransformContext context) { } - default void afterRun(TransformContext context) { + default boolean afterRun(TransformContext context) { + return true; } void visitFile(PsiFile psiFile, Replacements replacements); 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 a1963a9..890e0fa 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,4 @@ package net.neoforged.jst.api; -public record TransformContext(IntelliJEnvironment environment, FileSource source, FileSink sink) { +public record TransformContext(IntelliJEnvironment environment, FileSource source, FileSink sink, Logger logger) { } diff --git a/cli/build.gradle b/cli/build.gradle index 0900153..cd5ab50 100644 --- a/cli/build.gradle +++ b/cli/build.gradle @@ -20,6 +20,7 @@ dependencies { implementation "info.picocli:picocli:$picocli_version" implementation project(":parchment") implementation project(":accesstransformers") + implementation 'org.slf4j:slf4j-simple:2.0.13' testImplementation platform("org.junit:junit-bom:$junit_version") testImplementation 'org.junit.jupiter:junit-jupiter' 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 cef1d0c..3a41047 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/Main.java +++ b/cli/src/main/java/net/neoforged/jst/cli/Main.java @@ -1,5 +1,6 @@ package net.neoforged.jst.cli; +import net.neoforged.jst.api.Logger; import net.neoforged.jst.api.SourceTransformer; import net.neoforged.jst.api.SourceTransformerPlugin; import net.neoforged.jst.cli.io.FileSinks; @@ -37,6 +38,9 @@ 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") + boolean debug = false; + private final HashSet enabledTransformers = new HashSet<>(); public static void main(String[] args) { @@ -59,9 +63,9 @@ public static int innerMain(String... args) { @Override 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()) { + var processor = new SourceFileProcessor(logger)) { if (librariesList != null) { processor.addLibrariesList(librariesList); @@ -75,7 +79,10 @@ public Integer call() throws Exception { var orderedTransformers = new ArrayList<>(enabledTransformers); try (var sink = FileSinks.create(outputPath, outputFormat, source)) { - processor.process(source, sink, orderedTransformers); + if (!processor.process(source, sink, orderedTransformers)) { + logger.error("Transformation failed"); + return 1; + } } } 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 1d338f4..b3c97c3 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java +++ b/cli/src/main/java/net/neoforged/jst/cli/SourceFileProcessor.java @@ -5,6 +5,7 @@ import net.neoforged.jst.api.FileEntry; import net.neoforged.jst.api.FileSink; import net.neoforged.jst.api.FileSource; +import net.neoforged.jst.api.Logger; import net.neoforged.jst.api.Replacements; import net.neoforged.jst.api.SourceTransformer; import net.neoforged.jst.api.TransformContext; @@ -24,19 +25,22 @@ * https://github.com/JetBrains/kotlin/blob/22aa9ee65f759ad21aeaeb8ad9ac0b123b2c32fe/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt#L108 */ class SourceFileProcessor implements AutoCloseable { - private final IntelliJEnvironmentImpl ijEnv = new IntelliJEnvironmentImpl(); + private final IntelliJEnvironmentImpl ijEnv; private int maxQueueDepth = 50; + private final Logger logger; - public SourceFileProcessor() throws IOException { + public SourceFileProcessor(Logger logger) throws IOException { + this.logger = logger; + ijEnv = new IntelliJEnvironmentImpl(logger); ijEnv.addCurrentJdkToClassPath(); } - public void process(FileSource source, FileSink sink, List transformers) throws IOException { + public boolean process(FileSource source, FileSink sink, List transformers) throws IOException { if (source.canHaveMultipleEntries() && !sink.canHaveMultipleEntries()) { 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); + var context = new TransformContext(ijEnv, source, sink, logger); var sourceRoot = source.createSourceRoot(VirtualFileManager.getInstance()); ijEnv.addSourceRoot(sourceRoot); @@ -68,9 +72,12 @@ public void process(FileSource source, FileSink sink, List tr } } + boolean isOk = true; for (var transformer : transformers) { - transformer.afterRun(context); + isOk = isOk && transformer.afterRun(context); } + + return isOk; } private void processEntry(FileEntry entry, VirtualFile sourceRoot, List transformers, FileSink sink) throws IOException { @@ -131,11 +138,11 @@ public void setMaxQueueDepth(int maxQueueDepth) { } public void addLibrariesList(Path librariesList) throws IOException { - ClasspathSetup.addLibraries(librariesList, ijEnv); + ClasspathSetup.addLibraries(logger, librariesList, ijEnv); } public void addLibrary(Path library) { - ClasspathSetup.addLibrary(library, ijEnv); + ClasspathSetup.addLibrary(logger, library, ijEnv); } @Override diff --git a/cli/src/main/java/net/neoforged/jst/cli/intellij/ClasspathSetup.java b/cli/src/main/java/net/neoforged/jst/cli/intellij/ClasspathSetup.java index 111c492..ad19d4c 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/intellij/ClasspathSetup.java +++ b/cli/src/main/java/net/neoforged/jst/cli/intellij/ClasspathSetup.java @@ -4,6 +4,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.io.URLUtil; +import net.neoforged.jst.api.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,7 +22,7 @@ public final class ClasspathSetup { private ClasspathSetup() { } - public static void addJdkModules(Path jdkHome, JavaCoreProjectEnvironment javaEnv) { + public static void addJdkModules(Logger logger, Path jdkHome, JavaCoreProjectEnvironment javaEnv) { var jrtFileSystem = javaEnv.getEnvironment().getJrtFileSystem(); if (jrtFileSystem == null) { throw new IllegalStateException("No JRT file system was configured"); @@ -29,13 +30,13 @@ public static void addJdkModules(Path jdkHome, JavaCoreProjectEnvironment javaEn VirtualFile jdkVfsRoot = jrtFileSystem.findFileByPath(jdkHome.toAbsolutePath() + URLUtil.JAR_SEPARATOR); if (jdkVfsRoot == null) { - System.err.println("Failed to load VFS-entry for JDK home " + jdkHome + ". Is it missing?"); + logger.error("Failed to load VFS-entry for JDK home " + jdkHome + ". Is it missing?"); return; } var modulesFolder = jdkVfsRoot.findChild("modules"); if (modulesFolder == null) { - System.err.println("VFS for JDK " + jdkHome + " doesn't have a modules subfolder"); + logger.error("VFS for JDK " + jdkHome + " doesn't have a modules subfolder"); return; } @@ -45,7 +46,7 @@ public static void addJdkModules(Path jdkHome, JavaCoreProjectEnvironment javaEn for (String module : modules) { var moduleRoot = modulesFolder.findChild(module); if (moduleRoot == null || !moduleRoot.isDirectory()) { - System.err.println("Couldn't find module " + module + " even though it was listed in the release file of JDK " + jdkHome); + logger.error("Couldn't find module " + module + " even though it was listed in the release file of JDK " + jdkHome); } else { javaEnv.addSourcesToClasspath(moduleRoot); moduleCount++; @@ -61,10 +62,10 @@ public static void addJdkModules(Path jdkHome, JavaCoreProjectEnvironment javaEn } } - System.out.println("Added " + moduleCount + " modules from " + jdkHome); + logger.debug("Added %s modules from %s", moduleCount, jdkHome); } - public static void addLibraries(Path librariesPath, IntelliJEnvironmentImpl ijEnv) throws IOException { + public static void addLibraries(Logger logger, Path librariesPath, IntelliJEnvironmentImpl ijEnv) throws IOException { for (String libraryLine : Files.readAllLines(librariesPath)) { libraryLine = libraryLine.trim(); @@ -77,7 +78,7 @@ public static void addLibraries(Path librariesPath, IntelliJEnvironmentImpl ijEn continue; } - addLibrary(Paths.get(libraryLine), ijEnv); + addLibrary(logger, Paths.get(libraryLine), ijEnv); } } @@ -98,12 +99,12 @@ public static void addLibraries(Path librariesPath, IntelliJEnvironmentImpl ijEn return null; } - public static void addLibrary(Path libraryPath, IntelliJEnvironmentImpl ijEnv) { + public static void addLibrary(Logger logger, Path libraryPath, IntelliJEnvironmentImpl ijEnv) { // Add an explicit check since PSI doesn't throw if it doesn't exist if (!Files.exists(libraryPath)) { throw new UncheckedIOException(new NoSuchFileException(libraryPath.toString())); } ijEnv.addJarToClassPath(libraryPath); - System.out.println("Added " + libraryPath); + logger.debug("Added %s", libraryPath); } } diff --git a/cli/src/main/java/net/neoforged/jst/cli/intellij/IntelliJEnvironmentImpl.java b/cli/src/main/java/net/neoforged/jst/cli/intellij/IntelliJEnvironmentImpl.java index bccce97..ea3f479 100644 --- a/cli/src/main/java/net/neoforged/jst/cli/intellij/IntelliJEnvironmentImpl.java +++ b/cli/src/main/java/net/neoforged/jst/cli/intellij/IntelliJEnvironmentImpl.java @@ -33,6 +33,7 @@ import com.intellij.psi.impl.source.tree.TreeGenerator; import com.intellij.psi.util.JavaClassSupers; import net.neoforged.jst.api.IntelliJEnvironment; +import net.neoforged.jst.api.Logger; import org.jetbrains.annotations.VisibleForTesting; import java.io.IOException; @@ -43,13 +44,15 @@ public class IntelliJEnvironmentImpl implements IntelliJEnvironment, AutoCloseable { + private final Logger logger; private final Disposable rootDisposable; private final Path tempDir; private final MockProject project; private final JavaCoreProjectEnvironment javaEnv; private final PsiManager psiManager; - public IntelliJEnvironmentImpl() throws IOException { + public IntelliJEnvironmentImpl(Logger logger) throws IOException { + this.logger = logger; System.setProperty("java.awt.headless", "true"); tempDir = Files.createTempDirectory("jst"); @@ -111,7 +114,7 @@ public void addSourceRoot(VirtualFile sourceRoot) { public void addCurrentJdkToClassPath() { // Add the Java Runtime we are currently running in var javaHome = Paths.get(System.getProperty("java.home")); - ClasspathSetup.addJdkModules(javaHome, javaEnv); + ClasspathSetup.addJdkModules(logger, javaHome, javaEnv); } @Override diff --git a/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java b/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java index 3088f10..d3f508d 100644 --- a/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java +++ b/cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java @@ -4,6 +4,7 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiMethod; import com.intellij.psi.util.PsiTreeUtil; +import net.neoforged.jst.api.Logger; import net.neoforged.jst.api.PsiHelper; import net.neoforged.jst.cli.intellij.IntelliJEnvironmentImpl; import org.intellij.lang.annotations.Language; @@ -22,7 +23,7 @@ class PsiHelperTest { @BeforeAll static void setUp() throws IOException { - ijEnv = new IntelliJEnvironmentImpl(); + ijEnv = new IntelliJEnvironmentImpl(new Logger(null, null)); } @AfterAll diff --git a/settings.gradle b/settings.gradle index 6d9bf66..eb1ac12 100644 --- a/settings.gradle +++ b/settings.gradle @@ -28,6 +28,18 @@ dependencyResolutionManagement { url "https://maven.neoforged.net/releases/" } } + repositories { + maven { + name 'Maven for PR #10' // https://github.com/neoforged/AccessTransformers/pull/10 + url 'https://prmaven.neoforged.net/AccessTransformers/pr10' + content { + includeModule('net.neoforged.accesstransformers', 'at-modlauncher') + includeModule('net.neoforged.accesstransformers', 'at-parser') + includeModule('net.neoforged.accesstransformers', 'at-cli') + includeModule('net.neoforged', 'accesstransformers') + } + } + } } rootProject.name = 'JavaSourceTransformer' diff --git a/tests/data/accesstransformer/interfaces/accesstransformer.cfg b/tests/data/accesstransformer/interfaces/accesstransformer.cfg index 20c2704..77694b6 100644 --- a/tests/data/accesstransformer/interfaces/accesstransformer.cfg +++ b/tests/data/accesstransformer/interfaces/accesstransformer.cfg @@ -1 +1,2 @@ public If1 callThingy(I)V # Make sure that private interface methods become default when made public +protected If1 callThingy2()V # Make sure that it's an error diff --git a/tests/data/accesstransformer/interfaces/expected.log b/tests/data/accesstransformer/interfaces/expected.log new file mode 100644 index 0000000..d2ad1dd --- /dev/null +++ b/tests/data/accesstransformer/interfaces/expected.log @@ -0,0 +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. diff --git a/tests/data/accesstransformer/interfaces/expected/If1.java b/tests/data/accesstransformer/interfaces/expected/If1.java index 4930849..b8ec7b7 100644 --- a/tests/data/accesstransformer/interfaces/expected/If1.java +++ b/tests/data/accesstransformer/interfaces/expected/If1.java @@ -2,4 +2,6 @@ public interface If1 { default void callThingy(int i) { } + + private void callThingy2() {} } diff --git a/tests/data/accesstransformer/interfaces/source/If1.java b/tests/data/accesstransformer/interfaces/source/If1.java index 6cbc80f..fbba4b1 100644 --- a/tests/data/accesstransformer/interfaces/source/If1.java +++ b/tests/data/accesstransformer/interfaces/source/If1.java @@ -2,4 +2,6 @@ public interface If1 { private void callThingy(int i) { } + + private void callThingy2() {} } diff --git a/tests/data/accesstransformer/missing_target/accesstransformer.cfg b/tests/data/accesstransformer/missing_target/accesstransformer.cfg new file mode 100644 index 0000000..e01f675 --- /dev/null +++ b/tests/data/accesstransformer/missing_target/accesstransformer.cfg @@ -0,0 +1,5 @@ +public ExistingClass + +public ExistingClass notAField +public ExistingClass notAMethod()V +public DoesntExist diff --git a/tests/data/accesstransformer/missing_target/expected.log b/tests/data/accesstransformer/missing_target/expected.log new file mode 100644 index 0000000..0074d70 --- /dev/null +++ b/tests/data/accesstransformer/missing_target/expected.log @@ -0,0 +1,3 @@ +Access transformer PUBLIC LEAVE {atpath}:3, targeting ExistingClass FIELD notAField did not apply as its target doesn't exist +Access transformer PUBLIC LEAVE {atpath}:5, targeting DoesntExist CLASS did not apply as its target doesn't exist +Access transformer PUBLIC LEAVE {atpath}:4, targeting ExistingClass METHOD notAMethod()V did not apply as its target doesn't exist diff --git a/tests/data/accesstransformer/missing_target/expected/ExistingClass.java b/tests/data/accesstransformer/missing_target/expected/ExistingClass.java new file mode 100644 index 0000000..be68e15 --- /dev/null +++ b/tests/data/accesstransformer/missing_target/expected/ExistingClass.java @@ -0,0 +1 @@ +public class ExistingClass {} diff --git a/tests/data/accesstransformer/missing_target/source/ExistingClass.java b/tests/data/accesstransformer/missing_target/source/ExistingClass.java new file mode 100644 index 0000000..6913def --- /dev/null +++ b/tests/data/accesstransformer/missing_target/source/ExistingClass.java @@ -0,0 +1 @@ +class ExistingClass {} 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 c728640..817fc95 100644 --- a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java +++ b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java @@ -1,6 +1,7 @@ package net.neoforged.jst.tests; import net.neoforged.jst.cli.Main; +import org.assertj.core.util.CanIgnoreReturnValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -23,6 +24,7 @@ import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.zip.ZipEntry; @@ -264,19 +266,25 @@ void testNoModifiers() throws Exception { void testWildcardAndExplicit() throws Exception { runATTest("wildcard_and_explicit"); } + + @Test + void testMissingTarget() throws Exception { + runATTest("missing_target"); + } } protected final void runATTest(String testDirName) throws Exception { testDirName = "accesstransformer/" + testDirName; - runTest(testDirName, "--enable-accesstransformers", "--access-transformer", testDataRoot.resolve(testDirName).resolve("accesstransformer.cfg").toString()); + var atPath = testDataRoot.resolve(testDirName).resolve("accesstransformer.cfg"); + runTest(testDirName, txt -> txt.replace(atPath.toAbsolutePath().toString(), "{atpath}"), "--enable-accesstransformers", "--access-transformer", atPath.toString()); } protected final void runParchmentTest(String testDirName, String mappingsFilename) throws Exception { testDirName = "parchment/" + testDirName; - runTest(testDirName, "--enable-parchment", "--parchment-mappings", testDataRoot.resolve(testDirName).resolve(mappingsFilename).toString()); + runTest(testDirName, UnaryOperator.identity(), "--enable-parchment", "--parchment-mappings", testDataRoot.resolve(testDirName).resolve(mappingsFilename).toString()); } - protected final void runTest(String testDirName, String... args) throws Exception { + protected final void runTest(String testDirName, UnaryOperator consoleMapper, String... args) throws Exception { var testDir = testDataRoot.resolve(testDirName); var sourceDir = testDir.resolve("source"); var expectedDir = testDir.resolve("expected"); @@ -299,7 +307,7 @@ protected final void runTest(String testDirName, String... args) throws Exceptio arguments.addAll(Arrays.asList(args)); arguments.add(inputFile.toString()); arguments.add(outputFile.toString()); - runTool(arguments.toArray(String[]::new)); + var consoleOut = consoleMapper.apply(runTool(arguments.toArray(String[]::new))); try (var zipFile = new ZipFile(outputFile.toFile())) { var it = zipFile.entries().asIterator(); @@ -314,9 +322,15 @@ protected final void runTest(String testDirName, String... args) throws Exceptio assertEquals(expectedFile, actualFile); } } + + var expectedLog = testDir.resolve("expected.log"); + if (Files.exists(expectedLog)) { + assertThat(expectedLog).content().isEqualToNormalizingNewlines(consoleOut); + } } - protected void runTool(String... args) throws Exception { + @CanIgnoreReturnValue + protected String runTool(String... args) throws Exception { // This is thread hostile, but what can I do :-[ var oldOut = System.out; var oldErr = System.err; @@ -337,6 +351,8 @@ protected void runTool(String... args) throws Exception { if (exitCode != 0) { throw new RuntimeException("Process failed with exit code 0: " + capturedOutString); } + + return capturedOutString; } protected static String getRequiredSystemProperty(String key) { diff --git a/tests/src/test/java/net/neoforged/jst/tests/ExecutableJarTest.java b/tests/src/test/java/net/neoforged/jst/tests/ExecutableJarTest.java index cb53fce..2d27a64 100644 --- a/tests/src/test/java/net/neoforged/jst/tests/ExecutableJarTest.java +++ b/tests/src/test/java/net/neoforged/jst/tests/ExecutableJarTest.java @@ -10,7 +10,7 @@ */ public class ExecutableJarTest extends EmbeddedTest { @Override - protected void runTool(String... args) throws Exception { + protected String runTool(String... args) throws Exception { var javaExecutablePath = ProcessHandle.current() .info() .command() @@ -28,12 +28,13 @@ protected void runTool(String... args) throws Exception { process.getOutputStream().close(); // Close stdin to java - byte[] output = process.getInputStream().readAllBytes(); - System.out.println(new String(output)); + String output = new String(process.getInputStream().readAllBytes()); int exitCode = process.waitFor(); if (exitCode != 0) { - throw new RuntimeException(new String(output)); + throw new RuntimeException(output); } + + return output; } }