[
https://issues.apache.org/jira/browse/GROOVY-11986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18077935#comment-18077935
]
Paul King commented on GROOVY-11986:
------------------------------------
[~emilles] Here is AI's analysis:
{quote}
Two adjacent facts can give the impression get(Object) was supported:
1. The SET side is genuinely permissive. The corresponding isGenericSetMethod
in MetaClassHelper, going back to the SVN-era ded16e671d commit, only checks
name == "set" && params.length == 2 — any types. So set(Object, Object),
set(String, Object), set(Foo, Bar) all register. The asymmetry with the GET
side is real and longstanding.
2. Object get(Object key) does still work via direct invocation. c.get('foo')
finds it through normal method dispatch with String→Object widening. It just
isn't reachable as the property syntax c.foo because the property-resolution
path only consults genericGetMethod, which has always been exact-String.
Should we change to "superclass yes, superinterface no"?
* That's a strange API contract to document. It would silently shadow
propertyMissing(String) on every class that defines get(Object) — which is just
about every map-like / cache / id-lookup class. The original GROOVY-11986
reproducer is exactly this scenario (a GORM-style get(Serializable) on a trait
shadowing propertyMissing). The same hazard exists for get(Object).
* There's no test in the repo that asserts get(Object) is a generic getter, and
the empirical history says it never was.
{quote}
Do you remember what we were trying to account for? I looked at the current
changes at the time, but can't remember the details anymore.
> 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)
