remove getOrElseUpdate / updateWith overrides in non-concurrent map w…

…rapper
scala · Sep 13, 2022 · 7abe9b8 · 7abe9b8
1 parent 094d970
commit 7abe9b8
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 19 deletions.
diff --git a/src/library/scala/collection/convert/JavaCollectionWrappers.scala b/src/library/scala/collection/convert/JavaCollectionWrappers.scala
@@ -333,11 +333,6 @@ private[collection] object JavaCollectionWrappers extends Serializable {
       else
         None
     }
-    override def getOrElseUpdate(key: K, op: => V): V =
-      underlying.computeIfAbsent(key, _ => op) match {
-        case null => update(key, null.asInstanceOf[V]); null.asInstanceOf[V]
-        case v    => v
-      }
 
     def addOne(kv: (K, V)): this.type = { underlying.put(kv._1, kv._2); this }
     def subtractOne(key: K): this.type = { underlying remove key; this }
@@ -360,19 +355,6 @@ private[collection] object JavaCollectionWrappers extends Serializable {
 
     override def update(k: K, v: V): Unit = underlying.put(k, v)
 
-    override def updateWith(key: K)(remappingFunction: Option[V] => Option[V]): Option[V] = {
-      def remap(k: K, v: V): V =
-        remappingFunction(Option(v)) match {
-          case Some(null) => throw PutNull
-          case Some(x)    => x
-          case None       => null.asInstanceOf[V]
-        }
-      try Option(underlying.compute(key, remap))
-      catch {
-        case PutNull => update(key, null.asInstanceOf[V]); Some(null.asInstanceOf[V])
-      }
-    }
-
     // support Some(null) if currently bound to null
     override def remove(k: K): Option[V] = {
       var result: Option[V] = None
@@ -483,7 +465,7 @@ private[collection] object JavaCollectionWrappers extends Serializable {
     override def updateWith(key: K)(remappingFunction: Option[V] => Option[V]): Option[V] = {
       def remap(k: K, v: V): V =
         remappingFunction(Option(v)) match {
-          case Some(null) => throw PutNull
+          case Some(null) => throw PutNull // see scala/scala#10129
           case Some(x)    => x
           case None       => null.asInstanceOf[V]
         }

diff --git a/test/junit/scala/collection/convert/MapWrapperTest.scala b/test/junit/scala/collection/convert/MapWrapperTest.scala
@@ -155,4 +155,23 @@ class MapWrapperTest {
     wrapped.updateWith("K")(_ => Some(v))
     assertEquals(2, count)
   }
+
+  @Test def `getOrElseUpdate / updateWith support should insert null`: Unit = {
+    val jmap = new jutil.HashMap[String, String]()
+    val wrapped = jmap.asScala
+
+    wrapped.getOrElseUpdate("a", null)
+    assertTrue(jmap.containsKey("a"))
+
+    wrapped.getOrElseUpdate(null, "x")
+    assertTrue(jmap.containsKey(null))
+
+    jmap.clear()
+
+    wrapped.updateWith("b")(_ => Some(null))
+    assertTrue(jmap.containsKey("b"))
+
+    wrapped.updateWith(null)(_ => Some("x"))
+    assertTrue(jmap.containsKey(null))
+  }
 }