This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY_3_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit b8902bb4719ca2ed7af64706fb5ef7c86b7bd025 Author: Eric Milles <[email protected]> AuthorDate: Tue Aug 1 10:18:24 2023 -0500 GROOVY-10893: `collectEntries`/`collectMany` null return inserts nothing 3_0_X backport --- .../groovy/runtime/DefaultGroovyMethods.java | 17 ++++---- src/test/groovy/GroovyMethodsTest.groovy | 47 ++++++++++++++-------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java index f8917f4f5b..45d86f3c24 100644 --- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java +++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java @@ -3882,7 +3882,7 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { * <p> * <pre class="groovyTestCase"> * def map = [bread:3, milk:5, butter:2] - * def result = map.collectMany(['x']){ k, v {@code ->} k.startsWith('b') ? k.toList() : [] } + * def result = map.collectMany(['x']){ k, v {@code ->} if (k.startsWith('b')) k.toList() } * assert result == ['x', 'b', 'r', 'e', 'a', 'd', 'b', 'u', 't', 't', 'e', 'r'] * </pre> * @@ -3894,7 +3894,8 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { */ public static <T,K,V> Collection<T> collectMany(Map<K, V> self, Collection<T> collector, @ClosureParams(MapEntryOrKeyValue.class) Closure<? extends Collection<? extends T>> projection) { for (Map.Entry<K, V> entry : self.entrySet()) { - collector.addAll(callClosureForMapEntry(projection, entry)); + Collection items = callClosureForMapEntry(projection, entry); + if (items != null && !items.isEmpty()) collector.addAll(items); } return collector; } @@ -3967,7 +3968,7 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { * <p> * <pre class="groovyTestCase"> * def numsIter = [1, 2, 3, 4, 5, 6].iterator() - * def squaresAndCubesOfEvens = numsIter.collectMany{ it % 2 ? [] : [it**2, it**3] } + * def squaresAndCubesOfEvens = numsIter.collectMany{ if (it % 2 == 0) [it**2, it**3] } * assert squaresAndCubesOfEvens == [4, 8, 16, 64, 36, 216] * </pre> * @@ -3981,7 +3982,8 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { public static <T,E> Collection<T> collectMany(Iterator<E> self, Collection<T> collector, @ClosureParams(FirstParam.FirstGenericType.class) Closure<? extends Collection<? extends T>> projection) { while (self.hasNext()) { E next = self.next(); - collector.addAll(projection.call(next)); + Collection items = projection.call(next); + if (items != null) collector.addAll(items); } return collector; } @@ -4356,10 +4358,12 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { } private static <K, V> void addEntry(Map<K, V> result, Object newEntry) { - if (newEntry instanceof Map) { + if (newEntry == null) { + // GROOVY-10893: insert nothing + } else if (newEntry instanceof Map) { leftShift(result, (Map)newEntry); } else if (newEntry instanceof List) { - List list = (List) newEntry; + List list = (List)newEntry; // def (key, value) == list Object key = list.isEmpty() ? null : list.get(0); Object value = list.size() <= 1 ? null : list.get(1); @@ -4371,7 +4375,6 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport { Object value = array.length <= 1 ? null : array[1]; leftShift(result, new MapEntry(key, value)); } else { - // TODO: enforce stricter behavior? // given Map.Entry is an interface, we get a proxy which gives us lots // of flexibility but sometimes the error messages might be unexpected leftShift(result, asType(newEntry, Map.Entry.class)); diff --git a/src/test/groovy/GroovyMethodsTest.groovy b/src/test/groovy/GroovyMethodsTest.groovy index 0c9d5d39fb..0d750a3505 100644 --- a/src/test/groovy/GroovyMethodsTest.groovy +++ b/src/test/groovy/GroovyMethodsTest.groovy @@ -1459,32 +1459,47 @@ class GroovyMethodsTest extends GroovyTestCase { assert !iterable.containsAll([1, 6] as int[]) } - void testCollectEntriesIterator() { - def items = ['a', 'bb', 'ccc'].iterator() - def map = items.collectEntries { [it, it.size()] } - assert map == [a: 1, bb: 2, ccc: 3] + void testCollectEntriesArray() { + def arr = '1 San Francisco,2 Cupertino'.split(',') + def map = arr.collectEntries { it.split(' ', 2); } + assert map == ['1':'San Francisco', '2':'Cupertino'] } void testCollectEntriesIterable() { - def things = new Things() - def map = things.collectEntries { [it.toLowerCase(), it.toUpperCase()] } - assert map == [a: 'A', b: 'B', c: 'C'] + def obj = new Things() + def map = obj.collectEntries { [it.toLowerCase(), it.toUpperCase()] } + assert map == [a:'A', b:'B', c:'C'] } - void testCollectEntriesListFallbackCases() { - assert [[[1,'a'], [2,'b'], [3]].collectEntries(), - [[1,'a'], [2,'b'], []].collectEntries(), - [[1,'a'], [2,'b'], [3, 'c', 42]].collectEntries()] == [[1:'a', 2:'b', 3:null], [1:'a', 2:'b', (null):null], [1:'a', 2:'b', 3:'c']] + void testCollectEntriesIterator() { + def itr = ['a','bb','ccc'].iterator() + def map = itr.collectEntries { [(it): it.size()] } + assert map == [a:1, bb:2, ccc:3] + } + + void testCollectEntriesListReturn() { + def map = [[1,'a'], [2,'b'], [3]].collectEntries() + assert map == [1:'a', 2:'b', 3:null] + + map = [[1,'a'], [2,'b'], []].collectEntries() + assert map == [1:'a', 2:'b', (null):null] + + map = [[1,'a'], [2,'b'], [3, 'c', 42]].collectEntries() + assert map == [1:'a', 2:'b', 3:'c'] + shouldFail(NullPointerException) { [[1, 'a'], [3]].collectEntries(new Hashtable()) } } - void testCollectEntriesWithArray() { - def cityList = '1 San Francisco,2 Cupertino' - def cityMap = cityList.split(','). - collectEntries{ it.split(' ', 2) } - assert cityMap == ['1': 'San Francisco', '2': 'Cupertino'] + // GROOVY-10893 + void testCollectEntriesNullReturn() { + def map = ['a','b','c','d','e'].collectEntries { + if (it == 'a' || it == 'e') { + return [it, 'vowel'] + } + } + assert map == [a:'vowel', e:'vowel'] } void testListTakeWhile() {
