Skip to content

Commit d51a0b6

Browse files
eric-milleseric-milles
committed
GROOVY-7439: allow logging transform on trait class
1 parent 5374e52 commit d51a0b6

8 files changed

Lines changed: 36 additions & 29 deletions

‎src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,21 +280,24 @@ private void verifyAndAddTransform(final AnnotationNode annotation, final Class<
280280
if (!ASTTransformation.class.isAssignableFrom(transformClass)) {
281281
String error = "Not an ASTTransformation: " + transformClass.getName() + " declared by " + annotation.getClassNode().getName();
282282
source.getErrorCollector().addError(new SimpleMessage(error, source));
283+
return;
283284
}
284285

285286
GroovyASTTransformation transformationClass = transformClass.getAnnotation(GroovyASTTransformation.class);
286287
if (transformationClass == null) {
287288
String error = "AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + transformClass.getName() + " lacks this annotation.";
288289
source.getErrorCollector().addError(new SimpleMessage(error, source));
290+
return;
289291
}
290292

291293
CompilePhase specifiedCompilePhase = transformationClass.phase();
292294
if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) {
293295
String error = annotation.getClassNode().getName() + " is defined to be run in compile phase " + specifiedCompilePhase + ". Local AST transformations must run in SEMANTIC_ANALYSIS or later!";
294296
source.getErrorCollector().addError(new SimpleMessage(error, source));
297+
return;
295298
}
296299

297-
if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class || transformClass == SealedASTTransformation.class) {
300+
if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class || transformClass == SealedASTTransformation.class || transformClass == LogASTTransformation.class) {
298301
classNode.addTransform((Class<? extends ASTTransformation>) transformClass, annotation);
299302
}
300303
}

‎src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,21 +113,21 @@ public void visitClass(ClassNode classNode) {
113113
// only descend if we have annotations to look for
114114
Map<Class<? extends ASTTransformation>, Set<ASTNode>> baseTransforms = classNode.getTransforms(phase);
115115
if (!baseTransforms.isEmpty()) {
116-
final Map<Class<? extends ASTTransformation>, ASTTransformation> transformInstances = new HashMap<Class<? extends ASTTransformation>, ASTTransformation>();
116+
final Map<Class<? extends ASTTransformation>, ASTTransformation> transformInstances = new HashMap<>();
117117
for (Class<? extends ASTTransformation> transformClass : baseTransforms.keySet()) {
118118
try {
119119
transformInstances.put(transformClass, transformClass.getDeclaredConstructor().newInstance());
120120
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
121121
source.getErrorCollector().addError(
122122
new SimpleMessage(
123123
"Could not instantiate Transformation Processor " + transformClass
124-
, //+ " declared by " + annotation.getClassNode().getName(),
124+
/*+ " declared by " + annotation.getClassNode().getName()*/,
125125
source));
126126
}
127127
}
128128

129129
// invert the map, is now one to many
130-
transforms = new HashMap<ASTNode, List<ASTTransformation>>();
130+
transforms = new HashMap<>();
131131
for (Map.Entry<Class<? extends ASTTransformation>, Set<ASTNode>> entry : baseTransforms.entrySet()) {
132132
for (ASTNode node : entry.getValue()) {
133133
List<ASTTransformation> list = transforms.computeIfAbsent(node, k -> new ArrayList<>());
@@ -136,7 +136,7 @@ public void visitClass(ClassNode classNode) {
136136
}
137137
}
138138

139-
targetNodes = new LinkedList<ASTNode[]>();
139+
targetNodes = new LinkedList<>();
140140

141141
// first pass, collect nodes
142142
super.visitClass(classNode);
@@ -395,15 +395,15 @@ private static void addPhaseOperationsForGlobalTransforms(CompilationUnit compil
395395
private static class PriorityComparator implements Comparator<Tuple2<ASTTransformation, ASTNode[]>> {
396396
@Override
397397
public int compare(Tuple2<ASTTransformation, ASTNode[]> o1, Tuple2<ASTTransformation, ASTNode[]> o2) {
398-
Integer i1 = 0;
399-
Integer i2 = 0;
398+
int i1 = 0;
399+
int i2 = 0;
400400
if (o1.getV1() instanceof TransformWithPriority) {
401401
i1 = ((TransformWithPriority) o1.getV1()).priority();
402402
}
403403
if (o2.getV1() instanceof TransformWithPriority) {
404404
i2 = ((TransformWithPriority) o2.getV1()).priority();
405405
}
406-
return i2.compareTo(i1);
406+
return Integer.compare(i2, i1);
407407
}
408408
}
409409
}

‎src/main/java/org/codehaus/groovy/transform/BuilderASTTransformation.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,25 @@
5656
@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
5757
public class BuilderASTTransformation extends AbstractASTTransformation implements CompilationUnitAware, TransformWithPriority {
5858

59-
private static final Class MY_CLASS = Builder.class;
59+
private static final Class<?> MY_CLASS = Builder.class;
6060
private static final ClassNode MY_TYPE = make(MY_CLASS);
6161
public static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
6262
private static final ClassNode RECORD_TYPE = make(RecordBase.class, false);
6363
public static final ClassNode[] NO_EXCEPTIONS = ClassNode.EMPTY_ARRAY;
6464
public static final Parameter[] NO_PARAMS = Parameter.EMPTY_ARRAY;
6565

66+
@Override
67+
public int priority() {
68+
return -5;
69+
}
70+
6671
private CompilationUnit compilationUnit;
6772

73+
@Override
74+
public void setCompilationUnit(final CompilationUnit unit) {
75+
this.compilationUnit = unit;
76+
}
77+
6878
@Override
6979
public void visit(ASTNode[] nodes, SourceUnit source) {
7080
init(nodes, source);
@@ -101,11 +111,6 @@ public void visit(ASTNode[] nodes, SourceUnit source) {
101111
}
102112
}
103113

104-
@Override
105-
public int priority() {
106-
return -5;
107-
}
108-
109114
public interface BuilderStrategy {
110115
void build(BuilderASTTransformation transform, AnnotatedNode annotatedNode, AnnotationNode anno);
111116
}
@@ -295,9 +300,4 @@ private BuilderStrategy createBuilderStrategy(AnnotationNode anno, GroovyClassLo
295300
return null;
296301
}
297302
}
298-
299-
@Override
300-
public void setCompilationUnit(final CompilationUnit unit) {
301-
this.compilationUnit = unit;
302-
}
303303
}

‎src/main/java/org/codehaus/groovy/transform/ExternalizeVerifierASTTransformation.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@
3939
import static org.objectweb.asm.Opcodes.ACC_FINAL;
4040
import static org.objectweb.asm.Opcodes.ACC_TRANSIENT;
4141

42-
@org.codehaus.groovy.transform.GroovyASTTransformation(phase = CompilePhase.CLASS_GENERATION)
43-
public class ExternalizeVerifierASTTransformation extends org.codehaus.groovy.transform.AbstractASTTransformation {
44-
static final Class MY_CLASS = ExternalizeVerifier.class;
42+
@GroovyASTTransformation(phase = CompilePhase.CLASS_GENERATION)
43+
public class ExternalizeVerifierASTTransformation extends AbstractASTTransformation {
44+
45+
static final Class<?> MY_CLASS = ExternalizeVerifier.class;
4546
static final ClassNode MY_TYPE = make(MY_CLASS);
4647
static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
4748
private static final ClassNode EXTERNALIZABLE_TYPE = make(Externalizable.class);

‎src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
* This class provides an AST Transformation to add a log field to a class.
5757
*/
5858
@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
59-
public class LogASTTransformation extends AbstractASTTransformation implements CompilationUnitAware {
59+
public class LogASTTransformation extends AbstractASTTransformation implements CompilationUnitAware, TransformWithPriority {
6060

6161
/**
6262
* 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
6666

6767
public static final String DEFAULT_ACCESS_MODIFIER = "private";
6868

69+
@Override
70+
public int priority() {
71+
return 1; // GROOVY-7439
72+
}
73+
6974
private CompilationUnit compilationUnit;
7075

7176
@Override

‎src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@
8484
* Injects a set of Comparators and sort methods.
8585
*/
8686
@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
87-
8887
public class SortableASTTransformation extends AbstractASTTransformation {
88+
8989
private static final ClassNode MY_TYPE = make(Sortable.class);
9090
private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
9191
private static final ClassNode COMPARABLE_TYPE = makeClassSafe(Comparable.class);

‎src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,13 +375,13 @@ private static void addMopMethods(final ClassNode helper) {
375375

376376
/**
377377
* Copies annotations from the trait to the helper, excluding non-applicable
378-
* items such as {@link Trait @Trait} and {@link Sealed @Sealed}.
378+
* items such as {@link Trait @Trait}, {@link Sealed @Sealed} and logging transforms.
379379
*/
380380
private static void copyClassAnnotations(final ClassNode helper) {
381381
for (AnnotationNode annotation : helper.getOuterClass().getAnnotations()) {
382382
ClassNode annotationType = annotation.getClassNode();
383-
if (!annotationType.equals(Traits.TRAIT_CLASSNODE)
384-
&& !annotationType.equals(SEALED_TYPE)) {
383+
if (!annotationType.equals(Traits.TRAIT_CLASSNODE) && !annotationType.equals(SEALED_TYPE)
384+
&& !annotationType.getName().startsWith("groovy.util.logging.")) { // GROOVY-7439
385385
helper.addAnnotation(annotation);
386386
}
387387
}

‎src/test/groovy/groovy/util/logging/Slf4jTest.groovy‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import ch.qos.logback.classic.LoggerContext
2424
import ch.qos.logback.classic.spi.LoggingEvent
2525
import ch.qos.logback.core.OutputStreamAppender
2626
import ch.qos.logback.core.layout.EchoLayout
27-
import groovy.test.NotYetImplemented
2827
import org.junit.jupiter.api.AfterEach
2928
import org.junit.jupiter.api.BeforeEach
3029
import org.junit.jupiter.api.Test
@@ -374,7 +373,6 @@ final class Slf4jTest {
374373
}
375374

376375
// GROOVY-7439
377-
@NotYetImplemented
378376
@ParameterizedTest
379377
@ValueSource(strings=[
380378
'log', // Cannot find matching method Object#debug(String)

0 commit comments

Comments
 (0)