Skip to content

Groovy 5.0.x support for Grails 8 + Spring Boot 4#15557

Open
jamesfredley wants to merge 119 commits into
8.0.xfrom
grails8-groovy5-sb4
Open

Groovy 5.0.x support for Grails 8 + Spring Boot 4#15557
jamesfredley wants to merge 119 commits into
8.0.xfrom
grails8-groovy5-sb4

Conversation

@jamesfredley
Copy link
Copy Markdown
Contributor

@jamesfredley jamesfredley commented Apr 5, 2026

Status

Layered on 8.0.x (with the upgrade/gradle-9.3.1 work merged in: Gradle 9.4.1, Micronaut 4.10.10, Spring Boot 4.0.5, Spring 7.0.6). Locally verified end-to-end against Apache Groovy 5.0.7-SNAPSHOT (off GROOVY_5_0_X HEAD eca67326e) on JDK 21, including the -PgrailsIndy=false matrix that exposes Groovy 5 trait/interface bytecode bugs. Last full audit 2026-05-21; remaining workarounds re-verified against the current 5.0.7-SNAPSHOT during burn-down passes on 2026-05-27 and 2026-05-29 (the latter removed the @Builder/ConfigurationBuilder workaround after its GROOVY_5_0_X backport landed).

The PR tracks 5.0.7-SNAPSHOT from the GROOVY_5_0_X branch so post-GROOVY_5_0_6 fixes are picked up as they land. The Apache snapshots repository declaration in settings.gradle already includes org[.]apache[.]groovy.* so 5.0.7-SNAPSHOT resolves from https://repository.apache.org/content/groups/snapshots/ with no settings.gradle change; the same declaration lets the groovy-joint-workflow CI job swap in upstream Groovy snapshots when needed.

Burn-down since the original 6-item list:

  • Some fixes #5 (g.taglib STC) is fixed and removed - applied @paulk-asert's recommended Grails-side change from GROOVY-12041 (see Real bug fixes below).
  • The setVariableScope half of the old project folder in grails home should use application name, not parent folder name #2 is reclassified as an all-version real fix (it fails identically on Groovy 3.0.x/4.0.x, confirmed by Paul), leaving only the canonicalization guard as a workaround.
  • The ConfigurationBuilder @Builder-detection workaround is removed (db381c1ac6) - GROOVY-12040 (apache/groovy#2565) is now backported to GROOVY_5_0_X. The published 5.0.7-SNAPSHOT build 5.0.7-20260529.064309-2 restores @Builder to @Retention(RUNTIME) (verified directly: the prior 2026-05-22 build still carried SOURCE), so runtime Class.getAnnotation(Builder) detection works again and the isLikelyBuilderType() heuristic plus its three call-site disjuncts were dropped. The Spring 7 Map-to-typed-config conversion fallbacks in the same file are retained (independent of the Groovy version). The AbstractConstraint static-init fallback that used to share this row is a separate Groovy 5 bug and is retained below as remaining workaround Untitled #3.
  • 4 workarounds remain (below), each re-verified still failing on the current 5.0.7-SNAPSHOT with the workaround removed. (GROOVY-11996's groovy.truth.file.exists.enabled flag relates to the already-fixed File-truthiness change, not to any remaining item.)

Target stack

Component Version
Apache Groovy 5.0.7-SNAPSHOT (off GROOVY_5_0_X HEAD eca67326e)
Spock 2.4-groovy-5.0
Spring Boot 4.0.5
Spring Framework 7.0.6
Gradle 9.4.1
Micronaut 4.10.10 (used by Forge)
Jakarta EE 10 (jakarta.servlet, jakarta.validation, jakarta.inject, ...)
JDK 21+

Remaining workarounds

Four remain. Each has been re-verified still failing on the current 5.0.7-SNAPSHOT with the workaround removed.

# Site Real bug (verified) Reproducer Upstream status
1 AstUtils.processVariableScopes, GrailsASTUtils.processVariableScopes, and the AbstractMethodDecoratingTransformation canonicalisation guard - the try { VariableScopeVisitor... } catch (NullPointerException) {} (3 sites) Groovy 5 VariableScopeVisitor NPEs during the canonicalization phase on certain Grails AST-transform outputs. Re-proven on 5.0.7-SNAPSHOT: making the guards re-throw fails :grails-datamapping-tck:compileGroovy with BUG! exception in phase 'canonicalization' ... DataServiceRoutingProductDataService.groovy unexpected NullPointerException. This is distinct from the all-version ClosureWriter null-scope NPE, which the setVariableScope real fix handles (see Real bug fixes). groovy5-variablescope-canonicalization-bug - the standalone case isolates the all-version ClosureWriter NPE; the canonicalization NPE only reproduces inside the Grails transform chain (remove the guard → tck compile NPE above). Not yet filed. Clean in-grails-core reproduction available to attach to a new ticket.
2 gradle/boot4-disabled-integration-test-config.gradle applied to 5 grails-test-examples projects (app1, app3, exploded, mongodb/test-data-service, plugins/exploded) Two independent blockers. (a) Groovy 5 + indy=false: controller actions that declare parameters lose parameter scope - the parameter resolves to propertyMissing on the controller (via TagLibraryInvoker$Trait$Helper) instead of the local, after the full ControllerActionTransformer.convertToMethodAction chain. (b) SiteMesh3 decorator/layout is not compatible with Spring Framework 7. Functional tests (indy=true) PASS for the same projects. groovy5-controller-action-param-scope-bug - investigation harness. The bare try/catch wrap, the full wrapMethodBodyWithExceptionHandling shape, and an unresolved-local-ref variant all PASS standalone on every version/indy combo; the failure needs the live controller transform + request binding. Authoritative repro = the affected modules' integrationTest under -PgrailsIndy=false. Not yet filed (Groovy side); SiteMesh3 side is a Spring 7 compatibility gap.
3 AbstractConstraint.getDefaultMessage MESSAGE_BUNDLE fallback (grails-datamapping-validation) On Groovy 5 the ConstrainedProperty interface constant DEFAULT_MESSAGES is initialised with null values - its map initialiser runs before the DEFAULT_*_MESSAGE constants it references are assigned - so a direct DEFAULT_MESSAGES.get(code) returns null while MESSAGE_BUNDLE.getString(code) resolves the same code correctly. getDefaultMessage falls back to MESSAGE_BUNDLE when the map returns null. Re-proven on 5.0.7-SNAPSHOT: removing the fallback makes the new DefaultMessageResolutionSpec fail (the default message resolves to null); the pre-existing constraint specs pass only because they supply a MessageSource, which is why this went unnoticed before. In-tree: DefaultMessageResolutionSpec - constructs a constraint with a null MessageSource and asserts getDefaultMessage still resolves each code from the bundle. Not yet filed. Distinct from GROOVY-12040 - this is an interface-constant static-initialisation order regression and needs its own upstream Groovy ticket.
4 Validateable.resolveDefaultNullable() Method.invoke reflection bypass TraitReceiverTransformer rewrites this.defaultNullable() (called from inside the trait body) to a direct trait-helper static call, silently losing the implementing-class override. The reflection path is the only one that honours the override. Verified still failing on 5.0.6, 5.0.7-SNAPSHOT and 6.0.0-SNAPSHOT. groovy-trait-static-method-override-bug - reproduces (test 2 fails on all current snapshots). GROOVY-11985 (OPEN); PR apache/groovy#2529 open; root cause is the GROOVY-8854 (Sep 2023) TraitReceiverTransformer change.

Real bug fixes (not workarounds)

These fix latent or upstream-confirmed bugs and are not Groovy-version-conditional workarounds:

  • File.asBoolean silent-no-op in TemplateRendererImpl - rewrote render(Map) in grails-core (325e2fee08) and grails-shell-cli (faef56cfe2), and the typed render(CharSequence/File/Resource, File, Map, boolean) overloads in grails-shell-cli (43ad57a296), to use containsKey() / explicit == null checks. The old if (template && destination) guards silently no-opped because DefaultGroovyMethods.asBoolean(File) returns file.exists() && (isDirectory() OR length>0) for a yet-to-be-generated destination File. GenerateControllerCommand.generateFile calls the typed positional overload as defence-in-depth. Fix per @paulk-asert's confirmation. GROOVY-11996 ships an opt-out flag (groovy.truth.file.exists.enabled=false) but the rewrites do not depend on it.
  • setVariableScope(new VariableScope()) at AST-transform closure-synthesis sites (ResourceTransform, AbstractMethodDecoratingTransformation) - a synthesised ClosureExpression with a null VariableScope NPEs in ClosureWriter on every Groovy version (3.0.25 / 4.0.5 / 4.0.27 / 4.0.31 / 5.0.x / 6.0.x), confirmed by @paulk-asert. This is all-version AST hygiene, not a Groovy 5 workaround. (Distinct from the canonicalization-NPE guard in remaining workaround Grails fails to correctly identify Java Home in startGrails #1.)
  • g.taglib STC resolution from @CompileStatic GSP classes (GroovyPageTypeCheckingExtension, commit 649d7bbef4) - per GROOVY-12041, when a compiled GSP page inherits getProperty(String) the unresolvedVariable/unresolvedProperty callbacks no longer fire for g, so the node-identity match could never hit. methodNotFound now matches the taglib receiver by name against the allowed namespaces, per Paul's recommendation. The three @IgnoreIf({ ... && data.gDotPrefix }) skips in GspCompileStaticSpec are removed; grails-gsp-core (21 passed, 0 failed) and grails-web-gsp (13 passed, 0 failed) are green on 5.0.7-SNAPSHOT. Standalone reproducer + the name-based-fix extension: groovy5-stc-extension-node-identity-bug.
  • numberOfPessimisticUpdates typo in MongoCodecSession (4040590fd6).

Forge / generated-app coverage

The Forge generator produces consumer apps in grails-forge/test-core/src/test/groovy/.... Tests verify all generated apps:

  • Build (Groovy 5 + JDK 21+ default).
  • Pass runCommand round-trips for generate-controller, generate-service, generate-domain-class, generate-views, generate-interceptor, generate-taglib.
  • Pass functional tests against the generated app's GORM, GSP, Hibernate5, MongoDB, async, and security layers.
  • Resolve dependencies via the right repository chain - mavenLocal() for 8.0.0-SNAPSHOT, Maven Central / the Apache release repo for released artifacts, and the Apache snapshots repo for any in-flight org.apache.groovy.*-SNAPSHOT consumed by the groovy-joint-workflow job.

In addition, grails-test-examples/compile-static (cherry-picked from #15294) exercises GORM dynamic finders inside @GrailsCompileStatic services (Book.findAllByName('Joe')) - the GROOVY-11817 happy path - confirming that case works on 5.0.7-SNAPSHOT without the reflection workaround that remaining item #4 still needs for the trait-static-method-override path.


Reviewer notes

  • The bomDependencyVersions['groovy.version'] vs gradleBomDependencyVersions['gradle-groovy.version'] distinction is load-bearing. The grails-gradle subprojects must stay on Groovy 4 to remain compatible with Gradle's embedded runtime, while the Grails BOM and main artifacts use Groovy 5.
  • Each remaining Groovy 5 workaround above has an inline // Groovy 5 ... or // GROOVY-XXXXX ... comment that points at the actual upstream bug.
  • The new Java files in grails-views-gson (StreamingJsonBuilder.java, JsonGenerator.java, DefaultJsonGenerator.java) are deprecation shims so compiled .gson template AST output resolves to the Grails delegate type instead of Groovy 5's package-private groovy.json.StreamingJsonDelegate. Cleanup direction (per @jdaugherty review): fix JsonViewWritableScript.groovy to FQN-qualify groovy.json.StreamingJsonBuilder and stop synthesising the Grails inner-delegate alias - then the shims can be deleted. Tracked as a follow-up in an open review thread.
  • The update_release_draft job runs release-drafter against the PR base. With base = 8.0.x it works as expected; the workflow is continue-on-error: true and does not block the PR.

Open review threads (follow-up commits owed)

  • File upstream Groovy tickets (with the in-grails-core reproductions) for remaining workarounds Grails fails to correctly identify Java Home in startGrails #1 (VariableScopeVisitor canonicalization NPE on :grails-datamapping-tck:compileGroovy), project folder in grails home should use application name, not parent folder name #2 (controller action param-scope loss under indy=false), and Untitled #3 (ConstrainedProperty.DEFAULT_MESSAGES interface-constant static-init order; in-tree reproducer DefaultMessageResolutionSpec).
  • JsonViewTemplateResolverSpec @IgnoreIf - need to wire mock-maker-inline on the test runtime classpath (or rewrite against MockHttpServletRequest).
  • UrlMappingTagLib linkTagAttrs.clone() -> new LinkedHashMap(...) - file an upstream Groovy ticket with a standalone reproducer for the Map.clone() STC dispatch tightening.
  • RestfulServiceController Math.toIntExact(...) - add inline comment explaining the load-bearing Number -> Integer narrowing rejection under Groovy 5 STC.
  • Customer @GrailsCompileStatic removed - re-test restoring the annotation against 5.0.7-SNAPSHOT now that GROOVY-11907 / GROOVY-11968 are fixed; restore if the static-mapping closure VerifyError no longer fires.
  • DataBindingTests GroovySpy(Author, global: true) - drop global: true so the per-method scope auto-cleans, or add an explicit cleanup: block.
  • DefaultJsonGenerator.java / StreamingJsonBuilder.java / JsonGenerator.java shims - update JsonViewWritableScript.groovy to FQN-qualify groovy.json.StreamingJsonBuilder and remove the shims.
  • TraitPropertyAccessStrategy boolean-getter fallback - either delete the fallback if it has no triggering callers in current GORM tests, or rewrite the surrounding code so the JavaBean-conventions intent is self-evident.
  • File a Groovy ticket for the File.asBoolean changed semantics (not explicitly listed in the 5.0 release notes per Paul) so the breaking change is recorded against a JIRA. GROOVY-11996 is the opt-out flag, not the change-log entry.

matrei and others added 30 commits May 15, 2025 10:51
# Conflicts:
#	build.gradle
#	dependencies.gradle
#	grails-forge/build.gradle
#	grails-gradle/build.gradle
# Conflicts:
#	buildSrc/build.gradle
#	dependencies.gradle
#	grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy
#	grails-gradle/buildSrc/build.gradle
# Conflicts:
#	dependencies.gradle
#	gradle/test-config.gradle
#	grails-forge/settings.gradle
#	settings.gradle
# Conflicts:
#	gradle.properties
#	grails-core/src/test/groovy/org/grails/plugins/BinaryPluginSpec.groovy
Cherry-picked comprehensive Groovy 5 compat from 9574fe8.

Conflict resolutions:
- dependencies.gradle: Groovy 5.0.5 GA (not SNAPSHOT) + Jackson 2.21.2
- LoggingTransformer: Keep manual log field injection (avoids Groovy 5 VariableScopeVisitor NPE entirely)
- TransactionalTransformSpec: Remove direct Spock feature method invocation (Groovy 5/Spock 2.x incompatible)
- grails-test-core/build.gradle: Remove spock-core transitive=false, keep junit-platform-suite
- grails-test-suite-uber/build.gradle: Remove spock-core transitive=false and explicit byte-buddy
@jamesfredley
Copy link
Copy Markdown
Contributor Author

Will switch to Groovy 5.0.7-SNAPSHOT after apache/groovy#2547

Resolve conflict in dependencies.gradle by keeping Groovy 5.0.6
(required by this branch's purpose) alongside the new graphql-java
and graphql-java-extended-scalars version entries introduced on 8.0.x.

Assisted-by: claude-code:claude-4.7-opus
Track the GROOVY_5_0_X branch so post-5.0.6 fixes are picked up as they
land. Diff from the GROOVY_5_0_6 release tag to GROOVY_5_0_X HEAD
(eca67326e) is 4 substantive commits: GROOVY-11989 (javaparser bump),
GROOVY-11990 (jackson bump), a metadata update, and GROOVY-11996 (the
`groovy.truth.file.exists.enabled=false` opt-out flag already
cross-referenced in workaround #1 of the PR description; the PR's
real-fix rewrites do not depend on the flag).

None of these commits eliminate any of the 6 remaining workarounds.
The Apache snapshots repository declaration in settings.gradle already
includes `org[.]apache[.]groovy.*` so `5.0.7-SNAPSHOT` resolves from
https://repository.apache.org/content/groups/snapshots/ with no
settings.gradle change required.

Verified by compiling the workaround-site modules against
5.0.7-SNAPSHOT (build 5.0.7-20260520.205749-1):
:grails-core, :grails-validation, :grails-datamapping-tck,
:grails-views-gson, :grails-shell-cli - all green with the
workarounds in place.

Assisted-by: claude-code:claude-opus-4-7
@jamesfredley jamesfredley changed the base branch from 8.0.x to fix/8.0.x-merge-sb4-fallout May 22, 2026 01:40
Base automatically changed from fix/8.0.x-merge-sb4-fallout to 8.0.x May 22, 2026 16:16
…regressions

Four issues were causing the PR CI to fail; this commit addresses all of
them with minimal, scoped changes.

1. Validate Dependency Versions

   `dependencies.gradle` was pinning `groovy.version` to `5.0.6` in the
   `grails-micronaut-bom` overrides while the main BOM had moved to
   `5.0.7-SNAPSHOT` (commit f1b78b7). The strictly-pinned 5.0.6 then
   failed `:grails-micronaut:validateDependencyVersions`:

       org.apache.groovy:groovy-bom - resolved 5.0.7-SNAPSHOT, expected 5.0.6

   Bumped the micronaut-bom override to `5.0.7-SNAPSHOT` so it matches
   `bomDependencyVersions['groovy.version']` again (the inline comment
   already documents this invariant).

2. Code Style (Core Projects), CodeQL Analyze, macOS Build

   These three CI jobs all short-circuit on `compileGroovy` failures in
   `:grails-data-graphql-core` (and the macOS job additionally fails in
   `:grails-data-mongodb-core` because it does not stop on first failure).

   2a. `Arguable.groovy:43` and `ComplexTyped.groovy:134`

       Both files are traits that `extends ExecutesClosures` and call the
       static `withDelegate(Closure, Object)` declared on the parent trait.
       Groovy 5 `@CompileStatic` STC no longer resolves a parent trait's
       static method from a child trait that extends it:

           [Static type checking] - Cannot find matching method
           org.grails.gorm.graphql.entity.dsl.helpers.Arguable#withDelegate(
               groovy.lang.Closure, java.lang.Object)

       An explicit `ExecutesClosures.withDelegate(...)` qualification also
       fails STC ("Cannot find ... static method ExecutesClosures#withDelegate"),
       because traits compile static methods onto a `$Trait$Helper` rather
       than the trait interface. Converting `withDelegate` to an instance
       method breaks the two `static build(...)` call sites in
       `GraphQLMapping` and `GraphQLPropertyMapping`.

       Inlined the 5-line body at the two affected trait sites. The static
       `withDelegate` on `ExecutesClosures` is left untouched so all
       implementing-class call sites (`GraphQLMapping`, `GraphQLPropertyMapping`,
       `LazyGraphQLMapping`, `ComplexArgument`, `ComplexOperation`,
       `ComplexGraphQLProperty`) keep working without changes.

   2b. `PersistentEntityCodec.groovy:404-405`

       The embedded-update branch added in `e50bf4ff42` introduced a new
       call site for `PropertyEncoder#encode(...)` that declared the local
       variable as `PropertyEncoder<? extends PersistentProperty>`. Under
       Groovy 5 STC, calling `.encode(..., prop, ...)` through a receiver
       with `capture-of ? extends PersistentProperty` does not accept a
       plain `PersistentProperty` argument:

           [Static type checking] - Cannot call
           org.grails.datastore.bson.codecs.PropertyEncoder#encode(...
           capture-of ? extends PersistentProperty, ...) with arguments [...
           PersistentProperty, ...]

       Switched this site to the existing pattern already used in two
       other branches of the same method (lines 267-268 and 358-359):
       declare `propKind` as `Class<? extends PersistentProperty>` and
       erase the wildcard via an unchecked
       `(PropertyEncoder<PersistentProperty>)` cast. Behaviour is
       identical to the surrounding code, which is already exercised by
       the existing test suite.

Verification

Ran locally (Groovy 5.0.7-SNAPSHOT, JDK 21, Windows):

    .\gradlew :grails-data-graphql-core:compileGroovy              # BUILD SUCCESSFUL
    .\gradlew :grails-data-mongodb-core:compileGroovy              # BUILD SUCCESSFUL
    .\gradlew :grails-micronaut:validateDependencyVersions         # BUILD SUCCESSFUL
    .\gradlew validateDependencyVersions                           # BUILD SUCCESSFUL (all BOMs)
    .\gradlew :grails-data-graphql:build :grails-data-mongodb:build -x test  # BUILD SUCCESSFUL

Assisted-by: claude-code:claude-opus-4-7
Groovy 5 enforces what the JVM has always enforced: generic type arguments
are erased at runtime, so `instanceof List<FieldError>` cannot be verified
and is now a compile-time error:

    DefaultGraphQLErrorsResponseHandlerSpec.groovy: 100:
    Cannot perform instanceof check against parameterized type List<FieldError>

This was the new fault-line surfaced by the previous commit unblocking the
`:grails-data-graphql-core:compileGroovy` step; the CI then proceeded into
`compileTestGroovy` and failed there.

Replaced the parameterized check with `instanceof List`. The element-type
intent is still expressed by the cast on the very next line
(`((List<FieldError>) errorsFetcher.get(mockObjectEnv)).size() == 1`)
which is what the test was actually asserting.

Verification

    .\gradlew :grails-data-graphql-core:compileTestGroovy   # BUILD SUCCESSFUL

Assisted-by: claude-code:claude-opus-4-7
@paulk-asert
Copy link
Copy Markdown
Contributor

Remaining workarounds assessment:
Row 2: Main appears to fail on old and new Groovy versions
Row 3: Reproducer seems to pass on old and new Groovy versions
Row 4: should be fixed in GROOVY-12040 (MERGED)
Row 5: has GROOVY-12041 filed with a recommendation of a small change on the Grails side.

…operty (GROOVY-12041)

GroovyPageTypeCheckingExtension recorded the taglib-namespace receiver (e.g. `g`)
in unresolvedVariable / unresolvedProperty and matched it back by AST node
identity in methodNotFound. On Groovy 5, when the compiled GSP page inherits
getProperty(String) from its base class, those callbacks no longer fire for the
namespace, so the receiver is never recorded and `g.message(...)` is rejected by
@CompileStatic with "Cannot find matching method java.lang.Object#message(...)".

Per GROOVY-12041, match the call receiver by name against the allowed taglib
namespaces in methodNotFound, independently of whether the earlier callback
fired. This is resilient to the getProperty(String) short-circuit and does not
rely on node identity. Re-enables the previously @IgnoreIf-skipped g.* prefix
cases in GspCompileStaticSpec on Groovy 5.

Standalone reproducer (matrix across 4.0.31 / 5.0.6 / 5.0.7-SNAPSHOT / 6.0.0-SNAPSHOT):
https://github.com/jamesfredley/groovy5-stc-extension-node-identity-bug

Verified: grails-gsp-core test on 5.0.7-SNAPSHOT - 21 passed, 0 failed, 2 skipped
(the 2 skips are an unrelated undeclared-variable concern, left as-is).

Assisted-by: claude-code:claude-4.8-opus
@jamesfredley
Copy link
Copy Markdown
Contributor Author

Burn-down pass following @paulk-asert's remaining-workarounds assessment

Thanks @paulk-asert. I worked through your assessment row by row, re-ran every reproducer against multiple Groovy versions (4.0.31, released 5.0.6, current 5.0.7-SNAPSHOT off GROOVY_5_0_X, and 6.0.0-SNAPSHOT), and where relevant under both indy=true / indy=false. The reproducer repos have each been corrected and pushed; details + per-repo replies follow on the individual commit comments. Summary:

# Workaround Your note Verified outcome
1 render(Map) File truthiness (issue #1) use containsKey/== null Already done; real fix, no compiler-bug claim. Kept.
2 VariableScopeVisitor / setVariableScope "Main fails on old and new" Confirmed. Reclassified - see below.
3 controller action param scope (indy=false) "Reproducer passes on old and new" Confirmed. Not standalone-reproducible - see below.
4 @Builder(SimpleStrategy) "fixed in GROOVY-12040 (MERGED)" Confirmed root cause; fix is on master only, not yet on GROOVY_5_0_X.
5 g.taglib STC "GROOVY-12041 filed, minor change on the Grails side" Fixed and removed following your recommendation.
6 Validateable trait static override GROOVY-11985 Still reproduces; reflection shim kept.

Row 2 - setVariableScope is all-version hygiene, not a Groovy 5 workaround

You're right that the reproducer's Main fails on old Groovy too. I reproduced the ClosureWriter.createClosureClass NPE on 4.0.31, 5.0.6 and 5.0.7-SNAPSHOT alike - a synthesised ClosureExpression with a null VariableScope has always NPE'd; it was just never exercised. So closureExpression.setVariableScope(new VariableScope()) in ResourceTransform / AbstractMethodDecoratingTransformation is a real, all-version fix (like the row 1 File.asBoolean one), not a Groovy-5-conditional workaround, and I've reclassified it as such. The genuinely Groovy-5-specific item is the separate VariableScopeVisitor NPE during canonicalization (:grails-datamapping-tck:compileGroovy), which is what the try/catch guard actually addresses; that piece is not isolable standalone yet.

Row 3 - not standalone-reproducible; workaround retained

Confirmed the bare try/catch reproducer passes everywhere. I went further and faithfully replayed the full wrapMethodBodyWithExceptionHandling shape (prepended allowed-methods block, the real catch that declares $method = getExceptionHandlerMethodFor(...) and references this, and the finally touching this.request) at the CANONICALIZATION phase, plus a variant injecting an unresolved local-var reference post scope-resolution. All shapes still PASS on every version/indy combination. The runtime MissingPropertyException only arises from the complete convertToMethodAction chain (param rewritten to a request-bound local + the kept parameterised method + the generated no-arg delegating wrapper) interacting with live request binding - not isolable into a standalone script. The workaround stays; note it also covers an unrelated SiteMesh3 + Spring 7 incompatibility, so it can't be removed on the Groovy axis alone.

Row 4 - GROOVY-12040 confirmed, fix not yet on GROOVY_5_0_X

Confirmed the root cause is @Builder carrying @Retention(SOURCE) since 5.0.0 (GROOVY-10855 scope-creep) - getAnnotation(Builder) returns RUNTIME/non-null on 4.0.31 and SOURCE/null on 5.0.6. GROOVY-12040 restores RUNTIME and is merged on master (fix-version 5.0.7 + 6.0.0-alpha-2), but it is not yet backported to GROOVY_5_0_X, so the current 5.0.7-SNAPSHOT this PR consumes still returns null (verified). The ConfigurationBuilder workaround therefore stays until the backport lands; I'll drop it and re-verify once a fixed snapshot is published.

Row 5 - fixed per your GROOVY-12041 recommendation, workaround removed

This one was misdiagnosed in the PR (it was framed as "node identity changes between callbacks"). Your GROOVY-12041 write-up nailed it: when the receiver inherits getProperty(String) (every compiled GSP page does), unresolvedVariable/unresolvedProperty never fire for g, so the receiver is never recorded. I reworked the reproducer so the subject inherits getProperty(String) and it now reproduces exactly your table - PASS on 4.0.31, FAIL on 5.0.6 / 5.0.7-SNAPSHOT / 6.0.0-SNAPSHOT - and a name-based-match extension repairs every cell.

Applied the Grails-side change you recommended: GroovyPageTypeCheckingExtension.methodNotFound now matches the call receiver by name against the allowed taglib namespaces instead of by recorded node identity. The three @IgnoreIf({ ... && data.gDotPrefix }) skips are removed and GspCompileStaticSpec passes on 5.0.7-SNAPSHOT (21 passed, 0 failed; the 2 remaining skips are an unrelated undeclared-variable check). This workaround is burned down.

Row 6 - GROOVY-11985 still open, shim kept

Validateable's this.defaultNullable() override is still hijacked by TraitReceiverTransformer on 5.0.6, 5.0.7-SNAPSHOT and 6.0.0-SNAPSHOT (reproducer test 2 fails on all three); the reflection shim's reflection path is the only one that honours the override. GROOVY-11985 is Open and apache/groovy#2529 isn't merged, so the shim stays.

Net

Row 5 removed (1 burn-down); Row 2 reclassified from "workaround" to "real fix". Rows 3, 4 and 6 are genuinely still required on 5.0.7-SNAPSHOT today - Row 4 will drop the moment GROOVY-12040 reaches GROOVY_5_0_X. @paulk-asert a backport of GROOVY-12040 to GROOVY_5_0_X would let us drop row 4 immediately; and any movement on GROOVY-11985 / GROOVY-12041 would close out rows 6 and the upstream side of 5.

@jamesfredley
Copy link
Copy Markdown
Contributor Author

Description updated - burn-down pass complete

I've rewritten the PR description to reflect the post-review state. Summary of what changed this session:

Removed (fixed):

  • Some fixes #5 g.taglib STC - applied @paulk-asert's GROOVY-12041 recommendation: GroovyPageTypeCheckingExtension.methodNotFound now matches the taglib receiver by name instead of by recorded node identity (which never gets recorded on Groovy 5, because the GSP page's inherited getProperty(String) suppresses the unresolvedVariable/unresolvedProperty callback). Removed the three @IgnoreIf skips; grails-gsp-core (21/0) and grails-web-gsp (13/0) are green on 5.0.7-SNAPSHOT. Pushed as 649d7bbef4.

Reclassified:

Verified still required (with fresh evidence):

Reproducers: all six repos were corrected/updated and pushed to main, and Paul's commit comments on each have replies with what was wrong / what was fixed.

Net: 6 → 4 remaining workarounds. Of the 4, #3 and #4 are tracked by JIRA (GROOVY-12040 fixed-pending-backport, GROOVY-11985 open); #1 and #2 still need upstream tickets filed (reproductions ready).

@jamesfredley jamesfredley marked this pull request as ready for review May 29, 2026 03:38
Copilot AI review requested due to automatic review settings May 29, 2026 03:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Brings in Gradle 9.5.1, Spring Boot 4.0.6, and related dependency updates.

Resolved dependencies.gradle conflict by keeping the Groovy 5 branch
additions (kotlin, liquibase-hibernate5, mockito) and adopting
mongodb 5.6.5 from 8.0.x to align with Spring Boot 4.0.6.

Assisted-by: claude-code:claude-opus-4-8
@jamesfredley jamesfredley requested a review from jdaugherty May 29, 2026 04:42
Resolve conflict in .github/workflows/groovy-joint-workflow.yml: keep the
branch's dynamic Apache Groovy branch derivation (GROOVY_<major>_0_X from
dependencies.gradle) while adopting the pinned actions/checkout SHA from
8.0.x.

Assisted-by: claude-code:claude-4.8-opus
@paulk-asert
Copy link
Copy Markdown
Contributor

GROOVY_5_0_X should have GROOVY-12040 fixed now.

…12040 backported to GROOVY_5_0_X)

GROOVY-12040 (apache/groovy#2565) restores @builder to @retention(RUNTIME) and is now
backported to GROOVY_5_0_X. Confirmed against the published 5.0.7-SNAPSHOT build
5.0.7-20260529.064309-2, whose groovy.transform.builder.Builder again carries RUNTIME
retention (the prior cached build dated 2026-05-22 still had SOURCE).

The isLikelyBuilderType() heuristic was introduced only because Class.getAnnotation(Builder)
returned null under the SOURCE-retention regression. With runtime annotation detection working
again, the heuristic and its three call-site disjuncts are removed and builder detection
reverts to the pre-Groovy-5 getAnnotation(Builder) form. The Spring 7 Map-to-typed-config
conversion fallbacks (handleConverterNotFoundException, handleConversionException) are retained;
they are independent of the Groovy version.

While auditing the remainder of the former combined row, the AbstractConstraint.getDefaultMessage
fallback was confirmed to be a genuinely required, SEPARATE Groovy 5 bug (not GROOVY-12040): on
Groovy 5 the ConstrainedProperty interface constant DEFAULT_MESSAGES is initialised with null
values (its map initialiser runs before the DEFAULT_*_MESSAGE constants it references are
assigned), so a direct DEFAULT_MESSAGES.get(code) returns null while MESSAGE_BUNDLE resolves the
same code correctly. The MESSAGE_BUNDLE fallback is kept, its comment corrected to describe the
real root cause, and a new reproducer/regression test (DefaultMessageResolutionSpec) is added to
guard it.

Verified on 5.0.7-SNAPSHOT (5.0.7-20260529.064309-2) / JDK 21:
  :grails-datastore-core:test --tests *ConfigurationBuilder*  -> 9/9 passed
  :grails-datamapping-validation:test  -> all passed, incl new DefaultMessageResolutionSpec
  :grails-datastore-core:codeStyle and :grails-datamapping-validation:codeStyle  -> green

Assisted-by: claude-code:claude-4.8-opus
@jamesfredley
Copy link
Copy Markdown
Contributor Author

Burn-down: GROOVY-12040 backported to GROOVY_5_0_X - @Builder workaround removed

Following @paulk-asert's note that GROOVY-12040 is now fixed in GROOVY_5_0_X, I verified it against the consumed snapshot and removed the ConfigurationBuilder @Builder-detection workaround on this branch (db381c1ac6).

Verification that the fix is actually in the snapshot we consume

The published 5.0.7-SNAPSHOT advanced to 5.0.7-20260529.064309-2 (2026-05-29 06:43 UTC). I inspected groovy.transform.builder.Builder in the old vs new jar directly:

Snapshot build @Builder retention
...20260522... (prior cached jar) SOURCE - workaround was genuinely needed
5.0.7-20260529.064309-2 (current) RUNTIME - workaround removable

Class.getAnnotation(Builder) is non-null again, so the isLikelyBuilderType() heuristic and its three call-site disjuncts are gone and detection reverts to the pre-Groovy-5 getAnnotation(Builder) form. The Spring 7 Map-to-typed-config conversion fallbacks in the same file are retained (independent of the Groovy version).

Verified on 5.0.7-20260529.064309-2 / JDK 21 (forced --refresh-dependencies --rerun-tasks --no-build-cache):

  • :grails-datastore-core:test --tests "*ConfigurationBuilder*" -> 9/9 passed
  • :grails-datastore-core:codeStyle -> green

Finding: the AbstractConstraint half of the old row is a separate, genuinely-required bug

Old row 3 bundled AbstractConstraint static-init together with the @Builder issue, so I tried to drop it too - and a new test immediately caught a real regression. On Groovy 5 ConstrainedProperty.DEFAULT_MESSAGES.get(code) returns null for every code (the interface-constant map initialiser runs before the DEFAULT_*_MESSAGE constants it references are assigned), while MESSAGE_BUNDLE.getString(code) resolves the same code correctly. The existing constraint specs never caught this because they all supply a MessageSource.

So the MESSAGE_BUNDLE fallback stays, now:

  • decoupled from GROOVY-12040 and listed as its own remaining workaround Untitled #3,
  • documented inline with the real root cause, and
  • guarded by a new in-tree reproducer / regression test, DefaultMessageResolutionSpec (:grails-datamapping-validation:test green). This is the standalone reproducer prior audits said was still owed; it needs its own upstream Groovy ticket.

Reproducer scan

I went through every standalone reproducer repo (groovy5-builder-simplestrategy-runtime-bug, groovy-trait-static-method-override-bug, groovy5-variablescope-canonicalization-bug, groovy5-controller-action-param-scope-bug, groovy5-stc-extension-node-identity-bug, groovy5-compiledynamic-trait-bug). The most recent @paulk-asert comments there are 2026-05-24..2026-05-27 and are all already actioned (GROOVY-12040; GROOVY-12041 g.taglib; the setVariableScope and File-truthiness all-version reclassifications). Nothing newer is outstanding.

Accepted workaround set for Grails 8 on Groovy 5 (4 remain)

# Workaround Why it stays
1 VariableScopeVisitor canonicalization guards (3 sites) NPE in the canonicalization phase still reproduces on :grails-datamapping-tck:compileGroovy; not yet filed
2 boot4-disabled-integration-test-config.gradle (5 projects) indy=false controller param-scope loss and a SiteMesh3 / Spring 7 incompatibility - cannot drop on the Groovy axis alone
3 AbstractConstraint MESSAGE_BUNDLE fallback DEFAULT_MESSAGES static-init returns null on Groovy 5 (now reproduced by DefaultMessageResolutionSpec); not yet filed
4 Validateable.resolveDefaultNullable() reflection GROOVY-11985 / apache/groovy#2529 still OPEN

#4 is the next removal once #2529 merges and a fixed snapshot publishes - @paulk-asert's earlier validation already confirmed #2529 fixes it end-to-end.

Assisted-by: claude-code:claude-4.8-opus

@testlens-app
Copy link
Copy Markdown

testlens-app Bot commented May 29, 2026

✅ All tests passed ✅

🏷️ Commit: db381c1
▶️ Tests: 14454 executed
⚪️ Checks: 35/35 completed


Learn more about TestLens at testlens.app.

}
}
project.tasks.withType(Test).configureEach {
it.jvmArgs('-Dspock.iKnowWhatImDoing.disableGroovyVersionCheck=true')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is only needed if on a snapshot? If we're going to keep this, it should be on only tests

apply {
from rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
from rootProject.layout.projectDirectory.file('gradle/test-webjar-asset-config.gradle')
from rootProject.layout.projectDirectory.file('gradle/boot4-disabled-integration-test-config.gradle')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to remove this to know your build works?

// Known blockers:
// - SiteMesh3: Decorator/layout not compatible with Spring Framework 7
// - SiteMesh3: Decorator/layout not compatible with Spring Framework 7.
// - Groovy 5 + indy=false: Controller action methods that declare parameters
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants