jamesfredley commented on PR #15558:
URL: https://github.com/apache/grails-core/pull/15558#issuecomment-4322403674
## Groovy 5 → Groovy 6 workaround audit (latest snapshot)
Re-tested every source-level Groovy 5 workaround carried into this canary
against the **latest published Apache Groovy 6.0.0-SNAPSHOT (build 508 from
`repository.apache.org/snapshots`)**. Methodology: for each carried workaround,
removed the workaround code, ran `--refresh-dependencies` to ensure the latest
snapshot artifacts, ran the affected module's full test suite, and either
confirmed the workaround removable or restored it with a failing-test citation.
Local environment: JDK 21 (Corretto 21.0.10), Gradle 9.4.1, Groovy
6.0.0-SNAPSHOT artifact
(`d63a1f6ad3d38992c5e4c540953e49135003f4a6/groovy-6.0.0-SNAPSHOT.jar`, 8113515
bytes, fetched 2026-04-26 13:47 UTC).
### ✅ Workarounds REMOVED (no longer needed on Groovy 6)
| File | Workaround removed | Module tests run on Groovy 6 | Result |
|---|---|---|---|
| `grails-datamapping-validation/.../AbstractConstraint.java` |
`getDefaultMessageFromBundle()` helper that fell back to
`MESSAGE_BUNDLE.getString(code)` when the static `DEFAULT_MESSAGES` map was
empty (Groovy 5 interface static-init order regression). Reverted to direct
`ConstrainedProperty.DEFAULT_MESSAGES.get(code)` lookup, exactly the
pre-Groovy-5 form. | `:grails-datamapping-validation:test --rerun-tasks`,
`:grails-validation:test`, `:grails-data-hibernate5-core:test` (validation
specs run there too) | All green |
### ❌ Workarounds RE-VERIFIED still required on Groovy 6 (kept; comments
updated "Groovy 5" → "Groovy 5/6")
These were tentatively removed on Groovy 6, the affected module's test suite
was re-run, and the listed failure was reproduced. Each listed test failure is
reproducible against Groovy 6.0.0-SNAPSHOT build 508:
| File | Workaround | Failing test on Groovy 6 (after removal) |
|---|---|---|
| `grails-core/.../GrailsASTUtils.java` `processVariableScopes(SourceUnit,
ClassNode, MethodNode)` | `try { ... } catch (NullPointerException) {}` around
`VariableScopeVisitor.visitMethod` |
`org.grails.plugins.web.rest.transform.ResourceTransformSpec.Test that the
resource transform creates a controller class when namespace is #namespace` -
`org.codehaus.groovy.GroovyBugError: BUG! exception in phase 'canonicalization'
... unexpected NullPointerException ... Cannot read the array length because
"<local3>" is null` at `VariableScopeVisitor.visitConstructorOrMethod:510 ->
ClassCodeVisitorSupport.visitMethod:107 ->
GrailsASTUtils.processVariableScopes:1515 -> ResourceTransform.visit:214` |
| `grails-datastore-core/.../mapping/reflect/AstUtils.groovy`
`processVariableScopes(SourceUnit, ClassNode, MethodNode)` | Same NPE catch |
`grails.gorm.services.ServiceTransformSpec.test @Query update annotation with
default transaction attributes at class level` - same canonicalization NPE at
`AstUtils.processVariableScopes:253 ->
AbstractMethodDecoratingTransformation.weaveNewMethod:243` |
|
`grails-datamapping-core/.../transform/AbstractMethodDecoratingTransformation.groovy`
| (a) `closureExpression.setVariableScope(variableScope != null ?
variableScope : new VariableScope())` null-guard. (b) `try {
scopeVisitor.visitMethod(renamedMethodNode) } catch (NullPointerException) {}`
NPE catch | Same `ServiceTransformSpec` canonicalization NPE without the catch |
| `grails-rest-transforms/.../transform/ResourceTransform.groovy` |
`urlMappingsClosure.setVariableScope(new VariableScope())` (Groovy 6
ClosureWriter still NPEs on null VariableScope, separate from the AstUtils
path) | `ResourceTransformSpec.Test that the resource transform creates a
controller class when namespace is v2` - canonicalization NPE during the
closure body's bytecode emission |
| `grails-logging/.../compiler/logging/LoggingTransformer.java` | Manual
`final Logger log = LoggerFactory.getLogger(ClassName.class)` field injection
that bypasses `LogASTTransformation.visit()`. Reverting to standard
`LogASTTransformation.visit()` reproduces the Groovy 5 NPE on every domain
class compile. | `:grails-test-examples-database-cleanup:compileGroovy` (also
`:grails-test-examples-datasources:compileGroovy`,
`:grails-test-examples-issue-698-domain-save-npe:compileGroovy`) - `BUG!
exception in phase 'canonicalization' in source unit
'.../dbcleanup/Author.groovy' Cannot read the array length because "<local3>"
is null` at `LoggingTransformer.java:84` |
| `grails-validation/.../validation/Validateable.groovy` |
`resolveDefaultNullable(Class)` reflection-based dispatch |
`grails.validation.ValidateableTraitSpec.Test that constraints are nullable by
default if overridden` - test expected `constraints.size() == 3`, got `4`. The
implementing class's `defaultNullable -> true` override was hijacked back to
the trait's `false`, so a stray `nullable: true` constraint was added to the
property that should have been left untouched |
| `grails-bootstrap/.../config/NavigableMap.groovy` | Shallow + lazy
`convertConfigObjectToMap()`; called from both `merge(Map)` and `mergeMapEntry`
| `grails.config.ConfigMapSpec.should support merging ConfigObject maps` -
`java.lang.StackOverflowError` in `NavigableMap.mergeMapEntry:292 ->
mergeMaps:168 -> mergeMapEntry:292 -> mergeMaps:168 -> ...` (infinite recursion
through `ConfigObject`'s dynamic property creation) |
### 🛡️ Kept defensively (not directly verifiable in unit tests)
| File | Workaround | Why kept |
|---|---|---|
| `grails-data-mongodb/bson/.../BsonPersistentEntityCodec.groovy` |
`resolvePropertyType()` walker for synthetic intermediate superclasses on
anonymous-inner-class properties from `MappingFactory` | BSON unit tests pass
with the simple `propertyClass.superclass` form, but the synthetic-class
scenario only surfaces during MongoDB persistence with `@CompileStatic`
anonymous inner classes. Without a live MongoDB integration run we can't
confirm Groovy 6 generates non-synthetic superclasses for these cases. The
walker is safe and idempotent (returns the same answer as direct `.superclass`
when no synthetic ancestor is in the way). |
### ⏭️ Not retested in this round (still in place from #15557)
These were not retested for time reasons; verification requires multi-module
integration tests (forge generated apps, hibernate5 functional, mongodb
functional). All remain unchanged from #15557:
- `grails-core/.../GrailsApplicationCommand.groovy` (trait → abstract class
for `@Delegate`)
- `grails-core/.../template/TemplateRendererImpl.groovy` (explicit
`Resource`/`File` type checks bypassing `@Delegate` named-arg bridge)
- `grails-scaffolding/.../GenerateControllerCommand.groovy` (explicit 4-arg
`templateRenderer.render(Resource, File, Model, boolean)` call - bypasses the
named-arg `render(Map)` `@Delegate` bridge)
- `grails-core/.../core/cfg/GroovyConfigPropertySourceLoader.groovy`
(recursive ConfigObject normalization - parallel to `NavigableMap` which was
retested and confirmed still required, so this is almost certainly still
required too; the shape of the bug is identical)
- `grails-datastore-core/.../config/ConfigurationBuilder.groovy` (Map
subtype handling, abstract/interface exclusion, `handleConversionException`
Spring 7 fallback)
- `grails-data-hibernate5/.../HibernateEntityTransformation.groovy`
(`InnerClassNode.isAssignableFrom` instead of `instanceof` - `@CompileStatic`
checkcast bytecode bug)
- `grails-geb/...` testFixtures - already retested twice in `b8ee60d460`,
`0804a4fc73`, `b0c7f347e8` and confirmed still required for the
`-PgrailsIndy=false` matrix:
- `ContainerGebConfiguration.groovy` (interface→trait,
`IncompatibleClassChangeError` from `$getCallSiteArray()` on interfaces under
indy=false consumers)
- `support/ContainerSupport.groovy` (`@CompileDynamic`, GROOVY-11907
incomplete fix for indy=false static-setter helpers)
- `WebDriverContainerHolder.groovy` (`getOverriddenProperties()`
static-type-check helper)
### Already removed in earlier branch commits (no action required)
These workarounds from #15557 were tested for removal in earlier commits on
this branch, and the removals stuck:
- `HibernateEntity` static SQL methods (commit `8af5d1dc4c`)
- `CommandLineHelper` static finals
- `JspTagImpl` `@CompileDynamic` (commit `2bb0930a5d`)
- `ClassPropertyFetcherTests` generic trait detection (commit `a71c8b5ebb`)
- `MongoCodecSession` `[name]++` arithmetic
- `GormEntityTransformation` AST shim (now unconditionally added; commit
`8e9cdbc50f`)
- GROOVY-11907 trait static members in Geb and scaffolding helpers (commit
`a290b37156`)
### Net result
- **1** Groovy 5 workaround **removed** in this round (`AbstractConstraint`).
- **7** workarounds **re-verified still needed** on Groovy 6 with
reproducible test failures cited above.
- **1** workaround kept defensively pending MongoDB integration verification.
- **6** workarounds inherited from #15557 not retested in this round;
tracked for a follow-up.
The `VariableScopeVisitor` NPE class of regressions (4 sites) is the single
largest source of carried workarounds and would unblock the most code if filed
and fixed upstream.
Assisted-by: claude-code:claude-opus-4-7
--
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]
