Skip to content

Commit 6b5956a

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Avoid calling checkNotNull on nullable values except during actual precondition checks.
It's not that we're not going to make such calls illegal, I promise :) I mean, we certainly aren't going to _in general_, but I am tempted for `com.google.common`, as discussed on cl/372346107 :) (It would have caught the problem of cl/612591080!) I'm testing what would happen if we did do it for `com.google.common` in case it shakes out any more bugs. It does reveal that I didn't complete the cleanup of cl/612591080. And it reveals a few places where we'd normally use `requireNonNull`, since the checks aren't "preconditions" in the sense of "the caller did something wrong" (from cl/15376243 and cl/526930990). I've made those changes. (I would have made some more changes if I had tried to address more of `com.google.common`. But I stuck to the "main" packages, and I didn't even fix enough errors to see full results.) Honestly, the more interesting thing that this exercise revealed was that there are more cases in which I'm especially sympathetic to calling `checkNotNull` on nullable values: - `DummyProxy` is making an `InvocationHandler` perform automatic precondition tests based on annotations on the interface it's implementing. - `EqualsTester` and Truth have permissive signatures because they're test utilities, as documented in cl/578260904 and discussed during the Truth CLs. And the yet more interesting thing that it revealed is that we may want to use `@NonNull` here in the future, similar to what we've discussed in #6824. RELNOTES=n/a PiperOrigin-RevId: 612937549
1 parent 0b1c477 commit 6b5956a

File tree

6 files changed

+22
-22
lines changed

6 files changed

+22
-22
lines changed

android/guava-testlib/src/com/google/common/testing/FreshValueGenerator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Throwables.throwIfUnchecked;
21+
import static java.util.Objects.requireNonNull;
2122

2223
import com.google.common.annotations.GwtIncompatible;
2324
import com.google.common.annotations.J2ktIncompatible;
@@ -211,7 +212,7 @@ final <T> T newFreshProxy(final Class<T> interfaceType) {
211212
return pickInstance(rawType.getEnumConstants(), null);
212213
}
213214
if (type.isArray()) {
214-
TypeToken<?> componentType = checkNotNull(type.getComponentType());
215+
TypeToken<?> componentType = requireNonNull(type.getComponentType());
215216
Object array = Array.newInstance(componentType.getRawType(), 1);
216217
Array.set(array, 0, generate(componentType));
217218
return array;

android/guava/src/com/google/common/collect/TableCollectors.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,12 @@ void merge(V value, BinaryOperator<V> mergeFunction) {
199199
}
200200
}
201201

202-
private static <
203-
R extends @Nullable Object, C extends @Nullable Object, V extends @Nullable Object>
204-
void mergeTables(
205-
Table<R, C, V> table,
206-
@ParametricNullness R row,
207-
@ParametricNullness C column,
208-
@ParametricNullness V value,
209-
BinaryOperator<V> mergeFunction) {
202+
private static <R extends @Nullable Object, C extends @Nullable Object, V> void mergeTables(
203+
Table<R, C, V> table,
204+
@ParametricNullness R row,
205+
@ParametricNullness C column,
206+
V value,
207+
BinaryOperator<V> mergeFunction) {
210208
checkNotNull(value);
211209
V oldValue = table.get(row, column);
212210
if (oldValue == null) {

android/guava/src/com/google/common/collect/TreeMultimap.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.common.collect;
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
20+
import static java.util.Objects.requireNonNull;
2021

2122
import com.google.common.annotations.GwtCompatible;
2223
import com.google.common.annotations.GwtIncompatible;
@@ -217,8 +218,8 @@ private void writeObject(ObjectOutputStream stream) throws IOException {
217218
@SuppressWarnings("unchecked") // reading data stored by writeObject
218219
private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
219220
stream.defaultReadObject();
220-
keyComparator = checkNotNull((Comparator<? super K>) stream.readObject());
221-
valueComparator = checkNotNull((Comparator<? super V>) stream.readObject());
221+
keyComparator = requireNonNull((Comparator<? super K>) stream.readObject());
222+
valueComparator = requireNonNull((Comparator<? super V>) stream.readObject());
222223
setMap(new TreeMap<K, Collection<V>>(keyComparator));
223224
Serialization.populateMultimap(this, stream);
224225
}

guava-testlib/src/com/google/common/testing/FreshValueGenerator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Throwables.throwIfUnchecked;
21+
import static java.util.Objects.requireNonNull;
2122

2223
import com.google.common.annotations.GwtIncompatible;
2324
import com.google.common.annotations.J2ktIncompatible;
@@ -215,7 +216,7 @@ final <T> T newFreshProxy(final Class<T> interfaceType) {
215216
return pickInstance(rawType.getEnumConstants(), null);
216217
}
217218
if (type.isArray()) {
218-
TypeToken<?> componentType = checkNotNull(type.getComponentType());
219+
TypeToken<?> componentType = requireNonNull(type.getComponentType());
219220
Object array = Array.newInstance(componentType.getRawType(), 1);
220221
Array.set(array, 0, generate(componentType));
221222
return array;

guava/src/com/google/common/collect/TableCollectors.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,12 @@ void merge(V value, BinaryOperator<V> mergeFunction) {
195195
}
196196
}
197197

198-
private static <
199-
R extends @Nullable Object, C extends @Nullable Object, V extends @Nullable Object>
200-
void mergeTables(
201-
Table<R, C, V> table,
202-
@ParametricNullness R row,
203-
@ParametricNullness C column,
204-
@ParametricNullness V value,
205-
BinaryOperator<V> mergeFunction) {
198+
private static <R extends @Nullable Object, C extends @Nullable Object, V> void mergeTables(
199+
Table<R, C, V> table,
200+
@ParametricNullness R row,
201+
@ParametricNullness C column,
202+
V value,
203+
BinaryOperator<V> mergeFunction) {
206204
checkNotNull(value);
207205
V oldValue = table.get(row, column);
208206
if (oldValue == null) {

guava/src/com/google/common/collect/TreeMultimap.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.common.collect;
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
20+
import static java.util.Objects.requireNonNull;
2021

2122
import com.google.common.annotations.GwtCompatible;
2223
import com.google.common.annotations.GwtIncompatible;
@@ -217,8 +218,8 @@ private void writeObject(ObjectOutputStream stream) throws IOException {
217218
@SuppressWarnings("unchecked") // reading data stored by writeObject
218219
private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
219220
stream.defaultReadObject();
220-
keyComparator = checkNotNull((Comparator<? super K>) stream.readObject());
221-
valueComparator = checkNotNull((Comparator<? super V>) stream.readObject());
221+
keyComparator = requireNonNull((Comparator<? super K>) stream.readObject());
222+
valueComparator = requireNonNull((Comparator<? super V>) stream.readObject());
222223
setMap(new TreeMap<K, Collection<V>>(keyComparator));
223224
Serialization.populateMultimap(this, stream);
224225
}

0 commit comments

Comments
 (0)