GROOVY-9585: CompilerConfiguration copy constructor does not copy com…

COMPATIBILITY.md

@@ -206,6 +206,48 @@ planned for deprecation/removal in a future Groovy version. Formal
 `@Deprecated` may be restored before 6 GA once beta feedback confirms
 indy remains acceptable for those use cases.
 
+### Groovy 6 — `CompilerConfiguration` copy constructor copies customizers (GROOVY-9585)
+
+`CompilerConfiguration(CompilerConfiguration)` now copies the source
+configuration's compilation customizers along with every other setting.
+Before Groovy 6 it copied everything *except* customizers.
+
+**Who is affected.** Code that derives a configuration from an existing
+one and then registers its own customizers:
+
+```java
+CompilerConfiguration child = new CompilerConfiguration(parent);
+child.addCompilationCustomizers(mine);   // now: parent's customizers *and* mine
+```
+
+The change is silent — no exception, just customizers running that
+previously did not. An `ImportCustomizer` adds its imports twice; an
+`ASTTransformationCustomizer`, which carries mutable `applied` state, is
+invoked for a second compilation.
+
+**To restore the old behaviour**, use the two-argument copy constructor added in
+6.0.0:
+
+```java
+CompilerConfiguration child = new CompilerConfiguration(parent, false);
+```
+
+**Nested compilation.** A copied customizer is invoked for every primary
+class node of the child compilation, including nodes added via
+`CompilationUnit.addClassNode`, which have no `SourceUnit` — as that
+method's javadoc warns. A customizer that dereferences the `SourceUnit`
+it is handed will therefore throw `NullPointerException`; Gradle's
+incremental-compilation customizer is one such, and Groovy's own
+`SourceAwareCustomizer` is another. A customizer may equally assume the
+class nodes it sees belong to the compilation it was registered for.
+Groovy's own nested compilations —
+`StaticTypeCheckingSupport.evaluateExpression`, which compiles a
+synthetic expression holder, and `GroovyTypeCheckingExtensionSupport`,
+which compiles a type checking DSL script — therefore pass `false`.
+**Prefer `new CompilerConfiguration(parent, false)` whenever you derive a
+configuration for a nested compilation**, and null-check the `SourceUnit`
+in any customizer that might be applied to one.
+
 ## The binary-compatibility check
 
 The [`subprojects/binary-compatibility/`](subprojects/binary-compatibility)

src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java

@@ -605,10 +605,33 @@ private void handleOptimizationOption(String key, String val) {
      * CompilerConfiguration myConfiguration = new CompilerConfiguration(CompilerConfiguration.DEFAULT);
      * myConfiguration.setDebug(true);
      * </pre></blockquote>
+     * <p>
+     * The supplied configuration's {@link #getCompilationCustomizers() compilation customizers}
+     * are copied along with everything else. Prior to Groovy 6, they were not; code which copies a
+     * configuration and then adds its own customizers will now see the copied ones as well as its
+     * own. Use {@link #CompilerConfiguration(CompilerConfiguration, boolean)} with {@code false} if
+     * you want only your own.
      *
      * @param configuration The configuration to copy.
      */
     public CompilerConfiguration(final CompilerConfiguration configuration) {
+        this(configuration, true);
+    }
+
+    /**
+     * Copy constructor which optionally omits the supplied configuration's
+     * {@link #getCompilationCustomizers() compilation customizers}.
+     * <p>
+     * Pass {@code false} when deriving a configuration for a <em>nested</em> compilation, such as
+     * compiling a synthetic class node or a DSL script encountered while compiling something else.
+     * A customizer is registered for a particular compilation and commonly assumes its source units
+     * and class nodes, so applying one to a nested compilation may misbehave or fail outright.
+     *
+     * @param configuration The configuration to copy.
+     * @param copyCustomizers whether to also copy the compilation customizers
+     * @since 6.0.0
+     */
+    public CompilerConfiguration(final CompilerConfiguration configuration, final boolean copyCustomizers) {
         setWarningLevel(configuration.getWarningLevel());
         setTargetDirectory(configuration.getTargetDirectory());
         setClasspathList(configuration.getClasspath());
@@ -637,8 +660,9 @@ public CompilerConfiguration(final CompilerConfiguration configuration) {
         Map<String, Object> jointCompilationOptions = configuration.getJointCompilationOptions();
         setJointCompilationOptions(null != jointCompilationOptions ? new HashMap<>(jointCompilationOptions) : jointCompilationOptions);
 
-        // TODO GROOVY-9585: add line below once gradle build issues fixed
-//        compilationCustomizers.addAll(configuration.getCompilationCustomizers());
+        if (copyCustomizers) {
+            compilationCustomizers.addAll(configuration.getCompilationCustomizers());
+        }
     }
 
     /**

src/main/java/org/codehaus/groovy/transform/stc/GroovyTypeCheckingExtensionSupport.java

@@ -204,7 +204,12 @@ public void setup() {
                 "org.codehaus.groovy.ast.ClassHelper",
                 "org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport");
 
-        CompilerConfiguration config = new CompilerConfiguration().addCompilationCustomizers(ic);
+        // inherit the enclosing compilation's settings (bytecode target, preview features,
+        // optimization options, encoding, ...) but not its customizers: those are registered
+        // for the outer compilation and commonly assume its source units and class nodes
+        CompilerConfiguration config = new CompilerConfiguration(
+                typeCheckingVisitor.getSourceUnit().getConfiguration(), false)
+                .addCompilationCustomizers(ic);
         config.setScriptBaseClass("org.codehaus.groovy.transform.stc.GroovyTypeCheckingExtensionSupport$TypeCheckingDSL");
 
         final GroovyClassLoader transformLoader = compilationUnit!=null?compilationUnit.getTransformLoader():typeCheckingVisitor.getSourceUnit().getClassLoader();

src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java

@@ -2390,8 +2390,10 @@ public static Object evaluateExpression(final Expression expr, final CompilerCon
         ClassNode classNode = new ClassNode(className, Opcodes.ACC_PUBLIC, OBJECT_TYPE);
         addGeneratedMethod(classNode, "eval", Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, new ReturnStatement(expr));
 
-        // adjust configuration so class can be executed by this JVM
-        CompilerConfiguration cc = new CompilerConfiguration(config);
+        // adjust configuration so class can be executed by this JVM; the class node below is
+        // added without a source unit, so a customizer expecting one would fail, and customizers
+        // have no business running on an internal, throw-away expression holder in any case
+        CompilerConfiguration cc = new CompilerConfiguration(config, false);
         cc.setPreviewFeatures(false);
         cc.setScriptBaseClass(null);
         cc.setTargetBytecode(CompilerConfiguration.DEFAULT.getTargetBytecode());

src/test/groovy/groovy/transform/stc/TypeCheckingExtensionsTest.groovy

@@ -18,8 +18,14 @@
  */
 package groovy.transform.stc
 
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.classgen.GeneratorContext
+import org.codehaus.groovy.control.BytecodeProcessor
+import org.codehaus.groovy.control.CompilePhase
 import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.codehaus.groovy.control.SourceUnit
 import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
+import org.codehaus.groovy.control.customizers.CompilationCustomizer
 import org.junit.jupiter.api.Test
 
 import static groovy.test.GroovyAssert.shouldFail
@@ -576,4 +582,41 @@ final class TypeCheckingExtensionsTest extends StaticTypeCheckingTestCase {
         ''',
         'Error thrown from extension in onMethodSelection'
     }
+
+    @Test
+    void testExtensionScriptInheritsConfigurationButNotCustomizers() {
+        Set<String> postprocessed = []
+        Set<String> customized = []
+
+        // not a customizer, so it reaches the extension script only if that script's
+        // compiler configuration was derived from the enclosing compilation's
+        config.bytecodePostprocessor = { String name, byte[] bytes ->
+            postprocessed << name
+            bytes
+        } as BytecodeProcessor
+
+        // a customizer, registered for the enclosing compilation: it must not reach
+        // the extension script, since customizers commonly assume the source units
+        // and class nodes of the compilation they were registered for
+        config.addCompilationCustomizers(new CompilationCustomizer(CompilePhase.CANONICALIZATION) {
+            @Override
+            void call(SourceUnit source, GeneratorContext context, ClassNode classNode) {
+                customized << classNode.name
+            }
+        })
+
+        extension = 'groovy/transform/stc/SetupTestExtension.groovy'
+        assertScript '''
+            class A {}
+            new A()
+        '''
+
+        // the enclosing compilation is named TestScript<uuid>; the extension script is
+        // compiled separately by GroovyTypeCheckingExtensionSupport and named Script<n>
+        assert customized.contains('A'), 'customizer should see the enclosing compilation'
+        assert postprocessed.any { it.startsWith('Script') },
+            'extension script should inherit the enclosing compiler configuration'
+        assert !customized.any { it.startsWith('Script') },
+            'extension script should not inherit the enclosing compilation customizers'
+    }
 }

src/test/groovy/org/codehaus/groovy/control/CompilerConfigurationTest.java

@@ -181,8 +181,31 @@ public void testCopyConstructor1() {
         assertEquals(pluginFactory, config.getPluginFactory());
         assertTrue(config.isLogClassgen());
         assertEquals(100, config.getLogClassgenStackTraceMaxDepth());
-        // TODO GROOVY-9585: re-enable below assertion once prod code is fixed
-//        assertEquals(1, config.getCompilationCustomizers().size());
+        assertEquals(1, config.getCompilationCustomizers().size());
+    }
+
+    @Test
+    public void testCopyConstructorWithoutCustomizers() {
+        CompilerConfiguration init = new CompilerConfiguration();
+        init.setScriptBaseClass("blarg.foo.WhatSit");
+        init.setSourceEncoding("LEAD-123");
+        init.setTargetBytecode(CompilerConfiguration.JDK17);
+        init.addCompilationCustomizers(new ImportCustomizer().addStarImports("groovy.transform"));
+        assertEquals(1, init.getCompilationCustomizers().size());
+
+        CompilerConfiguration withCustomizers = new CompilerConfiguration(init, true);
+        assertEquals(1, withCustomizers.getCompilationCustomizers().size());
+
+        CompilerConfiguration withoutCustomizers = new CompilerConfiguration(init, false);
+        assertTrue(withoutCustomizers.getCompilationCustomizers().isEmpty());
+
+        // everything other than the customizers is copied either way
+        assertEquals("blarg.foo.WhatSit", withoutCustomizers.getScriptBaseClass());
+        assertEquals("LEAD-123", withoutCustomizers.getSourceEncoding());
+        assertEquals(CompilerConfiguration.JDK17, withoutCustomizers.getTargetBytecode());
+
+        // the source configuration is left alone
+        assertEquals(1, init.getCompilationCustomizers().size());
     }
 
     @Test