paulk-asert commented on PR #2842: URL: https://github.com/apache/groovy/pull/2842#issuecomment-5494204819
**Summary of what this PR now includes** (after the review rework; the branch is squashed to a single commit covering GROOVY-12314 plus a directly-related GROOVY-12310 fix that builds on the same machinery). **Runtime / MOP (GROOVY-12314).** Property or attribute access to a field whose reflective access cannot be forced — e.g. a non-public field of a strongly encapsulated JDK class, absent `--add-opens` — now degrades to the normal missing-member handling (`MissingPropertyException` / `MissingFieldException`, and `ReadOnlyPropertyException` on the write side), as in 4.x, instead of `GroovyBugError` or a raw `IllegalAccessException`. Following the review discussion, this ended up with **no cached, caller-independent accessibility verdict anywhere**: selection in `MetaClassImpl` no longer predicts what reflection can do, and the reflective path reports its own failure at the point of use (conversions are gated on an `IllegalAccessException` cause, so genuine errors still propagate). The `isAccessEstablishable` API that earlier iterations added to `CachedField` is gone. `makeAccessible` is attempted at most once, race-safely (the attempt completes before being recorded — a stress- test-caught fix; the once-only latch briefly made a concurrent first access observable as a spurious missing property). **Runtime / indy.** Per the `ARCHITECTURE.md` note in #2843, the call-site lookup is the sole access authority: field read and write handles unreflect against `callSite.getLookup()` (never a lookup fabricated from Groovy's own module), so e.g. a `FilterReader` subclass reaches the inherited protected `in` field through its own access rights — the case that motivated the ticket. On refusal, the handle is left unset and the full MOP adapter path applies; the earlier retry block that reached around the metaclass is gone. Map semantics are unaffected by construction: map-entry-over-non-public-field precedence (GROOVY-5001/5491/11367) is enforced solely by the metaclass ordering, and is now pinned by an additional `MapTest` case for `HashMap` subclass property *reads*. **STC.** Two accessibility gaps behind property syntax are now compile-time errors, matching javac: - *GROOVY-12314:* an inaccessible package-private or protected field declared by the receiver's own class reports `Cannot access field: ... of class: ... from class: ...` instead of "No such property" (resolution still falls through to accessor/extension/map/list first). A private field stays hidden (GROOVY-12290), and an inaccessible inherited field still reports missing (JLS 8.2; GROOVY-9093, GROOVY-9293). - *GROOVY-12310:* a member reached through an inaccessible **qualifying type** — the reported shape is `UIInput.PropertyKeys.localValueSet`, a public enum constant of a package-private nested enum — previously compiled to a direct reference that failed at runtime with `IllegalAccessError`; it is now rejected by the type checker (JLS 6.6.1, via a new `hasAccessToClass` check). The qualifying type is checked rather than the declaring class, so an inherited public field remains reachable through an accessible subtype. Dynamic access, class references, and subscript access are unaffected, and a type-checking extension can still resolve such a reference dynamically via `unresolvedProperty`/`makeDynamic`. The `valueOf(...)` method-call variant needs the same check in method selection and is left for a follow-up. **Classgen.** The property-access safety-net error is now well-formed: a synthetic receiver no longer yields `@ line -1, column -1`, and a type-parameter receiver reports its erasure rather than `E` (while a parameterized receiver keeps its type arguments — `List<String>`, not `List<E>`). Separately, the `size`/`length`-to-`size()` rewrite for `Collection` receivers is dropped: STC no longer admits either name unless a genuine member backs it, so `list.size` under `@CompileStatic` is now a compile error rather than a silently rewritten value — a deliberate source-breaking alignment with dynamic Groovy, which already throws for it (tests using a genuine `size` field member pin that real members still win). **Tests.** New `Groovy12314` suite (the JDK-field cases guard themselves with an assumption and skip under `--add-opens`; permissive dynamic read *and* write to open classes pinned; public-field-of-package-private-class pinned through the forced-access retry), new `Groovy12310` suite, the `MapTest` read-precedence pin, an STC extension resource for the parameterized-receiver classgen message, and updated package-scope/STC expectations for the new compile-time errors. **Behaviour changes to call out:** cross-package field misuse behind property syntax under STC is now a compile error; `list.size`/`list.length` under `@CompileStatic` no longer compile; strongly encapsulated JDK fields report missing instead of crashing. Dynamic Groovy's permissive access to open/class-path classes is unchanged throughout. **Follow-ups deliberately out of scope:** the method-path twins of the field fix (`CachedMethod` forcing access before `Lookup.unreflect` can check it, and `Java9.checkAccessible` hand-rolling module rules during linkage), the `valueOf` case of 12310, and removing the `effective*` machinery from indy — the latter expected to be subsumed by the realm-aware invokedynamic work under GEP-31 in Groovy 7. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
