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 3fc6748..93f9269 100644 --- a/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java +++ b/accesstransformers/src/main/java/net/neoforged/jst/accesstransformers/ApplyATsVisitor.java @@ -14,6 +14,7 @@ import com.intellij.psi.PsiRecursiveElementVisitor; import com.intellij.psi.PsiWhiteSpace; import com.intellij.psi.util.ClassUtil; +import java.util.HashSet; import net.neoforged.accesstransformer.parser.AccessTransformerFiles; import net.neoforged.accesstransformer.parser.Target; import net.neoforged.accesstransformer.parser.Transformation; @@ -51,6 +52,7 @@ class ApplyATsVisitor extends PsiRecursiveElementVisitor { private final Map pendingATs; private final Logger logger; private final ProblemReporter problemReporter; + private final Set allTargetedClasses = new HashSet<>();; boolean errored = false; public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements, Map pendingATs, Logger logger, ProblemReporter problemReporter) { @@ -59,68 +61,95 @@ public ApplyATsVisitor(AccessTransformerFiles ats, Replacements replacements, Ma this.logger = logger; this.pendingATs = pendingATs; this.problemReporter = problemReporter; + + // To skip any class that has no access transformer targeting any part of it (incl. inner classes or anonymous classes), + // take the set of targeted classes and create a new set that consists of those classes and all of their parent classes + // (Example: "example.C$Inner$1" results in "example.C", "example.C$Inner", and "example.C$Inner$1") + // + // This means we may have to iterate over elements that might not be targeted at all. (In the previous example, that'd + // be methods and fields in "example.C" and "example.C$Inner" if the only target is "example.C$Inner$1") + // However, that is the price we pay to ensure any targeted class is actually visited, even anonymous ones. + for (String target : ats.getTargets()) { + int lastIndex = target.length(); + do { + target = target.substring(0, lastIndex); + allTargetedClasses.add(target); + lastIndex = target.lastIndexOf('$'); + } while (lastIndex != -1); + } } @Override public void visitElement(@NotNull PsiElement element) { if (element instanceof PsiClass psiClass) { - if (psiClass.getQualifiedName() != null) { - String className = ClassUtil.getJVMClassName(psiClass); - if (!ats.containsClassTarget(className)) { - // Skip this class and its children, but not the inner classes - for (PsiClass innerClass : psiClass.getInnerClasses()) { - visitElement(innerClass); - } - return; + String className = getJVMClassName(psiClass); + if (!allTargetedClasses.contains(className)) { + // Skip this class and its children, but not the inner classes + for (PsiClass innerClass : psiClass.getInnerClasses()) { + visitElement(innerClass); } + return; + } - var classAt = pendingATs.remove(new Target.ClassTarget(className)); - apply(classAt, 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 classAt = pendingATs.remove(new Target.ClassTarget(className)); + apply(classAt, 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(getJVMClassName(parent), className)); + } - checkImplicitConstructor(psiClass, className, classAt); + checkImplicitConstructor(psiClass, className, classAt); - 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 - 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 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 + 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 = 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 - 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); - } + 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 + 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); } } } else if (element instanceof PsiField field) { final var cls = field.getContainingClass(); if (cls != null && cls.getQualifiedName() != null) { - String className = ClassUtil.getJVMClassName(cls); + String className = getJVMClassName(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(pendingATs.remove(method(className, method)), method, cls); + if (cls != null) { + var className = getJVMClassName(cls); + if (!className.isEmpty()) { + apply(pendingATs.remove(method(className, method)), method, cls); + } } } element.acceptChildren(this); } + // This returns a JVM class name like ClassUtil#getJVMClassName, but accounts for anonymous classes + private String getJVMClassName(PsiClass aClass) { + // Quick-path + final String qName = ClassUtil.getJVMClassName(aClass); + if (qName != null) return qName; + + var sb = new StringBuilder(); + PsiHelper.getBinaryClassName(aClass, sb); // This returns a binary class name with '/'s; convert it to a JVM class name with '.'s + return sb.toString().replace('/', '.'); + } + private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiClass containingClass) { if (at == null) return; if (!at.isValid()) { @@ -132,7 +161,7 @@ private void apply(@Nullable Transformation at, PsiModifierListOwner owner, PsiC @Override public String toString() { if (owner instanceof PsiClass cls) { - return ClassUtil.getJVMClassName(cls); + return getJVMClassName(cls); } String memberName; if (owner instanceof PsiMethod mtd && mtd.isConstructor()) { @@ -140,7 +169,7 @@ public String toString() { } else { memberName = ((NavigationItem) owner).getName(); } - return memberName + " of " + ClassUtil.getJVMClassName(containingClass); + return memberName + " of " + getJVMClassName(containingClass); } }; logger.debug("Applying AT %s to %s", at, targetInfo); diff --git a/tests/data/accesstransformer/anonymous_classes/accesstransformer.cfg b/tests/data/accesstransformer/anonymous_classes/accesstransformer.cfg new file mode 100644 index 0000000..677b740 --- /dev/null +++ b/tests/data/accesstransformer/anonymous_classes/accesstransformer.cfg @@ -0,0 +1,2 @@ +public example.C$1 method()V +public example.C$2 *() diff --git a/tests/data/accesstransformer/anonymous_classes/expected/example/C.java b/tests/data/accesstransformer/anonymous_classes/expected/example/C.java new file mode 100644 index 0000000..32e829a --- /dev/null +++ b/tests/data/accesstransformer/anonymous_classes/expected/example/C.java @@ -0,0 +1,25 @@ +package example; + +public class C { + static C REGULAR = new C() { + @Override + public void method() { + } + }; + + static C WILDCARD = new C() { + @Override + public void method() { + } + + @Override + public void method2() { + } + }; + + protected void method() { + } + + protected void method2() { + } +} diff --git a/tests/data/accesstransformer/anonymous_classes/expected_report.json b/tests/data/accesstransformer/anonymous_classes/expected_report.json new file mode 100644 index 0000000..0637a08 --- /dev/null +++ b/tests/data/accesstransformer/anonymous_classes/expected_report.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/tests/data/accesstransformer/anonymous_classes/source/example/C.java b/tests/data/accesstransformer/anonymous_classes/source/example/C.java new file mode 100644 index 0000000..244ddcc --- /dev/null +++ b/tests/data/accesstransformer/anonymous_classes/source/example/C.java @@ -0,0 +1,25 @@ +package example; + +public class C { + static C REGULAR = new C() { + @Override + protected void method() { + } + }; + + static C WILDCARD = new C() { + @Override + protected void method() { + } + + @Override + protected void method2() { + } + }; + + protected void method() { + } + + protected void method2() { + } +} 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 57515a6..785dc82 100644 --- a/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java +++ b/tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java @@ -308,6 +308,11 @@ void testIllegal() throws Exception { void testFolderClasspathEntries() throws Exception { runATTest("folder_classpath_entry", "--classpath=" + testDataRoot.resolve("accesstransformer/folder_classpath_entry/deps")); } + + @Test + void testAnonymousClasses() throws Exception { + runATTest("anonymous_classes"); + } } @Nested