Skip to content

Commit

Permalink
Roll back J2KT Map.merge nullness annotations to the more flexible …
Browse files Browse the repository at this point in the history
…(if possibly less convenient) original version, updating Guava.

This is another annotation from the [On Leveraging Tests to Infer Nullable Annotations](google/guava#6510) data.

PiperOrigin-RevId: 546272042
  • Loading branch information
cpovirk authored and Copybara-Service committed Jul 7, 2023
1 parent b7ef8b4 commit 62ef5b7
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 22 deletions.
3 changes: 2 additions & 1 deletion j2kt/jre/java/common/java/util/Map.java
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ default void forEach(BiConsumer<? super K, ? super V> action) {
Set<K> keySet();

@KtName("java_merge")
default V merge(K key, V value, BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
default @Nullable V merge(
K key, V value, BiFunction<? super V, ? super V, ? extends @Nullable V> remappingFunction) {
return ktNative();
}

Expand Down
17 changes: 9 additions & 8 deletions j2kt/jre/java/common/javaemul/lang/JavaMap.kt
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ interface JavaMap<K, V> : MutableMap<K, V> {

// The Java `merge` function exists on Kotlin/JVM but is undocumented. So we rename our `merge` to
// avoid a collision.
fun java_merge(key: K, value: V, remap: BiFunction<in V, in V, out V>): V =
fun java_merge(key: K, value: V, remap: BiFunction<in V, in V, out V?>): V? =
default_merge(key, value, remap)

fun java_putAll(t: MutableMap<out K, out V>)
Expand Down Expand Up @@ -122,9 +122,11 @@ fun <K, V> MutableMap<K, V>.java_getOrDefault(key: Any?, defaultValue: V?): V? =
if (this is JavaMap) java_getOrDefault(key, defaultValue)
else default_getOrDefault(key, defaultValue)

// TODO(b/275568112): Consider allowing BiFunction<in V, in V, out V?>
fun <K, V> MutableMap<K, V>.java_merge(key: K, value: V, remap: BiFunction<in V, in V, out V>): V =
if (this is JavaMap) java_merge(key, value, remap) else default_merge(key, value, remap)
fun <K, V> MutableMap<K, V>.java_merge(
key: K,
value: V,
remap: BiFunction<in V, in V, out V?>
): V? = if (this is JavaMap) java_merge(key, value, remap) else default_merge(key, value, remap)

fun <K, V> MutableMap<K, V>.java_putIfAbsent(key: K, value: V): V? =
if (this is JavaMap) java_putIfAbsent(key, value) else default_putIfAbsent(key, value)
Expand Down Expand Up @@ -204,11 +206,10 @@ private fun <K, V> MutableMap<K, V>.default_getOrDefault(key: Any?, defaultValue
private fun <K, V> MutableMap<K, V>.default_merge(
key: K,
value: V,
// TODO(b/275568112): Consider allowing BiFunction<in V, in V, out V?>
remap: BiFunction<in V, in V, out V>
): V {
remap: BiFunction<in V, in V, out V?>
): V? {
val oldValue = get(key)
val newValue: V = if (oldValue == null) value else remap.apply(oldValue, value)
val newValue: V? = if (oldValue == null) value else remap.apply(oldValue, value)
if (newValue == null) {
remove(key)
} else {
Expand Down
29 changes: 16 additions & 13 deletions j2kt/jre/javatests/smoke/CollectionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,10 @@ public void forEach(BiConsumer<? super K, ? super V> action) {
}

@Override
@SuppressWarnings("nullness:override.param") // Checker expects @PolyNull, JSpecify doesn't
public V merge(K key, V value, BiFunction<? super V, ? super V, ? extends V> remap) {
// Checker framework uses @PolyNull V, JSpecify uses @Nullable V
@SuppressWarnings("nullness:override")
public @Nullable V merge(
K key, V value, BiFunction<? super V, ? super V, ? extends @Nullable V> remap) {
return super.merge(key, value, remap);
}

Expand Down Expand Up @@ -503,17 +505,18 @@ public void testVector() {
assertEquals(1, list.size());
}

// TODO(b/275568193): Consider allowing sort(null).
// @Test
// public void testListSort() {
// List<Integer> list = new ArrayList<>();
// list.add(1);
// list.add(4);
// list.add(2);
// list.add(3);
// list.sort(null);
// assertEquals(new Integer[] {1, 2, 3, 4}, list.toArray());
// }
@Test
// TODO: b/275568193: Remove suppression after CF permits sort(null).
@SuppressWarnings("nullness:argument.type")
public void testListSort() {
List<Integer> list = new ArrayList<>();
list.add(1);
list.add(4);
list.add(2);
list.add(3);
list.sort(null);
assertEquals(new Integer[] {1, 2, 3, 4}, list.toArray());
}

@Test
public void testListSortComparator() {
Expand Down

0 comments on commit 62ef5b7

Please sign in to comment.