Paul King created GROOVY-12093:
----------------------------------
Summary: Static method override on trait implementer ignored when
called via this in trait body (cont'd)
Key: GROOVY-12093
URL: https://issues.apache.org/jira/browse/GROOVY-12093
Project: Groovy
Issue Type: Bug
Reporter: Paul King
Assignee: Paul King
Fix For: 6.0.0-alpha-2, 5.0.7
Analysis by AI of the description in the Grails canary build comments yielded
the following:
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:java|title=Validateable.groovy}
trait Validateable {
static boolean defaultNullable() {
false
}
static boolean defaultNullableSeenByTrait() {
// expected to dispatch to the implementing class override
this.defaultNullable()
}
}
{code}
{code:java|title=MyNullableValidateable.groovy}
class MyNullableValidateable implements Validateable {
static boolean defaultNullable() {
true
}
}
{code}
{code:java|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):
{quote}write {{T.m(p)}} as {{this.m($static$self,p)}} not {{$self.m(p)}}
{quote}
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)
