Skip to content

Commit

Permalink
SI-9113 Converting null collection Java<->Scala yields null
Browse files Browse the repository at this point in the history 
`scala.collection.{JavaConversions, JavaConverters}` no longer wrap
a `null` collection, but return `null` instead. Before, a wrapper would
hold the original `null` collections, delaying the NPE until the first
operation on the collection was called, with no way of knowing whether we
were holding such a time bomb wrapper until the NPE was thrown.

For example, this now holds: `(null : java.util.Iterator[_]).asScala ==
null`. Before, a wrapper would silently be created that would NPE on any
further interaction.

An example of this issue in the wild can be seen at
scalafx/scalafx#178, and an example of code
that generates such a wrapper [before the issue was fixed](https://github.com/scalafx/scalafx/blob/ad60d5faee687b71d3c0474510d
3f3dd081ea351/scalafx/src/main/scala/scalafx/stage/FileChooser.scala#L138).
  • Loading branch information
@RomanHargrave @adriaanm
RomanHargrave authored and adriaanm committed Feb 13, 2015
1 parent 73478ea commit 08dd95a
Show file tree
Hide file tree
Showing 3 changed files with 334 additions and 36 deletions.
56 changes: 33 additions & 23 deletions src/library/scala/collection/convert/WrapAsJava.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ trait WrapAsJava {
* @return A Java Iterator view of the argument.
*/
implicit def asJavaIterator[A](it: Iterator[A]): ju.Iterator[A] = it match {
case JIteratorWrapper(wrapped) => wrapped.asInstanceOf[ju.Iterator[A]]
case _ => IteratorWrapper(it)
case null => null
case JIteratorWrapper(wrapped) => wrapped.asInstanceOf[ju.Iterator[A]]
case _ => IteratorWrapper(it)
}

/**
Expand All @@ -48,8 +49,9 @@ trait WrapAsJava {
* @return A Java Enumeration view of the argument.
*/
implicit def asJavaEnumeration[A](it: Iterator[A]): ju.Enumeration[A] = it match {
case null => null
case JEnumerationWrapper(wrapped) => wrapped.asInstanceOf[ju.Enumeration[A]]
case _ => IteratorWrapper(it)
case _ => IteratorWrapper(it)
}

/**
Expand All @@ -66,8 +68,9 @@ trait WrapAsJava {
* @return A Java Iterable view of the argument.
*/
implicit def asJavaIterable[A](i: Iterable[A]): jl.Iterable[A] = i match {
case JIterableWrapper(wrapped) => wrapped.asInstanceOf[jl.Iterable[A]]
case _ => IterableWrapper(i)
case null => null
case JIterableWrapper(wrapped) => wrapped.asInstanceOf[jl.Iterable[A]]
case _ => IterableWrapper(i)
}

/**
Expand All @@ -82,8 +85,9 @@ trait WrapAsJava {
* @return A Java Collection view of the argument.
*/
implicit def asJavaCollection[A](it: Iterable[A]): ju.Collection[A] = it match {
case JCollectionWrapper(wrapped) => wrapped.asInstanceOf[ju.Collection[A]]
case _ => new IterableWrapper(it)
case null => null
case JCollectionWrapper(wrapped) => wrapped.asInstanceOf[ju.Collection[A]]
case _ => new IterableWrapper(it)
}

/**
Expand All @@ -100,8 +104,9 @@ trait WrapAsJava {
* @return A Java List view of the argument.
*/
implicit def bufferAsJavaList[A](b: mutable.Buffer[A]): ju.List[A] = b match {
case JListWrapper(wrapped) => wrapped
case _ => new MutableBufferWrapper(b)
case null => null
case JListWrapper(wrapped) => wrapped
case _ => new MutableBufferWrapper(b)
}

/**
Expand All @@ -118,8 +123,9 @@ trait WrapAsJava {
* @return A Java List view of the argument.
*/
implicit def mutableSeqAsJavaList[A](seq: mutable.Seq[A]): ju.List[A] = seq match {
case JListWrapper(wrapped) => wrapped
case _ => new MutableSeqWrapper(seq)
case null => null
case JListWrapper(wrapped) => wrapped
case _ => new MutableSeqWrapper(seq)
}

/**
Expand All @@ -136,8 +142,9 @@ trait WrapAsJava {
* @return A Java List view of the argument.
*/
implicit def seqAsJavaList[A](seq: Seq[A]): ju.List[A] = seq match {
case JListWrapper(wrapped) => wrapped.asInstanceOf[ju.List[A]]
case _ => new SeqWrapper(seq)
case null => null
case JListWrapper(wrapped) => wrapped.asInstanceOf[ju.List[A]]
case _ => new SeqWrapper(seq)
}

/**
Expand All @@ -154,8 +161,9 @@ trait WrapAsJava {
* @return A Java Set view of the argument.
*/
implicit def mutableSetAsJavaSet[A](s: mutable.Set[A]): ju.Set[A] = s match {
case null => null
case JSetWrapper(wrapped) => wrapped
case _ => new MutableSetWrapper(s)
case _ => new MutableSetWrapper(s)
}

/**
Expand All @@ -172,8 +180,9 @@ trait WrapAsJava {
* @return A Java Set view of the argument.
*/
implicit def setAsJavaSet[A](s: Set[A]): ju.Set[A] = s match {
case null => null
case JSetWrapper(wrapped) => wrapped
case _ => new SetWrapper(s)
case _ => new SetWrapper(s)
}

/**
Expand All @@ -190,9 +199,9 @@ trait WrapAsJava {
* @return A Java Map view of the argument.
*/
implicit def mutableMapAsJavaMap[A, B](m: mutable.Map[A, B]): ju.Map[A, B] = m match {
//case JConcurrentMapWrapper(wrapped) => wrapped
case null => null
case JMapWrapper(wrapped) => wrapped
case _ => new MutableMapWrapper(m)
case _ => new MutableMapWrapper(m)
}

/**
Expand All @@ -210,9 +219,9 @@ trait WrapAsJava {
* @return A Java `Dictionary` view of the argument.
*/
implicit def asJavaDictionary[A, B](m: mutable.Map[A, B]): ju.Dictionary[A, B] = m match {
//case JConcurrentMapWrapper(wrapped) => wrapped
case JDictionaryWrapper(wrapped) => wrapped
case _ => new DictionaryWrapper(m)
case null => null
case JDictionaryWrapper(wrapped) => wrapped
case _ => new DictionaryWrapper(m)
}

/**
Expand All @@ -230,9 +239,9 @@ trait WrapAsJava {
* @return A Java `Map` view of the argument.
*/
implicit def mapAsJavaMap[A, B](m: Map[A, B]): ju.Map[A, B] = m match {
//case JConcurrentMapWrapper(wrapped) => wrapped
case null => null
case JMapWrapper(wrapped) => wrapped.asInstanceOf[ju.Map[A, B]]
case _ => new MapWrapper(m)
case _ => new MapWrapper(m)
}

/**
Expand All @@ -251,8 +260,9 @@ trait WrapAsJava {
* @return A Java `ConcurrentMap` view of the argument.
*/
implicit def mapAsJavaConcurrentMap[A, B](m: concurrent.Map[A, B]): juc.ConcurrentMap[A, B] = m match {
case null => null
case JConcurrentMapWrapper(wrapped) => wrapped
case _ => new ConcurrentMapWrapper(m)
case _ => new ConcurrentMapWrapper(m)
}
}

Expand Down
35 changes: 22 additions & 13 deletions src/library/scala/collection/convert/WrapAsScala.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ trait WrapAsScala {
* @return A Scala `Iterator` view of the argument.
*/
implicit def asScalaIterator[A](it: ju.Iterator[A]): Iterator[A] = it match {
case null => null
case IteratorWrapper(wrapped) => wrapped
case _ => JIteratorWrapper(it)
case _ => JIteratorWrapper(it)
}

/**
Expand All @@ -48,8 +49,9 @@ trait WrapAsScala {
* @return A Scala Iterator view of the argument.
*/
implicit def enumerationAsScalaIterator[A](i: ju.Enumeration[A]): Iterator[A] = i match {
case null => null
case IteratorWrapper(wrapped) => wrapped
case _ => JEnumerationWrapper(i)
case _ => JEnumerationWrapper(i)
}

/**
Expand All @@ -67,8 +69,9 @@ trait WrapAsScala {
* @return A Scala Iterable view of the argument.
*/
implicit def iterableAsScalaIterable[A](i: jl.Iterable[A]): Iterable[A] = i match {
case null => null
case IterableWrapper(wrapped) => wrapped
case _ => JIterableWrapper(i)
case _ => JIterableWrapper(i)
}

/**
Expand All @@ -82,8 +85,9 @@ trait WrapAsScala {
* @return A Scala Iterable view of the argument.
*/
implicit def collectionAsScalaIterable[A](i: ju.Collection[A]): Iterable[A] = i match {
case null => null
case IterableWrapper(wrapped) => wrapped
case _ => JCollectionWrapper(i)
case _ => JCollectionWrapper(i)
}

/**
Expand All @@ -101,8 +105,9 @@ trait WrapAsScala {
* @return A Scala mutable `Buffer` view of the argument.
*/
implicit def asScalaBuffer[A](l: ju.List[A]): mutable.Buffer[A] = l match {
case MutableBufferWrapper(wrapped) => wrapped
case _ =>new JListWrapper(l)
case null => null
case MutableBufferWrapper(wrapped) => wrapped
case _ => new JListWrapper(l)
}

/**
Expand All @@ -119,8 +124,9 @@ trait WrapAsScala {
* @return A Scala mutable Set view of the argument.
*/
implicit def asScalaSet[A](s: ju.Set[A]): mutable.Set[A] = s match {
case null => null
case MutableSetWrapper(wrapped) => wrapped
case _ =>new JSetWrapper(s)
case _ => new JSetWrapper(s)
}

/**
Expand All @@ -144,9 +150,9 @@ trait WrapAsScala {
* @return A Scala mutable Map view of the argument.
*/
implicit def mapAsScalaMap[A, B](m: ju.Map[A, B]): mutable.Map[A, B] = m match {
//case ConcurrentMapWrapper(wrapped) => wrapped
case null => null
case MutableMapWrapper(wrapped) => wrapped
case _ => new JMapWrapper(m)
case _ => new JMapWrapper(m)
}

/**
Expand All @@ -163,8 +169,9 @@ trait WrapAsScala {
* @return A Scala mutable ConcurrentMap view of the argument.
*/
implicit def mapAsScalaConcurrentMap[A, B](m: juc.ConcurrentMap[A, B]): concurrent.Map[A, B] = m match {
case cmw: ConcurrentMapWrapper[a, b] => cmw.underlying
case _ => new JConcurrentMapWrapper(m)
case null => null
case cmw: ConcurrentMapWrapper[A, B] => cmw.underlying
case _ => new JConcurrentMapWrapper(m)
}

/**
Expand All @@ -179,8 +186,9 @@ trait WrapAsScala {
* @return A Scala mutable Map[String, String] view of the argument.
*/
implicit def dictionaryAsScalaMap[A, B](p: ju.Dictionary[A, B]): mutable.Map[A, B] = p match {
case null => null
case DictionaryWrapper(wrapped) => wrapped
case _ => new JDictionaryWrapper(p)
case _ => new JDictionaryWrapper(p)
}

/**
Expand All @@ -194,7 +202,8 @@ trait WrapAsScala {
* @return A Scala mutable Map[String, String] view of the argument.
*/
implicit def propertiesAsScalaMap(p: ju.Properties): mutable.Map[String, String] = p match {
case _ => new JPropertiesWrapper(p)
case null => null
case _ => new JPropertiesWrapper(p)
}
}

Expand Down

0 comments on commit 08dd95a

Please sign in to comment.