Skip to content

Commit 1046226

Browse files
klueverError Prone Team
kluever
authored and
Error Prone Team
committed
Warn on futureA.equals(futureB) (and popular subtypes). Futures do not have well-defined equality semantics.
PiperOrigin-RevId: 698960797
1 parent 99d9f97 commit 1046226

3 files changed

Lines changed: 58 additions & 28 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ public enum TypesWithUndefinedEquality {
2828
CHAR_SEQUENCE("CharSequence", "java.lang.CharSequence"),
2929
COLLECTION("Collection", "java.util.Collection"),
3030
DATE("Date", "java.util.Date"),
31+
FUTURE(
32+
"Future",
33+
"java.util.concurrent.Future",
34+
"java.util.concurrent.CompletableFuture",
35+
"com.google.common.util.concurrent.ListenableFuture",
36+
"com.google.common.util.concurrent.SettableFuture"),
3137
IMMUTABLE_COLLECTION("ImmutableCollection", "com.google.common.collect.ImmutableCollection"),
3238
IMMUTABLE_MULTIMAP("ImmutableMultimap", "com.google.common.collect.ImmutableMultimap"),
3339
ITERABLE("Iterable", "com.google.common.collect.FluentIterable", "java.lang.Iterable"),
@@ -45,6 +51,7 @@ public enum TypesWithUndefinedEquality {
4551
private final String shortName;
4652
private final ImmutableSet<String> typeNames;
4753

54+
// TODO: kak - should we add a bit which determines if we should check isSameType or isSubtype?
4855
TypesWithUndefinedEquality(String shortName, String... typeNames) {
4956
this.shortName = shortName;
5057
this.typeNames = ImmutableSet.copyOf(typeNames);

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

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.collect.Iterables.getLast;
2020
import static com.google.common.collect.Iterables.getOnlyElement;
2121
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
22+
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
2223
import static com.google.errorprone.matchers.Matchers.allOf;
2324
import static com.google.errorprone.matchers.Matchers.anyOf;
2425
import static com.google.errorprone.matchers.Matchers.assertEqualsInvocation;
@@ -55,7 +56,7 @@
5556
* @author [email protected] (Eleanor Harris)
5657
*/
5758
@BugPattern(
58-
summary = "This type is not guaranteed to implement a useful #equals method.",
59+
summary = "This type is not guaranteed to implement a useful equals() method.",
5960
severity = WARNING)
6061
public final class UndefinedEquals extends BugChecker implements MethodInvocationTreeMatcher {
6162

@@ -102,13 +103,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
102103
.map(
103104
b ->
104105
buildDescription(tree)
105-
.setMessage(
106-
"Subtypes of "
107-
+ b.shortName()
108-
+ " are not guaranteed to implement a useful #equals method.")
109-
.addFix(
110-
generateFix(tree, state, receiver, argument)
111-
.orElse(SuggestedFix.emptyFix()))
106+
.setMessage(b.shortName() + " does not have well-defined equality semantics")
107+
.addFix(generateFix(tree, state, receiver, argument).orElse(emptyFix()))
112108
.build())
113109
.orElse(Description.NO_MATCH);
114110
}
@@ -140,18 +136,18 @@ && isSubtype(getType(receiver), type, state)) {
140136

141137
// If both are subtypes of Iterable, rewrite
142138
Type iterableType = state.getSymtab().iterableType;
143-
Type multimapType = COM_GOOGLE_COMMON_COLLECT_MULTIMAP.get(state);
139+
Type multimapType = MULTIMAP.get(state);
144140
Optional<SuggestedFix> fix =
145-
firstPresent(
146-
generateTruthFix.apply(iterableType, "containsExactlyElementsIn"),
147-
generateTruthFix.apply(multimapType, "containsExactlyEntriesIn"));
141+
generateTruthFix
142+
.apply(iterableType, "containsExactlyElementsIn")
143+
.or(() -> generateTruthFix.apply(multimapType, "containsExactlyEntriesIn"));
148144
if (fix.isPresent()) {
149145
return fix;
150146
}
151147
}
152148

153149
// Generate fix for CharSequence
154-
Type charSequenceType = JAVA_LANG_CHARSEQUENCE.get(state);
150+
Type charSequenceType = CHAR_SEQUENCE.get(state);
155151
BiFunction<Tree, Tree, Optional<SuggestedFix>> generateCharSequenceFix =
156152
(maybeCharSequence, maybeString) -> {
157153
if (charSequenceType != null
@@ -161,23 +157,14 @@ && isSameType(getType(maybeString), state.getSymtab().stringType, state)) {
161157
}
162158
return Optional.empty();
163159
};
164-
return firstPresent(
165-
generateCharSequenceFix.apply(receiver, argument),
166-
generateCharSequenceFix.apply(argument, receiver));
160+
return generateCharSequenceFix
161+
.apply(receiver, argument)
162+
.or(() -> generateCharSequenceFix.apply(argument, receiver));
167163
}
168164

169-
private static <T> Optional<T> firstPresent(Optional<T>... optionals) {
170-
for (Optional<T> optional : optionals) {
171-
if (optional.isPresent()) {
172-
return optional;
173-
}
174-
}
175-
return Optional.empty();
176-
}
177-
178-
private static final Supplier<Type> COM_GOOGLE_COMMON_COLLECT_MULTIMAP =
165+
private static final Supplier<Type> MULTIMAP =
179166
VisitorState.memoize(state -> state.getTypeFromString("com.google.common.collect.Multimap"));
180167

181-
private static final Supplier<Type> JAVA_LANG_CHARSEQUENCE =
168+
private static final Supplier<Type> CHAR_SEQUENCE =
182169
VisitorState.memoize(state -> state.getTypeFromString("java.lang.CharSequence"));
183170
}

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void positiveInstanceEquals() {
4444
4545
class Test {
4646
void f(Queue a, Queue b) {
47-
// BUG: Diagnostic contains: Subtypes of Queue are not guaranteed
47+
// BUG: Diagnostic contains: Queue does not have well-defined equality semantics
4848
a.equals(b);
4949
}
5050
}
@@ -514,4 +514,40 @@ void f(CharSequence a, String b) {
514514
""")
515515
.doTest();
516516
}
517+
518+
@Test
519+
public void futures() {
520+
compilationHelper
521+
.addSourceLines(
522+
"Test.java",
523+
"""
524+
import com.google.common.util.concurrent.ListenableFuture;
525+
import com.google.common.util.concurrent.SettableFuture;
526+
import java.util.concurrent.CompletableFuture;
527+
import java.util.concurrent.Future;
528+
529+
class Test {
530+
void listenableFuture(ListenableFuture a, ListenableFuture b) {
531+
// BUG: Diagnostic contains: Future does not have well-defined equality semantics
532+
a.equals(b);
533+
}
534+
535+
void settableFuture(SettableFuture a, SettableFuture b) {
536+
// BUG: Diagnostic contains: Future does not have well-defined equality semantics
537+
a.equals(b);
538+
}
539+
540+
void completableFuture(CompletableFuture a, CompletableFuture b) {
541+
// BUG: Diagnostic contains: Future does not have well-defined equality semantics
542+
a.equals(b);
543+
}
544+
545+
void future(Future a, Future b) {
546+
// BUG: Diagnostic contains: Future does not have well-defined equality semantics
547+
a.equals(b);
548+
}
549+
}
550+
""")
551+
.doTest();
552+
}
517553
}

0 commit comments

Comments
 (0)