Jave Map wrapper respects put/remove contract

If map contains key, always return nonEmpty.
If binding to null, return Some(null) instead of None.
This behavior is consistent with get, which is the basis
for getOrElse, lift, and unapply.

src/library/scala/collection/convert/JavaCollectionWrappers.scala

@@ -319,11 +319,12 @@ private[collection] object JavaCollectionWrappers extends Serializable {
 
     override def size = underlying.size
 
+    // support Some(null) if currently bound to null
     def get(k: K) = {
-      val v = underlying get k
+      val v = underlying.get(k)
       if (v != null)
         Some(v)
-      else if (underlying containsKey k)
+      else if (underlying.containsKey(k))
         Some(null.asInstanceOf[V])
       else
         None
@@ -332,11 +333,18 @@ private[collection] object JavaCollectionWrappers extends Serializable {
     def addOne(kv: (K, V)): this.type = { underlying.put(kv._1, kv._2); this }
     def subtractOne(key: K): this.type = { underlying remove key; this }
 
-    override def put(k: K, v: V): Option[V] = Option(underlying.put(k, v))
+    // support Some(null) if currently bound to null
+    override def put(k: K, v: V): Option[V] = {
+      val present = underlying.containsKey(k)
+      val result  = underlying.put(k, v)
+      if (present) Some(result) else None
+    }
 
-    override def update(k: K, v: V): Unit = { underlying.put(k, v) }
+    override def update(k: K, v: V): Unit = underlying.put(k, v)
 
-    override def remove(k: K): Option[V] = Option(underlying remove k)
+    // support Some(null) if currently bound to null
+    override def remove(k: K): Option[V] =
+      if (underlying.containsKey(k)) Some(underlying.remove(k)) else None
 
     def iterator: Iterator[(K, V)] = new AbstractIterator[(K, V)] {
       val ui = underlying.entrySet.iterator

test/junit/scala/collection/convert/CollectionConvertersTest.scala

@@ -0,0 +1,80 @@
+/*
+ * Scala (https://www.scala-lang.org)
+ *
+ * Copyright EPFL and Lightbend, Inc.
+ *
+ * Licensed under Apache License 2.0
+ * (http://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package scala.collection.convert
+
+import java.util.{
+  Dictionary,
+  HashMap    => JMap,
+  Hashtable  => JTable,
+  Properties => JProperties,
+}
+import java.util.concurrent.{
+  ConcurrentHashMap => JCMap,
+}
+
+import org.junit.Assert.{assertEquals, assertNull, assertTrue}
+import org.junit.Test
+
+import scala.jdk.CollectionConverters._
+import scala.tools.testkit.AssertUtil.assertThrows
+
+class CollectionConvertersTest {
+  // Binding to null is represented as Some(null).
+  // This is consistent with get, which is the basis for getOrElse, lift, unapply
+  //
+  @Test def `t11894 Map wrapper respects put contract`(): Unit = {
+    val sut = new JMap[String, String].asScala
+    assertTrue(sut.isInstanceOf[JavaCollectionWrappers.JMapWrapper[_, _]])
+    assertThrows[NoSuchElementException](sut("one"))
+    assertEquals(None, sut.unapply("one"))
+    assertEquals(None, sut.put("one", null))
+    assertNull(sut("one"))
+    assertEquals(Some(null), sut.unapply("one"))
+    assertEquals(Some(null), sut.lift("one"))
+    assertEquals(Some(null), sut.put("one", "eins"))
+    assertEquals(Some("eins"), sut.put("one", "uno"))
+    assertEquals(Some("uno"), sut.remove("one"))
+    assertEquals(None, sut.remove("one"))
+    assertEquals(None, sut.put("one", null))
+    assertEquals(Some(null), sut.remove("one"))
+    assertEquals(None, sut.remove("one"))
+  }
+  @Test def `t11894 Map wrapper iterator witnesses null values`(): Unit = {
+    val sut = new JMap[String, String].asScala
+    assertTrue(sut.isInstanceOf[JavaCollectionWrappers.JMapWrapper[_, _]])
+    assertEquals(None, sut.put("one", null))
+    assertEquals(None, sut.put("deuce", "zwey"))
+    assertTrue(sut.iterator.find { case (_, v) => v == null }.nonEmpty)
+  }
+  @Test def `t11894 Dictionary wrapper disallows nulls`(): Unit = {
+    val tbl = new JTable[String, String]
+    val sut = tbl.asInstanceOf[Dictionary[String, String]].asScala
+    assertTrue(sut.isInstanceOf[JavaCollectionWrappers.JDictionaryWrapper[_, _]])
+    assertThrows[NullPointerException](sut.put("any", null))
+  }
+  @Test def `t11894 Properties wrapper enforces Strings`(): Unit = {
+    val ps  = new JProperties()
+    val sut = ps.asScala
+    assertTrue(sut.isInstanceOf[JavaCollectionWrappers.JPropertiesWrapper])
+    assertThrows[NullPointerException](sut.put("any", null))
+    assertEquals(None, sut.put("one", "eins"))
+    assertEquals("eins", sut("one"))
+    ps.put("one", new Object())
+    assertThrows[ClassCastException](sut.put("one", "uno"))
+  }
+  @Test def `t11894 JDK concurrent Map impl disallows nulls`(): Unit = {
+    val sut = new JCMap[String, String].asScala
+    assertTrue(sut.isInstanceOf[JavaCollectionWrappers.JConcurrentMapWrapper[_, _]])
+    assertThrows[NullPointerException](sut.put("any", null))
+  }
+}

test/junit/scala/collection/convert/NullSafetyToJavaTest.scala

@@ -3,73 +3,74 @@ package scala.collection.convert
 import java.{lang => jl}
 
 import org.junit.Test
+import org.junit.Assert.assertNull
 
 import scala.collection.{concurrent, mutable}
 import scala.jdk.CollectionConverters._
 
-// scala/bug#9113: tests to insure that wrappers return null instead of wrapping it as a collection
+// scala/bug#9113: tests to ensure that wrappers return null instead of wrapping it as a collection
 class NullSafetyToJavaTest {
   @Test def testIterableWrapping(): Unit = {
     val nullIterable: Iterable[AnyRef] = null
     val iterable: jl.Iterable[AnyRef] = nullIterable.asJava
 
-    assert(iterable == null)
+    assertNull(iterable)
   }
 
   // Implicit conversion to ju.Properties is not available
 
   @Test def testIteratorDecoration(): Unit = {
     val nullIterator: Iterator[AnyRef] = null
 
-    assert(nullIterator.asJava == null)
+    assertNull(nullIterator.asJava)
   }
 
   @Test def testEnumerationDecoration(): Unit = {
     val nullEnumeration: Iterator[AnyRef] = null
 
-    assert(nullEnumeration.asJavaEnumeration == null)
+    assertNull(nullEnumeration.asJavaEnumeration)
   }
 
   @Test def testIterableDecoration(): Unit = {
     val nullIterable: Iterable[AnyRef] = null
 
-    assert(nullIterable.asJava == null)
+    assertNull(nullIterable.asJava)
   }
 
   @Test def testCollectionDecoration(): Unit = {
     val nullCollection: Iterable[AnyRef] = null
 
-    assert(nullCollection.asJavaCollection == null)
+    assertNull(nullCollection.asJavaCollection)
   }
 
   @Test def testBufferDecoration(): Unit = {
     val nullBuffer: mutable.Buffer[AnyRef] = null
 
-    assert(nullBuffer.asJava == null)
+    assertNull(nullBuffer.asJava)
   }
 
   @Test def testSetDecoration(): Unit = {
     val nullSet: Set[AnyRef] = null
 
-    assert(nullSet.asJava == null)
+    assertNull(nullSet.asJava)
   }
 
   @Test def testMapDecoration(): Unit = {
     val nullMap: mutable.Map[AnyRef, AnyRef] = null
 
-    assert(nullMap.asJava == null)
+    assertNull(nullMap.asJava)
   }
 
   @Test def testConcurrentMapDecoration(): Unit = {
     val nullConMap: concurrent.Map[AnyRef, AnyRef] = null
 
-    assert(nullConMap.asJava == null)
+    assertNull(nullConMap.asJava)
   }
 
   @Test def testDictionaryDecoration(): Unit = {
     val nullDict: mutable.Map[AnyRef, AnyRef] = null
 
-    assert(nullDict.asJavaDictionary == null)
+    assertNull(nullDict.asJavaDictionary)
   }
 
   // Decorator conversion to ju.Properties is not available

test/junit/scala/collection/convert/NullSafetyToScalaTest.scala

@@ -4,69 +4,70 @@ import java.util.{concurrent => juc}
 import java.{lang => jl, util => ju}
 
 import org.junit.Test
+import org.junit.Assert.assertNull
 
 import scala.jdk.CollectionConverters._
 
-// scala/bug#9113: tests to insure that wrappers return null instead of wrapping it as a collection
+// scala/bug#9113: tests to ensure that wrappers return null instead of wrapping it as a collection
 
 class NullSafetyToScalaTest {
   @Test def testIteratorDecoration(): Unit = {
     val nullJIterator: ju.Iterator[AnyRef] = null
 
-    assert(nullJIterator.asScala == null)
+    assertNull(nullJIterator.asScala)
   }
 
   @Test def testEnumerationDecoration(): Unit = {
     val nullJEnumeration: ju.Enumeration[AnyRef] = null
 
-    assert(nullJEnumeration.asScala == null)
+    assertNull(nullJEnumeration.asScala)
   }
 
   @Test def testIterableDecoration(): Unit = {
     val nullJIterable: jl.Iterable[AnyRef] = null
 
-    assert(nullJIterable.asScala == null)
+    assertNull(nullJIterable.asScala)
   }
 
   @Test def testCollectionDecoration(): Unit = {
     val nullJCollection: ju.Collection[AnyRef] = null
 
-    assert(nullJCollection.asScala == null)
+    assertNull(nullJCollection.asScala)
   }
 
   @Test def testBufferDecoration(): Unit = {
     val nullJBuffer: ju.List[AnyRef] = null
 
-    assert(nullJBuffer.asScala == null)
+    assertNull(nullJBuffer.asScala)
   }
 
   @Test def testSetDecoration(): Unit = {
     val nullJSet: ju.Set[AnyRef] = null
 
-    assert(nullJSet.asScala == null)
+    assertNull(nullJSet.asScala)
   }
 
   @Test def testMapDecoration(): Unit = {
     val nullJMap: ju.Map[AnyRef, AnyRef] = null
 
-    assert(nullJMap.asScala == null)
+    assertNull(nullJMap.asScala)
   }
 
   @Test def testConcurrentMapDecoration(): Unit = {
     val nullJConMap: juc.ConcurrentMap[AnyRef, AnyRef] = null
 
-    assert(nullJConMap.asScala == null)
+    assertNull(nullJConMap.asScala)
   }
 
   @Test def testDictionaryDecoration(): Unit = {
     val nullJDict: ju.Dictionary[AnyRef, AnyRef] = null
 
-    assert(nullJDict.asScala == null)
+    assertNull(nullJDict.asScala)
   }
 
   @Test def testPropertiesDecoration(): Unit = {
     val nullJProperties: ju.Properties = null
 
-    assert(nullJProperties.asScala == null)
+    assertNull(nullJProperties.asScala)
   }
 }