Skip to content

Commit 6588f72

Browse files
octylFractalpaulk-asert
authored andcommitted
GROOVY-11308: Restore compatibility of Collection#unique
The documentation for this method states that when `mutate` is false, a new collection is _always_ returned. The previous code only did so when self.size() was greater than 1, and otherwise returned null.
1 parent f3f6f1c commit 6588f72

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

Diff for: src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -1304,19 +1304,19 @@ public static <T> List<T> unique(List<T> self) {
13041304
* </pre>
13051305
*
13061306
* @param self a collection
1307-
* @param mutate false will cause a new list containing unique items from the collection to be created, true will mutate collections in place
1308-
* @return the now modified collection
1307+
* @param mutate false will return a new collection containing the unique items from the collection, true will mutate collections in place and return the original collection
1308+
* @return a collection with unique elements
13091309
* @since 1.8.1
13101310
*/
13111311
public static <T> Collection<T> unique(Collection<T> self, boolean mutate) {
1312-
Collection<T> answer = null;
1313-
if (mutate || (self != null && self.size() > 1)) {
1314-
answer = uniqueItems(self);
1312+
Objects.requireNonNull(self);
1313+
if (mutate && self.size() <= 1) {
1314+
return self;
13151315
}
1316+
Collection<T> answer = uniqueItems(self);
13161317
if (mutate) {
13171318
self.clear();
13181319
self.addAll(answer);
1319-
13201320
return self;
13211321
} else {
13221322
return answer;

Diff for: src/test/groovy/UniqueOnCollectionTest.groovy

+18
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,22 @@ class UniqueOnCollectionTest extends GroovyTestCase {
112112
assert orig == [1, 3, 2, 3]
113113
assert uniq == [1, 3, 2]
114114
}
115+
116+
/** GROOVY-11308 */
117+
void testNonMutatingEmpty() {
118+
def it = []
119+
def uniq = it.unique( false )
120+
assert uniq !== it : "Must be a different instance to preserve documented behavior"
121+
assert uniq == []
122+
assert it == []
123+
}
124+
125+
/** GROOVY-11308 */
126+
void testNonMutatingSingleElement() {
127+
def it = [1]
128+
def uniq = it.unique( false )
129+
assert uniq !== it : "Must be a different instance to preserve documented behavior"
130+
assert uniq == [1]
131+
assert it == [1]
132+
}
115133
}

0 commit comments

Comments
 (0)