From 5816c519042fb1b9790cd2f00c07b0e13e2c9a0a Mon Sep 17 00:00:00 2001 From: Matyrobbrt <65940752+Matyrobbrt@users.noreply.github.com> Date: Wed, 26 Jun 2024 19:22:34 +0300 Subject: [PATCH] Fix inner class AT validation always reporting missing targets (#20) --- .../accesstransformers/AccessTransformersTransformer.java | 7 ++++++- .../neoforged/jst/accesstransformers/ApplyATsVisitor.java | 5 +++++ .../accesstransformer/inner_classes/accesstransformer.cfg | 3 +++ tests/data/accesstransformer/inner_classes/expected.log | 1 + 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/data/accesstransformer/inner_classes/expected.log 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 0309019..c70f4fb 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/AccessTransformersTransformer.java @@ -50,7 +50,12 @@ public void beforeRun(TransformContext context) { @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)); + 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); + }); 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 b18c847..7fef6cd 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java @@ -61,6 +61,11 @@ public void visitElement(@NotNull PsiElement element) { } apply(pendingATs.remove(new Target.ClassTarget(className)), psiClass, psiClass); + // We also remove any possible inner class ATs declared for that class as all class targets targeting inner classes + // generate a InnerClassTarget AT + if (psiClass.getParent() instanceof PsiClass parent) { + pendingATs.remove(new Target.InnerClassTarget(ClassUtil.getJVMClassName(parent), className)); + } var fieldWildcard = pendingATs.remove(new Target.WildcardFieldTarget(className)); if (fieldWildcard != null) { diff --git a/tests/data/accesstransformer/inner_classes/accesstransformer.cfg b/tests/data/accesstransformer/inner_classes/accesstransformer.cfg index 9e44386..3a0ce3d 100644 --- a/tests/data/accesstransformer/inner_classes/accesstransformer.cfg +++ b/tests/data/accesstransformer/inner_classes/accesstransformer.cfg @@ -1,2 +1,5 @@ public com.example.RootClass$InnerClass1 protected-f com.example.RootClass$InnerClass1$InnerClassNested + +# Inner class ATs with missing targets should only generate one log entry +public com.example.RootClass$InnerDoesntExist diff --git a/tests/data/accesstransformer/inner_classes/expected.log b/tests/data/accesstransformer/inner_classes/expected.log new file mode 100644 index 0000000..5f8bcfd --- /dev/null +++ b/tests/data/accesstransformer/inner_classes/expected.log @@ -0,0 +1 @@ +Access transformer PUBLIC LEAVE {atpath}:5, targeting com.example.RootClass INNERCLASS InnerDoesntExist did not apply as its target doesn't exist