Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -124,31 +124,23 @@ public void isEqualTo(@Nullable Object expected) {
}

/**
* @deprecated A Builder can never compare equal to a MessageLite instance. Use {@code build()},
* or {@code buildPartial()} on the argument to get a MessageLite for comparison instead. Or,
* if you are passing {@code null}, use {@link #isNull()}.
* <b>DO NOT CALL THIS METHOD!</b>. A {@link MessageLite.Builder} will never compare equal to a
* MessageLite instance. Use {@code build()}, or {@code buildPartial()} on the argument to get a
* MessageLite for comparison instead. Or, if you are passing {@code null}, use {@link #isNull()}.
*/
/*
* TODO(cpovirk): Consider @DoNotCall -- or probably some other static analysis, given the problem
* discussed in the rest of this comment.
*
* The problem: isEqualTo(null) resolves to this overload (since this overload is more specific
* than isEqualTo(Object)), so @DoNotCall would break all assertions of that form.
* NOTE: we don't actually mark this as deprecated (or @DoNotCall) because isEqualTo(null)
* resolves to this overload (since this overload is more specific than isEqualTo(Object)).
*
* To address that, we could try also adding something like `<NullT extends Impossible &
* MessageLite.Builder> void isEqualTo(NullT)` and hoping that isEqualTo(null) would resolve to
* that instead. That would also have the benefit of making isEqualTo(null) not produce a
* deprecation warning (though of course people "should" use isNull(): b/17294077). But yuck.
*
* Given the null issue, maybe we should never have added this overload in the first place,
* instead adding static analysis specific to MessageLite-MessageLite.Builder comparisons. (Sadly,
* we can't remove it now without breaking binary compatibility.)
*
* Still, we could add static analysis to produce a compile error for isEqualTo(Builder) this even
* today, even without using @DoNotCall. And then we could consider removing @Deprecated to stop
* spamming the people who call isEqualTo(null).
* Given the null issue, maybe we should never have added this overload in the first place!
* In cl/839267698, we added static analysis to MessageLite-MessageLite.Builder comparisons.
* However, we cannot remove this method without breaking binary compatibility.
*/
@Deprecated
public void isEqualTo(MessageLite.@Nullable Builder builder) {
isEqualTo((Object) builder);
}
Expand All @@ -175,12 +167,11 @@ public void isNotEqualTo(@Nullable Object expected) {
}

/**
* @deprecated A Builder will never compare equal to a MessageLite instance. Use {@code build()},
* or {@code buildPartial()} on the argument to get a MessageLite for comparison instead. Or,
* if you are passing {@code null}, use {@link #isNotNull()}.
* <b>DO NOT CALL THIS METHOD!</b>. A {@link MessageLite.Builder} will never compare equal to a
* {@link MessageLite} instance. Use {@code build()}, or {@code buildPartial()} on the argument to
* get a {@link MessageLite} for comparison instead. Or, if you are passing {@code null}, use
* {@link #isNotNull()}.
*/
// TODO(cpovirk): Consider @DoNotCall or other static analysis. (See isEqualTo(Builder).)
@Deprecated
public void isNotEqualTo(MessageLite.@Nullable Builder builder) {
isNotEqualTo((Object) builder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ public void subjectMethods() {
}

@Test
@SuppressWarnings("DoNotCall")
public void isEqualTo_success() {
expectThat(null).isEqualTo(null);
expectThat(null).isNull();
Expand Down