From 276de93316bb547a2b26f449add7e04f3877e430 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Sat, 9 Nov 2024 14:54:21 -0500 Subject: [PATCH] Replace Collectors.toUnmodifiableList() with Stream#toList() [JDK 16 (in JDK-8256441)](https://bugs.openjdk.org/browse/JDK-8256441) added [`Stream#toList()`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#toList()) provides an optimized version that uses JDK internals to avoid some of the additional array copies that make stream collection expensive. --- .../baseline/errorprone/StreamToList.java | 75 +++++++++++++++++++ .../baseline/errorprone/StreamToListTest.java | 63 ++++++++++++++++ .../BaselineErrorProneExtension.java | 1 + 3 files changed, 139 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamToList.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamToListTest.java diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamToList.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamToList.java new file mode 100644 index 000000000..0a413068c --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StreamToList.java @@ -0,0 +1,75 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.ChildMultiMatcher.MatchType; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import java.util.stream.Collector; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +@AutoService(BugChecker.class) +@BugPattern( + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + linkType = BugPattern.LinkType.CUSTOM, + severity = BugPattern.SeverityLevel.WARNING, + summary = "`Stream.toList()` is more efficient than `Stream.collect(Collectors.toUnmodifiableList())`") +public final class StreamToList extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + + private static final Matcher STREAM_COLLECT = MethodMatchers.instanceMethod() + .onDescendantOf(Stream.class.getName()) + .namedAnyOf("collect") + .withParameters(Collector.class.getName()); + + private static final Matcher COLLECTORS_TO_UNMODIFIABLE_LIST = MethodMatchers.staticMethod() + .onDescendantOf(Collectors.class.getName()) + .namedAnyOf("toUnmodifiableList") + .withNoParameters(); + + private static final Matcher STREAM_COLLECT_TO_UNMODIFIABLE_LIST = Matchers.methodInvocation( + STREAM_COLLECT, + // Any of the three MatchTypes are reasonable in this case, given a single arg + MatchType.LAST, + COLLECTORS_TO_UNMODIFIABLE_LIST); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (STREAM_COLLECT_TO_UNMODIFIABLE_LIST.matches(tree, state)) { + ExpressionTree receiver = ASTHelpers.getReceiver(tree.getMethodSelect()); + if (receiver != null) { + return buildDescription(tree) + .addFix(SuggestedFix.builder() + .replace(state.getEndPosition(receiver), state.getEndPosition(tree), ".toList()") + .build()) + .build(); + } + } + return Description.NO_MATCH; + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamToListTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamToListTest.java new file mode 100644 index 000000000..d889fb10e --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StreamToListTest.java @@ -0,0 +1,63 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import org.junit.jupiter.api.Test; + +class StreamToListTest { + @Test + public void test() { + fix().addInputLines( + "Test.java", + "import java.util.List;", + "import java.util.stream.Collectors;", + "import java.util.stream.Stream;", + "public class Test {", + " List f0(Stream in) {", + " return in.toList();", + " }", + // Collectors.toList() supports nulls & is mutable while Stream#toList() does not + " List f1(Stream in) {", + " return in.collect(Collectors.toList());", + " }", + " List f2(Stream in) {", + " return in.collect(Collectors.toUnmodifiableList());", + " }", + "}") + .addOutputLines( + "Test.java", + "import java.util.List;", + "import java.util.stream.Collectors;", + "import java.util.stream.Stream;", + "public class Test {", + " List f0(Stream in) {", + " return in.toList();", + " }", + " List f1(Stream in) {", + " return in.collect(Collectors.toList());", + " }", + " List f2(Stream in) {", + " return in.toList();", + " }", + "}") + .doTest(); + } + + private RefactoringValidator fix() { + return RefactoringValidator.of(StreamToList.class, getClass()); + } +} diff --git a/gradle-baseline-java/src/main/java/com/palantir/baseline/extensions/BaselineErrorProneExtension.java b/gradle-baseline-java/src/main/java/com/palantir/baseline/extensions/BaselineErrorProneExtension.java index bf3ff82bb..33cf83574 100644 --- a/gradle-baseline-java/src/main/java/com/palantir/baseline/extensions/BaselineErrorProneExtension.java +++ b/gradle-baseline-java/src/main/java/com/palantir/baseline/extensions/BaselineErrorProneExtension.java @@ -73,6 +73,7 @@ public class BaselineErrorProneExtension { "Slf4jThrowable", "StreamOfEmpty", "StreamFlatMapOptional", + "StreamToList", "StrictUnusedVariable", "StringBuilderConstantParameters", "ThrowError",