[ 
https://issues.apache.org/jira/browse/GROOVY-12215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100404#comment-18100404
 ] 

ASF GitHub Bot commented on GROOVY-12215:
-----------------------------------------

Copilot commented on code in PR #2750:
URL: https://github.com/apache/groovy/pull/2750#discussion_r3682395496


##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -15229,6 +15604,25 @@ public static Collection split(Object self, Closure 
closure) {
         return split(closure, accept, reject, InvokerHelper.asIterator(self));
     }
 
+    /**
+     * Splits all items into two collections based on the predicate.
+     * A "fat-free" variant of {@link #split(Object, Closure)} accepting a 
{@link Predicate}.
+     * The first list contains all items which match the given predicate, the 
second list all those that don't.
+     * <pre class="language-groovy groovyTestCase">
+     * assert [[2, 4], [1, 3]] == [1, 2, 3, 4].split(n -&gt; n % 2 == 0)
+     * </pre>
+     *
+     * @param self      an Object with an Iterator returning its values
+     * @param predicate a predicate used to determine the target collection
+     * @return a List whose first item is the accepted values and whose second 
item is the rejected values
+     * @since 6.0.0
+     */
+    public static Collection split(Object self, Predicate predicate) {

Review Comment:
   This overload mirrors split(Object, Closure) but currently lacks the 
`@SuppressWarnings` used for the same raw/unchecked usage. With a raw Predicate 
plus raw Lists, this can introduce new 'rawtypes'/'unchecked' compiler warnings 
in a core runtime class.
   
   This issue also appears in the following locations of the same file:
   - line 15726
   - line 15760



##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -16327,7 +16894,7 @@ private void advance() {
             exhausted = !delegate.hasNext();
             if (!exhausted) {
                 next = delegate.next();
-                if (!condition.call(next)) {
+                if (!condition.test(next)) {
                     exhausted = true;
                     next = null;
                 }

Review Comment:
   TakeWhileIterator prefetches via delegate.next() inside advance() (called 
from next()). Because of that, calling remove() after next() will delegate to 
the underlying iterator after an additional delegate.next() has already 
happened, which can remove the wrong element. Either rework iteration to avoid 
prefetching before remove, or (simplest) make remove() unsupported like other 
lazy iterators here.





> Fat-free variants for collectMany, countBy, split, take/dropWhile, groupBy
> --------------------------------------------------------------------------
>
>                 Key: GROOVY-12215
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12215
>             Project: Groovy
>          Issue Type: Sub-task
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> h3. Summary
> Sub-task of GROOVY-12205. Adds the straightforward "fat-free" 
> functional-interface twins — a single {{java.util.function}} overload beside 
> each existing {{Closure}} overload — for the mapping/filtering DGM methods 
> that currently have none. These are mechanical, following the conventions 
> established in GROOVY-12054; no design decisions are involved.
> h3. Methods
> ||Method||Closure overloads without a twin||Functional type||Receivers||
> |{{collectMany}}|8|{{Function<? super E, ? extends Collection<? extends 
> T>>}}|Iterable (x2), Map (x2), Iterator (x2), {{E[]}} (x2)|
> |{{collectingMany}}|1|{{Function}} (lazy)|Iterator|
> |{{countBy}}|4|{{Function<? super E, ? extends K>}}|Iterable, Iterator, Map, 
> {{E[]}}|
> |{{split}}|5|{{Predicate<? super T>}}|Object, Collection, List, Set, {{T[]}}|
> |{{groupBy}} (single-key)|2|{{Function<? super T, ? extends K>}}|Map, 
> {{Object[]}} (the Iterable twin already exists)|
> |{{takeWhile}}|6|{{Predicate<? super T>}}|List, Iterable, SortedSet, Map, 
> Iterator, {{T[]}}|
> |{{dropWhile}}|6|{{Predicate<? super T>}}|SortedSet, List, Iterable, Map, 
> Iterator, {{T[]}}|
> Total: 32 new overloads.
> h3. Conventions (per GROOVY-12054)
> * PECS wildcards on the functional parameter; {{@since 6.0.0}}.
> * Inline javadoc example using {{<pre class="language-groovy 
> groovyTestCase">}}, cross-linking the {{Closure}} counterpart via {{@link}}.
> * Plain single-SAM overloads only — *not* {{@Incubating}}; that annotation is 
> reserved for the curried {{(BiXxx, param)}} forms, which are a separate item.
> h3. Exclusions
> * {{groupBy}} multi-key forms ({{Object...}} / {{List<Closure>}}) — no clean 
> functional analogue.
> * {{takeWhile}}/{{dropWhile}} on {{CharSequence}}/{{GString}} (the SGM 
> char-by-char forms) — a String-domain 
> {{Predicate<Character>}}/{{IntPredicate}} decision, out of scope for this 
> collection-focused sub-task (keeps the count at 6 each).
> h3. Compatibility
> Existing closure/lambda call sites are unaffected: a {{Closure}} literal 
> binds to the {{Closure}} overload, while method references and typed 
> functional values select the twin under {{@CompileStatic}} (per 
> GROOVY-12214). Only check: an existing {{@CompileStatic}} call passing a 
> literal {{null}} to one of these currently-single-overload methods would 
> become ambiguous — a rare, grep-checkable case, resolved with a cast if found.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to