[
https://issues.apache.org/jira/browse/GROOVY-12534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137094#comment-18137094
]
ASF GitHub Bot commented on GROOVY-12534:
-----------------------------------------
paulk-asert commented on PR #3058:
URL: https://github.com/apache/groovy/pull/3058#issuecomment-6599659848
+1 on the approach. I ran a cross-version probe of the JIRA reproducer,
Jochen's `People` case and a few neighbours (dynamic / TC / CS on 2.4.12,
2.5.23, 3.0.9, 3.0.25, 4.0.5, 4.0.24–4.0.33, 5.0.8, 5.1.3, 6.0.0, master and
this PR, JDK 17).
For reference, dynamic Groovy (unchanged since at least 2.4.12) treats
`list.prop` as `list*.prop` unless the list itself has `prop`, and a field,
getter or extension getter on the list all count. An explicit `list*.prop`
always reads the elements. Dynamic and TC agree in every version tested.
With this PR, CS now follows the same rule:
- `words.joined` returns `'x,y'`.
- `[].joined` returns `''`, not `[]`.
- `people*.name`, where the list declares `name`, returns `[Alice, Bob]`.
Every version back to 2.4 gave `'the collection'`.
- On a List subclass whose `name` comes from an extension method, both
`.name` and `*.name` now work.
A list subclass that declares only a one-argument setter still writes the
list, not its elements, in every mode.
A few observations:
1. **Set subclasses.** `class PeopleSet extends LinkedHashSet<Person> {
String name = 'the set' }` then `s*.name` under CS throws `GroovyCastException`
(Person to PeopleSet) on master and with this PR. It worked up to 4.0.5 and
fails from at least 4.0.24. Spread over a List subclass with an extension
`name` regressed in the same window, and this PR fixes that one. The likely
cause is 9e2ed767e3 (GROOVY-5568/10815, 4.0.7), though I haven't confirmed it.
The new spread skip in STC is limited to `LIST_TYPE`, which is why Sets still
fail. I'm happy for that to be a follow-up issue rather than widening this PR.
2. **Mismatched extension generics.** The one case where the modes still
differ on "does the list have the property" is an extension whose receiver
generics don't match, e.g. `getJoined(List<String>)` used on a `List<Row>`.
Dynamic matches by erasure and picks the extension, then throws
`ClassCastException`; TC does the same at runtime. CS respects the generics and
reads each element (`['p','p']`). That's the same in every version, and I think
CS's answer is the right one. Since the test asserts `rows.joined ==
['p','p']`, a comment saying that value is CS-specific would stop it reading as
cross-mode semantics.
3. **Test extensions.** The new `joined` / `plural` / `arrayJoined`
extensions are global to the test JVM. There are no collisions today. Minor:
`m12n.TestStringExtension` now depends on `bugs.Groovy12534People`.
4. **Backport.** If this goes to 6.0.x (or 5.x), the CS results above change
for existing code, and the new results are correct. That's worth a line in the
release notes.
> SC: a property defined by an extension method of List is read from each
> element of the list
> -------------------------------------------------------------------------------------------
>
> Key: GROOVY-12534
> URL: https://issues.apache.org/jira/browse/GROOVY-12534
> Project: Groovy
> Issue Type: Bug
> Components: Static compilation
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Assignee: Daniel Sun
> Priority: Major
>
> Under {{@CompileStatic}}, a property whose getter is an extension method of
> {{List}}, or of a class that implements {{List}}, is compiled as a property
> of the list's elements: the getter is called once for each element, with the
> element cast to the extension's receiver type. With this extension module:
> {code:java}
> package demo;
> import java.util.List;
> public class ListExtension {
> public static String getJoined(List<String> self) { return
> String.join(",", self); }
> }
> {code}
> {code:groovy}
> @groovy.transform.CompileStatic
> class C {
> def test(List<String> words) { words.joined }
> }
> new C().test(['x', 'y']) // GroovyCastException: Cannot cast object 'x' with
> class 'java.lang.String' to class 'java.util.List'
> {code}
> {{words.getJoined()}} returns 'x,y' in every mode, and dynamic Groovy and
> {{@TypeChecked}} return 'x,y' for {{words.joined}} too. On an empty list,
> {{words.joined}} evaluates to {{[]}} instead of an empty string.
> {{StaticCompilationVisitor.existsProperty}} marks the property expression
> spread-safe when the getter's declaring class implements {{List}} but does
> not declare the getter, which is meant for a property of the element type.
> The declaring class of an extension method is its receiver type, which never
> declares it. Extension getters declared for {{Collection}} or {{Iterable}},
> such as {{getIndices}} and {{isSorted}}, are not affected.
> {{DefaultGroovyMethodsSTCTest.testPropertySemantics2}}, which
> {{StaticCompileDGMTest}} also runs, keeps the property forms of
> {{getSequence(List)}} and {{getString(List)}} from {{TestStringExtension}}
> commented out. Seen in 4.0.33, 5.0.8, 6.0.0 and master.
> Expected: the property calls the extension getter on the list, as in dynamic
> Groovy and {{@TypeChecked}}.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
