Index: src/main/groovy/grape/GrabAnnotationTransformation.java =================================================================== --- src/main/groovy/grape/GrabAnnotationTransformation.java (revision 18167) +++ src/main/groovy/grape/GrabAnnotationTransformation.java (working copy) @@ -24,6 +24,7 @@ import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.transform.ASTTransformation; +import org.codehaus.groovy.transform.ASTTransformationVisitor; import org.codehaus.groovy.transform.GroovyASTTransformation; import java.util.*; @@ -179,6 +180,8 @@ try { Grape.grab(basicArgs, grabMaps.toArray(new Map[grabMaps.size()])); + // grab may have added more transformations through new URLs added to classpath, so do one more scan + ASTTransformationVisitor.addGlobalTransformsAfterGrab(); } catch (RuntimeException re) { // Decided against syntax exception since this is not a syntax error. // The down side is we lose line number information for the offending Index: src/main/org/codehaus/groovy/control/CompilationUnit.java =================================================================== --- src/main/org/codehaus/groovy/control/CompilationUnit.java (revision 18167) +++ src/main/org/codehaus/groovy/control/CompilationUnit.java (working copy) @@ -83,8 +83,8 @@ protected OptimizerVisitor optimizer; LinkedList[] phaseOperations; + LinkedList[] newPhaseOperations; - /** * Initializes the CompilationUnit with defaults. */ @@ -152,8 +152,10 @@ this.optimizer = new OptimizerVisitor(this); phaseOperations = new LinkedList[Phases.ALL + 1]; + newPhaseOperations = new LinkedList[Phases.ALL + 1]; for (int i = 0; i < phaseOperations.length; i++) { phaseOperations[i] = new LinkedList(); + newPhaseOperations[i] = new LinkedList(); } addPhaseOperation(new SourceUnitOperation() { public void call(SourceUnit source) throws CompilationFailedException { @@ -202,6 +204,10 @@ phaseOperations[Phases.OUTPUT].addFirst(op); } + public void addNewPhaseOperation(SourceUnitOperation op, int phase) { + if (phase < 0 || phase > Phases.ALL) throw new IllegalArgumentException("phase " + phase + " is unknown"); + newPhaseOperations[phase].add(op); + } /** * Configures its debugging mode and classloader classpath from a given compiler configuration. @@ -456,16 +462,9 @@ while (throughPhase >= phase && phase <= Phases.ALL) { - for (Iterator it = phaseOperations[phase].iterator(); it.hasNext();) { - Object operation = it.next(); - if (operation instanceof PrimaryClassNodeOperation) { - applyToPrimaryClassNodes((PrimaryClassNodeOperation) operation); - } else if (operation instanceof SourceUnitOperation) { - applyToSourceUnits((SourceUnitOperation) operation); - } else { - applyToGeneratedGroovyClasses((GroovyClassOperation) operation); - } - } + processPhaseOperations(phaseOperations[phase]); + // Grab processing may have brought in new AST transforms into various phases, process them as well + processPhaseOperations(newPhaseOperations[phase]); if (progressCallback != null) progressCallback.call(this, phase); completePhase(); @@ -482,6 +481,19 @@ errorCollector.failIfErrors(); } + + private void processPhaseOperations(List ops) { + for (Iterator it = ops.iterator(); it.hasNext();) { + Object operation = it.next(); + if (operation instanceof PrimaryClassNodeOperation) { + applyToPrimaryClassNodes((PrimaryClassNodeOperation) operation); + } else if (operation instanceof SourceUnitOperation) { + applyToSourceUnits((SourceUnitOperation) operation); + } else { + applyToGeneratedGroovyClasses((GroovyClassOperation) operation); + } + } + } private void sortClasses() throws CompilationFailedException { Iterator modules = this.ast.getModules().iterator(); Index: src/main/org/codehaus/groovy/transform/ASTTransformationVisitor.java =================================================================== --- src/main/org/codehaus/groovy/transform/ASTTransformationVisitor.java (revision 18167) +++ src/main/org/codehaus/groovy/transform/ASTTransformationVisitor.java (working copy) @@ -58,6 +58,8 @@ private List targetNodes; private Map> transforms; private Map, ASTTransformation> transformInstances; + private static CompilationUnit compUnit; + private static Set globalTransformNames = new HashSet(); private ASTTransformationVisitor(CompilePhase phase) { this.phase = phase; @@ -176,10 +178,19 @@ } } } + + public static void addGlobalTransformsAfterGrab() { + doAddGlobalTransforms(compUnit, false); + } + + public static void addGlobalTransforms(CompilationUnit compilationUnit) { + compUnit = compilationUnit; + doAddGlobalTransforms(compilationUnit, true); + } - public static void addGlobalTransforms(CompilationUnit compilationUnit) { + private static void doAddGlobalTransforms(CompilationUnit compilationUnit, boolean isFirstScan) { GroovyClassLoader transformLoader = compilationUnit.getTransformLoader(); - LinkedHashMap globalTransformNames = new LinkedHashMap(); + Map transformNames = new LinkedHashMap(); try { Enumeration globalServices = transformLoader.getResources("META-INF/services/org.codehaus.groovy.transform.ASTTransformation"); while (globalServices.hasMoreElements()) { @@ -196,12 +207,12 @@ } while (className != null) { if (!className.startsWith("#") && className.length() > 0) { - if (globalTransformNames.containsKey(className)) { - if (!service.equals(globalTransformNames.get(className))) { + if (transformNames.containsKey(className)) { + if (!service.equals(transformNames.get(className))) { compilationUnit.getErrorCollector().addWarning( WarningMessage.POSSIBLE_ERRORS, "The global transform for class " + className + " is defined in both " - + globalTransformNames.get(className).toExternalForm() + + transformNames.get(className).toExternalForm() + " and " + service.toExternalForm() + " - the former definition will be used and the latter ignored.", @@ -210,7 +221,7 @@ } } else { - globalTransformNames.put(className, service); + transformNames.put(className, service); } } try { @@ -237,7 +248,7 @@ StringBuffer sb = new StringBuffer(); sb.append("Global ASTTransformations are not enabled in retro builds of groovy.\n"); sb.append("The following transformations will be ignored:"); - for (Map.Entry entry : globalTransformNames.entrySet()) { + for (Map.Entry entry : transformNames.entrySet()) { sb.append('\t'); sb.append(entry.getKey()); sb.append('\n'); @@ -246,7 +257,31 @@ WarningMessage.POSSIBLE_ERRORS, sb.toString(), null, null)); return; } - for (Map.Entry entry : globalTransformNames.entrySet()) { + + // record the transforms found in the first scan, so that in the 2nd scan, phase operations + // can be added for only for new transforms that have come in + if(isFirstScan) { + for (Map.Entry entry : transformNames.entrySet()) { + globalTransformNames.add(entry.getKey()); + } + addPhaseOperationsForGlobalTransforms(compilationUnit, transformNames, isFirstScan); + } else { + Iterator> it = transformNames.entrySet().iterator(); + while(it.hasNext()) { + Map.Entry entry = it.next(); + if(!globalTransformNames.add(entry.getKey())) { + // phase operations for this transform class have already been added before, so remove from current scan cycle + it.remove(); + } + } + addPhaseOperationsForGlobalTransforms(compilationUnit, transformNames, isFirstScan); + } + } + + private static void addPhaseOperationsForGlobalTransforms(CompilationUnit compilationUnit, + Map transformNames, boolean isFirstScan) { + GroovyClassLoader transformLoader = compilationUnit.getTransformLoader(); + for (Map.Entry entry : transformNames.entrySet()) { try { Class gTransClass = transformLoader.loadClass(entry.getKey(), false, true, false); //no inspection unchecked @@ -263,11 +298,16 @@ } if (ASTTransformation.class.isAssignableFrom(gTransClass)) { final ASTTransformation instance = (ASTTransformation)gTransClass.newInstance(); - compilationUnit.addPhaseOperation(new CompilationUnit.SourceUnitOperation() { + CompilationUnit.SourceUnitOperation suOp = new CompilationUnit.SourceUnitOperation() { public void call(SourceUnit source) throws CompilationFailedException { instance.visit(new ASTNode[] {source.getAST()}, source); } - }, transformAnnotation.phase().getPhaseNumber()); + }; + if(isFirstScan) { + compilationUnit.addPhaseOperation(suOp, transformAnnotation.phase().getPhaseNumber()); + } else { + compilationUnit.addNewPhaseOperation(suOp, transformAnnotation.phase().getPhaseNumber()); + } } else { compilationUnit.getErrorCollector().addError(new SimpleMessage( "Transform Class " + entry.getKey() + " specified at " Index: src/test/groovy/bugs/vm5/Groovy3861Bug.groovy =================================================================== --- src/test/groovy/bugs/vm5/Groovy3861Bug.groovy (revision 0) +++ src/test/groovy/bugs/vm5/Groovy3861Bug.groovy (revision 0) @@ -0,0 +1,25 @@ +package groovy.bugs.vm5 + +class Groovy3861Bug extends GroovyTestCase { + void testGrabOfSpockThatBrigsInItsNewASTTransform() { + def spockScript = """ + import spock.lang.* + + @Grab(group='org.spockframework', module='spock-core', version='0.2') + class HelloSpock extends Specification { + def "can you figure out what I'm up to?"() { + expect: + name.size() == length + + where: + name << ["Kirk", "Spock", "Scotty"] + length << [4, 5, 6] + } + } + """ + + def shell = new GroovyShell() + + shell.run(spockScript, 'Groovy3861BugSpockScript.groovy', []) + } +}