This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11903 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit af4a1510b5900a8edc7c8dec8912cac656b74c4a Author: Eric Milles <[email protected]> AuthorDate: Sat Apr 4 10:30:45 2026 -0500 GROOVY-11903: ensure minus doesn't modify inputs and use fewer iterators --- .../groovy/runtime/DefaultGroovyMethods.java | 83 +++++++++++----------- 1 file changed, 43 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java index 2c97d5933c..1649342c46 100644 --- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java +++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java @@ -10794,9 +10794,13 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { } /** - * Create a List composed of the elements of the first list minus + * Create a List composed of the elements of the given List minus * every occurrence of elements of the given Collection. - * <pre class="groovyTestCase">assert [1, "a", true, true, false, 5.3] - [true, 5.3] == [1, "a", false]</pre> + * <pre class="groovyTestCase"> + * def one = [1, "a", true, true, false, 5.3, null], two = [null, true, 5.3] + * def sub = one.asUnmodifiable() - two.asUnmodifiable() + * assert sub == [1, "a", false] + * </pre> * * @param self a List * @param removeMe the elements to exclude @@ -10875,7 +10879,9 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { * Create a new Collection composed of the elements of the first Iterable minus * every matching occurrence as determined by the condition comparator of elements of the given Iterable. * <pre class="groovyTestCase"> - * assert ['a', 'B', 'c', 'D', 'E'].minus(['b', 'C', 'D'], {@code (i, j) -> i.toLowerCase() <=> j.toLowerCase()}) == ['a', 'E'] + * List<String> one = ['a', 'B', 'c', 'D', 'E'], two = ['b', 'C', 'D'] + * def sub = one.minus(two, Comparator.comparing(String::toLowerCase)) + * assert sub == ['a', 'E'] * </pre> * * @param self an Iterable @@ -10884,48 +10890,45 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { * @return a new Collection * @since 4.0.0 */ - @SuppressWarnings("unchecked") public static <T> Collection<T> minus(Iterable<T> self, Iterable<?> removeMe, Comparator<? super T> comparator) { - Collection<T> ansCollection = createSimilarCollection(self); - if (!self.iterator().hasNext()) - return ansCollection; - T head = self.iterator().next(); - - // We can't use the same tactic as for intersection - // since AbstractCollection only does a remove on the first - // element it encounters. - boolean nlgnSort = sameType(new Iterable[]{self, removeMe}); - - if (nlgnSort && (head instanceof Comparable)) { - //n*LOG(n) version - Set<T> removeMe2 = new TreeSet<>(comparator); - for(Object o: removeMe) { - removeMe2.add((T) o); - } - for (T o : self) { - if (!removeMe2.contains(o)) - ansCollection.add(o); - } - } else { - //n*n version - Collection<T> tmpAnswer = asCollection(self); - for (Iterator<T> iter = self.iterator(); iter.hasNext();) { - T element = iter.next(); - boolean elementRemoved = false; - for (Iterator<?> iterator = removeMe.iterator(); iterator.hasNext() && !elementRemoved;) { - Object elt = iterator.next(); - if (DefaultTypeTransformation.compareEqual(element, elt)) { - iter.remove(); - elementRemoved = true; - } + Collection<T> answer = createSimilarCollection(self); + Iterator<T> iterator = self.iterator(); + if (iterator.hasNext()) { + T next = iterator.next(); + boolean more = iterator.hasNext(); + Predicate exclude; // the elements of self are discarded if this returns true + + // We can't use the same tactic as for intersection, since AbstractCollection + // only does a remove on the first element it encounters. + if (next instanceof Comparable && sameType(new Iterable[]{self, removeMe})) { + // O(log(n)) version + Set removeMe2 = new TreeSet<>(comparator); + for (Object o : removeMe) { + removeMe2.add(o); } + exclude = removeMe2::contains; + } else { + // O(n) version + exclude = (o1) -> { + for (Object o2 : removeMe) { + if (DefaultTypeTransformation.compareEqual(o1, o2)) { + return true; + } + } + return false; + }; } - //remove duplicates - //can't use treeset since the base classes are different - ansCollection.addAll(tmpAnswer); + while (true) { + if (!exclude.test(next)) + answer.add(next); // include duplicates unless answer dedups + if (!more) break; else { + next = iterator.next(); + more = iterator.hasNext(); + } + } } - return ansCollection; + return answer; } /**
