From 9ba22491bf3e7e8a60ffc6e1371b8951a052dc53 Mon Sep 17 00:00:00 2001 From: Kurt Alfred Kluever Date: Wed, 3 Dec 2025 06:32:16 -0800 Subject: [PATCH] Add static analysis to prevent calling `liteProtoSubject.isEqualTo(Builder)` (and un-deprecate those methods at the same time since they're now uncallable). RELNOTES=n/a PiperOrigin-RevId: 839732476 --- .../extensions/proto/LiteProtoSubject.java | 33 +++++++------------ .../proto/LiteProtoSubjectTest.java | 1 + 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/extensions/liteproto/src/main/java/com/google/common/truth/extensions/proto/LiteProtoSubject.java b/extensions/liteproto/src/main/java/com/google/common/truth/extensions/proto/LiteProtoSubject.java index 0f476ad26..17544c705 100644 --- a/extensions/liteproto/src/main/java/com/google/common/truth/extensions/proto/LiteProtoSubject.java +++ b/extensions/liteproto/src/main/java/com/google/common/truth/extensions/proto/LiteProtoSubject.java @@ -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()}. + * DO NOT CALL THIS METHOD!. 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 ` 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); } @@ -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()}. + * DO NOT CALL THIS METHOD!. 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); } diff --git a/extensions/liteproto/src/test/java/com/google/common/truth/extensions/proto/LiteProtoSubjectTest.java b/extensions/liteproto/src/test/java/com/google/common/truth/extensions/proto/LiteProtoSubjectTest.java index 205867d67..787d320f8 100644 --- a/extensions/liteproto/src/test/java/com/google/common/truth/extensions/proto/LiteProtoSubjectTest.java +++ b/extensions/liteproto/src/test/java/com/google/common/truth/extensions/proto/LiteProtoSubjectTest.java @@ -161,6 +161,7 @@ public void subjectMethods() { } @Test + @SuppressWarnings("DoNotCall") public void isEqualTo_success() { expectThat(null).isEqualTo(null); expectThat(null).isNull();