Index: src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java =================================================================== --- src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java (revision 18267) +++ src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java (working copy) @@ -74,28 +74,51 @@ // skip if there is no such annotation continue; } - for (String transformClass : getTransformClasses(transformClassAnnotation)) { - try { - Class klass = transformLoader.loadClass(transformClass, false, true, false); - if (ASTTransformation.class.isAssignableFrom(klass)) { - classNode.addTransform(klass, annotation); - } else { - source.getErrorCollector().addError( - new SimpleMessage( - "Not an ASTTransformation: " + transformClass - + " declared by " + annotation.getClassNode().getName(), - source)); - } - } catch (ClassNotFoundException e) { - source.getErrorCollector().addErrorAndContinue( - new SimpleMessage( - "Could not find class for Transformation Processor " + transformClass - + " declared by " + annotation.getClassNode().getName(), - source)); - } + addTransformsToClassNode(annotation, transformClassAnnotation); + } + } + + private void addTransformsToClassNode(AnnotationNode annotation, Annotation transformClassAnnotation) { + String[] transformClassNames = getTransformClassNames(transformClassAnnotation); + Class[] transformClasses = getTransformClasses(transformClassAnnotation); + boolean transformClassNamesSpecified = (transformClassNames != null) && (transformClassNames.length > 0); + boolean transformClassesSpecified = (transformClasses != null) && (transformClasses.length > 0); + if(!transformClassNamesSpecified && !transformClassesSpecified) { + source.getErrorCollector().addError(new SimpleMessage("@GroovyASTTransformationClass in " + + annotation.getClassNode().getName() + " does not specify any transform class names/classes", source)); + } + if(transformClassNamesSpecified && transformClassesSpecified) { + source.getErrorCollector().addError(new SimpleMessage("@GroovyASTTransformationClass in " + + annotation.getClassNode().getName() + " should specify transforms only by class names or by classes and not by both", source)); + } + for (String transformClass : transformClassNames) { + try { + Class klass = transformLoader.loadClass(transformClass, false, true, false); + verifyClassAndAddTransform(annotation, klass); + } catch (ClassNotFoundException e) { + source.getErrorCollector().addErrorAndContinue( + new SimpleMessage( + "Could not find class for Transformation Processor " + transformClass + + " declared by " + annotation.getClassNode().getName(), + source)); } } + for (Class klass : transformClasses) { + verifyClassAndAddTransform(annotation, klass); + } } + + private void verifyClassAndAddTransform(AnnotationNode annotation, Class klass) { + if (ASTTransformation.class.isAssignableFrom(klass)) { + classNode.addTransform(klass, annotation); + } else { + source.getErrorCollector().addError( + new SimpleMessage( + "Not an ASTTransformation: " + klass.getName() + + " declared by " + annotation.getClassNode().getName(), + source)); + } + } private static Annotation getTransformClassAnnotation(ClassNode annotatedType) { if (!annotatedType.isResolved()) return null; @@ -112,7 +135,7 @@ return null; } - private String[] getTransformClasses(Annotation transformClassAnnotation) { + private String[] getTransformClassNames(Annotation transformClassAnnotation) { try { Method valueMethod = transformClassAnnotation.getClass().getMethod("value"); return (String[]) valueMethod.invoke(transformClassAnnotation); @@ -121,4 +144,14 @@ return new String[0]; } } + + private Class[] getTransformClasses(Annotation transformClassAnnotation) { + try { + Method classesMethod = transformClassAnnotation.getClass().getMethod("classes"); + return (Class[]) classesMethod.invoke(transformClassAnnotation); + } catch (Exception e) { + source.addException(e); + return new Class[0]; + } + } } Index: src/main/org/codehaus/groovy/transform/GroovyASTTransformationClass.java =================================================================== --- src/main/org/codehaus/groovy/transform/GroovyASTTransformationClass.java (revision 18267) +++ src/main/org/codehaus/groovy/transform/GroovyASTTransformationClass.java (working copy) @@ -23,5 +23,6 @@ // in the future the target will be wider than annotations, but for now it is just on annotations @Target(ElementType.ANNOTATION_TYPE) public @interface GroovyASTTransformationClass { - String[] value(); + String[] value() default {}; + Class[] classes() default {}; } Index: src/test/groovy/bugs/vm5/G3839A1.java =================================================================== --- src/test/groovy/bugs/vm5/G3839A1.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839A1.java (revision 0) @@ -0,0 +1,6 @@ +package groovy.bugs.vm5; + +import org.codehaus.groovy.transform.GroovyASTTransformationClass; + +@GroovyASTTransformationClass(classes=G3839Transform1.class) +public @interface G3839A1 {} Index: src/test/groovy/bugs/vm5/G3839A2.java =================================================================== --- src/test/groovy/bugs/vm5/G3839A2.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839A2.java (revision 0) @@ -0,0 +1,7 @@ +package groovy.bugs.vm5; + +import org.codehaus.groovy.transform.GroovyASTTransformationClass; + +@GroovyASTTransformationClass(classes={G3839Transform2.class, G3839Transform3.class}) +public @interface G3839A2 { +} Index: src/test/groovy/bugs/vm5/G3839A3.java =================================================================== --- src/test/groovy/bugs/vm5/G3839A3.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839A3.java (revision 0) @@ -0,0 +1,6 @@ +package groovy.bugs.vm5; + +import org.codehaus.groovy.transform.GroovyASTTransformationClass; + +@GroovyASTTransformationClass() // does not specify ast transform class names or classes +public @interface G3839A3 {} Index: src/test/groovy/bugs/vm5/G3839A4.java =================================================================== --- src/test/groovy/bugs/vm5/G3839A4.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839A4.java (revision 0) @@ -0,0 +1,7 @@ +package groovy.bugs.vm5; + +import org.codehaus.groovy.transform.GroovyASTTransformationClass; + +// add ast transforms both by class names and classes, which should result in an error +@GroovyASTTransformationClass(value="groovy.bugs.vm5.G3839Transform2", classes=G3839Transform3.class) +public @interface G3839A4 {} Index: src/test/groovy/bugs/vm5/G3839Transform1.java =================================================================== --- src/test/groovy/bugs/vm5/G3839Transform1.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839Transform1.java (revision 0) @@ -0,0 +1,17 @@ +package groovy.bugs.vm5; + +import org.objectweb.asm.Opcodes; + +import org.codehaus.groovy.ast.*; +import org.codehaus.groovy.control.*; +import org.codehaus.groovy.transform.*; + +@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) +public class G3839Transform1 implements ASTTransformation, Opcodes{ + + public void visit(ASTNode[] nodes, SourceUnit sourceUnit) { + ClassNode classNode = (ClassNode) nodes[1]; + classNode.addField(new FieldNode("f1", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, classNode, null)); + } + +} Index: src/test/groovy/bugs/vm5/G3839Transform2.java =================================================================== --- src/test/groovy/bugs/vm5/G3839Transform2.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839Transform2.java (revision 0) @@ -0,0 +1,17 @@ +package groovy.bugs.vm5; + +import org.objectweb.asm.Opcodes; + +import org.codehaus.groovy.ast.*; +import org.codehaus.groovy.control.*; +import org.codehaus.groovy.transform.*; + +@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) +public class G3839Transform2 implements ASTTransformation, Opcodes{ + + public void visit(ASTNode[] nodes, SourceUnit sourceUnit) { + ClassNode classNode = (ClassNode) nodes[1]; + classNode.addField(new FieldNode("f2", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, classNode, null)); + } + +} Index: src/test/groovy/bugs/vm5/G3839Transform3.java =================================================================== --- src/test/groovy/bugs/vm5/G3839Transform3.java (revision 0) +++ src/test/groovy/bugs/vm5/G3839Transform3.java (revision 0) @@ -0,0 +1,17 @@ +package groovy.bugs.vm5; + +import org.objectweb.asm.Opcodes; + +import org.codehaus.groovy.ast.*; +import org.codehaus.groovy.control.*; +import org.codehaus.groovy.transform.*; + +@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) +public class G3839Transform3 implements ASTTransformation, Opcodes{ + + public void visit(ASTNode[] nodes, SourceUnit sourceUnit) { + ClassNode classNode = (ClassNode) nodes[1]; + classNode.addField(new FieldNode("f3", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, classNode, null)); + } + +} Index: src/test/groovy/bugs/vm5/Groovy3839Bug.groovy =================================================================== --- src/test/groovy/bugs/vm5/Groovy3839Bug.groovy (revision 0) +++ src/test/groovy/bugs/vm5/Groovy3839Bug.groovy (revision 0) @@ -0,0 +1,56 @@ +package groovy.bugs.vm5 + +class Groovy3839Bug extends GroovyTestCase { + void testGroovyASTTransformationWithOneClass() { + assertScript """ + import groovy.bugs.vm5.* + + @G3839A1 + class G3839V1 {} + // verify if the ast transform added field 1 + assert G3839V1.class.fields.find{it.name == 'f1'} != null + """ + } + + void testGroovyASTTransformationWithMultipleClass() { + assertScript """ + import groovy.bugs.vm5.* + + @G3839A2 + class G3839V2 {} + // verify if the ast transforms added field f2 and f3 + assert G3839V2.class.fields.find{it.name == 'f2'} != null + assert G3839V2.class.fields.find{it.name == 'f3'} != null + """ + } + + void testGroovyASTTransformationWithNeitherTransClassNamesNorClasses() { + try { + assertScript """ + import groovy.bugs.vm5.* + + @G3839A3 + class G3839V3 {} + new G3839V3() + """ + fail('The script should have failed as @GroovyASTTransformationClass in GroovyASTTransformationClass does not specify transform class names or classes') + }catch(ex) { + assert ex.message.contains('@GroovyASTTransformationClass in groovy.bugs.vm5.G3839A3 does not specify any transform class names/classes') + } + } + + void testGroovyASTTransformationWithBothTransClassNamesAndClasses() { + try { + assertScript """ + import groovy.bugs.vm5.* + + @G3839A4 + class G3839V4 {} + new G3839V4() + """ + fail('The script should have failed as @GroovyASTTransformationClass in GroovyASTTransformationClass does specifies both transform class names and classes') + }catch(ex) { + assert ex.message.contains('@GroovyASTTransformationClass in groovy.bugs.vm5.G3839A4 should specify transforms only by class names or by classes and not by both') + } + } +}