[
https://issues.apache.org/jira/browse/GROOVY-12041?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18083467#comment-18083467
]
Paul King commented on GROOVY-12041:
------------------------------------
h2. Root cause
Commit [1ea6927ad9|https://github.com/apache/groovy/commit/1ea6927ad9]
(2023-08-16, Eric Milles) — _GROOVY-7024, GROOVY-10985: self property
precedence_ — broadened the GROOVY-7996 dynamic-lookup heuristic in
{{StaticTypeCheckingVisitor.existsProperty}} (current source ~lines 2306–2328):
# Removed the closure-only gate ({{typeCheckingContext.getEnclosingClosure() !=
null}}) — the heuristic now applies in plain method bodies too, not only inside
closures.
# Added {{getProperty(String)}} and {{setProperty(String, Object)}} to the MOP
methods that count as evidence of dynamic resolution. Previously only
{{get(String)}}, {{set(String, Object)}} and {{propertyMissing}} were checked.
Consequence for an extension-aware compile: when
{{tryVariableExpressionAsProperty(vexp, "g")}} synthesises {{this.g}} and calls
{{existsProperty}}, the inherited {{getProperty(String)}} on the superclass is
now found. {{existsProperty}} returns {{true}}, marks the property with
{{DYNAMIC_RESOLUTION=true}}, and that metadata is copied onto {{vexp}}. STC
then treats {{g}} as resolved and *skips* the
{{handleUnresolvedVariableExpression(vexp)}} / {{handleUnresolvedProperty(pe)}}
extension path.
h2. Behavioural contract change
{quote}
*Before Groovy 5:* in a non-closure method body, a bare identifier (or
{{this.foo}}) resolvable only via the receiver's {{getProperty(String)}} was
_unresolved_ from STC's perspective. {{unresolvedVariable}} /
{{unresolvedProperty}} fired.
*Since Groovy 5:* STC pre-resolves it as a dynamic property lookup. The
callbacks do *not* fire. Instead the receiver expression carries
{{StaticTypesMarker.DYNAMIC_RESOLUTION = true}} metadata.
{quote}
This is the contract change real-world extensions (notably Grails'
[GroovyPageTypeCheckingExtension|https://github.com/apache/grails-core/blob/grails8-groovy5-sb4/grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageTypeCheckingExtension.groovy])
have tripped over, and is the reason {{GspCompileStaticSpec}} cases that use
the {{g.}} prefix are currently {{@IgnoreIf}}'d on Groovy 5 in
[apache/grails-core PR #15557|https://github.com/apache/grails-core/pull/15557].
h2. Minor secondary observation
On Groovy 5, {{unresolvedVariable}} and {{unresolvedProperty}} are invoked
*twice* for the same node where Groovy 4 invoked them once. Node identity is
preserved so identity-based set bookkeeping is unaffected, but extensions
performing side effects in those callbacks may behave differently.
> STC: unresolvedVariable/unresolvedProperty no longer fires when receiver
> inherits getProperty(String)
> -----------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12041
> URL: https://issues.apache.org/jira/browse/GROOVY-12041
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> A {{TypeCheckingDSL}} extension that captures the receiver in
> {{unresolvedVariable}} / {{unresolvedProperty}} and later matches it by node
> identity in {{methodNotFound}} stops working in Groovy 5 when the enclosing
> class inherits {{getProperty(String)}} (e.g. extends {{Script}} or a
> GSP-style {{GroovyPage}} base). The {{unresolvedVariable}} /
> {{unresolvedProperty}} callback never fires, so the extension has no chance
> to record the receiver, and the downstream {{methodNotFound}} rejects the
> chained call.
> h2. Observed behaviour
> Reproducer extension (abridged):
> {code:groovy}
> newScope { dynamicProperties = [] as Set }
> unresolvedVariable { VariableExpression ve ->
> if (ve.name == 'g') { currentScope.dynamicProperties << ve; return
> makeDynamic(ve) }
> }
> unresolvedProperty { PropertyExpression pe ->
> if (pe.propertyAsString == 'g') { currentScope.dynamicProperties << pe;
> return makeDynamic(pe) }
> }
> methodNotFound { receiver, name, argList, argTypes, call ->
> if (call.objectExpression != null
> &&
> currentScope.dynamicProperties.contains(call.objectExpression)) {
> return makeDynamic(call)
> }
> }
> {code}
> Results across versions (running {{g.message(code: 'World')}} inside the
> {{Subject}}):
> ||Subject shape||4.0.27||5.0.5 / 5.0.6||
> |Plain class, {{g.message(...)}}|PASS ({{unresolvedVariable}} fires)|PASS
> (fires twice, but works)|
> |Plain class, {{this.g.message(...)}}|PASS ({{unresolvedProperty}}
> fires)|PASS|
> |{{extends Script}}, {{g.message(...)}}|*PASS*|*FAIL* —
> {{unresolvedVariable}} never fires|
> |{{extends Base}} with {{Object getProperty(String)}},
> {{g.message(...)}}|*PASS*|*FAIL* — same|
> |{{extends Script}}, {{this.g.message(...)}}|PASS|PASS|
> The Groovy 5 failure is:
> {noformat}
> [Static type checking] - Cannot find matching method
> java.lang.Object#message(LinkedHashMap<String,String>)
> {noformat}
> Diagnostic prints confirm that on the failing cases the extension is *not*
> called for {{g}} at all; {{methodNotFound}} fires with an empty
> {{dynamicProperties}} set. Where the callbacks do fire, AST node identity is
> *preserved* across callbacks on Groovy 5 — i.e. the originally reported "node
> identity changes between callbacks" hypothesis is not what is happening.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
