[
https://issues.apache.org/jira/browse/GROOVY-12412?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12412:
-------------------------------
Description:
In a subscript with several indices, a {{Range}} with a negative bound is no
longer resolved
against the size when the receiver is a {{List}}. The range is walked literally
instead.
{code:groovy}
assert (1..8)[3..-1] == [4, 5, 6, 7, 8] // single range:
still correct
assert 'abcdefgh'[0..2, 3..-1] == 'abcdefgh' // String: still
correct
assert (1..8)[0..2, 3..-1] == [1, 2, 3, 4, 5, 6, 7, 8] // fails from 5.0.0
// actual: [1, 2, 3, 4, 3, 2, 1, 8] -- 3..-1 walked as the indices 3, 2, 1, 0,
-1
{code}
An all-positive multi-index subscript is unaffected: {{(1..8)[0..2, 3..7]}}
still gives the
whole list.
h3. Scope
|| receiver || 4.0.33 || 6.0.0-RC-3 ||
| {{List}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} |
| {{Integer[]}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} |
| {{String}} | {{abcdefgh}} | {{abcdefgh}} |
| {{ListWithDefault}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 5, 6, 7,
8]}} |
| {{int[]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} |
Only the {{List}} row changed. 5.0.6 behaves the same as 6.0.0-RC-3. The
{{int[]}} row was
already inconsistent in 4.x through the primitive-array path, which looks like
a separate and
older problem.
h3. Cause
Commit 87b4109418 ("minor refactor - no need to funnel through InvokerHelper
here",
2022-08-24, no JIRA reference) changed one line in
{{DefaultGroovyMethods.getAt(List, Collection)}}:
{code:java}
- answer.addAll((List<T>) InvokerHelper.invokeMethod(self, "getAt", value));
+ answer.addAll(getAt(self, (Collection) value));
{code}
The dynamic call let the MOP select the most specific overload for the value's
runtime type, so
a {{Range}} element reached {{getAt(List, Range)}}, which resolves the bounds
through
{{subListBorders}}. The direct call binds {{getAt(List, Collection)}} at
compile time, so it
recurses and treats the range's own elements as indices.
The commit was never applied to {{GROOVY_4_0_X}}, so the 4.0.x line is
unaffected. It first
shipped in 5.0.0.
The sibling method immediately above it, {{getAt(ListWithDefault,
Collection)}}, still uses the
{{InvokerHelper.invokeMethod}} form, which is why {{ListWithDefault}} continues
to resolve
negative bounds. That asymmetry within the same file, together with the "minor
refactor"
wording, suggests the behaviour change was not intended.
h3. Suggested fix
Restore overload selection for {{Range}} values, either by reinstating the
dynamic call or by
testing for {{Range}} before the {{Collection}} branch, and add a test covering
a negative bound
in a multi-index subscript for {{List}}, {{Object[]}}, {{String}} and
{{ListWithDefault}}.
Found while re-testing the examples in the
[Using Gatherers with Groovy|https://groovy.apache.org/blog/groovy-gatherers]
blog post, where
{{assert (1..8)[0..2, 3..-1] == 1..8}} passes under Groovy 4 and fails from
5.0.0.
> getAt(List, Collection) walks a nested Range literally instead of resolving
> negative indices
> --------------------------------------------------------------------------------------------
>
> Key: GROOVY-12412
> URL: https://issues.apache.org/jira/browse/GROOVY-12412
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> In a subscript with several indices, a {{Range}} with a negative bound is no
> longer resolved
> against the size when the receiver is a {{List}}. The range is walked
> literally instead.
> {code:groovy}
> assert (1..8)[3..-1] == [4, 5, 6, 7, 8] // single range:
> still correct
> assert 'abcdefgh'[0..2, 3..-1] == 'abcdefgh' // String: still
> correct
> assert (1..8)[0..2, 3..-1] == [1, 2, 3, 4, 5, 6, 7, 8] // fails from 5.0.0
> // actual: [1, 2, 3, 4, 3, 2, 1, 8] -- 3..-1 walked as the indices 3, 2, 1,
> 0, -1
> {code}
> An all-positive multi-index subscript is unaffected: {{(1..8)[0..2, 3..7]}}
> still gives the
> whole list.
> h3. Scope
> || receiver || 4.0.33 || 6.0.0-RC-3 ||
> | {{List}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} |
> | {{Integer[]}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}}
> |
> | {{String}} | {{abcdefgh}} | {{abcdefgh}} |
> | {{ListWithDefault}} | {{[1, 2, 3, 4, 5, 6, 7, 8]}} | {{[1, 2, 3, 4, 5, 6,
> 7, 8]}} |
> | {{int[]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} | {{[1, 2, 3, 4, 3, 2, 1, 8]}} |
> Only the {{List}} row changed. 5.0.6 behaves the same as 6.0.0-RC-3. The
> {{int[]}} row was
> already inconsistent in 4.x through the primitive-array path, which looks
> like a separate and
> older problem.
> h3. Cause
> Commit 87b4109418 ("minor refactor - no need to funnel through InvokerHelper
> here",
> 2022-08-24, no JIRA reference) changed one line in
> {{DefaultGroovyMethods.getAt(List, Collection)}}:
> {code:java}
> - answer.addAll((List<T>) InvokerHelper.invokeMethod(self, "getAt",
> value));
> + answer.addAll(getAt(self, (Collection) value));
> {code}
> The dynamic call let the MOP select the most specific overload for the
> value's runtime type, so
> a {{Range}} element reached {{getAt(List, Range)}}, which resolves the bounds
> through
> {{subListBorders}}. The direct call binds {{getAt(List, Collection)}} at
> compile time, so it
> recurses and treats the range's own elements as indices.
> The commit was never applied to {{GROOVY_4_0_X}}, so the 4.0.x line is
> unaffected. It first
> shipped in 5.0.0.
> The sibling method immediately above it, {{getAt(ListWithDefault,
> Collection)}}, still uses the
> {{InvokerHelper.invokeMethod}} form, which is why {{ListWithDefault}}
> continues to resolve
> negative bounds. That asymmetry within the same file, together with the
> "minor refactor"
> wording, suggests the behaviour change was not intended.
> h3. Suggested fix
> Restore overload selection for {{Range}} values, either by reinstating the
> dynamic call or by
> testing for {{Range}} before the {{Collection}} branch, and add a test
> covering a negative bound
> in a multi-index subscript for {{List}}, {{Object[]}}, {{String}} and
> {{ListWithDefault}}.
> Found while re-testing the examples in the
> [Using Gatherers with Groovy|https://groovy.apache.org/blog/groovy-gatherers]
> blog post, where
> {{assert (1..8)[0..2, 3..-1] == 1..8}} passes under Groovy 4 and fails from
> 5.0.0.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
