From f748c611c79daea64758beaece3380ef99352e4d Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 2 Jan 2020 13:36:12 -0500 Subject: [PATCH 1/2] Migrate the CollectionStreamForEach refactor to error-prone --- README.md | 1 + .../errorprone/CollectionStreamForEach.java | 82 +++++++++++++++++++ .../CollectionStreamForEachTest.java | 29 ++++--- .../refaster/CollectionStreamForEach.java | 42 ---------- .../BaselineErrorProneExtension.java | 1 + 5 files changed, 102 insertions(+), 53 deletions(-) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CollectionStreamForEach.java rename {baseline-refaster-rules/src/test/java/com/palantir/baseline/refaster => baseline-error-prone/src/test/java/com/palantir/baseline/errorprone}/CollectionStreamForEachTest.java (60%) delete mode 100644 baseline-refaster-rules/src/main/java/com/palantir/baseline/refaster/CollectionStreamForEach.java diff --git a/README.md b/README.md index c8f53160e..39e4cba71 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `ThrowSpecificity`: Prefer to declare more specific `throws` types than Exception and Throwable. - `UnsafeGaugeRegistration`: Use TaggedMetricRegistry.registerWithReplacement over TaggedMetricRegistry.gauge. - `BracesRequired`: Require braces for loops and if expressions. +- `CollectionStreamForEach`: Collection.forEach is more efficient than Collection.stream().forEach. ### Programmatic Application diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CollectionStreamForEach.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CollectionStreamForEach.java new file mode 100644 index 000000000..eb7924019 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CollectionStreamForEach.java @@ -0,0 +1,82 @@ +/* + * (c) Copyright 2020 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.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.Collection; +import java.util.function.Consumer; +import java.util.stream.Stream; + +@AutoService(BugChecker.class) +@BugPattern( + name = "CollectionStreamForEach", + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + linkType = BugPattern.LinkType.CUSTOM, + severity = BugPattern.SeverityLevel.WARNING, + summary = "Collection.forEach is more efficient than Collection.stream().forEach") +public final class CollectionStreamForEach extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + + private static final Matcher STREAM_FOR_EACH = MethodMatchers.instanceMethod() + .onDescendantOf(Stream.class.getName()) + .namedAnyOf("forEach", "forEachOrdered") + .withParameters(Consumer.class.getName()); + + private static final Matcher COLLECTION_STREAM = MethodMatchers.instanceMethod() + .onDescendantOf(Collection.class.getName()) + .named("stream") + .withParameters(); + + private static final Matcher matcher = Matchers.allOf( + STREAM_FOR_EACH, + Matchers.receiverOfInvocation(COLLECTION_STREAM)); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (matcher.matches(tree, state)) { + ExpressionTree stream = ASTHelpers.getReceiver(tree); + if (stream == null) { + // Should be impossible. + return describeMatch(tree); + } + ExpressionTree collection = ASTHelpers.getReceiver(stream); + if (collection == null) { + // Should be impossible. + return describeMatch(tree); + } + return buildDescription(tree) + .addFix(SuggestedFix.builder() + // Replaces forEachOrdered with forEach + .merge(MoreSuggestedFixes.renameInvocationRetainingTypeArguments(tree, "forEach", state)) + .replace(stream, state.getSourceForNode(collection)) + .build()) + .build(); + } + return Description.NO_MATCH; + } +} diff --git a/baseline-refaster-rules/src/test/java/com/palantir/baseline/refaster/CollectionStreamForEachTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/CollectionStreamForEachTest.java similarity index 60% rename from baseline-refaster-rules/src/test/java/com/palantir/baseline/refaster/CollectionStreamForEachTest.java rename to baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/CollectionStreamForEachTest.java index 2b707d796..bf6f5122b 100644 --- a/baseline-refaster-rules/src/test/java/com/palantir/baseline/refaster/CollectionStreamForEachTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/CollectionStreamForEachTest.java @@ -1,5 +1,5 @@ /* - * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * (c) Copyright 2020 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. @@ -14,33 +14,40 @@ * limitations under the License. */ -package com.palantir.baseline.refaster; +package com.palantir.baseline.errorprone; -import org.junit.Test; - -public class CollectionStreamForEachTest { +import org.junit.jupiter.api.Test; +class CollectionStreamForEachTest { @Test public void test() { - RefasterTestHelper - .forRefactoring(CollectionStreamForEach.class) - .withInputLines( - "Test", + fix() + .addInputLines( + "Test.java", "import java.util.List;", "public class Test {", " void f(List in) {", " in.stream().forEach(System.out::println);", " in.stream().forEachOrdered(System.out::println);", + " in.stream().forEach(System.out::println);", + " in.stream().forEachOrdered(System.out::println);", " }", "}") - .hasOutputLines( + .addOutputLines( + "Test.java", "import java.util.List;", "public class Test {", " void f(List in) {", " in.forEach(System.out::println);", " in.forEach(System.out::println);", + " in.forEach(System.out::println);", + " in.forEach(System.out::println);", " }", - "}"); + "}") + .doTest(); } + private RefactoringValidator fix() { + return RefactoringValidator.of(new CollectionStreamForEach(), getClass()); + } } diff --git a/baseline-refaster-rules/src/main/java/com/palantir/baseline/refaster/CollectionStreamForEach.java b/baseline-refaster-rules/src/main/java/com/palantir/baseline/refaster/CollectionStreamForEach.java deleted file mode 100644 index 138acd603..000000000 --- a/baseline-refaster-rules/src/main/java/com/palantir/baseline/refaster/CollectionStreamForEach.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * (c) Copyright 2019 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.refaster; - -import com.google.errorprone.refaster.annotation.AfterTemplate; -import com.google.errorprone.refaster.annotation.BeforeTemplate; -import java.util.Collection; -import java.util.function.Consumer; - -public final class CollectionStreamForEach { - - @BeforeTemplate - @SuppressWarnings("SimplifyStreamApiCallChains") // This is what we're fixing - void before1(Collection target, Consumer consumer) { - target.stream().forEach(consumer); - } - - @BeforeTemplate - @SuppressWarnings("SimplifyStreamApiCallChains") // This is what we're fixing - void before2(Collection target, Consumer consumer) { - target.stream().forEachOrdered(consumer); - } - - @AfterTemplate - void after(Collection target, Consumer consumer) { - target.forEach(consumer); - } -} diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java index 332435d78..14134e055 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java @@ -25,6 +25,7 @@ public class BaselineErrorProneExtension { // Baseline checks "BracesRequired", "CatchBlockLogException", + "CollectionStreamForEach", // TODO(ckozak): re-enable pending scala check //"CatchSpecificity", "ExecutorSubmitRunnableFutureIgnored", From 81a37fbab382bc6adfc83a219914d94e3ad4a28f Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 2 Jan 2020 18:36:12 +0000 Subject: [PATCH 2/2] Add generated changelog entries --- changelog/@unreleased/pr-1139.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1139.v2.yml diff --git a/changelog/@unreleased/pr-1139.v2.yml b/changelog/@unreleased/pr-1139.v2.yml new file mode 100644 index 000000000..b2b5fd21e --- /dev/null +++ b/changelog/@unreleased/pr-1139.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Migrate the CollectionStreamForEach refactor to error-prone + links: + - https://github.com/palantir/gradle-baseline/pull/1139