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..ec87ce8 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java @@ -11,10 +11,14 @@ import picocli.CommandLine; import java.io.IOException; +import java.io.Reader; import java.io.UncheckedIOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; public class AccessTransformersTransformer implements SourceTransformer { @@ -25,8 +29,13 @@ public class AccessTransformersTransformer implements SourceTransformer { @CommandLine.Option(names = "--access-transformer-validation", description = "The level of validation to use for ats") public AccessTransformerValidation validation = AccessTransformerValidation.LOG; + // TODO: as is, either this or the normal option is required + @CommandLine.Option(names = "--access-transformer-no-validation") + public List atFilesWithoutValidation; + private AccessTransformerFiles ats; private Map pendingATs; + private Set sourcesWithoutValidation; private Logger logger; private volatile boolean errored; @@ -34,6 +43,7 @@ public class AccessTransformersTransformer implements SourceTransformer { public void beforeRun(TransformContext context) { ats = new AccessTransformerFiles(); logger = context.logger(); + sourcesWithoutValidation = new HashSet<>(); for (Path path : atFiles) { try { @@ -44,20 +54,44 @@ public void beforeRun(TransformContext context) { } } + for (Path path : atFilesWithoutValidation) { + var sourceName = path.toAbsolutePath().toString(); + try (Reader reader = Files.newBufferedReader(path)) { + ats.loadAT(reader, sourceName); + } catch (IOException ex) { + logger.error("Failed to parse access transformer file %s: %s", path, ex.getMessage()); + throw new UncheckedIOException(ex); + } + sourcesWithoutValidation.add(sourceName); + } + pendingATs = new ConcurrentHashMap<>(ats.getAccessTransformers()); } @Override public boolean afterRun(TransformContext context) { - if (!pendingATs.isEmpty()) { - pendingATs.forEach((target, transformation) -> { - // ClassTarget for inner classes have a corresponding InnerClassTarget which is more obvious for users - // 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); - }); + pendingATs.forEach((target, transformation) -> { + // Ignore targets that could not be applied, + // but are only referenced in AT files for which we don't want to perform any validation + boolean ignore = transformation.origins() + .stream() + .allMatch(origin -> { + var colonPos = origin.lastIndexOf(':'); + var sourceName = colonPos == -1 ? origin : origin.substring(0, colonPos); + return sourcesWithoutValidation.contains(sourceName); + }); + if (ignore) { + return; + } + // ClassTarget for inner classes have a corresponding InnerClassTarget which is more obvious for users + // 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); errored = true; - } + }); return !(errored && validation == AccessTransformerValidation.ERROR); } diff --git a/tests/data/accesstransformer/partially_invalid/accesstransformer-checked.cfg b/tests/data/accesstransformer/partially_invalid/accesstransformer-checked.cfg new file mode 100644 index 0000000..03fbbc4 --- /dev/null +++ b/tests/data/accesstransformer/partially_invalid/accesstransformer-checked.cfg @@ -0,0 +1 @@ +public C1 diff --git a/tests/data/accesstransformer/partially_invalid/accesstransformer-unchecked.cfg b/tests/data/accesstransformer/partially_invalid/accesstransformer-unchecked.cfg new file mode 100644 index 0000000..4ec0c33 --- /dev/null +++ b/tests/data/accesstransformer/partially_invalid/accesstransformer-unchecked.cfg @@ -0,0 +1 @@ +public Cdoesnotexist diff --git a/tests/data/accesstransformer/partially_invalid/expected/C1.java b/tests/data/accesstransformer/partially_invalid/expected/C1.java new file mode 100644 index 0000000..9784452 --- /dev/null +++ b/tests/data/accesstransformer/partially_invalid/expected/C1.java @@ -0,0 +1,3 @@ +public class C1 { + C1() {} +} diff --git a/tests/data/accesstransformer/partially_invalid/source/C1.java b/tests/data/accesstransformer/partially_invalid/source/C1.java new file mode 100644 index 0000000..44f95f7 --- /dev/null +++ b/tests/data/accesstransformer/partially_invalid/source/C1.java @@ -0,0 +1,2 @@ +class C1 { +} 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..ebb2669 100644 --- a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java +++ b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java @@ -297,6 +297,29 @@ void testIllegal() throws Exception { void testFolderClasspathEntries() throws Exception { runATTest("folder_classpath_entry", "--classpath=" + testDataRoot.resolve("accesstransformer/folder_classpath_entry/deps")); } + + @Test + void testPartialValidation() throws Exception { + var testDirName = "accesstransformer/partially_invalid"; + var checkedAtPath = testDataRoot.resolve(testDirName).resolve("accesstransformer-checked.cfg"); + var uncheckedAtPath = testDataRoot.resolve(testDirName).resolve("accesstransformer-unchecked.cfg"); + runTest(testDirName, txt -> txt, + "--enable-accesstransformers", + "--access-transformer", checkedAtPath.toString(), + "--access-transformer-no-validation", uncheckedAtPath.toString(), + "--access-transformer-validation=error" + ); + + // Swapping which file is validated should throw: + assertThrows(RuntimeException.class, () -> { + runTest(testDirName, txt -> txt, + "--enable-accesstransformers", + "--access-transformer", uncheckedAtPath.toString(), + "--access-transformer-no-validation", checkedAtPath.toString(), + "--access-transformer-validation=error" + ); + }); + } } @Nested