This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-7439 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 1c21cf936b34899f513fd311995f9af1c9a4499d Author: Eric Milles <[email protected]> AuthorDate: Fri May 9 09:56:07 2025 -0500 GROOVY-7439: allow logging transform on trait class --- .../ASTTransformationCollectorCodeVisitor.java | 5 ++++- .../groovy/transform/ASTTransformationVisitor.java | 14 +++++++------- .../groovy/transform/BuilderASTTransformation.java | 22 +++++++++++----------- .../ExternalizeVerifierASTTransformation.java | 7 ++++--- .../groovy/transform/LogASTTransformation.java | 7 ++++++- .../transform/SortableASTTransformation.java | 2 +- .../transform/trait/TraitASTTransformation.java | 6 +++--- .../groovy/groovy/util/logging/Slf4jTest.groovy | 2 -- 8 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java index 7c0a26279f..9df19d8e3b 100644 --- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java @@ -280,21 +280,24 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo if (!ASTTransformation.class.isAssignableFrom(transformClass)) { String error = "Not an ASTTransformation: " + transformClass.getName() + " declared by " + annotation.getClassNode().getName(); source.getErrorCollector().addError(new SimpleMessage(error, source)); + return; } GroovyASTTransformation transformationClass = transformClass.getAnnotation(GroovyASTTransformation.class); if (transformationClass == null) { String error = "AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + transformClass.getName() + " lacks this annotation."; source.getErrorCollector().addError(new SimpleMessage(error, source)); + return; } CompilePhase specifiedCompilePhase = transformationClass.phase(); if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) { String error = annotation.getClassNode().getName() + " is defined to be run in compile phase " + specifiedCompilePhase + ". Local AST transformations must run in SEMANTIC_ANALYSIS or later!"; source.getErrorCollector().addError(new SimpleMessage(error, source)); + return; } - if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class || transformClass == SealedASTTransformation.class) { + if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class || transformClass == SealedASTTransformation.class || transformClass == LogASTTransformation.class) { classNode.addTransform((Class<? extends ASTTransformation>) transformClass, annotation); } } diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java index af21d9ee75..1735b51d4e 100644 --- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java @@ -113,7 +113,7 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport { // only descend if we have annotations to look for Map<Class<? extends ASTTransformation>, Set<ASTNode>> baseTransforms = classNode.getTransforms(phase); if (!baseTransforms.isEmpty()) { - final Map<Class<? extends ASTTransformation>, ASTTransformation> transformInstances = new HashMap<Class<? extends ASTTransformation>, ASTTransformation>(); + final Map<Class<? extends ASTTransformation>, ASTTransformation> transformInstances = new HashMap<>(); for (Class<? extends ASTTransformation> transformClass : baseTransforms.keySet()) { try { transformInstances.put(transformClass, transformClass.getDeclaredConstructor().newInstance()); @@ -121,13 +121,13 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport { source.getErrorCollector().addError( new SimpleMessage( "Could not instantiate Transformation Processor " + transformClass - , //+ " declared by " + annotation.getClassNode().getName(), + /*+ " declared by " + annotation.getClassNode().getName()*/, source)); } } // invert the map, is now one to many - transforms = new HashMap<ASTNode, List<ASTTransformation>>(); + transforms = new HashMap<>(); for (Map.Entry<Class<? extends ASTTransformation>, Set<ASTNode>> entry : baseTransforms.entrySet()) { for (ASTNode node : entry.getValue()) { List<ASTTransformation> list = transforms.computeIfAbsent(node, k -> new ArrayList<>()); @@ -136,7 +136,7 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport { } } - targetNodes = new LinkedList<ASTNode[]>(); + targetNodes = new LinkedList<>(); // first pass, collect nodes super.visitClass(classNode); @@ -395,15 +395,15 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport { private static class PriorityComparator implements Comparator<Tuple2<ASTTransformation, ASTNode[]>> { @Override public int compare(Tuple2<ASTTransformation, ASTNode[]> o1, Tuple2<ASTTransformation, ASTNode[]> o2) { - Integer i1 = 0; - Integer i2 = 0; + int i1 = 0; + int i2 = 0; if (o1.getV1() instanceof TransformWithPriority) { i1 = ((TransformWithPriority) o1.getV1()).priority(); } if (o2.getV1() instanceof TransformWithPriority) { i2 = ((TransformWithPriority) o2.getV1()).priority(); } - return i2.compareTo(i1); + return Integer.compare(i2, i1); } } } diff --git a/src/main/java/org/codehaus/groovy/transform/BuilderASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/BuilderASTTransformation.java index ec77fc01e9..d367485017 100644 --- a/src/main/java/org/codehaus/groovy/transform/BuilderASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/BuilderASTTransformation.java @@ -56,15 +56,25 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.param; @GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS) public class BuilderASTTransformation extends AbstractASTTransformation implements CompilationUnitAware, TransformWithPriority { - private static final Class MY_CLASS = Builder.class; + private static final Class<?> MY_CLASS = Builder.class; private static final ClassNode MY_TYPE = make(MY_CLASS); public static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final ClassNode RECORD_TYPE = make(RecordBase.class, false); public static final ClassNode[] NO_EXCEPTIONS = ClassNode.EMPTY_ARRAY; public static final Parameter[] NO_PARAMS = Parameter.EMPTY_ARRAY; + @Override + public int priority() { + return -5; + } + private CompilationUnit compilationUnit; + @Override + public void setCompilationUnit(final CompilationUnit unit) { + this.compilationUnit = unit; + } + @Override public void visit(ASTNode[] nodes, SourceUnit source) { init(nodes, source); @@ -101,11 +111,6 @@ public class BuilderASTTransformation extends AbstractASTTransformation implemen } } - @Override - public int priority() { - return -5; - } - public interface BuilderStrategy { void build(BuilderASTTransformation transform, AnnotatedNode annotatedNode, AnnotationNode anno); } @@ -295,9 +300,4 @@ public class BuilderASTTransformation extends AbstractASTTransformation implemen return null; } } - - @Override - public void setCompilationUnit(final CompilationUnit unit) { - this.compilationUnit = unit; - } } diff --git a/src/main/java/org/codehaus/groovy/transform/ExternalizeVerifierASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ExternalizeVerifierASTTransformation.java index 8d88393391..aadc5ef930 100644 --- a/src/main/java/org/codehaus/groovy/transform/ExternalizeVerifierASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/ExternalizeVerifierASTTransformation.java @@ -39,9 +39,10 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.getInstancePropertyFiel import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_TRANSIENT; [email protected](phase = CompilePhase.CLASS_GENERATION) -public class ExternalizeVerifierASTTransformation extends org.codehaus.groovy.transform.AbstractASTTransformation { - static final Class MY_CLASS = ExternalizeVerifier.class; +@GroovyASTTransformation(phase = CompilePhase.CLASS_GENERATION) +public class ExternalizeVerifierASTTransformation extends AbstractASTTransformation { + + static final Class<?> MY_CLASS = ExternalizeVerifier.class; static final ClassNode MY_TYPE = make(MY_CLASS); static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final ClassNode EXTERNALIZABLE_TYPE = make(Externalizable.class); diff --git a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java index 971e0b9dad..1a1d925507 100644 --- a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java @@ -56,7 +56,7 @@ import static org.objectweb.asm.Opcodes.ACC_TRANSIENT; * This class provides an AST Transformation to add a log field to a class. */ @GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS) -public class LogASTTransformation extends AbstractASTTransformation implements CompilationUnitAware { +public class LogASTTransformation extends AbstractASTTransformation implements CompilationUnitAware, TransformWithPriority { /** * This is just a dummy value used because String annotations values can not be null. @@ -66,6 +66,11 @@ public class LogASTTransformation extends AbstractASTTransformation implements C public static final String DEFAULT_ACCESS_MODIFIER = "private"; + @Override + public int priority() { + return 1; // GROOVY-7439 + } + private CompilationUnit compilationUnit; @Override diff --git a/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java index ac163c9b2d..3260916c17 100644 --- a/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java @@ -84,8 +84,8 @@ import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC; * Injects a set of Comparators and sort methods. */ @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) - public class SortableASTTransformation extends AbstractASTTransformation { + private static final ClassNode MY_TYPE = make(Sortable.class); private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); private static final ClassNode COMPARABLE_TYPE = makeClassSafe(Comparable.class); diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java index addd4c5f14..50cf1eac1f 100644 --- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java @@ -375,13 +375,13 @@ public class TraitASTTransformation extends AbstractASTTransformation implements /** * Copies annotations from the trait to the helper, excluding non-applicable - * items such as {@link Trait @Trait} and {@link Sealed @Sealed}. + * items such as {@link Trait @Trait}, {@link Sealed @Sealed} and logging transforms. */ private static void copyClassAnnotations(final ClassNode helper) { for (AnnotationNode annotation : helper.getOuterClass().getAnnotations()) { ClassNode annotationType = annotation.getClassNode(); - if (!annotationType.equals(Traits.TRAIT_CLASSNODE) - && !annotationType.equals(SEALED_TYPE)) { + if (!annotationType.equals(Traits.TRAIT_CLASSNODE) && !annotationType.equals(SEALED_TYPE) + && !annotationType.getName().startsWith("groovy.util.logging.")) { // GROOVY-7439 helper.addAnnotation(annotation); } } diff --git a/src/test/groovy/groovy/util/logging/Slf4jTest.groovy b/src/test/groovy/groovy/util/logging/Slf4jTest.groovy index 5f7bcba37d..cb1c071d24 100644 --- a/src/test/groovy/groovy/util/logging/Slf4jTest.groovy +++ b/src/test/groovy/groovy/util/logging/Slf4jTest.groovy @@ -24,7 +24,6 @@ import ch.qos.logback.classic.LoggerContext import ch.qos.logback.classic.spi.LoggingEvent import ch.qos.logback.core.OutputStreamAppender import ch.qos.logback.core.layout.EchoLayout -import groovy.test.NotYetImplemented import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -374,7 +373,6 @@ final class Slf4jTest { } // GROOVY-7439 - @NotYetImplemented @ParameterizedTest @ValueSource(strings=[ 'log', // Cannot find matching method Object#debug(String)
