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.
--
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]
