[
https://issues.apache.org/jira/browse/GROOVY-11986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18078168#comment-18078168
]
Jochen Theodorou commented on GROOVY-11986:
-------------------------------------------
Actually I am not sure about the rationale of why the generic get and set was
added. I vaguely remember there was a usecase for it. Maybe it really was to
actually support map.foo. If so then get(Object) and set(Object,Object)
absolutely make sense from the perspective back then because that is what is
available already.
> 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
> Assignee: Paul King
> Priority: Major
> Fix For: 6.0.0-alpha-1
>
>
> 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)
