Skip to content

Commit

Permalink
Migrate the CollectionStreamForEach refactor to error-prone (#1139)
Browse files Browse the repository at this point in the history
Migrate the CollectionStreamForEach refactor to error-prone
  • Loading branch information
carterkozak authored and bulldozer-bot[bot] committed Jan 2, 2020
1 parent 505080f commit b3e9344
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 53 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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<ExpressionTree> STREAM_FOR_EACH = MethodMatchers.instanceMethod()
.onDescendantOf(Stream.class.getName())
.namedAnyOf("forEach", "forEachOrdered")
.withParameters(Consumer.class.getName());

private static final Matcher<ExpressionTree> COLLECTION_STREAM = MethodMatchers.instanceMethod()
.onDescendantOf(Collection.class.getName())
.named("stream")
.withParameters();

private static final Matcher<MethodInvocationTree> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<String> in) {",
" in.stream().forEach(System.out::println);",
" in.stream().forEachOrdered(System.out::println);",
" in.stream().<String>forEach(System.out::println);",
" in.stream().<String>forEachOrdered(System.out::println);",
" }",
"}")
.hasOutputLines(
.addOutputLines(
"Test.java",
"import java.util.List;",
"public class Test {",
" void f(List<String> in) {",
" in.forEach(System.out::println);",
" in.forEach(System.out::println);",
" in.<String>forEach(System.out::println);",
" in.<String>forEach(System.out::println);",
" }",
"}");
"}")
.doTest();
}

private RefactoringValidator fix() {
return RefactoringValidator.of(new CollectionStreamForEach(), getClass());
}
}

This file was deleted.

5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1139.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Migrate the CollectionStreamForEach refactor to error-prone
links:
- https://github.com/palantir/gradle-baseline/pull/1139
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class BaselineErrorProneExtension {
// Baseline checks
"BracesRequired",
"CatchBlockLogException",
"CollectionStreamForEach",
// TODO(ckozak): re-enable pending scala check
//"CatchSpecificity",
"ExecutorSubmitRunnableFutureIgnored",
Expand Down

0 comments on commit b3e9344

Please sign in to comment.