ImmutableChecker: handle method references.

I think the only requirement for a method reference is that the receiver should be of an immutable type. PiperOrigin-RevId: 442843308
google · Apr 19, 2022 · f174a80 · f174a80
1 parent c3a263c
commit f174a80
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 2 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java
@@ -21,10 +21,12 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
+import static com.google.errorprone.util.ASTHelpers.getReceiverType;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
 import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
 import static com.google.errorprone.util.ASTHelpers.isSubtype;
+import static com.google.errorprone.util.ASTHelpers.targetType;
 import static java.lang.String.format;
 import static java.util.stream.Collectors.joining;
 
@@ -244,17 +246,35 @@ private boolean hasImmutableAnnotation(TypeSymbol tsym, VisitorState state) {
         .anyMatch(annotation -> hasAnnotation(tsym, annotation, state));
   }
 
-  // check instantiations of `@ImmutableTypeParameter`s in method references
   @Override
   public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
+    // check instantiations of `@ImmutableTypeParameter`s in method references
     checkInvocation(tree, getSymbol(tree), ((JCMemberReference) tree).referentType, state);
     if (!matchLambdas) {
       return NO_MATCH;
     }
-    TypeSymbol lambdaType = getType(tree).tsym;
+    TypeSymbol lambdaType = targetType(state).type().tsym;
     if (!hasImmutableAnnotation(lambdaType, state)) {
       return NO_MATCH;
     }
+    if (getSymbol(getReceiver(tree)) instanceof ClassSymbol) {
+      return NO_MATCH;
+    }
+    var receiverType = getReceiverType(tree);
+    ImmutableAnalysis analysis = createImmutableAnalysis(state);
+    ImmutableSet<String> typarams =
+        immutableTypeParametersInScope(getSymbol(tree), state, analysis);
+    var violation =
+        analysis.isThreadSafeType(/* allowContainerTypeParameters= */ true, typarams, receiverType);
+    if (violation.isPresent()) {
+      return buildDescription(tree)
+          .setMessage(
+              "This method reference implements @Immutable interface "
+                  + lambdaType.getSimpleName()
+                  + ", but "
+                  + violation.message())
+          .build();
+    }
     return NO_MATCH;
   }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
@@ -2686,9 +2686,75 @@ public void methodReference_onMutableType() {
             "  @Immutable interface ImmutableFunction { String apply(String b); }",
             "  void test(ImmutableFunction f) {",
             "    Map<String, String> map = new HashMap<>();",
+            "    // BUG: Diagnostic contains:",
             "    test(map::get);",
             "  }",
             "}")
         .doTest();
   }
+
+  @Test
+  public void methodReference_onExpressionWithMutableType() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.common.collect.Maps;",
+            "import com.google.errorprone.annotations.Immutable;",
+            "abstract class Test {",
+            "  @Immutable interface ImmutableFunction { String apply(String b); }",
+            "  void test(ImmutableFunction f) {",
+            "    // BUG: Diagnostic contains:",
+            "    test(Maps.<String, String>newHashMap()::get);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReference_toStaticMethod() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.common.collect.Lists;",
+            "import com.google.errorprone.annotations.Immutable;",
+            "abstract class Test {",
+            "  @Immutable interface ImmutableProvider { Object get(); }",
+            "  void test(ImmutableProvider f) {",
+            "    test(Lists::newArrayList);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReference_toUnboundMethodReference() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.errorprone.annotations.Immutable;",
+            "import java.util.Set;",
+            "abstract class Test {",
+            "  @Immutable interface ImmutableBiConsumer { void accept(Set<String> xs, String x); }",
+            "  void test(ImmutableBiConsumer c) {",
+            "    test(Set::add);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReference_toConstructor() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.errorprone.annotations.Immutable;",
+            "import java.util.ArrayList;",
+            "abstract class Test {",
+            "  @Immutable interface ImmutableProvider { Object get(); }",
+            "  void test(ImmutableProvider f) {",
+            "    test(ArrayList::new);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }