jamesfredley commented on PR #15557: URL: https://github.com/apache/grails-core/pull/15557#issuecomment-4625879243
## Upstream Groovy bugs filed + root-cause records corrected The two previously-unfiled Groovy 5 defects in this PR now have tickets, each with a self-contained pure-Groovy reproducer (no Grails dependency), verified as a regression across 4.0.x -> 5.0.5 / 5.0.6 / 5.0.7-SNAPSHOT / 6.0.0-alpha-1 on JDK 21: - **[GROOVY-12062](https://issues.apache.org/jira/browse/GROOVY-12062)** - `OptimizingStatementWriter`'s `__$stMC` slow branch resolves a try-block local as `getProperty` when the method has a non-empty `finally` (classic `indy=false` codegen). This is the controller-action parameter-scope bug; the `ClassNodeSkip` workaround stays. - **[GROOVY-12063](https://issues.apache.org/jira/browse/GROOVY-12063)** - an anonymous inner class extending `Map` resolves enclosing-class field references in its instance initializer as a dynamic `getProperty` on `this`, which the MOP turns into a key lookup on the still-empty map (so every value is `null`). This is the `ConstrainedProperty.DEFAULT_MESSAGES` bug. ### Records corrected (commit `9b47e14932`) The earlier root-cause explanations were wrong and have been corrected in the description and in-code comments: - The GROOVY-12063 story was **not** "the interface field-initializer order changed / the map initialiser runs before the constants are assigned". A `javap` of the Groovy 5 bytecode shows the interface `<clinit>` assigns the `DEFAULT_*_MESSAGE` constants *before* the map is built; the real cause is the Map-MOP `getProperty` dispatch above. Corrected in `AbstractConstraint`, `DefaultMessageResolutionSpec`, and the PR description. - The GROOVY-12062 story was earlier (in now-deleted `boot4-disabled` comments) attributed to `TryCatchStatement` re-parenting / `VariableScopeVisitor` not rebinding. The real trigger is the non-empty `finally` causing a `CompileStack` scope restore that drops the locals between the two `__$stMC` emissions. Corrected in the `ControllerActionTransformer` comment + PR description. ### GROOVY-12063 now fixed at the source, not patched downstream Now that the mechanism is understood, the workaround is the most Grails-idiomatic form rather than a symptom patch: - `ConstrainedProperty.DEFAULT_MESSAGES` is built with a **Groovy map literal** instead of the `new HashMap() {{ put(...) }}` idiom, so the constant references resolve against the interface scope (correct on every Groovy version, for **all** consumers of the map). - The `AbstractConstraint.getDefaultMessage` `MESSAGE_BUNDLE` fallback is **reverted to the base `8.0.x` form** - it was only ever a symptom patch. - Fixed a latent gap surfaced in review: `default.not.unique.message` (used by `UniqueConstraint`) was missing from `DEFAULT_MESSAGES`. The map now covers all 16 keys in `DefaultErrorMessages.properties`, and `DefaultMessageResolutionSpec` now iterates the whole bundle (`MESSAGE_BUNDLE.keySet()`) so a missing key can never silently recur. `:grails-datamapping-validation:test` is green (16/16). Assisted-by: claude-code:claude-4.8-opus -- 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]
