Skip to content

Commit 29436ae

Browse files
klueverGoogle Java Core Libraries
authored andcommitted
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: 839267698
1 parent 657ecbd commit 29436ae

File tree

2 files changed

+13
-21
lines changed

2 files changed

+13
-21
lines changed

extensions/liteproto/src/main/java/com/google/common/truth/extensions/proto/LiteProtoSubject.java

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -124,31 +124,23 @@ public void isEqualTo(@Nullable Object expected) {
124124
}
125125

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

177169
/**
178-
* @deprecated A Builder will never compare equal to a MessageLite instance. Use {@code build()},
179-
* or {@code buildPartial()} on the argument to get a MessageLite for comparison instead. Or,
180-
* if you are passing {@code null}, use {@link #isNotNull()}.
170+
* <b>DO NOT CALL THIS METHOD!</b>. A {@link MessageLite.Builder} will never compare equal to a
171+
* {@link MessageLite} instance. Use {@code build()}, or {@code buildPartial()} on the argument to
172+
* get a {@link MessageLite} for comparison instead. Or, if you are passing {@code null}, use
173+
* {@link #isNotNull()}.
181174
*/
182-
// TODO(cpovirk): Consider @DoNotCall or other static analysis. (See isEqualTo(Builder).)
183-
@Deprecated
184175
public void isNotEqualTo(MessageLite.@Nullable Builder builder) {
185176
isNotEqualTo((Object) builder);
186177
}

extensions/liteproto/src/test/java/com/google/common/truth/extensions/proto/LiteProtoSubjectTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ public void subjectMethods() {
161161
}
162162

163163
@Test
164+
@SuppressWarnings("DoNotCall")
164165
public void isEqualTo_success() {
165166
expectThat(null).isEqualTo(null);
166167
expectThat(null).isNull();

0 commit comments

Comments
 (0)