[jira] [Comment Edited] (GROOVY-12063) Anonymous inner class extending Map: enclosing-class field references in the instance initializer compile to dynamic getProperty on 'this' and silently resolve to null - regression in 5.0.x

["Paul King (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GROOVY-12063?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18088451#comment-18088451 ] 

Paul King edited comment on GROOVY-12063 at 6/12/26 6:49 AM:
-------------------------------------------------------------

I know the exemplar isn't the production code, but using the preferred Groovy idiom is the other workaround:

{code:groovy}
class Holder {
    static final String VALUE = 'expected'
    static final Map<String, String> MAP = [literalKey: VALUE]
}

println "Groovy ${GroovySystem.version} -> ${Holder.MAP}"
assert Holder.MAP.get('literalKey') == 'expected'
{code}

The anonymous inner class initializer pattern. while useful for Java. is usually considered an anti-pattern for Groovy.



was (Author: paulk):
I know the exemplar isn't the production code, but using the preferred Groovy idiom is the other workaround:

{code:groovy}
class Holder {
    static final String VALUE = 'expected'
    static final Map<String, String> MAP = [literalKey: VALUE]
}

println "Groovy ${GroovySystem.version} -> ${Holder.MAP}"
assert Holder.MAP.get('literalKey') == 'expected'
{code}

The static inner initializer while useful for Java is usually an anti-pattern for Groovy.


> Anonymous inner class extending Map: enclosing-class field references in the instance initializer compile to dynamic getProperty on 'this' and silently resolve to null - regression in 5.0.x
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12063
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12063
>             Project: Groovy
>          Issue Type: Bug
>          Components: class generator, Compiler
>    Affects Versions: 6.0.0-alpha-1, 5.0.5, 5.0.6, 5.0.7
>         Environment: JDK 21; reproduces with and without @CompileStatic, and in both indy and classic codegen
>            Reporter: James Fredley
>            Priority: Major
>
> h2. Summary
> In an anonymous inner class that extends {{java.util.Map}} (e.g. {{HashMap}}), bareword references to the *enclosing class's fields* inside the AIC instance-initializer block are no longer resolved to those fields. As of Groovy 5.0.x they compile to dynamic property reads on {{this}} ({{this.getProperty("FIELD")}}); because {{this}} is a {{Map}}, the MOP turns that into a key lookup {{this.get("FIELD")}} on the still-empty map, so the reference silently evaluates to {{null}}. This is a regression: 3.0.x and 4.0.x resolve it correctly. The {{Map}} base class makes it fail *silently* - a non-Map AIC would instead throw {{MissingPropertyException}}.
> h2. Steps to reproduce
> Single self-contained script:
> {code:groovy}
> class Holder {
>     static final String VALUE = 'expected'
>     static final Map<String, String> MAP = new HashMap<String, String>() {
>         {
>             put('literalKey', VALUE)   // 'VALUE' is the enclosing class's static field
>         }
>     }
> }
> println "Groovy ${GroovySystem.version} -> ${Holder.MAP}"
> assert Holder.MAP.get('literalKey') == 'expected'
> {code}
> h2. Actual result (Groovy 5.0.5 / 5.0.6 / 5.0.7-SNAPSHOT / 6.0.0-alpha-1)
> {noformat}
> Groovy 5.0.7-SNAPSHOT -> [literalKey:null]
> Assertion failed:
> assert Holder.MAP.get('literalKey') == 'expected'
>        |      |   |                  |
>        |      |   null               false
> {noformat}
> h2. Expected result (as on Groovy 3.0.24 / 4.0.32)
> {noformat}
> Groovy 4.0.32 -> [literalKey:expected]
> (assertion passes)
> {noformat}
> h2. Affected versions
> || Groovy || Holder.MAP || Result ||
> | 3.0.24 | {{[literalKey:expected]}} | correct |
> | 4.0.32 | {{[literalKey:expected]}} | correct |
> | 5.0.5 | {{[literalKey:null]}} | broken |
> | 5.0.6 | {{[literalKey:null]}} | broken |
> | 5.0.7-SNAPSHOT | {{[literalKey:null]}} | broken |
> | 6.0.0-alpha-1 | {{[literalKey:null]}} | broken |
> Notes: reproduces for a plain class, a {{@CompileStatic}} class, and a {{@CompileStatic}} interface - so it is not specific to {{@CompileStatic}}. The literal key survives (an {{LDC}} constant); only the bareword field reference degrades to {{null}}. If both key and value are field references, the map collapses to {{[null:null]}}.
> h2. Bytecode evidence
> {{Holder$1.<init>}} (the anonymous HashMap initializer), {{javap -c}}, Groovy 5.0.7-SNAPSHOT:
> {noformat}
> 13: aload_0                                    // receiver = this (the HashMap)
> 14: ldc           "literalKey"                  // key: literal           -> correct
> 16: aload_0
> 17: invokedynamic #0:getProperty:(LHolder$1;)   // value: this.getProperty("VALUE") -> BUG
> 22: invokedynamic #1:invoke:(...)               // this.put(key, value), dynamic
> 28: return
> {noformat}
> The BootstrapMethods table confirms the call-site name is the source identifier ({{IndyInterface.bootstrap(..., "VALUE", 12)}}). The enclosing {{<clinit>}} is correct - {{VALUE}} is assigned before {{new Holder$1()}} runs - so the defect is purely the AIC initializer compiling the bareword as a dynamic read on {{this}} instead of {{GETSTATIC}}. At runtime {{MetaClassImpl.getProperty}} special-cases {{Map}} receivers as {{((Map) this).get(name)}}; the map is empty during its own initializer, so the read returns {{null}}. The JDK is not involved - the emitted bytecode is wrong.
> h2. Root cause
> * The bareword {{VALUE}} inside the AIC initializer is resolved as a dynamic property read on {{this}} ({{invokedynamic getProperty}}) rather than a {{GETSTATIC}} of the enclosing class's field.
> * Because the AIC extends {{Map}}, Groovy's MOP turns that property read into a map-key lookup {{this.get("VALUE")}}; the map is empty while its own initializer runs, so it returns {{null}} instead of failing fast.
> * Groovy 3.0.x / 4.0.x resolve the bareword to the enclosing-class field correctly.
> h2. Real-world impact
> Apache Grails {{grails.gorm.validation.ConstrainedProperty}} builds its default-messages map with exactly this idiom ({{new HashMap<String,String>() { { put(CODE, MESSAGE) } }}}); under Groovy 5 the map ends up effectively empty ({{[null:null]}}), so default validation messages fail to resolve (apache/grails-core PR #15557).
> h2. Workarounds
> * Avoid the anonymous-Map-with-instance-initializer idiom; use {{Map.of(...)}}, a literal map, or a statically compiled static block.
> * Or fully qualify the references: {{put('literalKey', Holder.VALUE)}}.
> h2. Suggested fix
> Resolve bareword identifiers inside an AIC instance-initializer against the lexical scope (enclosing-class members) before falling back to a dynamic property read on {{this}}, regardless of whether {{this}} is a {{Map}} - i.e. restore 4.0.x name-resolution for AIC initializer blocks.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)