diff --git a/src/library/scala/collection/convert/JavaCollectionWrappers.scala b/src/library/scala/collection/convert/JavaCollectionWrappers.scala index 011980b70aca..15f546d1d402 100644 --- a/src/library/scala/collection/convert/JavaCollectionWrappers.scala +++ b/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 diff --git a/test/junit/scala/collection/convert/CollectionConvertersTest.scala b/test/junit/scala/collection/convert/CollectionConvertersTest.scala new file mode 100644 index 000000000000..37d3b1579332 --- /dev/null +++ b/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)) + } +} diff --git a/test/junit/scala/collection/convert/NullSafetyToJavaTest.scala b/test/junit/scala/collection/convert/NullSafetyToJavaTest.scala index ef94216e0bb7..073674169f39 100644 --- a/test/junit/scala/collection/convert/NullSafetyToJavaTest.scala +++ b/test/junit/scala/collection/convert/NullSafetyToJavaTest.scala @@ -3,17 +3,18 @@ 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 @@ -21,55 +22,55 @@ class NullSafetyToJavaTest { @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 diff --git a/test/junit/scala/collection/convert/NullSafetyToScalaTest.scala b/test/junit/scala/collection/convert/NullSafetyToScalaTest.scala index fd078110e982..ba68faa57d76 100644 --- a/test/junit/scala/collection/convert/NullSafetyToScalaTest.scala +++ b/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) } }