[
https://issues.apache.org/jira/browse/GROOVY-11986?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-11986:
-------------------------------
Description:
h2. Summary
On Groovy 6.0.0-SNAPSHOT, dynamic property access on an instance whose class
inherits a single-arg {{get(X)}} method — where {{X}} is any supertype of
{{String}} ({{Serializable}}, {{Object}}, {{CharSequence}}, {{Comparable}},
{{Cloneable}}) — is incorrectly routed through that {{get}} method as the
{{genericGetMethod}}, bypassing {{propertyMissing(String)}}. The same code
works on Groovy 4.x and 5.x.
Discovered during the Grails 8 / Groovy 6 canary ([apache/grails-core PR
#15558|https://github.com/apache/grails-core/pull/15558]). GORM's
{{GormEntity}} trait declares {{static D get(Serializable id)}} as the
entity-by-ID lookup; on Groovy 6, {{book.someConnection}}, {{book.errors}},
etc. now hit {{get(Serializable)}} with the property name as the "id" argument
instead of falling through to {{propertyMissing}}. CI failures: 16 in
{{DataServiceConnectionRoutingSpec}}, similar pattern in
{{CrossLayerMultiDataSourceSpec}}.
h2. Reproducer
Standalone repro:
[https://github.com/jamesfredley/groovy6-get-as-generic-getter]
{code:title=EntityTrait.groovy}
trait EntityTrait {
Object get(Serializable id) { "no-such-entity:${id}".toString() }
Object propertyMissing(String name) { new DelegatingApi(name: name) }
}
{code}
{code:title=Book.groovy}
@CompileStatic
class Book implements EntityTrait {
String title
}
{code}
{code:title=Driver}
def book = new Book(title: 'Effective Java')
def r = book."secondary"
// Groovy 4.x / 5.x: r is DelegatingApi[secondary] (PASS)
// Groovy 6.0: r is "no-such-entity:secondary" (FAIL — hit
get(Serializable))
{code}
h2. Observed behaviour
|| Groovy version || {{book."secondary"}} ||
| 4.0.27 | DelegatingApi (PASS) — propertyMissing wins |
| 5.0.5 | DelegatingApi (PASS) — propertyMissing wins |
| 6.0.0-SNAPSHOT | *"no-such-entity:secondary" (FAIL) — get(Serializable)
wins* |
h2. Root cause
Commit {{7bc29825bc}} (GROOVY-11829, "check param types of standard methods and
keep best fit", Jan 2026, master only) replaced the strict
{{isGenericGetMethod}} check with an assignability-based {{checkMatch}}.
*Pre-change* ({{MetaClassImpl.isGenericGetMethod}}, Groovy 4.x and 5.x):
{code:java}
return "get".equals(method.getName())
&& parameterTypes.length == 1
&& parameterTypes[0].getTheClass() == String.class; // strict equality
{code}
The candidate had to take *exactly {{String}}*. {{get(Serializable)}},
{{get(Object)}}, etc. were rejected.
*Post-change* ({{MetaClassImpl.checkMatch}} / {{checkIfStdMethod}}, Groovy
6.0.0-SNAPSHOT):
{code:java}
case "get":
if (checkMatch(metaMethod, genericGetMethod, GETTER_MISSING_ARGS)
&& metaMethod.getReturnType() != Void.TYPE) {
genericGetMethod = metaMethod;
}
private static boolean checkMatch(MetaMethod newMethod, MetaMethod oldMethod,
Class<?>[] arguments) {
return newMethod.isValidExactMethod(arguments) && (oldMethod == null
|| MetaClassHelper.calculateParameterDistance(arguments, newMethod)
<= MetaClassHelper.calculateParameterDistance(arguments,
oldMethod));
}
{code}
{{isValidExactMethod(\{String.class})}} accepts any method whose parameter is a
*supertype of {{String}}* — {{String}}, {{CharSequence}}, {{Comparable}},
{{Serializable}}, {{Object}}, {{Cloneable}}. So inherited methods like
{{get(Serializable)}} now register as the generic getter, and dynamic property
reads route through them before {{propertyMissing}} is consulted.
h2. Scope of impact
Only signatures where String is assignable to the param trip the regression.
{{get(File)}}, {{get(Long)}}, {{get(UUID)}} are unaffected. Affected:
{{get(String)}}, {{get(CharSequence)}}, {{get(Comparable)}},
{{get(Serializable)}}, {{get(Object)}}, {{get(Cloneable)}}. {{Map}}-typed
classes ({{get(Object)}}) are within the impact zone in principle but Map
dispatch is special-cased elsewhere; GORM-style {{get(Serializable)}} is the
visible casualty.
h2. GROOVY-11829's intent vs side effect
The added tests target {{def get(String name)}} and a setter-precedence case
({{set(Object, Object)}} vs {{set(String, Object)}} — the more-specific one
needs to win). The "best-fit" {{<= calculateParameterDistance}} tie-breaker is
the actual goal. Folding the getter side into the same {{isValidExactMethod}}
check inadvertently relaxed the getter's historically strict "must be
{{String}} exactly" rule.
h2. Suggested options
# Restore the strict {{String}}-exact check on the getter side, while keeping
the new best-fit logic for the setter side. (The getter regression doesn't
appear to have been a GROOVY-11829 target.)
# Or keep the assignability check but require {{parameter == String.class}} for
*initial* registration, then use {{<=}} distance for tie-breaking further
candidates.
# Add a test mirroring this scenario — class with inherited
{{get(Serializable)}} (or {{get(Object)}}) + {{propertyMissing(String)}} — so
the regression can't recur.
h2. Workaround (already applied in Grails)
Per-entity AST-injected shim that gives Groovy's "best-fit" tie-breaker a
more-specific {{get(String)}} candidate than the inherited
{{get(Serializable)}}:
{code}
public Object get(String name) {
propertyMissing(name)
}
{code}
Verified locally: with this shim added, parameter distance to {{String}} is 0
for {{get(String)}} vs ≥1 for {{get(Serializable)}}, so the new tie-breaker
picks the shim and {{book."secondary"}} correctly routes to
{{propertyMissing}}. The shim should be removed once the upstream fix lands.
was:
h2. Summary
On Groovy 6.0.0-SNAPSHOT, dynamic property access on an instance whose class
inherits a single-arg {{get(X)}} method — where {{X}} is any supertype of
{{String}} ({{Serializable}}, {{Object}}, {{CharSequence}}, {{Comparable}},
{{Cloneable}}) — is incorrectly routed through that {{get}} method as the
{{genericGetMethod}}, bypassing {{propertyMissing(String)}}. The same code
works on Groovy 4.x and 5.x.
Discovered during the Grails 8 / Groovy 6 canary ([apache/grails-core PR
#15558|https://github.com/apache/grails-core/pull/15558]). GORM's
{{GormEntity}} trait declares {{static D get(Serializable id)}} as the
entity-by-ID lookup; on Groovy 6, {{book.someConnection}}, {{book.errors}},
etc. now hit {{get(Serializable)}} with the property name as the "id" argument
instead of falling through to {{propertyMissing}}. CI failures: 16 in
{{DataServiceConnectionRoutingSpec}}, similar pattern in
{{CrossLayerMultiDataSourceSpec}}.
h2. Reproducer
Standalone repro:
[https://github.com/jamesfredley/groovy6-get-as-generic-getter]
{code:title=EntityTrait.groovy}
trait EntityTrait {
Object get(Serializable id) { "no-such-entity:${id}".toString() }
Object propertyMissing(String name) { new DelegatingApi(name: name) }
}
{code}
{code:title=Book.groovy}
@CompileStatic
class Book implements EntityTrait {
String title
}
{code}
{code:title=Driver}
def book = new Book(title: 'Effective Java')
def r = book."secondary"
// Groovy 4.x / 5.x: r is DelegatingApi[secondary] (PASS)
// Groovy 6.0: r is "no-such-entity:secondary" (FAIL — hit
get(Serializable))
{code}
h2. Observed behaviour
|| Groovy version || {{book."secondary"}} ||
| 4.0.27 | DelegatingApi (PASS) — propertyMissing wins |
| 5.0.5 | DelegatingApi (PASS) — propertyMissing wins |
| 6.0.0-SNAPSHOT | *"no-such-entity:secondary" (FAIL) — get(Serializable)
wins* |
h2. Root cause
Commit {{7bc29825bc}} (GROOVY-11829, "check param types of standard methods and
keep best fit", Jan 2026, master only) replaced the strict
{{isGenericGetMethod}} check with an assignability-based {{checkMatch}}.
*Pre-change* ({{MetaClassImpl.isGenericGetMethod}}, Groovy 4.x and 5.x):
{code:java}
return "get".equals(method.getName())
&& parameterTypes.length == 1
&& parameterTypes[0].getTheClass() == String.class; // strict equality
{code}
The candidate had to take *exactly {{String}}*. {{get(Serializable)}},
{{get(Object)}}, etc. were rejected.
*Post-change* ({{MetaClassImpl.checkMatch}} / {{checkIfStdMethod}}, Groovy
6.0.0-SNAPSHOT):
{code:java}
case "get":
if (checkMatch(metaMethod, genericGetMethod, GETTER_MISSING_ARGS)
&& metaMethod.getReturnType() != Void.TYPE) {
genericGetMethod = metaMethod;
}
private static boolean checkMatch(MetaMethod newMethod, MetaMethod oldMethod,
Class<?>[] arguments) {
return newMethod.isValidExactMethod(arguments) && (oldMethod == null
|| MetaClassHelper.calculateParameterDistance(arguments, newMethod)
<= MetaClassHelper.calculateParameterDistance(arguments,
oldMethod));
}
{code}
{{isValidExactMethod({String.class})}} accepts any method whose parameter is a
*supertype of {{String}}* — {{String}}, {{CharSequence}}, {{Comparable}},
{{Serializable}}, {{Object}}, {{Cloneable}}. So inherited methods like
{{get(Serializable)}} now register as the generic getter, and dynamic property
reads route through them before {{propertyMissing}} is consulted.
h2. Scope of impact
Only signatures where String is assignable to the param trip the regression.
{{get(File)}}, {{get(Long)}}, {{get(UUID)}} are unaffected. Affected:
{{get(String)}}, {{get(CharSequence)}}, {{get(Comparable)}},
{{get(Serializable)}}, {{get(Object)}}, {{get(Cloneable)}}. {{Map}}-typed
classes ({{get(Object)}}) are within the impact zone in principle but Map
dispatch is special-cased elsewhere; GORM-style {{get(Serializable)}} is the
visible casualty.
h2. GROOVY-11829's intent vs side effect
The added tests target {{def get(String name)}} and a setter-precedence case
({{set(Object, Object)}} vs {{set(String, Object)}} — the more-specific one
needs to win). The "best-fit" {{<= calculateParameterDistance}} tie-breaker is
the actual goal. Folding the getter side into the same {{isValidExactMethod}}
check inadvertently relaxed the getter's historically strict "must be
{{String}} exactly" rule.
h2. Suggested options
# Restore the strict {{String}}-exact check on the getter side, while keeping
the new best-fit logic for the setter side. (The getter regression doesn't
appear to have been a GROOVY-11829 target.)
# Or keep the assignability check but require {{parameter == String.class}} for
*initial* registration, then use {{<=}} distance for tie-breaking further
candidates.
# Add a test mirroring this scenario — class with inherited
{{get(Serializable)}} (or {{get(Object)}}) + {{propertyMissing(String)}} — so
the regression can't recur.
h2. Workaround (already applied in Grails)
Per-entity AST-injected shim that gives Groovy's "best-fit" tie-breaker a
more-specific {{get(String)}} candidate than the inherited
{{get(Serializable)}}:
{code}
public Object get(String name) {
propertyMissing(name)
}
{code}
Verified locally: with this shim added, parameter distance to {{{String}}} is 0
for {{get(String)}} vs ≥1 for {{get(Serializable)}}, so the new tie-breaker
picks the shim and {{book."secondary"}} correctly routes to
{{propertyMissing}}. The shim should be removed once the upstream fix lands.
> genericGetMethod registration too permissive: matches any get(X) where X is a
> supertype of String
> -------------------------------------------------------------------------------------------------
>
> Key: GROOVY-11986
> URL: https://issues.apache.org/jira/browse/GROOVY-11986
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> h2. Summary
> On Groovy 6.0.0-SNAPSHOT, dynamic property access on an instance whose class
> inherits a single-arg {{get(X)}} method — where {{X}} is any supertype of
> {{String}} ({{Serializable}}, {{Object}}, {{CharSequence}}, {{Comparable}},
> {{Cloneable}}) — is incorrectly routed through that {{get}} method as the
> {{genericGetMethod}}, bypassing {{propertyMissing(String)}}. The same code
> works on Groovy 4.x and 5.x.
> Discovered during the Grails 8 / Groovy 6 canary ([apache/grails-core PR
> #15558|https://github.com/apache/grails-core/pull/15558]). GORM's
> {{GormEntity}} trait declares {{static D get(Serializable id)}} as the
> entity-by-ID lookup; on Groovy 6, {{book.someConnection}}, {{book.errors}},
> etc. now hit {{get(Serializable)}} with the property name as the "id"
> argument instead of falling through to {{propertyMissing}}. CI failures: 16
> in {{DataServiceConnectionRoutingSpec}}, similar pattern in
> {{CrossLayerMultiDataSourceSpec}}.
> h2. Reproducer
> Standalone repro:
> [https://github.com/jamesfredley/groovy6-get-as-generic-getter]
> {code:title=EntityTrait.groovy}
> trait EntityTrait {
> Object get(Serializable id) { "no-such-entity:${id}".toString() }
> Object propertyMissing(String name) { new DelegatingApi(name: name) }
> }
> {code}
> {code:title=Book.groovy}
> @CompileStatic
> class Book implements EntityTrait {
> String title
> }
> {code}
> {code:title=Driver}
> def book = new Book(title: 'Effective Java')
> def r = book."secondary"
> // Groovy 4.x / 5.x: r is DelegatingApi[secondary] (PASS)
> // Groovy 6.0: r is "no-such-entity:secondary" (FAIL — hit
> get(Serializable))
> {code}
> h2. Observed behaviour
> || Groovy version || {{book."secondary"}} ||
> | 4.0.27 | DelegatingApi (PASS) — propertyMissing wins |
> | 5.0.5 | DelegatingApi (PASS) — propertyMissing wins |
> | 6.0.0-SNAPSHOT | *"no-such-entity:secondary" (FAIL) — get(Serializable)
> wins* |
> h2. Root cause
> Commit {{7bc29825bc}} (GROOVY-11829, "check param types of standard methods
> and keep best fit", Jan 2026, master only) replaced the strict
> {{isGenericGetMethod}} check with an assignability-based {{checkMatch}}.
> *Pre-change* ({{MetaClassImpl.isGenericGetMethod}}, Groovy 4.x and 5.x):
> {code:java}
> return "get".equals(method.getName())
> && parameterTypes.length == 1
> && parameterTypes[0].getTheClass() == String.class; // strict equality
> {code}
> The candidate had to take *exactly {{String}}*. {{get(Serializable)}},
> {{get(Object)}}, etc. were rejected.
> *Post-change* ({{MetaClassImpl.checkMatch}} / {{checkIfStdMethod}}, Groovy
> 6.0.0-SNAPSHOT):
> {code:java}
> case "get":
> if (checkMatch(metaMethod, genericGetMethod, GETTER_MISSING_ARGS)
> && metaMethod.getReturnType() != Void.TYPE) {
> genericGetMethod = metaMethod;
> }
> private static boolean checkMatch(MetaMethod newMethod, MetaMethod oldMethod,
> Class<?>[] arguments) {
> return newMethod.isValidExactMethod(arguments) && (oldMethod == null
> || MetaClassHelper.calculateParameterDistance(arguments, newMethod)
> <= MetaClassHelper.calculateParameterDistance(arguments,
> oldMethod));
> }
> {code}
> {{isValidExactMethod(\{String.class})}} accepts any method whose parameter is
> a *supertype of {{String}}* — {{String}}, {{CharSequence}}, {{Comparable}},
> {{Serializable}}, {{Object}}, {{Cloneable}}. So inherited methods like
> {{get(Serializable)}} now register as the generic getter, and dynamic
> property reads route through them before {{propertyMissing}} is consulted.
> h2. Scope of impact
> Only signatures where String is assignable to the param trip the regression.
> {{get(File)}}, {{get(Long)}}, {{get(UUID)}} are unaffected. Affected:
> {{get(String)}}, {{get(CharSequence)}}, {{get(Comparable)}},
> {{get(Serializable)}}, {{get(Object)}}, {{get(Cloneable)}}. {{Map}}-typed
> classes ({{get(Object)}}) are within the impact zone in principle but Map
> dispatch is special-cased elsewhere; GORM-style {{get(Serializable)}} is the
> visible casualty.
> h2. GROOVY-11829's intent vs side effect
> The added tests target {{def get(String name)}} and a setter-precedence case
> ({{set(Object, Object)}} vs {{set(String, Object)}} — the more-specific one
> needs to win). The "best-fit" {{<= calculateParameterDistance}} tie-breaker
> is the actual goal. Folding the getter side into the same
> {{isValidExactMethod}} check inadvertently relaxed the getter's historically
> strict "must be {{String}} exactly" rule.
> h2. Suggested options
> # Restore the strict {{String}}-exact check on the getter side, while keeping
> the new best-fit logic for the setter side. (The getter regression doesn't
> appear to have been a GROOVY-11829 target.)
> # Or keep the assignability check but require {{parameter == String.class}}
> for *initial* registration, then use {{<=}} distance for tie-breaking further
> candidates.
> # Add a test mirroring this scenario — class with inherited
> {{get(Serializable)}} (or {{get(Object)}}) + {{propertyMissing(String)}} — so
> the regression can't recur.
> h2. Workaround (already applied in Grails)
> Per-entity AST-injected shim that gives Groovy's "best-fit" tie-breaker a
> more-specific {{get(String)}} candidate than the inherited
> {{get(Serializable)}}:
> {code}
> public Object get(String name) {
> propertyMissing(name)
> }
> {code}
> Verified locally: with this shim added, parameter distance to {{String}} is 0
> for {{get(String)}} vs ≥1 for {{get(Serializable)}}, so the new tie-breaker
> picks the shim and {{book."secondary"}} correctly routes to
> {{propertyMissing}}. The shim should be removed once the upstream fix lands.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
