Skip to content

Commit

Permalink
Introduce IsInstanceLambdaUsage check (#323)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ptijohn committed Nov 4, 2022
1 parent 7febccb commit 42e632e
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;

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.bugpatterns.BugChecker.LambdaExpressionTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.InstanceOfTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.Tree.Kind;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

/**
* A {@link BugChecker} that flags lambda expressions that can be replaced with a method reference
* of the form {@code T.class::isInstance}.
*
* @see MethodReferenceUsage
*/
// XXX: Consider folding this logic into the `MethodReferenceUsage` check.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer `Class::isInstance` method reference over equivalent lambda expression",
link = BUG_PATTERNS_BASE_URL + "IsInstanceLambdaUsage",
linkType = CUSTOM,
severity = SUGGESTION,
tags = SIMPLIFICATION)
public final class IsInstanceLambdaUsage extends BugChecker implements LambdaExpressionTreeMatcher {
private static final long serialVersionUID = 1L;

/** Instantiates a new {@link IsInstanceLambdaUsage} instance. */
public IsInstanceLambdaUsage() {}

@Override
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
if (tree.getKind() != Kind.LAMBDA_EXPRESSION || tree.getBody().getKind() != Kind.INSTANCE_OF) {
return Description.NO_MATCH;
}

return describeMatch(
tree,
SuggestedFix.replace(
tree,
SourceCode.treeToString(((InstanceOfTree) tree.getBody()).getType(), state)
+ ".class::isInstance"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

/**
* A {@link BugChecker} that flags lambda expressions that can be replaced with method references.
*
* @see IsInstanceLambdaUsage
*/
// XXX: Other custom expressions we could rewrite:
// - `a -> "str" + a` to `"str"::concat`. But only if `str` is provably non-null.
Expand All @@ -50,6 +52,9 @@
// `not(some::reference)`.
// XXX: This check is extremely inefficient due to its reliance on `SuggestedFixes.compilesWithFix`.
// Palantir's `LambdaMethodReference` check seems to suffer a similar issue at this time.
// XXX: Expressions of the form `i -> SomeType.class.isInstance(i)` are not replaced; fix that using
// a suitable generalization.
// XXX: Consider folding the `IsInstanceLambdaUsage` check into this class.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer method references over lambda expressions",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

final class IsInstanceLambdaUsageTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(IsInstanceLambdaUsage.class, getClass());
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(IsInstanceLambdaUsage.class, getClass());

@Test
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" // BUG: Diagnostic contains:",
" Stream.of(1).filter(i -> i instanceof Integer);",
" Stream.of(2).filter(Integer.class::isInstance);",
" }",
"}")
.doTest();
}

@Test
void replacement() {
refactoringTestHelper
.addInputLines(
"A.java",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" Stream.of(1).filter(i -> i instanceof Integer);",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.stream.Stream;",
"",
"class A {",
" void m() {",
" Stream.of(1).filter(Integer.class::isInstance);",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

0 comments on commit 42e632e

Please sign in to comment.