[
https://issues.apache.org/jira/browse/GROOVY-12314?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110315#comment-18110315
]
ASF GitHub Bot commented on GROOVY-12314:
-----------------------------------------
testlens-app[bot] commented on PR #2842:
URL: https://github.com/apache/groovy/pull/2842#issuecomment-5495450948
## ✅ All tests passed ✅
🏷️ Commit: 9f86231443072ebd8813a408c23f6733fc8c7067
▶️ Tests: 115372 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at
[testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
> Align field-backed property access across compilation modes
> -----------------------------------------------------------
>
> Key: GROOVY-12314
> URL: https://issues.apache.org/jira/browse/GROOVY-12314
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
> Labels: breaking
>
> Found while investigating GROOVY-12305 (see also GROOVY-12290, GROOVY-9967):
> a family of edge cases where property access that resolves to a *field*
> behaved differently under dynamic Groovy, {{@TypeChecked}} and
> {{@CompileStatic}} — different values, different failure modes, or silently
> wrong values. Each item is individually minor; taken together they justify
> aligning the three modes even though some behavior changes result.
> h3. Behavior before this ticket (measured on 4.0.33, 5.1.1, 6.0.0-beta-3 and
> master)
> *A. Non-public fields of foreign (JDK) classes*
> {code:groovy}
> def list = [1, 2]
> println list.modCount // protected field inherited from
> java.util.AbstractList
> println list.elementData // package-private field of java.util.ArrayList
> println list.@modCount // attribute access
> {code}
> ||access (dynamic, no add-opens)||4.0.33||5.1.1||beta-3 / master||
> |{{list.modCount}} (protected)|MissingPropertyException|*GroovyBugError*
> "BUG! UNCAUGHT EXCEPTION: member is protected"|*GroovyBugError*|
> |{{list.elementData}}
> (package-private)|MissingPropertyException|MissingPropertyException|MissingPropertyException|
> |{{list.@modCount}}|MissingFieldException|raw IllegalAccessException|raw
> IllegalAccessException|
> The GroovyBugError was a 4→5 regression:
> {{Selector$PropertySelector.chooseMeta}} wrapped the refused
> {{MethodHandles}} lookup in GroovyBugError (vmplugin/v8/Selector.java,
> GROOVY-9144/9596 code). It was not catchable as MissingPropertyException and
> presented as an internal bug. The {{.@}} error-shape change
> (MissingFieldException → raw IllegalAccessException escaping from
> {{CachedField.getProperty}}) was likewise a 4→5 regression.
> The static modes for the same expressions: {{@TypeChecked}} compiled the
> property reads ({{storeField}} admitted the fields — {{isFieldAccessible}}'s
> exact-receiver leniency still covered package-private, and {{storeField}}
> deliberately proceeded for inaccessible protected fields), then failed at
> runtime as above. {{@CompileStatic}} failed during class generation with
> {{Access to E#modCount is forbidden @ line -1, column -1}} — an unresolved
> type parameter as the receiver name and no source position.
> With {{--add-opens java.base/java.util=ALL-UNNAMED}}, every spelling above
> works and prints the field value, on all versions.
> *B. Collection {{size}}/{{length}} classgen shortcut*
> {{StaticTypesCallSiteWriter#makeGetPropertySite}} rewrote {{size}}/{{length}}
> on Collection receivers to {{size()}} *before* getter/map-rule/field lookup.
> Measured under {{@CompileStatic}} (the dynamic column is identical on all
> four versions):
> {code:groovy}
> class C { int getSize() { 999 } } // Groovy class
> // JColl: Java class extends ArrayList<Object> with public int size = 42,
> public int length = 99
> {code}
> ||scenario (CS)||4.0.33||5.1.1||beta-3 / master||dynamic (all versions)||
> |{{List l = \[1,2\]; l.size}}|2|2|STC error|MissingPropertyException|
> |{{l.length}}|STC error|STC error|STC error|MissingPropertyException|
> |Groovy class with {{getSize()}}: {{c.size}}|999|999|999|999|
> |Groovy class, public field {{size=42}}: {{d.size}}|42|42|42|42|
> |{{List<C> l; l.size}} (element {{getSize()}})|*2*|*2*|\[999, 999\]|\[999,
> 999\]|
> |Java class, public field {{size=42}}: {{j.size}}|*1*|*1*|*1*|42|
> |Java class, public field {{length=99}}: {{j.length}}|*1*|*1*|*1*|99|
> Notes:
> * The shortcut's only mainstream feeder was STC resolving {{l.size}} to
> ArrayList's *private* {{int size}} field via the exact-receiver leniency —
> closed by GROOVY-12290 — which is why rows 1 and 5 changed in 6.0.0-beta-3
> (aligning with dynamic semantics).
> * Row 5 on 4.x/5.x was a three-way divergence: dynamic gives \[999, 999\];
> {{@TypeChecked}} returned \[999, 999\] at runtime while statically typing the
> expression {{int}} (so {{int n = l.size}} type-checked cleanly then threw
> GroovyCastException); {{@CompileStatic}} gave 2.
> * Rows 6–7 were silently wrong values on *every* version: STC legitimately
> admits the public field, but the shortcut hijacked the access to {{size()}}.
> Groovy-class receivers were immune because
> {{makeGroovyObjectGetPropertySite}} has no such shortcut — the result
> differed depending on whether the receiver class was written in Java or
> Groovy.
> h3. Changes made (one commit each)
> # *MOP*: a field whose reflective access cannot be established (new
> {{CachedField#isAccessEstablishable}}, backed by {{checkCanSetAccessible}})
> is treated as absent during meta-property selection — property get/set and
> attribute get/set — so the normal missing-member handling applies. The indy
> selectors degrade to the generic MetaProperty or the sender-aware adapter
> path instead of throwing GroovyBugError. Forceable access (open modules,
> class-path classes, {{--add-opens}}) is unaffected. This commit alone fixes
> the 5.x GroovyBugError/IllegalAccessException regressions and is a back-port
> candidate for GROOVY_5_0_X.
> # *STC*: the GROOVY-12290 rule extended — the exact-receiver leniency no
> longer admits plain property syntax to *any* field of a foreign nest that
> Java access rules reject (was: private only), and an inaccessible protected
> field no longer backs a property at all. Resolution falls through to
> accessors, extensions or the map/list handling, so the checker rejects at
> compile time what the dynamic MOP reports as missing at run time. Escape
> hatches unchanged: attribute access, closure bodies, delegate-resolved
> access, nest-mates, and everything Java admits (same package, protected from
> a subclass).
> # *Classgen*: the Collection {{size}}/{{length}}-to-{{size()}} rewrite
> removed. Post-GROOVY-12290 it was vestigial, and the cases still reaching it
> produced wrong values; a public {{size}}/{{length}} field now resolves
> through the normal field handling.
> # *Tests*: the two {{DifferentPackageTest}} scenarios now expect the
> positioned type-checking error ("No such property") instead of class
> generation's "Access to ... is forbidden".
> # *Classgen*: the safety-net "Access to ... is forbidden" error now reports
> the placeholder's erasure (was "E") and falls back to the current statement's
> position (was line -1, column -1).
> # *indy*: the metaclass skips fields reflection cannot force, but that
> constraint belongs to the classic ({{Field.get}}) access path only — a sender
> that passes Java's access rules (e.g. a {{FilterReader}} subclass reading the
> protected {{in}} field) still reaches the field through its own
> {{MethodHandles}} lookup, exactly like javac-emitted bytecode. When the
> effective meta property comes back as a fallback, the property-get selector
> retries the raw field via the sender lookup before binding the fallback.
> (Selection itself must stay sender-blind: classic API calls pass the receiver
> class as the sender, which would otherwise grant phantom privileges.)
> h3. Resulting behavior (verified)
> ||scenario (no add-opens)||dynamic||@TypeChecked||@CompileStatic||
> |{{list.size}} / {{list.length}}|MissingPropertyException|STC error|STC error|
> |{{list.modCount}} read (protected, foreign sender)|MissingPropertyException
> (was BUG!)|STC error (was compiles→BUG!)|STC error (was line -1 classgen
> error)|
> |{{list.elementData}} read (package-private)|MissingPropertyException|STC
> error (was compiles→runtime MPE)|STC error (was line -1 classgen error)|
> |{{list.modCount}} write|ReadOnlyPropertyException (was BUG!-adjacent; 4.x
> threw IllegalArgumentException)|STC error|STC error|
> |{{list.@modCount}} read / write|MissingFieldException (was raw IAE)|STC
> error "Cannot access field" (unchanged)|STC error (unchanged)|
> |protected field of super class from a *subclass* (e.g.
> {{FilterReader#in}})|works (sender lookup)|works|works|
> |Groovy {{getSize()}} / Groovy public field / element spread|999 / 42 /
> \[999, 999\]|999 / 42 / \[999, 999\]|999 / 42 / \[999, 999\]|
> |Java public field {{size}}/{{length}}|42 / 99|42 / 99|42 / 99 (was 1)|
> ||with --add-opens||dynamic||@TypeChecked||@CompileStatic||
> |{{list.modCount}}|field value (kept)|STC error|STC error|
> |{{list.@modCount}}|field value|field value|field value|
> All three modes agree on every row: the same value, or the static modes
> rejecting at compile time exactly what dynamic reports as missing at run
> time. All failures are well-formed — catchable
> MissingPropertyException/MissingFieldException/ReadOnlyPropertyException at
> runtime, positioned STC errors at compile time; no GroovyBugError, no "line
> -1" classgen errors. The one deliberate asymmetry: with {{--add-opens}},
> dynamic property syntax can still read a protected field, while the static
> modes require the explicit {{.@}} spelling.
> h3. Behavior changes (accepted as the price of alignment)
> * {{@TypeChecked}}/{{@CompileStatic}} code reading package-private/protected
> foreign fields via property syntax stops compiling (previously it failed at
> runtime or with a malformed classgen error; {{.@}} remains rejected
> statically as before, and dynamic {{.@}} works under {{--add-opens}}).
> * Cross-package {{@PackageScope}} field misuse now fails during type checking
> with "No such property" instead of during class generation with "Access to
> ... is forbidden".
> * {{@CompileStatic}} on a Java Collection class with a public
> {{size}}/{{length}} field changes from the element count to the field value
> (bug fix, but observable).
> * A dynamic property *write* to a strongly encapsulated field now throws
> ReadOnlyPropertyException (accurate: the field exists but cannot be written
> that way; previously a raw IllegalAccessException-based failure,
> IllegalArgumentException on 4.x).
> * Already shipped in 6.0.0-beta-3 via GROOVY-12290, noted here for the
> migration notes: {{list.size}} under {{@CompileStatic}} is now a compile
> error; on 4.x/5.x it compiled and returned the element count.
> Validated with the full core test suite (17,244 tests) plus the complete
> scenario matrix above run against 4.0.33, 5.1.1, 6.0.0-beta-3 and the patched
> build, with and without {{--add-opens}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
