[
https://issues.apache.org/jira/browse/GROOVY-11964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18076272#comment-18076272
]
ASF GitHub Bot commented on GROOVY-11964:
-----------------------------------------
Copilot commented on code in PR #2489:
URL: https://github.com/apache/groovy/pull/2489#discussion_r3142776415
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -1374,6 +1406,298 @@ private boolean
typeCheckMultipleAssignmentAndContinue(final Expression leftExpr
return true;
}
+ /**
+ * GEP-20 rest binding under static checking. Handles both list-literal
RHS and
+ * declared-type RHS (List, array, String, Range, Set, Iterator, Stream —
anything with a
+ * typed {@code getAt(int)} or {@code iterator()} inferable element type).
The rest
+ * variable's inferred type tracks the runtime dispatch in
+ * {@code MultipleAssignmentSupport.tailRest}: {@code List<T>} for Path A
receivers
+ * (those that support {@code getAt(IntRange)}); {@code Stream<T>} for
{@code Stream}
+ * RHS (Path B', tail rest only); {@code Iterator<T>} for any other Path B
receiver
+ * (tail rest only). Head and middle rest reject Path B and Path B'
receivers as a
+ * static type error.
+ */
+ private boolean typeCheckMultipleAssignmentRest(final Expression
leftExpression, final Expression rightExpression) {
+ if (rightExpression instanceof ListExpression values) {
+ return typeCheckRestAgainstListLiteral((TupleExpression)
leftExpression, values, rightExpression);
+ }
+ // Any RHS with a statically-resolvable getAt(int) (or known shape via
declared type)
+ // routes through the declared-type path; this covers variables,
properties, method
+ // calls, constants (e.g. String literals), closure invocations, etc.
+ return typeCheckRestAgainstDeclaredType((TupleExpression)
leftExpression, rightExpression);
+ }
+
+ private boolean typeCheckRestAgainstListLiteral(final TupleExpression
tuple, final ListExpression values, final Expression rightExpression) {
+ List<Expression> tupleExpressions = tuple.getExpressions();
+ List<Expression> valueExpressions = values.getExpressions();
+ int tupleSize = tupleExpressions.size();
+ int valueSize = valueExpressions.size();
+
+ int restIndex = restIndexOf(tuple);
+ int fixedAfter = tupleSize - restIndex - 1;
+
+ // Per GEP-20: lower sizes are forgiving (getAt(int) returns null for
OOB,
+ // getAt(IntRange) returns empty for inverted ranges). Skip the strict
size check.
+
+ // Fixed slots before the rest binding: pairwise from index 0.
+ for (int i = 0; i < restIndex; i++) {
+ if (i >= valueSize) break;
+ ClassNode valueType = getType(valueExpressions.get(i));
+ ClassNode targetType = getType(tupleExpressions.get(i));
+ if (!isAssignableTo(valueType, targetType)) {
+ addStaticTypeError("Cannot assign value of type " +
prettyPrintType(valueType) + " to variable of type " +
prettyPrintType(targetType), rightExpression);
+ return false;
+ }
+ storeType(tupleExpressions.get(i), valueType);
+ }
+
+ // Fixed slots after the rest binding: anchored to the right of the
value list.
+ for (int j = 0; j < fixedAfter; j++) {
+ int lhsIdx = restIndex + 1 + j;
+ int rhsIdx = valueSize - fixedAfter + j;
+ if (rhsIdx < 0 || rhsIdx >= valueSize) continue;
+ ClassNode valueType = getType(valueExpressions.get(rhsIdx));
+ ClassNode targetType = getType(tupleExpressions.get(lhsIdx));
+ if (!isAssignableTo(valueType, targetType)) {
+ addStaticTypeError("Cannot assign value of type " +
prettyPrintType(valueType) + " to variable of type " +
prettyPrintType(targetType), rightExpression);
+ return false;
+ }
+ storeType(tupleExpressions.get(lhsIdx), valueType);
+ }
+
+ // Rest position: container type derived from the LUB of absorbed
value types.
+ int restFromIdx = Math.min(restIndex, valueSize);
+ int restToIdx = Math.max(restFromIdx, valueSize - fixedAfter);
+ Expression restExpr = tupleExpressions.get(restIndex);
+ ClassNode elementType;
+ if (restToIdx > restFromIdx) {
+ elementType = getType(valueExpressions.get(restFromIdx));
+ for (int k = restFromIdx + 1; k < restToIdx; k++) {
+ elementType = lowestUpperBound(elementType,
getType(valueExpressions.get(k)));
+ }
+ } else {
+ // Rest absorbs zero values (e.g. `def (Integer a, List<Integer>
*t, Integer b) = [1, 2]`).
+ // Default to OBJECT_TYPE, but if the rest binder is typed with a
single generic
+ // argument, use that as the element type so `List<Integer> *t`
doesn't get rejected
+ // as `List<Object>` not assignable to `List<Integer>`.
+ elementType = OBJECT_TYPE;
+ if (restExpr instanceof Variable v) {
+ ClassNode declared = v.getOriginType();
+ if (declared != null && !ClassHelper.isDynamicTyped(declared)
&& !declared.equals(OBJECT_TYPE)) {
+ GenericsType[] gts = declared.getGenericsTypes();
+ if (gts != null && gts.length == 1) {
+ elementType = getCombinedBoundType(gts[0]);
+ }
+ }
+ }
+ }
+ // Generics can't take primitives; box (int → Integer) so
List<Integer> matches.
+ elementType = ClassHelper.getWrapper(elementType);
+ ClassNode restType =
GenericsUtils.makeClassSafeWithGenerics(LIST_TYPE, new
GenericsType(elementType));
+ if (!checkRestAssignability(restExpr, restType, rightExpression))
return false;
+ storeType(restExpr, restType);
+
+ return true;
+ }
+
+ private boolean typeCheckRestAgainstDeclaredType(final TupleExpression
tuple, final Expression rightExpression) {
+ ClassNode rhsType =
Optional.ofNullable(getType(rightExpression)).orElseGet(rightExpression::getType);
+ ClassNode elementType = inferComponentType(rhsType, int_TYPE);
+ if (elementType == null) {
+ // Non-indexable RHS — let positional surface the existing
rejection.
+ return typeCheckMultipleAssignmentPositional(tuple,
rightExpression);
+ }
+
+ List<Expression> tupleExpressions = tuple.getExpressions();
+ int tupleSize = tupleExpressions.size();
+ int restIndex = restIndexOf(tuple);
+ boolean tailRest = restIndex == tupleSize - 1;
+ boolean pathA = rhsSupportsPathA(rhsType);
+
+ if (!tailRest && !pathA) {
+ addStaticTypeError("Head or middle rest binding requires an
indexable right-hand side; "
+ + prettyPrintType(rhsType) + " does not support
getAt(IntRange)", rightExpression);
+ return false;
+ }
+
+ // Non-rest positions all share the element type derived from
getAt(int).
+ for (int i = 0; i < tupleSize; i++) {
+ if (i == restIndex) continue;
+ ClassNode targetType = getType(tupleExpressions.get(i));
+ if (!isAssignableTo(elementType, targetType)) {
+ addStaticTypeError("Cannot assign value of type " +
prettyPrintType(elementType) + " to variable of type " +
prettyPrintType(targetType), rightExpression);
+ return false;
+ }
+ storeType(tupleExpressions.get(i), elementType);
+ }
+
+ // Rest position: List<T> for Path A, Stream<T> for Stream RHS (tail
rest only),
+ // Iterator<T> for any other Path B receiver (tail rest only).
+ // Box primitives so List<Integer>/Stream<Integer>/Iterator<Integer>
matches.
+ ClassNode container;
+ if (pathA) {
+ container = LIST_TYPE;
+ } else if (GeneralUtils.isOrImplements(rhsType, STREAM_TYPE)) {
+ container = STREAM_TYPE;
+ } else {
+ container = Iterator_TYPE;
+ }
+ ClassNode restType =
GenericsUtils.makeClassSafeWithGenerics(container, new
GenericsType(ClassHelper.getWrapper(elementType)));
Review Comment:
In `typeCheckRestAgainstDeclaredType`, Path A currently infers the rest
binder type as `List<T>` for all indexable RHS types. This is incorrect for
`CharSequence`/`String`/`GString` RHS: `MultipleAssignmentSupport.tailRest`
delegates to `getAt(IntRange)` which returns a `CharSequence` (or `String` for
`GString`), not a `List`. This can cause wrong inferred types under
STC/`@CompileStatic` and false negatives for typed rest binders like `def (c,
String *t) = 'hello'`.
Consider special-casing `CharSequence`/`GString` (and possibly `String`) so
the inferred rest type matches the `getAt(IntRange)` return type for those
receivers (instead of forcing `List<T>`), while keeping `List<T>` for
list/array/range Path A receivers.
##########
src/main/java/org/codehaus/groovy/runtime/MultipleAssignmentSupport.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.runtime;
+
+import groovy.lang.IntRange;
+
+import java.lang.reflect.Array;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Runtime helpers used by the GEP-20 multi-assignment destructuring bytecode.
+ *
+ * <p>These are called from generated bytecode only. User code should not rely
on them.</p>
+ */
+public final class MultipleAssignmentSupport {
+
+ private MultipleAssignmentSupport() {}
+
+ /**
+ * Resolve the value for a tail rest binding ({@code *t}) at declarator
index
+ * {@code fromIndex}. Picks Path A (slice via {@code getAt(IntRange)})
when the
+ * RHS is a list/string/array; Path B' (a fresh {@code Stream} wrapping the
+ * already-advanced iterator) when the RHS is a {@code Stream}; otherwise
Path B
+ * (returns the already-advanced iterator).
+ *
+ * <p>For Path B' the new Stream is sequential and reports no spliterator
+ * characteristics — capturing the original characteristics would require
reading
+ * the spliterator before the head was extracted, which is mutually
exclusive
+ * with the iterator path the head uses. The {@code onClose} handler of
the new
+ * Stream delegates to {@code rhs.close()} so that a typed source such as
+ * {@code Files.lines(p)} can be closed via the rest binder.</p>
+ *
+ * <p>Primitive streams ({@code IntStream}, {@code LongStream},
+ * {@code DoubleStream}) are not subtypes of {@code Stream<T>} and
therefore
+ * fall through to Path B; their elements are boxed by the underlying
iterator.</p>
+ *
+ * @param rhs the original RHS value
+ * @param fromIndex the declarator position of the rest binding
+ * @param seq the iterator that has already yielded the preceding
fixed-slot values
+ */
+ public static Object tailRest(final Object rhs, final int fromIndex, final
Iterator<?> seq) {
+ if (rhs instanceof List) {
+ if (fromIndex >= ((List<?>) rhs).size()) return
Collections.emptyList();
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs instanceof CharSequence) {
+ if (fromIndex >= ((CharSequence) rhs).length()) return "";
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs != null && rhs.getClass().isArray()) {
+ // Match the non-empty branch: ArrayGroovyMethods.getAt(T[],
IntRange) returns List<T>,
+ // so the empty case must be Collections.emptyList() — not an
empty T[] — for typed
+ // rest binders like `List<T> *t` to round-trip consistently.
+ if (fromIndex >= Array.getLength(rhs)) return
Collections.emptyList();
+ IntRange range = new IntRange(true, fromIndex, -1);
+ return InvokerHelper.invokeMethod(rhs, "getAt", range);
+ }
+ if (rhs instanceof Stream) {
+ Spliterator<?> spliterator =
Spliterators.spliteratorUnknownSize(seq, 0);
+ Stream<?> tail = StreamSupport.stream(spliterator, false);
+ return tail.onClose(((Stream<?>) rhs)::close);
+ }
+ return seq;
+ }
+
+ /**
+ * Resolve the value for a head or middle rest binding. Requires a sized,
indexable RHS
+ * (List, CharSequence, or array); for other RHS types this routes through
the MOP so that
+ * non-indexable sources (iterators, streams, sets) fail fast with a
+ * {@code MissingMethodException} — preventing silent materialisation of
unbounded sources.
Review Comment:
The Javadoc for `nonTailRestSlice` says non-indexable sources like streams
fail fast with `MissingMethodException`, but `Stream` actually has
`getAt(IntRange)` (see `StreamGroovyMethods`) and in the head/middle-rest case
the negative end index will typically trigger an `IllegalArgumentException` for
a reverse range instead. Please adjust the documentation to match the actual
failure modes (e.g., MissingMethodException for truly non-indexable RHS like
Set/Iterator, and IllegalArgumentException for Stream reverse ranges).
```suggestion
* (List, CharSequence, or array); for other RHS types this routes
through the MOP.
* Truly non-indexable sources such as iterators and sets typically fail
with a
* {@code MissingMethodException}, preventing silent materialisation of
unbounded sources.
* Streams are a special case: Groovy provides {@code getAt(IntRange)}
for
* {@link java.util.stream.Stream}, so in head/middle-rest scenarios an
inverted or otherwise
* unsupported range may instead fail with an {@code
IllegalArgumentException}.
```
> Additional multi-assignment forms
> ---------------------------------
>
> Key: GROOVY-11964
> URL: https://issues.apache.org/jira/browse/GROOVY-11964
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> Three idioms are common in modern languages but unavailable in Groovy:
> * Tail rest binding: {{def (h, *t) = list}} - separate the head from the
> remainder. Common in functional code (recursive list traversal,
> pipeline-style processing of streams and iterators).
> * Map-style destructuring: {{def (name: n, age: a) = person}} - extract named
> properties or map keys directly, without intermediate variables.
> * Rest in non-tail positions: {{def (*f, last) = list, def (l, *m, r) =
> list}} - useful when the boundaries are at the ends rather than the middle.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
