I see the following code (JDK 16, JDK 17):
public static <T>
Collector<T, ?, List<T>> toUnmodifiableList() {
return new CollectorImpl<>(ArrayList::new, List::add,
(left, right) -> { left.addAll(right); return left; },
list -> {
if (list.getClass() == ArrayList.class) { // ensure it's trusted
return SharedSecrets.getJavaUtilCollectionAccess()
.listFromTrustedArray(list.toArray());
} else {
throw new IllegalArgumentException();
}
},
CH_NOID);
}
What's the point of using if (list.getClass() == ArrayList.class) { check? Why not simply make this code as simple as
return new CollectorImpl<>(ArrayList::new, ArrayList::add,
(left, right) -> { left.addAll(right); return left; },
list -> SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(list.toArray()),
CH_NOID);
By replacing List::add with ArrayList::add we're providing the most specific method handle of the class, not an interface.
The last code uses an assumption that the accumulator instance will be passed to the 'accumulator', the 'combiner' and the 'finisher' parameters as an ArrayList instance (actually, it will, since CollectorImpl uses the same invariant generic variable A for all parameters). I don't see any difference between an IllegalArgumentException and a ClassCastException except the code complexity and an additional check in the finisher.