From 45182fbdd08e2994e477061efe29961e996f7b75 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 25 Jun 2020 12:54:37 -0400 Subject: [PATCH 1/3] fix #1431: Avoid rewriting lambdas into ambiguous references Note that this uses the expensive `SuggestedFixes.compilesWithFix` check which can result in poor performance, particularly on large projects. Fortunately this case isn't very common (no occurrences in a very large internal project). --- .../errorprone/LambdaMethodReference.java | 12 ++++----- .../errorprone/LambdaMethodReferenceTest.java | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java index 188a51a00..1447d47f0 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java @@ -42,11 +42,12 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.parser.Tokens; + +import javax.annotation.Nullable; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; -import javax.annotation.Nullable; @AutoService(BugChecker.class) @BugPattern( @@ -59,8 +60,6 @@ @SuppressWarnings("checkstyle:CyclomaticComplexity") public final class LambdaMethodReference extends BugChecker implements BugChecker.LambdaExpressionTreeMatcher { - private static final String MESSAGE = "Lambda should be a method reference"; - @Override public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) { LambdaExpressionTree.BodyKind bodyKind = tree.getBodyKind(); @@ -150,8 +149,7 @@ private Description convertVariableInstanceMethods( return Description.NO_MATCH; } return buildFix(methodSymbol, methodInvocation, root, state, isLocal(methodInvocation)) - .map(fix -> - buildDescription(root).setMessage(MESSAGE).addFix(fix).build()) + .map(fix -> buildDescription(root).addFix(fix).build()) .orElse(Description.NO_MATCH); } @@ -170,8 +168,8 @@ private Description convertMethodInvocations( } return buildFix(methodSymbol, methodInvocation, root, state, isLocal(methodInvocation)) - .map(fix -> - buildDescription(root).setMessage(MESSAGE).addFix(fix).build()) + .filter(fix -> SuggestedFixes.compilesWithFix(fix, state)) + .map(fix -> buildDescription(root).addFix(fix).build()) .orElse(Description.NO_MATCH); } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/LambdaMethodReferenceTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/LambdaMethodReferenceTest.java index b2bd0c435..33b1d7169 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/LambdaMethodReferenceTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/LambdaMethodReferenceTest.java @@ -727,4 +727,29 @@ public void testGuavaToJavaUtilOptional() { .expectUnchanged() .doTest(); } + + @Test + public void testSuperAmbiguous() { + refactor() + .addInputLines( + "Test.java", + "import java.util.function.BiConsumer;", + "import java.util.function.Consumer;", + "class Test {", + " interface One {", + " void call(Consumer a);", + " void call(BiConsumer a);", + " }", + " interface Two {", + " void apply(String a, String b);", + " void apply(String a);", + " }", + " void f(One one, Two two) {", + // Lambda conversion requires a cast which wouldn't necessarily be cleaner. + " one.call(value -> two.apply(value));", + " }", + "}") + .expectUnchanged() + .doTest(); + } } From f6c2a483ba13d1fdf16f68e5f54bd017efe79c6c Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 25 Jun 2020 16:54:37 +0000 Subject: [PATCH 2/3] Add generated changelog entries --- changelog/@unreleased/pr-1432.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1432.v2.yml diff --git a/changelog/@unreleased/pr-1432.v2.yml b/changelog/@unreleased/pr-1432.v2.yml new file mode 100644 index 000000000..9df9b3ac5 --- /dev/null +++ b/changelog/@unreleased/pr-1432.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Avoid rewriting lambdas into ambiguous references + links: + - https://github.com/palantir/gradle-baseline/pull/1432 From 4d0cdd3b70a003974da7526fb3c0fd904a5b2de4 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 25 Jun 2020 13:03:02 -0400 Subject: [PATCH 3/3] fmt --- .../palantir/baseline/errorprone/LambdaMethodReference.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java index 1447d47f0..3fad79386 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/LambdaMethodReference.java @@ -42,12 +42,11 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.parser.Tokens; - -import javax.annotation.Nullable; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; +import javax.annotation.Nullable; @AutoService(BugChecker.class) @BugPattern(