jamesfredley commented on PR #15557: URL: https://github.com/apache/grails-core/pull/15557#issuecomment-4570241641
## 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):** - **#5 g.taglib STC** - applied @paulk-asert's [GROOVY-12041](https://issues.apache.org/jira/browse/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:** - **old #2 `setVariableScope`** → moved to *Real bug fixes*. Per Paul, the null-`VariableScope` `ClosureWriter` NPE fails identically on Groovy 3.0.25 / 4.0.5 / 4.0.27 / 4.0.31 / 5.x / 6.x, so it's all-version AST hygiene, not a Groovy 5 workaround. **Verified still required (with fresh evidence):** - **#1 `VariableScopeVisitor` canonicalization guard** - I made the three `try/catch` guards re-throw and rebuilt on `5.0.7-SNAPSHOT`: `:grails-datamapping-tck:compileGroovy` fails with `BUG! exception in phase 'canonicalization' ... DataServiceRoutingProductDataService.groovy unexpected NullPointerException`. So the guard is genuinely needed (distinct from the `setVariableScope` fix). Needs an upstream ticket filed - clean in-grails-core repro now exists. - **#3 `@Builder`** - GROOVY-12040 is fixed on `master` (fix-version 5.0.7) but **not yet on `GROOVY_5_0_X`**; `getAnnotation(Builder)` still returns `null` on the current `5.0.7-SNAPSHOT`. Drops as soon as it backports. @paulk-asert - a cherry-pick to `GROOVY_5_0_X` would let this one go immediately. - **#4 `Validateable`** - GROOVY-11985 still OPEN (PR #2529 not merged); reproduces on 5.0.6, 5.0.7-SNAPSHOT and 6.0.0-SNAPSHOT. Reflection shim stays. - **#2 controller param-scope** - re-confirmed not isolable standalone (even the full `wrapMethodBodyWithExceptionHandling` shape passes); the workaround also independently covers the SiteMesh3 + Spring 7 gap. **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). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
