Fixes NPE using iterator with an XML attribute ...

Fixes NPE using iterator with an XML attribute being null or None      )
(SI-5052 Also fixes incorrect size method (SI-5115                     )

Contributed by Jordi Salvat i Alabart. Closes SI-5052, SI-5115, no
review.

5 files changed, 32 insertions, 4 deletions

diff --git a/src/library/scala/xml/Attribute.scala b/src/library/scala/xml/Attribute.scala
index 5b69017e70..ec366409f2 100644
--- a/src/library/scala/xml/Attribute.scala
+++ b/src/library/scala/xml/Attribute.scala
@@ -64,6 +64,17 @@ abstract trait Attribute extends MetaData {
     (next(arg, scope, key) == null) && (next wellformed scope)
   }
 
+  /** Returns an iterator on attributes */
+  override def iterator: Iterator[MetaData] = {
+    if (value == null) next.iterator
+    else Iterator.single(this) ++ next.iterator
+  }
+
+  override def size: Int = {
+    if (value == null) next.size
+    else 1 + next.size
+  }
+
   /** Appends string representation of only this attribute to stringbuffer.
    */
   def toString1(sb: StringBuilder) {
diff --git a/src/library/scala/xml/MetaData.scala b/src/library/scala/xml/MetaData.scala
index e3e025cf59..8043508046 100644
--- a/src/library/scala/xml/MetaData.scala
+++ b/src/library/scala/xml/MetaData.scala
@@ -139,10 +139,6 @@ abstract class MetaData extends Iterable[MetaData] with Equality with Serializab
   }
   def basisForHashCode: Seq[Any] = List(this.asAttrMap)
 
-  /** Returns an iterator on attributes */
-  def iterator: Iterator[MetaData] = Iterator.single(this) ++ next.iterator
-  override def size: Int = 1 + iterator.length
-
   /** filters this sequence of meta data */
   override def filter(f: MetaData => Boolean): MetaData =
     if (f(this)) copy(next filter f)
diff --git a/src/library/scala/xml/Null.scala b/src/library/scala/xml/Null.scala
index 0b527c91bd..3a75aeaca2 100644
--- a/src/library/scala/xml/Null.scala
+++ b/src/library/scala/xml/Null.scala
@@ -19,6 +19,7 @@ import scala.collection.Iterator
  */
 case object Null extends MetaData {
   override def iterator = Iterator.empty
+  override def size = 0
   override def append(m: MetaData, scope: NamespaceBinding = TopScope): MetaData = m
   override def filter(f: MetaData => Boolean): MetaData = this
 
diff --git a/test/files/run/t5052.scala b/test/files/run/t5052.scala
new file mode 100644
index 0000000000..9e418e8ac5
--- /dev/null
+++ b/test/files/run/t5052.scala
@@ -0,0 +1,6 @@
+object Test extends App {
+  assert(<elem attr={null:String} /> xml_== <elem />)
+  assert(<elem attr={None} /> xml_== <elem />)
+  assert(<elem /> xml_== <elem attr={null:String} />)
+  assert(<elem /> xml_== <elem attr={None} />)
+}
diff --git a/test/files/run/t5115.scala b/test/files/run/t5115.scala
new file mode 100644
index 0000000000..cf25214715
--- /dev/null
+++ b/test/files/run/t5115.scala
@@ -0,0 +1,14 @@
+import scala.collection.Iterable
+
+object Test extends App {
+  def assertHonorsIterableContract(i: Iterable[_]) = assert(i.size == i.iterator.size)
+
+  assertHonorsIterableContract(<a/>.attributes)
+  assertHonorsIterableContract(<a x=""/>.attributes)
+  assertHonorsIterableContract(<a y={None}/>.attributes)
+  assertHonorsIterableContract(<a y={None} x=""/>.attributes)
+  assertHonorsIterableContract(<a a="" y={None} />.attributes)
+  assertHonorsIterableContract(<a y={null:String}/>.attributes)
+  assertHonorsIterableContract(<a y={null:String} x=""/>.attributes)
+  assertHonorsIterableContract(<a a="" y={null:String} />.attributes)
+}