Paul King created GROOVY-11985:
----------------------------------

             Summary: Static method override on trait implementer ignored when 
called via this in trait body
                 Key: GROOVY-11985
                 URL: https://issues.apache.org/jira/browse/GROOVY-11985
             Project: Groovy
          Issue Type: Bug
            Reporter: Paul King


{code:title=JIRA description}
h2. Summary

In Groovy 4.x, calling {{this.someStaticMethod()}} from inside a trait's static 
method body dispatched dynamically and honoured a static method override 
declared on the implementing class. In Groovy 5.x and 6.0.0-SNAPSHOT, the same 
call is rewritten by {{TraitReceiverTransformer}} to dispatch through the trait 
helper, which can never see the implementing-class override. The override is 
silently lost — no exception, no compile warning, the trait's default value 
just always wins.

This was discovered as part of the Grails 8 / Groovy 5 migration 
(apache/grails-core PRs 
[#15557|https://github.com/apache/grails-core/pull/15557] and 
[#15558|https://github.com/apache/grails-core/pull/15558]) and motivated a 
reflection-based workaround in {{Validateable.resolveDefaultNullable(Class)}}.

h2. Reproducer

Standalone repro: 
[https://github.com/jamesfredley/groovy-trait-static-method-override-bug]

{code:title=Validateable.groovy}
trait Validateable {
    static boolean defaultNullable() {
        false
    }
    static boolean defaultNullableSeenByTrait() {
        // expected to dispatch to the implementing class override
        this.defaultNullable()
    }
}
{code}

{code:title=MyNullableValidateable.groovy}
class MyNullableValidateable implements Validateable {
    static boolean defaultNullable() {
        true
    }
}
{code}

{code:title=Driver}
assert MyNullableValidateable.defaultNullable()           // direct call: true 
on every version
assert MyNullableValidateable.defaultNullableSeenByTrait() // expected true; 
gets false on 5.x / 6.0
{code}

h2. Observed behaviour

|| Groovy version || Direct call || {{this.defaultNullable()}} from trait body 
||
| 4.0.27          | true (PASS)  | true (PASS)                                  
|
| 5.0.5           | true (PASS)  | *false (FAIL)*                               
|
| 6.0.0-SNAPSHOT  | true (PASS)  | *false (FAIL)*                               
|

h2. Root cause

The bytecode emitted for the trait helper's 
{{defaultNullableSeenByTrait(Class)}} method changed shape:

{noformat}
// Groovy 4.0.27                                 // Groovy 5.0.5 / 
6.0.0-SNAPSHOT
0: aload_0                                       0: ldc          // 
Validateable$Trait$Helper.class
1: invokedynamic invoke:                         2: aload_0
   (Ljava/lang/Class;)Ljava/lang/Object;         3: invokedynamic invoke:
                                                    
(Ljava/lang/Class;Ljava/lang/Class;)Ljava/lang/Object;
{noformat}

In 4.x the indy receiver is {{aload_0}} — the implementing class — so the 
dynamic dispatch resolves {{defaultNullable()}} against 
{{MyNullableValidateable}} and finds the override. In 5.x+ the receiver is 
hard-coded {{Validateable$Trait$Helper.class}} (an {{ldc}}) and the 
implementing class is demoted to an argument, so the dispatch resolves 
{{defaultNullable(Class)}} on the trait helper itself and always lands on the 
lowered trait default.

The behaviour change was introduced by commit {{0aa78d0a33}} (GROOVY-8854, Sep 
2023):

bq. write {{T.m(p)}} as {{this.m($static$self,p)}} not {{$self.m(p)}}

That commit rewrote {{TraitReceiverTransformer.transformMethodCallOnThis}} so 
that {{this.someStaticMethod()}} inside a trait body is rewritten as {{(this | 
T$Trait$Helper).m((Class)$self.getClass(), args)}} — routed through the trait 
helper's lowered static, with the implementing class passed as the 
{{$static$self}} argument. The existing trait static method test coverage in 
{{TraitASTTransformationTest.testTraitStaticMethod}} (including the GROOVY-8854 
case at line 2218) doesn't exercise the override-on-implementing-class 
scenario, so the regression wasn't caught.

h2. Tradeoff

Groovy 5's behaviour is arguably closer to Java semantics: static methods are 
not virtual, and {{this}} inside a Java static method doesn't exist, so 
"{{this.staticMethod()}} virtually dispatches to the implementing class's 
static" was always Groovy-specific magic that relied on MOP. However:

* The Groovy 4 behaviour was depended on by real code — Grails {{Validateable}} 
is the visible canary; the same idiom likely exists elsewhere.
* The failure mode is silent — no exception, no compile warning, the trait 
default just wins every time.
* The change surfaced as a side effect of GROOVY-8854 (whose ticket was about 
something else), not as a deliberate "trait statics are no longer virtual" 
decision, and it isn't called out in the Groovy 5 release notes.

h2. Suggested options

# Restore Groovy 4 semantics for {{this.staticMethod()}} in trait bodies — emit 
a dynamic lookup whose receiver is {{$static$self}} (the implementing class) 
rather than the trait helper.
# At minimum, emit a compile-time warning when a trait body calls a same-named 
static and the dispatch will provably not hit any override on the implementing 
class.
# Document the new contract explicitly in the Groovy 5 release notes and trait 
docs, so consumers can adapt at the trait level instead of debugging silent 
no-ops.

h2. Workaround (already applied in Grails)

Bypass {{TraitReceiverTransformer}} via plain Java reflection, which it can't 
see through:

{code:groovy}
private static boolean resolveDefaultNullable(Class<?> clazz) {
    try {
        return clazz.getMethod('defaultNullable').invoke(null) as boolean
    } catch (NoSuchMethodException ignored) {
        return false
    }
}
{code}




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

Reply via email to