From 0911e6f2afc9c2867381f307f70a037f0263442f Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 3 May 2017 00:29:42 -0400 Subject: [PATCH 1/4] Improve junit tests to use assertEquals Using assertTrue and == won't show expected versus result. --- jvm/src/test/scala/scala/xml/XMLTest.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/jvm/src/test/scala/scala/xml/XMLTest.scala b/jvm/src/test/scala/scala/xml/XMLTest.scala index 1b4058629..cc40e998e 100644 --- a/jvm/src/test/scala/scala/xml/XMLTest.scala +++ b/jvm/src/test/scala/scala/xml/XMLTest.scala @@ -40,8 +40,8 @@ class XMLTestJVM { override def text = "" } - assertTrue(c == parsedxml11) - assertTrue(parsedxml1 == parsedxml11) + assertEquals(c, parsedxml11) + assertEquals(parsedxml1, parsedxml11) assertTrue(List(parsedxml1) sameElements List(parsedxml11)) assertTrue(Array(parsedxml1).toList sameElements List(parsedxml11)) @@ -50,10 +50,10 @@ class XMLTestJVM { val i = new InputSource(new StringReader(x2)) val x2p = scala.xml.XML.load(i) - assertTrue(x2p == Elem(null, "book", e, sc, + assertEquals(Elem(null, "book", e, sc, Elem(null, "author", e, sc, Text("Peter Buneman")), Elem(null, "author", e, sc, Text("Dan Suciu")), - Elem(null, "title", e, sc, Text("Data on ze web")))) + Elem(null, "title", e, sc, Text("Data on ze web"))), x2p) } @@ -431,16 +431,16 @@ class XMLTestJVM { @UnitTest def t6939 = { val foo = - assertTrue(foo.child.head.scope.toString == """ xmlns:x="http://bar.com/"""") + assertEquals(foo.child.head.scope.toString, """ xmlns:x="http://bar.com/"""") val fooDefault = - assertTrue(fooDefault.child.head.scope.toString == """ xmlns="http://bar.com/"""") + assertEquals(fooDefault.child.head.scope.toString, """ xmlns="http://bar.com/"""") val foo2 = scala.xml.XML.loadString("""""") - assertTrue(foo2.child.head.scope.toString == """ xmlns:x="http://bar.com/"""") + assertEquals(foo2.child.head.scope.toString, """ xmlns:x="http://bar.com/"""") val foo2Default = scala.xml.XML.loadString("""""") - assertTrue(foo2Default.child.head.scope.toString == """ xmlns="http://bar.com/"""") + assertEquals(foo2Default.child.head.scope.toString, """ xmlns="http://bar.com/"""") } @UnitTest From a30715e26d0540963f5c985c66b9e986977136b4 Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 3 May 2017 00:38:29 -0400 Subject: [PATCH 2/4] Replace deprecated stack with list shared/src/main/scala/scala/xml/dtd/impl/SubsetConstruction.scala:35: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. val rest = new mutable.Stack[immutable.BitSet] ^ shared/src/main/scala/scala/xml/include/sax/XIncluder.scala:129: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. private val entities = new mutable.Stack[String]() ^ shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala:42: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. val attribStack = new mutable.Stack[MetaData] ^ shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala:43: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. val hStack = new mutable.Stack[Node] // [ element ] contains siblings ^ shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala:44: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. val tagStack = new mutable.Stack[String] ^ shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala:45: class Stack in package mutable is deprecated (since 2.12.0): Stack is an inelegant and potentially poorly-performing wrapper around List. Use a List assigned to a var instead. var scopeStack = new mutable.Stack[NamespaceBinding] ^ --- .../xml/dtd/impl/SubsetConstruction.scala | 9 +++-- .../scala/scala/xml/factory/XMLLoader.scala | 4 +- .../scala/xml/include/sax/XIncluder.scala | 11 +++-- .../scala/xml/parsing/FactoryAdapter.scala | 40 +++++++++++-------- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/shared/src/main/scala/scala/xml/dtd/impl/SubsetConstruction.scala b/shared/src/main/scala/scala/xml/dtd/impl/SubsetConstruction.scala index 93ca0cfcd..b046a34e3 100644 --- a/shared/src/main/scala/scala/xml/dtd/impl/SubsetConstruction.scala +++ b/shared/src/main/scala/scala/xml/dtd/impl/SubsetConstruction.scala @@ -32,9 +32,9 @@ private[dtd] class SubsetConstruction[T <: AnyRef](val nfa: NondetWordAutom[T]) val delta = new mutable.HashMap[immutable.BitSet, mutable.HashMap[T, immutable.BitSet]] var deftrans = mutable.Map(q0 -> sink, sink -> sink) // initial transitions var finals: mutable.Map[immutable.BitSet, Int] = mutable.Map() - val rest = new mutable.Stack[immutable.BitSet] + var rest = immutable.List.empty[immutable.BitSet] - rest.push(sink, q0) + rest = q0 :: sink :: rest def addFinal(q: immutable.BitSet) { if (nfa containsFinal q) @@ -43,7 +43,7 @@ private[dtd] class SubsetConstruction[T <: AnyRef](val nfa: NondetWordAutom[T]) def add(Q: immutable.BitSet) { if (!states(Q)) { states += Q - rest push Q + rest = Q :: rest addFinal(Q) } } @@ -51,7 +51,8 @@ private[dtd] class SubsetConstruction[T <: AnyRef](val nfa: NondetWordAutom[T]) addFinal(q0) // initial state may also be a final state while (!rest.isEmpty) { - val P = rest.pop() + val P = rest.head + rest = rest.tail // assign a number to this bitset indexMap = indexMap.updated(P, ix) invIndexMap = invIndexMap.updated(ix, P) diff --git a/shared/src/main/scala/scala/xml/factory/XMLLoader.scala b/shared/src/main/scala/scala/xml/factory/XMLLoader.scala index 0604282ea..e08b41e94 100644 --- a/shared/src/main/scala/scala/xml/factory/XMLLoader.scala +++ b/shared/src/main/scala/scala/xml/factory/XMLLoader.scala @@ -37,9 +37,9 @@ trait XMLLoader[T <: Node] { def loadXML(source: InputSource, parser: SAXParser): T = { val newAdapter = adapter - newAdapter.scopeStack push TopScope + newAdapter.scopeStack = TopScope :: newAdapter.scopeStack parser.parse(source, newAdapter) - newAdapter.scopeStack.pop() + newAdapter.scopeStack = newAdapter.scopeStack.tail newAdapter.rootElem.asInstanceOf[T] } diff --git a/shared/src/main/scala/scala/xml/include/sax/XIncluder.scala b/shared/src/main/scala/scala/xml/include/sax/XIncluder.scala index ae1cd9dd0..ec5da7618 100644 --- a/shared/src/main/scala/scala/xml/include/sax/XIncluder.scala +++ b/shared/src/main/scala/scala/xml/include/sax/XIncluder.scala @@ -10,7 +10,6 @@ package scala package xml package include.sax -import scala.collection.mutable import org.xml.sax.{ ContentHandler, Locator, Attributes } import org.xml.sax.ext.LexicalHandler import java.io.{ OutputStream, OutputStreamWriter, IOException } @@ -126,7 +125,7 @@ class XIncluder(outs: OutputStream, encoding: String) extends ContentHandler wit // LexicalHandler methods private var inDTD: Boolean = false - private val entities = new mutable.Stack[String]() + private var entities = List.empty[String] def startDTD(name: String, publicID: String, systemID: String) { inDTD = true @@ -145,12 +144,12 @@ class XIncluder(outs: OutputStream, encoding: String) extends ContentHandler wit } def endDTD() {} - def startEntity(name: String) { - entities push name + def startEntity(name: String): Unit = { + entities = name :: entities } - def endEntity(name: String) { - entities.pop() + def endEntity(name: String): Unit = { + entities = entities.tail } def startCDATA() {} diff --git a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala index a8b5c887f..41dd60388 100644 --- a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala +++ b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala @@ -10,7 +10,6 @@ package scala package xml package parsing -import scala.collection.{ mutable, Iterator } import org.xml.sax.Attributes import org.xml.sax.helpers.DefaultHandler @@ -39,10 +38,10 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node var rootElem: Node = null val buffer = new StringBuilder() - val attribStack = new mutable.Stack[MetaData] - val hStack = new mutable.Stack[Node] // [ element ] contains siblings - val tagStack = new mutable.Stack[String] - var scopeStack = new mutable.Stack[NamespaceBinding] + var attribStack = List.empty[MetaData] + var hStack = List.empty[Node] // [ element ] contains siblings + var tagStack = List.empty[String] + var scopeStack = List.empty[NamespaceBinding] var curTag: String = null var capture: Boolean = false @@ -122,17 +121,17 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node attributes: Attributes): Unit = { captureText() - tagStack push curTag + tagStack = curTag :: tagStack curTag = qname val localName = splitName(qname)._2 capture = nodeContainsText(localName) - hStack push null + hStack = null :: hStack var m: MetaData = Null var scpe: NamespaceBinding = if (scopeStack.isEmpty) TopScope - else scopeStack.top + else scopeStack.head for (i <- 0 until attributes.getLength()) { val qname = attributes getQName i @@ -147,8 +146,8 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node m = Attribute(Option(pre), key, Text(value), m) } - scopeStack push scpe - attribStack push m + scopeStack = scpe :: scopeStack + attribStack = m :: attribStack } /** @@ -156,7 +155,7 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node */ def captureText(): Unit = { if (capture && buffer.length > 0) - hStack push createText(buffer.toString) + hStack = createText(buffer.toString) :: hStack buffer.clear() } @@ -170,17 +169,24 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node */ override def endElement(uri: String, _localName: String, qname: String): Unit = { captureText() - val metaData = attribStack.pop() + val metaData = attribStack.head + attribStack = attribStack.tail // reverse order to get it right - val v = (Iterator continually hStack.pop takeWhile (_ != null)).toList.reverse + val v = hStack.takeWhile(_ != null).reverse + hStack = hStack.dropWhile(_ != null) match { + case null :: hs => hs + case hs => hs + } val (pre, localName) = splitName(qname) - val scp = scopeStack.pop() + val scp = scopeStack.head + scopeStack = scopeStack.tail // create element rootElem = createNode(pre, localName, metaData, scp, v) - hStack push rootElem - curTag = tagStack.pop() + hStack = rootElem :: hStack + curTag = tagStack.head + tagStack = tagStack.tail capture = curTag != null && nodeContainsText(curTag) // root level } @@ -189,6 +195,6 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node */ override def processingInstruction(target: String, data: String) { captureText() - hStack pushAll createProcInstr(target, data) + hStack = hStack.reverse_:::(createProcInstr(target, data).toList) } } From 913247e717a351813d7fbb4b7e512c8b0354105e Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 10 May 2017 23:20:01 -0400 Subject: [PATCH 3/4] Add mima exclude filters for mutable.Stack Scala 2.12.2 produced a lot more warnings about deprecation. One of them that affected scala-xml suggested dropping mutable.Stack in favor of immutable.List and var. I tried to preserve binary compatibility by creating members that use the collection.mutable.Stack.apply factory method, since it won't produce deprecation warnings. Must be only the constructor has deprecation warnings? Regardless, I was committing fraudulent binary compatibility: I created members of the type mima wants, but the values are not actually used. In all likelihood, no one uses the members of FactoryAdapter. We will just change everything to List, drop the use of mutable.Stack entirely, break bincompat, add entries to mimaBinaryIssueFilters, inform users, and call it fixed. --- build.sbt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/build.sbt b/build.sbt index 4782aca5c..85bf1ef87 100644 --- a/build.sbt +++ b/build.sbt @@ -36,6 +36,19 @@ lazy val xml = crossProject.in(file(".")) libraryDependencies += "junit" % "junit" % "4.11" % "test", libraryDependencies += "com.novocode" % "junit-interface" % "0.10" % "test", libraryDependencies += ("org.scala-lang" % "scala-compiler" % scalaVersion.value % "test").exclude("org.scala-lang.modules", s"scala-xml_${scalaVersion.value}"), + mimaBinaryIssueFilters ++= { + import com.typesafe.tools.mima.core._ + import com.typesafe.tools.mima.core.ProblemFilters._ + Seq( + // Scala 2.12 deprecated mutable.Stack, so we broke + // binary compatability for 1.0.7 in the following way: + exclude[IncompatibleMethTypeProblem]("scala.xml.parsing.FactoryAdapter.scopeStack_="), + exclude[IncompatibleResultTypeProblem]("scala.xml.parsing.FactoryAdapter.hStack"), + exclude[IncompatibleResultTypeProblem]("scala.xml.parsing.FactoryAdapter.scopeStack"), + exclude[IncompatibleResultTypeProblem]("scala.xml.parsing.FactoryAdapter.attribStack"), + exclude[IncompatibleResultTypeProblem]("scala.xml.parsing.FactoryAdapter.tagStack") + ) + }, mimaPreviousVersion := Some("1.0.6")): _*) .jsConfigure(_.enablePlugins(ScalaJSJUnitPlugin)) From 424ca626a06d446498f4a2cec70b3201f5f8adc8 Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Thu, 11 May 2017 00:53:07 -0400 Subject: [PATCH 4/4] Document dropping mutuable.Stack Make a note of binary compatability change in scaladoc for members that were converted to mutable.Stack. --- .../scala/xml/parsing/FactoryAdapter.scala | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala index 41dd60388..c74754d86 100644 --- a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala +++ b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala @@ -38,9 +38,33 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node var rootElem: Node = null val buffer = new StringBuilder() + /** List of attributes + * + * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * + * @since 1.0.7 + */ var attribStack = List.empty[MetaData] + /** List of elements + * + * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * + * @since 1.0.7 + */ var hStack = List.empty[Node] // [ element ] contains siblings + /** List of element names + * + * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * + * @since 1.0.7 + */ var tagStack = List.empty[String] + /** List of namespaces + * + * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * + * @since 1.0.7 + */ var scopeStack = List.empty[NamespaceBinding] var curTag: String = null