Skip to content

Commit 4dbf78d

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Improve nullness of types in some APIs related to Map merging, and fix Collectors.toMap null-handling.
- Restrict `Collections.toMap` value-type arguments to non-nullable types. - ...in J2KT, following what [we'd found in JSpecify research](jspecify/jdk@15eda89) - Fix `Collections.toMap` to remove the key in question when `mergeFunction` returns `null`. - ...in J2KT - ...in J2CL - Use `@NonNull` / `& Any` in a few places in `Map.merge` and `Map.computeIfPresent`. - ...in J2KT - ...in Guava `Map` implementations, even though we don't yet include `@NonNull` annotations in the JDK APIs that we build Guava against. (See post-submit discussion on cl/559605577.) - Use `@Nullable` (to match the existing Kotlin `?` types) in the return types of `Map.computeIfPresent` and `Map.compute`. - ...in J2KT - Test a bunch of this. Note that the test for `mergeFunction` has to work around an overly restricted `toMap` signature that J2KT inherited from JSpecify. As discussed in a code comment there, this is fundamentally the same issue as we have in Guava with `ImmutableMap.toImmutableMap`, which is discussed as part of #6824. RELNOTES=n/a PiperOrigin-RevId: 580659517
1 parent fe62266 commit 4dbf78d

File tree

2 files changed

+24
-22
lines changed

2 files changed

+24
-22
lines changed

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

+18-14
Original file line numberDiff line numberDiff line change
@@ -1735,15 +1735,15 @@ public V computeIfAbsent(
17351735
throw new UnsupportedOperationException();
17361736
}
17371737

1738-
/*
1739-
* TODO(cpovirk): Uncomment the @NonNull annotations below once our JDK stubs and J2KT
1740-
* emulations include them.
1741-
*/
17421738
@Override
17431739
@CheckForNull
1740+
/*
1741+
* Our checker arguably should produce a nullness error here until we see @NonNull in JDK APIs.
1742+
* But it doesn't, which may be a sign that we still permit parameter contravariance in some
1743+
* cases?
1744+
*/
17441745
public V computeIfPresent(
1745-
K key,
1746-
BiFunction<? super K, ? super /*@NonNull*/ V, ? extends @Nullable V> remappingFunction) {
1746+
K key, BiFunction<? super K, ? super @NonNull V, ? extends @Nullable V> remappingFunction) {
17471747
throw new UnsupportedOperationException();
17481748
}
17491749

@@ -1757,11 +1757,11 @@ public V compute(
17571757

17581758
@Override
17591759
@CheckForNull
1760+
@SuppressWarnings("nullness") // TODO(b/262880368): Remove once we see @NonNull in JDK APIs
17601761
public V merge(
17611762
K key,
1762-
/*@NonNull*/ V value,
1763-
BiFunction<? super /*@NonNull*/ V, ? super /*@NonNull*/ V, ? extends @Nullable V>
1764-
function) {
1763+
@NonNull V value,
1764+
BiFunction<? super @NonNull V, ? super @NonNull V, ? extends @Nullable V> function) {
17651765
throw new UnsupportedOperationException();
17661766
}
17671767

@@ -3659,9 +3659,13 @@ public V computeIfAbsent(
36593659
*/
36603660
@Override
36613661
@CheckForNull
3662+
/*
3663+
* Our checker arguably should produce a nullness error here until we see @NonNull in JDK APIs.
3664+
* But it doesn't, which may be a sign that we still permit parameter contravariance in some
3665+
* cases?
3666+
*/
36623667
public V computeIfPresent(
3663-
K key,
3664-
BiFunction<? super K, ? super /*@NonNull*/ V, ? extends @Nullable V> remappingFunction) {
3668+
K key, BiFunction<? super K, ? super @NonNull V, ? extends @Nullable V> remappingFunction) {
36653669
throw new UnsupportedOperationException();
36663670
}
36673671

@@ -3675,11 +3679,11 @@ public V compute(
36753679

36763680
@Override
36773681
@CheckForNull
3682+
@SuppressWarnings("nullness") // TODO(b/262880368): Remove once we see @NonNull in JDK APIs
36783683
public V merge(
36793684
K key,
3680-
/*@NonNull*/ V value,
3681-
BiFunction<? super /*@NonNull*/ V, ? super /*@NonNull*/ V, ? extends @Nullable V>
3682-
function) {
3685+
@NonNull V value,
3686+
BiFunction<? super @NonNull V, ? super @NonNull V, ? extends @Nullable V> function) {
36833687
throw new UnsupportedOperationException();
36843688
}
36853689

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

+6-8
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import java.util.function.UnaryOperator;
5050
import java.util.stream.Stream;
5151
import javax.annotation.CheckForNull;
52+
import org.checkerframework.checker.nullness.qual.NonNull;
5253
import org.checkerframework.checker.nullness.qual.Nullable;
5354

5455
/**
@@ -1187,15 +1188,11 @@ public V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction
11871188
}
11881189
}
11891190

1190-
/*
1191-
* TODO(cpovirk): Uncomment the @NonNull annotations below once our JDK stubs and J2KT
1192-
* emulations include them.
1193-
*/
11941191
@Override
11951192
@CheckForNull
1193+
@SuppressWarnings("nullness") // TODO(b/262880368): Remove once we see @NonNull in JDK APIs
11961194
public V computeIfPresent(
1197-
K key,
1198-
BiFunction<? super K, ? super /*@NonNull*/ V, ? extends @Nullable V> remappingFunction) {
1195+
K key, BiFunction<? super K, ? super @NonNull V, ? extends @Nullable V> remappingFunction) {
11991196
synchronized (mutex) {
12001197
return delegate().computeIfPresent(key, remappingFunction);
12011198
}
@@ -1213,10 +1210,11 @@ public V compute(
12131210

12141211
@Override
12151212
@CheckForNull
1213+
@SuppressWarnings("nullness") // TODO(b/262880368): Remove once we see @NonNull in JDK APIs
12161214
public V merge(
12171215
K key,
1218-
/*@NonNull*/ V value,
1219-
BiFunction<? super /*@NonNull*/ V, ? super /*@NonNull*/ V, ? extends @Nullable V>
1216+
@NonNull V value,
1217+
BiFunction<? super @NonNull V, ? super @NonNull V, ? extends @Nullable V>
12201218
remappingFunction) {
12211219
synchronized (mutex) {
12221220
return delegate().merge(key, value, remappingFunction);

0 commit comments

Comments
 (0)