GROOVY-11964: Additional multi-assignment forms

[paulk-asert]

No description provided.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.41667% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.1650%. Comparing base (f972c37) to head (a41a7a8).

Files with missing lines	Patch %	Lines
...roovy/transform/stc/StaticTypeCheckingVisitor.java	81.3187%	12 Missing and 22 partials ⚠️
...haus/groovy/runtime/MultipleAssignmentSupport.java	78.7879%	0 Missing and 7 partials ⚠️
...us/groovy/classgen/asm/BinaryExpressionHelper.java	93.1034%	2 Missing and 4 partials ⚠️
...va/org/apache/groovy/parser/antlr4/AstBuilder.java	95.8333%	1 Missing ⚠️
...m/sc/transformers/BinaryExpressionTransformer.java	85.7143%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2489        +/-   ##
==================================================
+ Coverage     67.1114%   67.1650%   +0.0536%     
- Complexity      31606      31708       +102     
==================================================
  Files            1451       1453         +2     
  Lines          122556     122878       +322     
  Branches        22006      22097        +91     
==================================================
+ Hits            82249      82531       +282     
- Misses          33224      33232         +8     
- Partials         7083       7115        +32     

Files with missing lines	Coverage Δ
...odehaus/groovy/ast/MultipleAssignmentMetadata.java	100.0000% <100.0000%> (ø)
...va/org/apache/groovy/parser/antlr4/AstBuilder.java	86.6753% <95.8333%> (+0.1653%)	⬆️
...m/sc/transformers/BinaryExpressionTransformer.java	92.1875% <85.7143%> (-0.2125%)	⬇️
...us/groovy/classgen/asm/BinaryExpressionHelper.java	89.9671% <93.1034%> (+0.3249%)	⬆️
...haus/groovy/runtime/MultipleAssignmentSupport.java	78.7879% <78.7879%> (ø)
...roovy/transform/stc/StaticTypeCheckingVisitor.java	87.5000% <81.3187%> (-0.3930%)	⬇️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blackdrag]

I have the feeling it would be better if rest where a stream. to be specific:
´´´
def s = Stream.of('a', 'b', 'c')
def (first, *rest) = s
assert first == 'a'
assert rest instanceof Stream
assert rest == s
´´´

[paulk-asert]

I was going for a simple mental model: RHS supports getAt(IntRange) or fallback to iterator(), and a Stream is only a .stream() call away.

But I agree, Stream is important, maybe it is worth have a third alternative in the model. I'll see if any complications arise.

[paulk-asert]

In the GEP, we show these lowerings:

def (h, *t) = list

becomes:

// Path A — RHS supports getAt(IntRange):
def __rhs = rhs
def h = __rhs.getAt(0)
def t = __rhs.getAt(1..-1)

 // Path B — RHS iterable but not range-indexable:
 def __it = rhs.iterator()
 def h = __it.hasNext() ? __it.next() : null
 def t = __it

For the Stream case, the simplest implementation would just detect instanceof Stream and produce a lowering something like:

// Path C — RHS is a Stream:
def __rhs = rhs
def __it  = __rhs.iterator()
def h     = __it.hasNext() ? __it.next() : null
def t     = StreamSupport.stream(
                Spliterators.spliteratorUnknownSize(__it, 0),
                false
            ).onClose { __rhs.close() }

And this has some simplifications:

• The wrapped tail is sequential and reports no spliterator characteristics (no SIZED, ORDERED, IMMUTABLE, etc.). Capturing the original characteristics requires reading __rhs.spliterator() instead of __rhs.iterator() — but they're mutually exclusive terminal-class ops, so head extraction would have to switch to Spliterator.tryAdvance, which is implementable but adds machinery that doesn't change semantics for the typical destructuring use case.
• Primitive streams (IntStream, LongStream, DoubleStream) fall back to the iterator() path unless you call .boxed().

I do think the Stream use case probably still justifies this but it increases the complexity of the mental model and the education costs in describing the limitations. The lowering above is in some sense an implementation detail. I will look at changing to above and we can over time handle more cases if demand warrants it.

[paulk-asert]

The PR now incorporates the above suggestion.

[blackdrag]

To explain a bit more my way of thinking... What if the stream is "endless" or just "big" (data does not fit in memory)? For example a generator of some kind, or a stream representing a giant file, or... We could do an iterator that wraps the stream and then works one take(1) base I guess, but in a,*b we never return the iterator, but but a list or something alike for b. Of course I am aware that everything falls apart for a,*b,c Here b cannot be the original stream, for this to work it cannot be an endless stream either. Or even a stream that produces more data, that fits in memory.

[paulk-asert]

These work if that is what you are getting at:

def (hs, *ts) = Stream.iterate(0, n -> n + 2)
assert hs == 0 && ts.limit(4).toList() == [2, 4, 6, 8]

def (hi, *ti) = Iterators.iterate(0, n -> n + 2)
assert hi == 0 && ti.take(4).toList() == [2, 4, 6, 8]

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.