RedundantMethodReference check to avoid unnecessary method references (…

…#1069)

RedundantMethodReference check to avoid unnecessary method references

README.md

@@ -181,6 +181,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
 - `Slf4jThrowable`: Slf4j loggers require throwables to be the last parameter otherwise a stack trace is not produced.
 - `JooqResultStreamLeak`: Autocloseable streams and cursors from jOOQ results should be obtained in a try-with-resources statement.
 - `StreamOfEmpty`: Stream.of() should be replaced with Stream.empty() to avoid unnecessary varargs allocation.
+- `RedundantMethodReference`: Redundant method reference to the same type.
 
 ### Programmatic Application
 

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/RedundantMethodReference.java

@@ -0,0 +1,88 @@
+/*
+ * (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.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.util.ASTHelpers;
+import com.sun.source.tree.MemberReferenceTree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.tree.JCTree;
+import java.util.Set;
+import javax.lang.model.element.Modifier;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        name = "RedundantMethodReference",
+        link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
+        linkType = BugPattern.LinkType.CUSTOM,
+        severity = BugPattern.SeverityLevel.SUGGESTION,
+        summary = "Redundant method reference to the same type")
+public final class RedundantMethodReference extends BugChecker implements BugChecker.MemberReferenceTreeMatcher {
+
+    @Override
+    public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
+        if (tree.getMode() != MemberReferenceTree.ReferenceMode.INVOKE) {
+            return Description.NO_MATCH;
+        }
+        if (!(tree instanceof JCTree.JCMemberReference)) {
+            return Description.NO_MATCH;
+        }
+        JCTree.JCMemberReference jcMemberReference = (JCTree.JCMemberReference) tree;
+        // Only support expression::method, not static method references or unbound method references (e.g. List::add)
+        if (jcMemberReference.kind != JCTree.JCMemberReference.ReferenceKind.BOUND) {
+            return Description.NO_MATCH;
+        }
+        Type rawResultType = ASTHelpers.getResultType(tree.getQualifierExpression());
+        if (rawResultType == null) {
+            return Description.NO_MATCH;
+        }
+        Type treeType = ASTHelpers.getType(tree);
+        if (treeType == null) {
+            return Description.NO_MATCH;
+        }
+        if (!state.getTypes().isAssignable(rawResultType, treeType)) {
+            return Description.NO_MATCH;
+        }
+        Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
+        if (methodSymbol == null) {
+            return Description.NO_MATCH;
+        }
+        // Make sure the same method is being overridden, it's important not to change method invocations on types
+        // that also happen to implement the resulting functional interface.
+        Set<Symbol.MethodSymbol> matching = ASTHelpers.findMatchingMethods(
+                methodSymbol.name,
+                symbol -> symbol.getModifiers().contains(Modifier.ABSTRACT)
+                        && methodSymbol.overrides(symbol, symbol.enclClass(), state.getTypes(), true),
+                treeType,
+                state.getTypes());
+        // Do not allow any ambiguity, size must be exactly one.
+        if (matching.size() != 1) {
+            return Description.NO_MATCH;
+        }
+        return buildDescription(tree)
+                .addFix(SuggestedFix.builder()
+                        .replace(tree, state.getSourceForNode(tree.getQualifierExpression()))
+                        .build())
+                .build();
+    }
+}

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/RedundantMethodReferenceTest.java

@@ -0,0 +1,85 @@
+/*
+ * (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.errorprone;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Execution;
+import org.junit.jupiter.api.parallel.ExecutionMode;
+
+@Execution(ExecutionMode.CONCURRENT)
+class RedundantMethodReferenceTest {
+
+    @Test
+    void testFix() {
+        fix()
+                .addInputLines(
+                        "Test.java",
+                        "import java.util.*;",
+                        "import java.util.function.*;",
+                        "class Test {",
+                        "  String func(Optional<String> in, Supplier<String> defaultValue, Collection<String> col) {",
+                        "    Comparator c = Comparator.comparing(Objects::nonNull);",
+                        "    in.ifPresent(col::add);",
+                        "    return in.orElseGet(defaultValue::get);",
+                        "  }",
+                        "}")
+                .addOutputLines(
+                        "Test.java",
+                        "import java.util.*;",
+                        "import java.util.function.*;",
+                        "class Test {",
+                        "  String func(Optional<String> in, Supplier<String> defaultValue, Collection<String> col) {",
+                        "    Comparator c = Comparator.comparing(Objects::nonNull);",
+                        "    in.ifPresent(col::add);",
+                        "    return in.orElseGet(defaultValue);",
+                        "  }",
+                        "}")
+                .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+    }
+
+    @Test
+    void testFunctionalInterfaceAdditionalMethod() {
+        fix()
+                .addInputLines(
+                        "Test.java",
+                        "import java.util.*;",
+                        "import java.util.stream.*;",
+                        "import java.util.function.*;",
+                        "class Test {",
+                        "  void func(Stream<String> in, List<String> items) {",
+                        "    Ambiguous ambiguous = new Ambiguous();",
+                        "    in.forEach(ambiguous::printError);",
+                        "  }",
+                        "  static class Ambiguous implements Consumer<String> {",
+                        "    @Override",
+                        "    public void accept(String value) {",
+                        "        System.out.println(value);",
+                        "    }",
+                        "    public void printError(String value) {",
+                        "        System.err.println(value);",
+                        "    }",
+                        "  }",
+                        "}")
+                .expectUnchanged()
+                .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+    }
+
+    private RefactoringValidator fix() {
+        return RefactoringValidator.of(new RedundantMethodReference(), getClass());
+    }
+}

changelog/@unreleased/pr-1069.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: RedundantMethodReference check to avoid unnecessary method references
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1069

gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java

@@ -34,6 +34,7 @@ public class BaselineErrorProneExtension {
             "PreferSafeLoggableExceptions",
             "PreferSafeLoggingPreconditions",
             "ReadReturnValueIgnored",
+            "RedundantMethodReference",
             "RedundantModifier",
             "Slf4jLevelCheck",
             "Slf4jLogsafeArgs",