diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypesWithUndefinedEquality.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypesWithUndefinedEquality.java index 1785dbb849e..8ca1ba87995 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TypesWithUndefinedEquality.java +++ b/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 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); diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UndefinedEquals.java b/core/src/main/java/com/google/errorprone/bugpatterns/UndefinedEquals.java index 7e6afc1d022..73a93b82813 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UndefinedEquals.java +++ b/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 eleanorh@google.com (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 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> 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 Optional firstPresent(Optional... optionals) { - for (Optional optional : optionals) { - if (optional.isPresent()) { - return optional; - } - } - return Optional.empty(); - } - - private static final Supplier COM_GOOGLE_COMMON_COLLECT_MULTIMAP = + private static final Supplier MULTIMAP = VisitorState.memoize(state -> state.getTypeFromString("com.google.common.collect.Multimap")); - private static final Supplier JAVA_LANG_CHARSEQUENCE = + private static final Supplier CHAR_SEQUENCE = VisitorState.memoize(state -> state.getTypeFromString("java.lang.CharSequence")); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UndefinedEqualsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UndefinedEqualsTest.java index 5745ff18e4f..cffeb83f7ca 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/UndefinedEqualsTest.java +++ b/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(); + } }