From f927ff635af6b149f8d31cab7d136413b96e3c0a Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 10 Jun 2025 17:50:37 +0200 Subject: [PATCH] =?UTF-8?q?Revise=20@=E2=81=A0Nullable=20declarations=20fo?= =?UTF-8?q?r=20contains*()=20in=20CollectionUtils?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes gh-35023 --- .../springframework/util/CollectionUtils.java | 41 +++++++---- .../util/CollectionUtilsTests.java | 69 ++++++++++++------- 2 files changed, 75 insertions(+), 35 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/CollectionUtils.java b/spring-core/src/main/java/org/springframework/util/CollectionUtils.java index d1c0c46a70..bab4651257 100644 --- a/spring-core/src/main/java/org/springframework/util/CollectionUtils.java +++ b/spring-core/src/main/java/org/springframework/util/CollectionUtils.java @@ -45,6 +45,7 @@ import org.springframework.lang.Contract; * @author Juergen Hoeller * @author Rob Harrop * @author Arjen Poutsma + * @author Sam Brannen * @since 1.1.3 */ public abstract class CollectionUtils { @@ -195,13 +196,15 @@ public abstract class CollectionUtils { /** - * Check whether the given Iterator contains the given element. - * @param iterator the Iterator to check + * Check whether the given {@link Iterator} contains the given element. + * @param iterator the {@code Iterator} to check * @param element the element to look for * @return {@code true} if found, {@code false} otherwise */ @Contract("null, _ -> false") - public static boolean contains(@Nullable Iterator iterator, Object element) { + public static boolean contains(@Nullable Iterator iterator, + @Nullable Object element) { + if (iterator != null) { while (iterator.hasNext()) { Object candidate = iterator.next(); @@ -214,13 +217,15 @@ public abstract class CollectionUtils { } /** - * Check whether the given Enumeration contains the given element. - * @param enumeration the Enumeration to check + * Check whether the given {@link Enumeration} contains the given element. + * @param enumeration the {@code Enumeration} to check * @param element the element to look for * @return {@code true} if found, {@code false} otherwise */ @Contract("null, _ -> false") - public static boolean contains(@Nullable Enumeration enumeration, Object element) { + public static boolean contains(@Nullable Enumeration enumeration, + @Nullable Object element) { + if (enumeration != null) { while (enumeration.hasMoreElements()) { Object candidate = enumeration.nextElement(); @@ -233,15 +238,17 @@ public abstract class CollectionUtils { } /** - * Check whether the given Collection contains the given element instance. + * Check whether the given {@link Collection} contains the given element instance. *

Enforces the given instance to be present, rather than returning * {@code true} for an equal element as well. - * @param collection the Collection to check + * @param collection the {@code Collection} to check * @param element the element to look for * @return {@code true} if found, {@code false} otherwise */ @Contract("null, _ -> false") - public static boolean containsInstance(@Nullable Collection collection, Object element) { + public static boolean containsInstance(@Nullable Collection collection, + @Nullable Object element) { + if (collection != null) { for (Object candidate : collection) { if (candidate == element) { @@ -255,12 +262,22 @@ public abstract class CollectionUtils { /** * Return {@code true} if any element in '{@code candidates}' is * contained in '{@code source}'; otherwise returns {@code false}. - * @param source the source Collection + * @param source the source {@link Collection} * @param candidates the candidates to search for * @return whether any of the candidates has been found */ - public static boolean containsAny(Collection source, Collection candidates) { - return findFirstMatch(source, candidates) != null; + public static boolean containsAny(Collection source, + Collection candidates) { + + if (isEmpty(source) || isEmpty(candidates)) { + return false; + } + for (Object candidate : candidates) { + if (source.contains(candidate)) { + return true; + } + } + return false; } /** diff --git a/spring-core/src/test/java/org/springframework/util/CollectionUtilsTests.java b/spring-core/src/test/java/org/springframework/util/CollectionUtilsTests.java index a1ad7390db..0322b34d09 100644 --- a/spring-core/src/test/java/org/springframework/util/CollectionUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/util/CollectionUtilsTests.java @@ -18,10 +18,10 @@ package org.springframework.util; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; -import java.util.Hashtable; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -42,6 +42,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Rob Harrop * @author Juergen Hoeller * @author Rick Evans + * @author Sam Brannen */ class CollectionUtilsTests { @@ -100,19 +101,24 @@ class CollectionUtilsTests { } @Test - void contains() { + void containsWithIterator() { assertThat(CollectionUtils.contains((Iterator) null, "myElement")).isFalse(); - assertThat(CollectionUtils.contains((Enumeration) null, "myElement")).isFalse(); - assertThat(CollectionUtils.contains(new ArrayList().iterator(), "myElement")).isFalse(); - assertThat(CollectionUtils.contains(new Hashtable().keys(), "myElement")).isFalse(); + assertThat(CollectionUtils.contains(List.of().iterator(), "myElement")).isFalse(); - List list = new ArrayList<>(); - list.add("myElement"); + List list = Arrays.asList("myElement", null); assertThat(CollectionUtils.contains(list.iterator(), "myElement")).isTrue(); + assertThat(CollectionUtils.contains(list.iterator(), null)).isTrue(); + } - Hashtable ht = new Hashtable<>(); - ht.put("myElement", "myValue"); - assertThat(CollectionUtils.contains(ht.keys(), "myElement")).isTrue(); + @Test + void containsWithEnumeration() { + assertThat(CollectionUtils.contains((Enumeration) null, "myElement")).isFalse(); + assertThat(CollectionUtils.contains(Collections.enumeration(List.of()), "myElement")).isFalse(); + + List list = Arrays.asList("myElement", null); + Enumeration enumeration = Collections.enumeration(list); + assertThat(CollectionUtils.contains(enumeration, "myElement")).isTrue(); + assertThat(CollectionUtils.contains(enumeration, null)).isTrue(); } @Test @@ -128,39 +134,49 @@ class CollectionUtilsTests { candidates.add("abc"); assertThat(CollectionUtils.containsAny(source, candidates)).isTrue(); + candidates.remove("def"); assertThat(CollectionUtils.containsAny(source, candidates)).isTrue(); + candidates.remove("abc"); assertThat(CollectionUtils.containsAny(source, candidates)).isFalse(); + + source.add(null); + assertThat(CollectionUtils.containsAny(source, candidates)).isFalse(); + + candidates.add(null); + assertThat(CollectionUtils.containsAny(source, candidates)).isTrue(); } @Test void containsInstanceWithNullCollection() { - assertThat(CollectionUtils.containsInstance(null, this)).as("Must return false if supplied Collection argument is null").isFalse(); + assertThat(CollectionUtils.containsInstance(null, this)).isFalse(); } @Test void containsInstanceWithInstancesThatAreEqualButDistinct() { - List list = new ArrayList<>(); - list.add(new Instance("fiona")); - assertThat(CollectionUtils.containsInstance(list, new Instance("fiona"))).as("Must return false if instance is not in the supplied Collection argument").isFalse(); + List list = List.of(new Instance("fiona")); + assertThat(CollectionUtils.containsInstance(list, new Instance("fiona"))).isFalse(); } @Test void containsInstanceWithSameInstance() { - List list = new ArrayList<>(); - list.add(new Instance("apple")); - Instance instance = new Instance("fiona"); - list.add(instance); - assertThat(CollectionUtils.containsInstance(list, instance)).as("Must return true if instance is in the supplied Collection argument").isTrue(); + Instance fiona = new Instance("fiona"); + Instance apple = new Instance("apple"); + + List list = List.of(fiona, apple); + assertThat(CollectionUtils.containsInstance(list, fiona)).isTrue(); } @Test void containsInstanceWithNullInstance() { - List list = new ArrayList<>(); - list.add(new Instance("apple")); - list.add(new Instance("fiona")); - assertThat(CollectionUtils.containsInstance(list, null)).as("Must return false if null instance is supplied").isFalse(); + Instance fiona = new Instance("fiona"); + + List list = List.of(fiona); + assertThat(CollectionUtils.containsInstance(list, null)).isFalse(); + + list = Arrays.asList(fiona, null); + assertThat(CollectionUtils.containsInstance(list, null)).isTrue(); } @Test @@ -176,6 +192,13 @@ class CollectionUtilsTests { candidates.add("abc"); assertThat(CollectionUtils.findFirstMatch(source, candidates)).isEqualTo("def"); + + source.clear(); + source.add(null); + assertThat(CollectionUtils.findFirstMatch(source, candidates)).isNull(); + + candidates.add(null); + assertThat(CollectionUtils.findFirstMatch(source, candidates)).isNull(); } @Test