From e0e7df1b7e9fef64372560cf8e149a58ee3ec00f Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 3 May 2017 00:29:42 -0400 Subject: [PATCH 1/5] 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 37d45bccc..11387171b 100644 --- a/jvm/src/test/scala/scala/xml/XMLTest.scala +++ b/jvm/src/test/scala/scala/xml/XMLTest.scala @@ -41,8 +41,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)) @@ -51,10 +51,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) } @@ -455,16 +455,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 39753a6bc7211cca56add4cedee230713c768d43 Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 3 May 2017 00:38:29 -0400 Subject: [PATCH 2/5] 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 | 7 ++-- .../scala/xml/parsing/FactoryAdapter.scala | 40 +++++++++++-------- 4 files changed, 33 insertions(+), 27 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 2bc2c97b1..e3d2ba194 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]] val deftrans = mutable.Map(q0 -> sink, sink -> sink) // initial transitions val 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): Unit = { if (nfa containsFinal q) @@ -43,7 +43,7 @@ private[dtd] class SubsetConstruction[T <: AnyRef](val nfa: NondetWordAutom[T]) def add(Q: immutable.BitSet): Unit = { 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(P) = ix invIndexMap(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 b84f42af4..e9a534fbd 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 2f6284a6c..bf65cfafc 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): Unit = { inDTD = true @@ -146,11 +145,11 @@ class XIncluder(outs: OutputStream, encoding: String) extends ContentHandler wit def endDTD(): Unit = {} def startEntity(name: String): Unit = { - entities push name + entities = name :: entities } def endEntity(name: String): Unit = { - entities.pop() + entities = entities.tail } def startCDATA(): Unit = {} diff --git a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala index cbacb0492..5751bf670 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 scala.collection.Seq import org.xml.sax.Attributes import org.xml.sax.helpers.DefaultHandler @@ -40,10 +39,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 @@ -123,17 +122,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 @@ -148,8 +147,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 } /** @@ -157,7 +156,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() } @@ -171,17 +170,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 } @@ -190,6 +196,6 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node */ override def processingInstruction(target: String, data: String): Unit = { captureText() - hStack pushAll createProcInstr(target, data) + hStack = hStack.reverse_:::(createProcInstr(target, data).toList) } } From 64b7dbd283f52f8daf4573dded5687e0add8176c Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Wed, 10 May 2017 23:20:01 -0400 Subject: [PATCH 3/5] 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 54e744115..a18bc22bc 100644 --- a/build.sbt +++ b/build.sbt @@ -24,6 +24,19 @@ lazy val xml = crossProject(JSPlatform, JVMPlatform) mimaPreviousVersion := { Some("1.2.0") }, + 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 compatibility for 1.1.0 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") + ) + }, unmanagedSourceDirectories in Compile ++= { (unmanagedSourceDirectories in Compile).value.map { dir => From 35f452c01cf1826b6697ce96c7324ec0889f8e02 Mon Sep 17 00:00:00 2001 From: "Aaron S. Hawley" Date: Thu, 11 May 2017 00:53:07 -0400 Subject: [PATCH 4/5] 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 5751bf670..ad863d4fc 100644 --- a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala +++ b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala @@ -39,9 +39,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 From 27404951320cd56a465d3e101893f10ddbfd5e9e Mon Sep 17 00:00:00 2001 From: Joe Barnes Date: Sat, 5 Aug 2017 23:20:57 -0400 Subject: [PATCH 5/5] Improve wording on stack deprecation --- .../scala/scala/xml/parsing/FactoryAdapter.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala index ad863d4fc..608646f2a 100644 --- a/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala +++ b/shared/src/main/scala/scala/xml/parsing/FactoryAdapter.scala @@ -41,30 +41,30 @@ abstract class FactoryAdapter extends DefaultHandler with factory.XMLLoader[Node val buffer = new StringBuilder() /** List of attributes * - * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * Previously was a mutable [[scala.collection.mutable.Stack Stack]], but is now a mutable reference to an immutable [[scala.collection.immutable.List List]]. * - * @since 1.0.7 + * @since 1.1.0 */ var attribStack = List.empty[MetaData] /** List of elements * - * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * Previously was a mutable [[scala.collection.mutable.Stack Stack]], but is now a mutable reference to an immutable [[scala.collection.immutable.List List]]. * - * @since 1.0.7 + * @since 1.1.0 */ 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]]. + * Previously was a mutable [[scala.collection.mutable.Stack Stack]], but is now a mutable reference to an immutable [[scala.collection.immutable.List List]]. * - * @since 1.0.7 + * @since 1.1.0 */ var tagStack = List.empty[String] /** List of namespaces * - * Previously was a [[scala.collection.mutable.Stack]], but is now a mutable [[scala.collection.immutable.List]]. + * Previously was a mutable [[scala.collection.mutable.Stack Stack]], but is now a mutable reference to an immutable [[scala.collection.immutable.List List]]. * - * @since 1.0.7 + * @since 1.1.0 */ var scopeStack = List.empty[NamespaceBinding]