From 62ef5b74f9bdd754774d3cdbbbd522ff4fde6210 Mon Sep 17 00:00:00 2001
From: cpovirk <cpovirk@google.com>
Date: Fri, 7 Jul 2023 07:11:04 -0700
Subject: [PATCH] Roll back J2KT `Map.merge` nullness annotations to the more
 flexible (if possibly less convenient) original version, updating Guava.

This is another annotation from the [On Leveraging Tests to Infer Nullable Annotations](https://github.com/google/guava/issues/6510) data.

PiperOrigin-RevId: 546272042
---
 j2kt/jre/java/common/java/util/Map.java       |  3 +-
 j2kt/jre/java/common/javaemul/lang/JavaMap.kt | 17 ++++++-----
 j2kt/jre/javatests/smoke/CollectionsTest.java | 29 ++++++++++---------
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/j2kt/jre/java/common/java/util/Map.java b/j2kt/jre/java/common/java/util/Map.java
index c2b4029..aecae0d 100644
--- a/j2kt/jre/java/common/java/util/Map.java
+++ b/j2kt/jre/java/common/java/util/Map.java
@@ -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();
   }
 
diff --git a/j2kt/jre/java/common/javaemul/lang/JavaMap.kt b/j2kt/jre/java/common/javaemul/lang/JavaMap.kt
index daddc7f..e728c25 100644
--- a/j2kt/jre/java/common/javaemul/lang/JavaMap.kt
+++ b/j2kt/jre/java/common/javaemul/lang/JavaMap.kt
@@ -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>)
@@ -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)
@@ -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 {
diff --git a/j2kt/jre/javatests/smoke/CollectionsTest.java b/j2kt/jre/javatests/smoke/CollectionsTest.java
index a116541..eecf429 100644
--- a/j2kt/jre/javatests/smoke/CollectionsTest.java
+++ b/j2kt/jre/javatests/smoke/CollectionsTest.java
@@ -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);
     }
 
@@ -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() {