Warn on futureA.equals(futureB) (and popular subtypes). Futures do …

…not have well-defined equality semantics.

PiperOrigin-RevId: 698360881

core/src/main/java/com/google/errorprone/bugpatterns/TypesWithUndefinedEquality.java

@@ -28,6 +28,12 @@ public enum TypesWithUndefinedEquality {
   CHAR_SEQUENCE("CharSequence", "java.lang.CharSequence"),
   COLLECTION("Collection", "java.util.Collection"),
   DATE("Date", "java.util.Date"),
+  FUTURE(
+      "Future",
+      "java.util.concurrent.Future",
+      "java.util.concurrent.CompletableFuture",
+      "com.google.common.util.concurrent.ListenableFuture",
+      "com.google.common.util.concurrent.SettableFuture"),
   IMMUTABLE_COLLECTION("ImmutableCollection", "com.google.common.collect.ImmutableCollection"),
   IMMUTABLE_MULTIMAP("ImmutableMultimap", "com.google.common.collect.ImmutableMultimap"),
   ITERABLE("Iterable", "com.google.common.collect.FluentIterable", "java.lang.Iterable"),
@@ -45,6 +51,7 @@ public enum TypesWithUndefinedEquality {
   private final String shortName;
   private final ImmutableSet<String> typeNames;
 
+  // TODO: kak - should we add a bit which determines if we should check isSameType or isSubtype?
   TypesWithUndefinedEquality(String shortName, String... typeNames) {
     this.shortName = shortName;
     this.typeNames = ImmutableSet.copyOf(typeNames);

core/src/main/java/com/google/errorprone/bugpatterns/UndefinedEquals.java

@@ -19,6 +19,7 @@
 import static com.google.common.collect.Iterables.getLast;
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
 import static com.google.errorprone.matchers.Matchers.allOf;
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.assertEqualsInvocation;
@@ -55,7 +56,7 @@
  * @author [email protected] (Eleanor Harris)
  */
 @BugPattern(
-    summary = "This type is not guaranteed to implement a useful #equals method.",
+    summary = "This type is not guaranteed to implement a useful equals() method.",
     severity = WARNING)
 public final class UndefinedEquals extends BugChecker implements MethodInvocationTreeMatcher {
 
@@ -102,13 +103,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
         .map(
             b ->
                 buildDescription(tree)
-                    .setMessage(
-                        "Subtypes of "
-                            + b.shortName()
-                            + " are not guaranteed to implement a useful #equals method.")
-                    .addFix(
-                        generateFix(tree, state, receiver, argument)
-                            .orElse(SuggestedFix.emptyFix()))
+                    .setMessage(b.shortName() + " does not have well-defined equality semantics")
+                    .addFix(generateFix(tree, state, receiver, argument).orElse(emptyFix()))
                     .build())
         .orElse(Description.NO_MATCH);
   }
@@ -140,18 +136,18 @@ && isSubtype(getType(receiver), type, state)) {
 
       // If both are subtypes of Iterable, rewrite
       Type iterableType = state.getSymtab().iterableType;
-      Type multimapType = COM_GOOGLE_COMMON_COLLECT_MULTIMAP.get(state);
+      Type multimapType = MULTIMAP.get(state);
       Optional<SuggestedFix> fix =
-          firstPresent(
-              generateTruthFix.apply(iterableType, "containsExactlyElementsIn"),
-              generateTruthFix.apply(multimapType, "containsExactlyEntriesIn"));
+          generateTruthFix
+              .apply(iterableType, "containsExactlyElementsIn")
+              .or(() -> generateTruthFix.apply(multimapType, "containsExactlyEntriesIn"));
       if (fix.isPresent()) {
         return fix;
       }
     }
 
     // Generate fix for CharSequence
-    Type charSequenceType = JAVA_LANG_CHARSEQUENCE.get(state);
+    Type charSequenceType = CHAR_SEQUENCE.get(state);
     BiFunction<Tree, Tree, Optional<SuggestedFix>> generateCharSequenceFix =
         (maybeCharSequence, maybeString) -> {
           if (charSequenceType != null
@@ -161,23 +157,14 @@ && isSameType(getType(maybeString), state.getSymtab().stringType, state)) {
           }
           return Optional.empty();
         };
-    return firstPresent(
-        generateCharSequenceFix.apply(receiver, argument),
-        generateCharSequenceFix.apply(argument, receiver));
+    return generateCharSequenceFix
+        .apply(receiver, argument)
+        .or(() -> generateCharSequenceFix.apply(argument, receiver));
   }
 
-  private static <T> Optional<T> firstPresent(Optional<T>... optionals) {
-    for (Optional<T> optional : optionals) {
-      if (optional.isPresent()) {
-        return optional;
-      }
-    }
-    return Optional.empty();
-  }
-
-  private static final Supplier<Type> COM_GOOGLE_COMMON_COLLECT_MULTIMAP =
+  private static final Supplier<Type> MULTIMAP =
       VisitorState.memoize(state -> state.getTypeFromString("com.google.common.collect.Multimap"));
 
-  private static final Supplier<Type> JAVA_LANG_CHARSEQUENCE =
+  private static final Supplier<Type> CHAR_SEQUENCE =
       VisitorState.memoize(state -> state.getTypeFromString("java.lang.CharSequence"));
 }

core/src/test/java/com/google/errorprone/bugpatterns/UndefinedEqualsTest.java

@@ -44,7 +44,7 @@ public void positiveInstanceEquals() {
 
             class Test {
               void f(Queue a, Queue b) {
-                // BUG: Diagnostic contains: Subtypes of Queue are not guaranteed
+                // BUG: Diagnostic contains: Queue does not have well-defined equality semantics
                 a.equals(b);
               }
             }
@@ -514,4 +514,40 @@ void f(CharSequence a, String b) {
             """)
         .doTest();
   }
+
+  @Test
+  public void futures() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            import com.google.common.util.concurrent.ListenableFuture;
+            import com.google.common.util.concurrent.SettableFuture;
+            import java.util.concurrent.CompletableFuture;
+            import java.util.concurrent.Future;
+
+            class Test {
+              void listenableFuture(ListenableFuture a, ListenableFuture b) {
+                // BUG: Diagnostic contains: Future does not have well-defined equality semantics
+                a.equals(b);
+              }
+
+              void settableFuture(SettableFuture a, SettableFuture b) {
+                // BUG: Diagnostic contains: Future does not have well-defined equality semantics
+                a.equals(b);
+              }
+
+              void completableFuture(CompletableFuture a, CompletableFuture b) {
+                // BUG: Diagnostic contains: Future does not have well-defined equality semantics
+                a.equals(b);
+              }
+
+              void future(Future a, Future b) {
+                // BUG: Diagnostic contains: Future does not have well-defined equality semantics
+                a.equals(b);
+              }
+            }
+            """)
+        .doTest();
+  }
 }