Move isUnmodifiable assertion to Iterables

assertj-core/src/main/java/org/assertj/core/api/AbstractCollectionAssert.java

@@ -12,43 +12,42 @@
  */
 package org.assertj.core.api;
 
-import static java.util.function.UnaryOperator.identity;
 import static org.assertj.core.error.ShouldBeUnmodifiable.shouldBeUnmodifiable;
 
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.NavigableSet;
+import java.util.Optional;
 import java.util.Set;
-
 import org.assertj.core.annotations.Beta;
 
 /**
  * Base class for all implementations of assertions for {@link Collection}s.
- * @param <SELF> the "self" type of this assertion class. Please read &quot;<a href="http://bit.ly/1IZIRcY"
- *          target="_blank">Emulating 'self types' using Java Generics to simplify fluent API implementation</a>&quot;
- *          for more details.
+ *
+ * @param <SELF> the "self" type of this assertion class. Please read &quot;<a
+ *     href="http://bit.ly/1IZIRcY" target="_blank">Emulating 'self types' using Java Generics to
+ *     simplify fluent API implementation</a>&quot; for more details.
  * @param <ACTUAL> the type of the "actual" value.
  * @param <ELEMENT> the type of elements of the "actual" value.
  * @param <ELEMENT_ASSERT> used for navigational assertions to return the right assert type.
- *
  * @since 3.21.0
  */
-//@format:off
-public abstract class AbstractCollectionAssert<SELF extends AbstractCollectionAssert<SELF, ACTUAL, ELEMENT, ELEMENT_ASSERT>,
-                                               ACTUAL extends Collection<? extends ELEMENT>,
-                                               ELEMENT,
-                                               ELEMENT_ASSERT extends AbstractAssert<ELEMENT_ASSERT, ELEMENT>>
+// @format:off
+public abstract class AbstractCollectionAssert<
+        SELF extends AbstractCollectionAssert<SELF, ACTUAL, ELEMENT, ELEMENT_ASSERT>,
+        ACTUAL extends Collection<? extends ELEMENT>,
+        ELEMENT,
+        ELEMENT_ASSERT extends AbstractAssert<ELEMENT_ASSERT, ELEMENT>>
     extends AbstractIterableAssert<SELF, ACTUAL, ELEMENT, ELEMENT_ASSERT> {
-//@format:on
+  // @format:on
 
   protected AbstractCollectionAssert(ACTUAL actual, Class<?> selfType) {
     super(actual, selfType);
   }
 
   /**
-   * Verifies that the actual collection is unmodifiable, i.e., throws an {@link UnsupportedOperationException} with
-   * any attempt to modify the collection.
+   * Verifies that the actual collection is unmodifiable, i.e., throws an
+   * {@link UnsupportedOperationException} with any attempt to modify the collection.
    * <p>
    * Example:
    * <pre><code class='java'> // assertions will pass
@@ -58,77 +57,18 @@ protected AbstractCollectionAssert(ACTUAL actual, Class<?> selfType) {
    *
    * // assertions will fail
    * assertThat(new ArrayList&lt;&gt;()).isUnmodifiable();
-   * assertThat(new HashSet&lt;&gt;()).isUnmodifiable();</code></pre>
+   * assertThat(new HashSet&lt;&gt;()).isUnmodifiable();
+   * assertThat(new MyCustomIterable&lt;&gt;()).isUnmodifiable();</code></pre>
    *
    * @return {@code this} assertion object.
    * @throws AssertionError if the actual collection is modifiable.
-   * @see Collections#unmodifiableCollection(Collection)
-   * @see Collections#unmodifiableList(List)
-   * @see Collections#unmodifiableSet(Set)
+   * @see java.util.Collections#unmodifiableCollection(Collection)
+   * @see java.util.Collections#unmodifiableList(List)
+   * @see java.util.Collections#unmodifiableSet (java.util.Set)
    */
   @Beta
   public SELF isUnmodifiable() {
-    isNotNull();
-    assertIsUnmodifiable();
-    return myself;
+    // this method is included for binary backwards compatibility
+    return super.isUnmodifiable();
   }
-
-  @SuppressWarnings("unchecked")
-  private void assertIsUnmodifiable() {
-    switch (actual.getClass().getName()) {
-    case "java.util.Collections$EmptyList":
-    case "java.util.Collections$EmptyNavigableSet":
-    case "java.util.Collections$EmptySet":
-    case "java.util.Collections$EmptySortedSet":
-    case "java.util.Collections$SingletonList":
-    case "java.util.Collections$SingletonSet":
-      // immutable by contract, although not all methods throw UnsupportedOperationException
-      return;
-    }
-
-    expectUnsupportedOperationException(() -> actual.add(null), "Collection.add(null)");
-    expectUnsupportedOperationException(() -> actual.addAll(emptyCollection()), "Collection.addAll(emptyCollection())");
-    expectUnsupportedOperationException(() -> actual.clear(), "Collection.clear()");
-    expectUnsupportedOperationException(() -> actual.iterator().remove(), "Collection.iterator().remove()");
-    expectUnsupportedOperationException(() -> actual.remove(null), "Collection.remove(null)");
-    expectUnsupportedOperationException(() -> actual.removeAll(emptyCollection()), "Collection.removeAll(emptyCollection())");
-    expectUnsupportedOperationException(() -> actual.removeIf(element -> true), "Collection.removeIf(element -> true)");
-    expectUnsupportedOperationException(() -> actual.retainAll(emptyCollection()), "Collection.retainAll(emptyCollection())");
-
-    if (actual instanceof List) {
-      List<ELEMENT> list = (List<ELEMENT>) actual;
-      expectUnsupportedOperationException(() -> list.add(0, null), "List.add(0, null)");
-      expectUnsupportedOperationException(() -> list.addAll(0, emptyCollection()), "List.addAll(0, emptyCollection())");
-      expectUnsupportedOperationException(() -> list.listIterator().add(null), "List.listIterator().add(null)");
-      expectUnsupportedOperationException(() -> list.listIterator().remove(), "List.listIterator().remove()");
-      expectUnsupportedOperationException(() -> list.listIterator().set(null), "List.listIterator().set(null)");
-      expectUnsupportedOperationException(() -> list.remove(0), "List.remove(0)");
-      expectUnsupportedOperationException(() -> list.replaceAll(identity()), "List.replaceAll(identity())");
-      expectUnsupportedOperationException(() -> list.set(0, null), "List.set(0, null)");
-      expectUnsupportedOperationException(() -> list.sort((o1, o2) -> 0), "List.sort((o1, o2) -> 0)");
-    }
-
-    if (actual instanceof NavigableSet) {
-      NavigableSet<ELEMENT> set = (NavigableSet<ELEMENT>) actual;
-      expectUnsupportedOperationException(() -> set.descendingIterator().remove(), "NavigableSet.descendingIterator().remove()");
-      expectUnsupportedOperationException(() -> set.pollFirst(), "NavigableSet.pollFirst()");
-      expectUnsupportedOperationException(() -> set.pollLast(), "NavigableSet.pollLast()");
-    }
-  }
-
-  private void expectUnsupportedOperationException(Runnable runnable, String method) {
-    try {
-      runnable.run();
-      throwAssertionError(shouldBeUnmodifiable(method));
-    } catch (UnsupportedOperationException e) {
-      // happy path
-    } catch (RuntimeException e) {
-      throwAssertionError(shouldBeUnmodifiable(method, e));
-    }
-  }
-
-  private <E extends ELEMENT> Collection<E> emptyCollection() {
-    return Collections.emptyList();
-  }
-
 }

assertj-core/src/main/java/org/assertj/core/api/AbstractIterableAssert.java

@@ -18,6 +18,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.filter.Filters.filter;
 import static org.assertj.core.description.Description.mostRelevantDescription;
+import static org.assertj.core.error.ShouldBeUnmodifiable.shouldBeUnmodifiable;
 import static org.assertj.core.error.ShouldNotBeNull.shouldNotBeNull;
 import static org.assertj.core.extractor.Extractors.byName;
 import static org.assertj.core.extractor.Extractors.extractedDescriptionOf;
@@ -41,6 +42,8 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.function.BiConsumer;
@@ -2619,7 +2622,7 @@ public RecursiveComparisonAssert<?> usingRecursiveComparison(RecursiveComparison
    *
    * <p>The recursive algorithm employs cycle detection, so object graphs with cyclic references can safely be asserted over without causing looping.</p>
    *
-   * <p>This method enables recursive asserting using default configuration, which means all fields of all objects have the   
+   * <p>This method enables recursive asserting using default configuration, which means all fields of all objects have the
    * {@link java.util.function.Predicate} applied to them (including primitive fields), no fields are excluded, but:
    * <ul>
    *   <li>The recursion does not enter into Java Class Library types (java.*, javax.*)</li>
@@ -3258,7 +3261,7 @@ public SELF filteredOnAssertions(Consumer<? super ELEMENT> elementAssertions) {
    * Filters the iterable under test keeping only elements matching the given assertions specified with a {@link ThrowingConsumer}.
    * <p>
    * This is the same assertion as {@link #filteredOnAssertions(Consumer)} but the given consumer can throw checked exceptions.<br>
-   * More precisely, {@link RuntimeException} and {@link AssertionError} are rethrown as they are and {@link Throwable} wrapped in a {@link RuntimeException}. 
+   * More precisely, {@link RuntimeException} and {@link AssertionError} are rethrown as they are and {@link Throwable} wrapped in a {@link RuntimeException}.
    * <p>
    * Example: check young hobbits whose age &lt; 34:
    *
@@ -3522,8 +3525,8 @@ public ELEMENT_ASSERT element(int index) {
   }
 
   /**
-   * Allow to perform assertions on the elements corresponding to the given indices 
-   * (the iterable {@link Iterable} under test is changed to an iterable with the selected elements).  
+   * Allow to perform assertions on the elements corresponding to the given indices
+   * (the iterable {@link Iterable} under test is changed to an iterable with the selected elements).
    * <p>
    * Example:
    * <pre><code class='java'> Iterable&lt;TolkienCharacter&gt; hobbits = newArrayList(frodo, sam, pippin);
@@ -3578,6 +3581,45 @@ private static void assertIndicesIsNotEmpty(int[] indices) {
     if (indices.length == 0) throw new IllegalArgumentException("indices must not be empty");
   }
 
+  /**
+   * If the iterable is a collection, verifies that the actual collection is unmodifiable, i.e., throws an
+   * {@link UnsupportedOperationException} with any attempt to modify the collection. Iterables
+   * that are instance of {@code java.util.Collection} aren't supported, since there isn't a way to
+   * know if the actual class has any mutating methods.
+   * <p>
+   * Example:
+   * <pre><code class='java'> // assertions will pass
+   * assertThat(Collections.unmodifiableCollection(new ArrayList&lt;&gt;())).isUnmodifiable();
+   * assertThat(Collections.unmodifiableList(new ArrayList&lt;&gt;())).isUnmodifiable();
+   * assertThat(Collections.unmodifiableSet(new HashSet&lt;&gt;())).isUnmodifiable();
+   *
+   * // assertions will fail
+   * assertThat(new ArrayList&lt;&gt;()).isUnmodifiable();
+   * assertThat(new HashSet&lt;&gt;()).isUnmodifiable();
+   * assertThat(new MyCustomIterable&lt;&gt;()).isUnmodifiable();</code></pre>
+   *
+   * @return {@code this} assertion object.
+   * @throws AssertionError if the actual collection is modifiable.
+   * @see java.util.Collections#unmodifiableCollection(Collection)
+   * @see java.util.Collections#unmodifiableList(List)
+   * @see java.util.Collections#unmodifiableSet (java.util.Set)
+   */
+  @Beta
+  public SELF isUnmodifiable() {
+    isNotNull();
+
+    try {
+      CollectionVisitor<Optional<String>> finder = new MutatingMethodFinder();
+      acceptVisitor(finder).ifPresent(method -> throwAssertionError(shouldBeUnmodifiable(method)));
+    } catch (CollectionVisitor.TargetException e) {
+      throwAssertionError(shouldBeUnmodifiable(e.getMessage(), e.getException()));
+    }
+
+    return myself;
+  }
+
+  protected abstract <T> T acceptVisitor(CollectionVisitor<? extends T> visitor);
+
   private void checkIndexValidity(int index, List<ELEMENT> indexedActual) {
     assertThat(indexedActual).describedAs("check actual size is enough to get element[" + index + "]")
                              .hasSizeGreaterThan(index);
@@ -3704,7 +3746,7 @@ public ELEMENT_ASSERT singleElement() {
   }
 
   /**
-   * Verifies that the {@link Iterable} under test contains a single element and allows to perform assertions on that element, 
+   * Verifies that the {@link Iterable} under test contains a single element and allows to perform assertions on that element,
    * the assertions are strongly typed according to the given {@link AssertFactory} parameter.
    * <p>
    * This is a shorthand for <code>hasSize(1).first(assertFactory)</code>.

assertj-core/src/main/java/org/assertj/core/api/AbstractListAssert.java

@@ -14,6 +14,7 @@
 
 import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Consumer;
 
 import org.assertj.core.data.Index;
@@ -397,4 +398,9 @@ public SELF withThreadDumpOnError() {
     return super.withThreadDumpOnError();
   }
 
+  @Override
+  protected final <T> T acceptVisitor(CollectionVisitor<? extends T> visitor) {
+    Objects.requireNonNull(visitor, "visitor can't be null");
+    return visitor.visitList(actual);
+  }
 }

assertj-core/src/main/java/org/assertj/core/api/AbstractMapAssert.java

@@ -28,11 +28,10 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.NavigableMap;
+import java.util.Optional;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -1421,52 +1420,15 @@ protected SELF containsExactlyForProxy(Map.Entry<? extends K, ? extends V>[] ent
   @Beta
   public SELF isUnmodifiable() {
     isNotNull();
-    assertIsUnmodifiable();
-    return myself;
-  }
-
-  @SuppressWarnings("unchecked")
-  private void assertIsUnmodifiable() {
-    switch (actual.getClass().getName()) {
-    case "java.util.Collections$EmptyNavigableMap":
-    case "java.util.Collections$EmptyMap":
-    case "java.util.Collections$EmptySortedMap":
-    case "java.util.Collections$SingletonMap":
-      // unmodifiable by contract, although not all methods throw UnsupportedOperationException
-      return;
-    }
 
-    expectUnsupportedOperationException(() -> actual.clear(), "Map.clear()");
-    expectUnsupportedOperationException(() -> actual.compute(null, (k, v) -> v), "Map.compute(null, (k, v) -> v)");
-    expectUnsupportedOperationException(() -> actual.computeIfAbsent(null, k -> null), "Map.computeIfAbsent(null, k -> null)");
-    expectUnsupportedOperationException(() -> actual.computeIfPresent(null, (k, v) -> v),
-                                        "Map.computeIfPresent(null, (k, v) -> v)");
-    expectUnsupportedOperationException(() -> actual.merge(null, null, (v1, v2) -> v1), "Map.merge(null, null, (v1, v2) -> v1))");
-    expectUnsupportedOperationException(() -> actual.put(null, null), "Map.put(null, null)");
-    expectUnsupportedOperationException(() -> actual.putAll(new HashMap<>()), "Map.putAll(new HashMap<>())");
-    expectUnsupportedOperationException(() -> actual.putIfAbsent(null, null), "Map.putIfAbsent(null, null)");
-    expectUnsupportedOperationException(() -> actual.replace(null, null, null), "Map.replace(null, null, null)");
-    expectUnsupportedOperationException(() -> actual.replace(null, null), "Map.replace(null, null)");
-    expectUnsupportedOperationException(() -> actual.remove(null), "Map.remove(null)");
-    expectUnsupportedOperationException(() -> actual.remove(null, null), "Map.remove(null, null)");
-    expectUnsupportedOperationException(() -> actual.replaceAll((k, v) -> v), "Map.replaceAll((k, v) -> v)");
-
-    if (actual instanceof NavigableMap) {
-      NavigableMap<K, V> navigableMap = (NavigableMap<K, V>) actual;
-      expectUnsupportedOperationException(() -> navigableMap.pollFirstEntry(), "NavigableMap.pollFirstEntry()");
-      expectUnsupportedOperationException(() -> navigableMap.pollLastEntry(), "NavigableMap.pollLastEntry()");
-    }
-  }
-
-  private void expectUnsupportedOperationException(Runnable runnable, String method) {
     try {
-      runnable.run();
-      throwAssertionError(shouldBeUnmodifiable(method));
-    } catch (UnsupportedOperationException e) {
-      // happy path
-    } catch (RuntimeException e) {
-      throwAssertionError(shouldBeUnmodifiable(method, e));
+      Optional<String> mutatingMethod = new MutatingMethodFinder().visitMap(actual);
+      mutatingMethod.ifPresent(method -> throwAssertionError(shouldBeUnmodifiable(method)));
+    } catch (CollectionVisitor.TargetException e) {
+      throwAssertionError(shouldBeUnmodifiable(e.getMessage(), e.getException()));
     }
+
+    return myself;
   }
 
   /**
@@ -2113,7 +2075,7 @@ public RecursiveComparisonAssert<?> usingRecursiveComparison(RecursiveComparison
    *
    * <p>The recursive algorithm employs cycle detection, so object graphs with cyclic references can safely be asserted over without causing looping.</p>
    *
-   * <p>This method enables recursive asserting using default configuration, which means all fields of all objects have the   
+   * <p>This method enables recursive asserting using default configuration, which means all fields of all objects have the
    * {@link java.util.function.Predicate} applied to them (including primitive fields), no fields are excluded, but:
    * <ul>
    *   <li>The recursion does not enter into Java Class Library types (java.*, javax.*)</li>

assertj-core/src/main/java/org/assertj/core/api/ClassBasedNavigableIterableAssert.java

@@ -12,7 +12,10 @@
  */
 package org.assertj.core.api;
 
+import org.assertj.core.internal.Objects;
+
 import java.lang.reflect.Constructor;
+import java.util.Collection;
 
 /**
  * Build the Assert instance by reflection.
@@ -70,4 +73,11 @@ private <V> ELEMENT_ASSERT buildAssert(V value, String description, Class<?> cla
       throw new RuntimeException("Failed to build an assert object with " + value + ": " + e.getMessage(), e);
     }
   }
+
+  @Override
+  protected <T> T acceptVisitor(CollectionVisitor<? extends T> visitor) {
+    java.util.Objects.requireNonNull(visitor, "visitor can't be null");
+    Objects.instance().assertIsInstanceOf(info, actual, Collection.class);
+    return visitor.visitCollection((Collection<?>) actual);
+  }
 }

assertj-core/src/main/java/org/assertj/core/api/CollectionAssert.java

@@ -13,8 +13,11 @@
 package org.assertj.core.api;
 
 import static org.assertj.core.util.Lists.newArrayList;
+import static org.assertj.core.util.Preconditions.checkNotNull;
 
 import java.util.Collection;
+import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Assertion methods for {@link Collection}s.
@@ -47,4 +50,9 @@ protected CollectionAssert<ELEMENT> newAbstractIterableAssert(Iterable<? extends
     return new CollectionAssert<>(newArrayList(iterable));
   }
 
+  @Override
+  protected <T> T acceptVisitor(CollectionVisitor<? extends T> visitor) {
+    Objects.requireNonNull(visitor, "visitor can't be null");
+    return visitor.visitCollection(actual);
+  }
 }