Replace Collectors.toUnmodifiableList() with Stream#toList()

[schlosna]

Before this PR

Uses of stream.collect(Collectors.toUnmodifiableList()) could be replaced with stream.toList() added in JDK 16 and provides a more efficient lower allocation implementation that returns an immutable List that does not contain null elements.

Similar to https://github.com/palantir/gradle-baseline/pull/2946

After this PR

==COMMIT_MSG== JDK 16 (in JDK-8256441) added Stream#toList() provides an optimized version that uses JDK internals to avoid some of the additional array copies that make stream collection expensive. ==COMMIT_MSG==

Possible downsides?

This error-prone check does not modify usages of Collectors.toList() as that returns a modifiable List that may contain null elements. It also does not modify usages of ImmutableList.toImmutableList() as some use sites leverage Guava ImmutableList types to avoid copies via ImmutableList.copyOf(someImmutableList).

[changelog-app[bot]]

Generate changelog in changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration, performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• [ ] Feature
• [ ] Improvement
• [ ] Fix
• [ ] Break
• [ ] Deprecation
• [ ] Manual task
• [ ] Migration

Description JDK 16 (in JDK-8256441) added Stream#toList() provides an optimized version that uses JDK internals to avoid some of the additional array copies that make stream collection expensive.

Check the box to generate changelog(s)

• [ ] Generate changelog entry

[pkoenig10]

It's not always possible to replace toUnmodifiableList() with toList() when using subclasses. The type signatures of these methods are:

// Stream.collect 
<R, A> R collect(Collector<? super T, A, R> collector) 

// Collectors.toUnmodifiableList 
Collector<T, ?, List<T>> toUnmodifiableList()

// Stream.toList 
List<T> toList()

So if I have a Stream<Dog> that I want to turn into a List<Animal>, I can't use toList().

This is a not uncommon pattern that I've hit a couple times. For example:

List<Foo> foos = arguments.stream()
    .map(args -> new FooImpl(args))
    // This returns List<FooImpl>, not List<Foo>
    .toList();

You can work around this by explicitly declaring the map type parameter, but it is a but cumbersome. I don't think it would be unacceptable to say that we require this, but I just want to point this out.

List<Foo> foos = arguments.stream()
    .<Foo>map(args -> new FooImpl(args))
    .toList();

[carterkozak]

I think we can compare the generic component of the Stream receiver to the generic component of the toUnmodifiableList() method invocation, and only propose replacement when they match.

(unverified)

[stale[bot]]

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.