Skip to content

Change Node.child and Node.attribute return type to immutable.Seq on 2.13+ #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sbtcrossproject.CrossPlugin.autoImport.{crossProject, CrossType}
import com.typesafe.tools.mima.core._
import sbtcrossproject.CrossPlugin.autoImport.{CrossType, crossProject}

publish / skip := true // root project

Expand Down Expand Up @@ -68,6 +69,51 @@ lazy val xml = crossProject(JSPlatform, JVMPlatform, NativePlatform)
//import com.typesafe.tools.mima.core.{}
//import com.typesafe.tools.mima.core.ProblemFilters
Seq( // exclusions for all Scala versions
// new method in `Node` with return type `immutable.Seq`
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.Node.child"),

// these used to be declared methods, but are now bridges without generic signature
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Node.nonEmptyChildren"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Group.child"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.SpecialNode.child"),

// new methods with return type immutable.Seq
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.Attribute.apply"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.Attribute.value"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.MetaData.apply"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.MetaData.value"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.xml.NodeSeq.theSeq"),

// Synthetic static accessors (for Java interop) have a changed return type
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.xml.Null.apply"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.xml.Null.value"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.xml.Utility.parseAttributeValue"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.xml.Utility.trimProper"),

// used to be a declared method, now a bridge without generic signature
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.MetaData.apply"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Null.apply"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Null.value"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.PrefixedAttribute.apply"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.PrefixedAttribute.value"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.UnprefixedAttribute.apply"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.UnprefixedAttribute.value"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Document.theSeq"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Group.theSeq"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Node.theSeq"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.TextBuffer.toText"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Utility.trimProper"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Utility.parseAttributeValue"),

// Option[c.Seq] => Option[i.Seq] results in a changed generic signature
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.MetaData.get"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Null.get"),
ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.xml.Node.attribute"),

// trait Attribute now extends trait ScalaVersionSpecificMetaData to ensure the previous signatures
// with return type `collection.Seq` remain valid.
// (trait Attribute extends MetaData, but that parent is not present in bytecode because it's a class.)
ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("scala.xml.Attribute.apply"),
) ++ (CrossVersion.partialVersion(scalaVersion.value) match {
case Some((3, _)) => Seq( // Scala 3-specific exclusions
)
Expand Down
2 changes: 1 addition & 1 deletion jvm/src/test/scala/scala/xml/SerializationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class SerializationTest {
def implicitConversion(): Unit = {
val parent: Elem = <parent><child></child><child/></parent>
val children: Seq[Node] = parent.child
val asNodeSeq: NodeSeq = children
val asNodeSeq: NodeSeq = children // implicit seqToNodeSeq
assertEquals(asNodeSeq, JavaByteSerialization.roundTrip(asNodeSeq))
}
}
11 changes: 10 additions & 1 deletion shared/src/main/scala-2.12/scala/xml/ScalaVersionSpecific.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ private[xml] object ScalaVersionSpecific {
override def apply(from: Coll): mutable.Builder[Node, NodeSeq] = NodeSeq.newBuilder
override def apply(): mutable.Builder[Node, NodeSeq] = NodeSeq.newBuilder
}
type SeqNodeUnapplySeq = scala.collection.Seq[Node]
type SeqOfNode = scala.collection.Seq[Node]
type SeqOfText = scala.collection.Seq[Text]
}

private[xml] trait ScalaVersionSpecificNodeSeq extends SeqLike[Node, NodeSeq] { self: NodeSeq =>
Expand All @@ -33,3 +34,11 @@ private[xml] trait ScalaVersionSpecificNodeSeq extends SeqLike[Node, NodeSeq] {
private[xml] trait ScalaVersionSpecificNodeBuffer { self: NodeBuffer =>
override def stringPrefix: String = "NodeBuffer"
}

private[xml] trait ScalaVersionSpecificNode { self: Node => }

private[xml] trait ScalaVersionSpecificMetaData { self: MetaData => }

private[xml] trait ScalaVersionSpecificTextBuffer { self: TextBuffer => }

private[xml] trait ScalaVersionSpecificUtility { self: Utility.type => }
29 changes: 28 additions & 1 deletion shared/src/main/scala-2.13+/scala/xml/ScalaVersionSpecific.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ private[xml] object ScalaVersionSpecific {
def newBuilder(from: Coll): Builder[Node, NodeSeq] = NodeSeq.newBuilder
def fromSpecific(from: Coll)(it: IterableOnce[Node]): NodeSeq = (NodeSeq.newBuilder ++= from).result()
}
type SeqNodeUnapplySeq = scala.collection.immutable.Seq[Node]
type SeqOfNode = scala.collection.immutable.Seq[Node]
type SeqOfText = scala.collection.immutable.Seq[Text]
}

private[xml] trait ScalaVersionSpecificNodeSeq
Expand All @@ -48,8 +49,34 @@ private[xml] trait ScalaVersionSpecificNodeSeq
fromSpecific(new View.Map(this, f))
def flatMap(f: Node => IterableOnce[Node]): NodeSeq =
fromSpecific(new View.FlatMap(this, f))

def theSeq: scala.collection.Seq[Node]
}

private[xml] trait ScalaVersionSpecificNodeBuffer { self: NodeBuffer =>
override def className: String = "NodeBuffer"
}

private[xml] trait ScalaVersionSpecificNode { self: Node =>
// These methods are overridden in Node with return type `immutable.Seq`. The declarations here result
// in a bridge method in `Node` with result type `collection.Seq` which is needed for binary compatibility.
def child: scala.collection.Seq[Node]
def nonEmptyChildren: scala.collection.Seq[Node]
}

private[xml] trait ScalaVersionSpecificMetaData { self: MetaData =>
def apply(key: String): scala.collection.Seq[Node]
def apply(namespace_uri: String, owner: Node, key: String): scala.collection.Seq[Node]
def apply(namespace_uri: String, scp: NamespaceBinding, k: String): scala.collection.Seq[Node]

def value: scala.collection.Seq[Node]
}

private[xml] trait ScalaVersionSpecificTextBuffer { self: TextBuffer =>
def toText: scala.collection.Seq[Text]
}

private[xml] trait ScalaVersionSpecificUtility { self: Utility.type =>
def trimProper(x: Node): scala.collection.Seq[Node]
def parseAttributeValue(value: String): scala.collection.Seq[Node]
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ private[xml] object ScalaVersionSpecificReturnTypes { // should be
type NullNext = scala.Null
type NullKey = scala.Null
type NullValue = scala.Null
type NullApply1 = scala.collection.Seq[Node] // scala.Null
type NullApply3 = scala.Null
type NullRemove = Null.type
type SpecialNodeChild = Nil.type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@ package scala.xml
What should have been specified explicitly is given in the comments;
next time we break binary compatibility the types should be changed in the code and this class removed.
*/
private[xml] object ScalaVersionSpecificReturnTypes { // should be
type ExternalIDAttribute = MetaData // Null.type
type NoExternalIDId = String // scala.Null
type NodeNoAttributes = MetaData // Null.type
type NullFilter = MetaData // Null.type
type NullGetNamespace = String // scala.Null
type NullNext = MetaData // scala.Null
type NullKey = String // scala.Null
type NullValue = scala.collection.Seq[Node] // scala.Null
type NullApply1 = scala.collection.Seq[Node] // scala.Null
type NullApply3 = scala.collection.Seq[Node] // scala.Null
type NullRemove = MetaData // Null.type
type SpecialNodeChild = scala.collection.Seq[Node] // Nil.type
type GroupChild = scala.collection.Seq[Node] // Nothing
private[xml] object ScalaVersionSpecificReturnTypes { // should be
type ExternalIDAttribute = MetaData // Null.type
type NoExternalIDId = String // scala.Null
type NodeNoAttributes = MetaData // Null.type
type NullFilter = MetaData // Null.type
type NullGetNamespace = String // scala.Null
type NullNext = MetaData // scala.Null
type NullKey = String // scala.Null
type NullValue = scala.collection.immutable.Seq[Node] // scala.Null
type NullApply3 = scala.collection.immutable.Seq[Node] // scala.Null
type NullRemove = MetaData // Null.type
type SpecialNodeChild = scala.collection.immutable.Seq[Node] // Nil.type
type GroupChild = scala.collection.immutable.Seq[Node] // Nothing
}
8 changes: 4 additions & 4 deletions shared/src/main/scala/scala/xml/Attribute.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ object Attribute {
*
* @author Burak Emir
*/
trait Attribute extends MetaData {
trait Attribute extends MetaData with ScalaVersionSpecificMetaData {
def pre: String // will be null if unprefixed
override val key: String
override val value: Seq[Node]
override val value: ScalaVersionSpecific.SeqOfNode
override val next: MetaData

override def apply(key: String): Seq[Node]
override def apply(namespace: String, scope: NamespaceBinding, key: String): Seq[Node]
override def apply(key: String): ScalaVersionSpecific.SeqOfNode
override def apply(namespace: String, scope: NamespaceBinding, key: String): ScalaVersionSpecific.SeqOfNode
override def copy(next: MetaData): Attribute

override def remove(key: String): MetaData =
Expand Down
4 changes: 2 additions & 2 deletions shared/src/main/scala/scala/xml/Document.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Document extends NodeSeq with Serializable {
* excluded. If there is a document type declaration, the list also
* contains a document type declaration information item.
*/
var children: Seq[Node] = _
var children: Seq[Node] = _ // effectively an `immutable.Seq`, not changed due to binary compatibility

/** The element information item corresponding to the document element. */
var docElem: Node = _
Expand Down Expand Up @@ -96,7 +96,7 @@ class Document extends NodeSeq with Serializable {

// methods for NodeSeq

override def theSeq: Seq[Node] = this.docElem
override def theSeq: ScalaVersionSpecific.SeqOfNode = this.docElem

override def canEqual(other: Any): Boolean = other match {
case _: Document => true
Expand Down
6 changes: 3 additions & 3 deletions shared/src/main/scala/scala/xml/Elem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ object Elem {
def apply(prefix: String, label: String, attributes: MetaData, scope: NamespaceBinding, minimizeEmpty: Boolean, child: Node*): Elem =
new Elem(prefix, label, attributes, scope, minimizeEmpty, child: _*)

def unapplySeq(n: Node): Option[(String, String, MetaData, NamespaceBinding, ScalaVersionSpecific.SeqNodeUnapplySeq)] =
def unapplySeq(n: Node): Option[(String, String, MetaData, NamespaceBinding, ScalaVersionSpecific.SeqOfNode)] =
n match {
case _: SpecialNode | _: Group => None
case _ => Some((n.prefix, n.label, n.attributes, n.scope, n.child.toSeq))
case _ => Some((n.prefix, n.label, n.attributes, n.scope, n.child))
}
}

Expand Down Expand Up @@ -104,7 +104,7 @@ class Elem(
scope: NamespaceBinding = this.scope,
minimizeEmpty: Boolean = this.minimizeEmpty,
child: Seq[Node] = this.child
): Elem = Elem(prefix, label, attributes, scope, minimizeEmpty, child: _*)
): Elem = Elem(prefix, label, attributes, scope, minimizeEmpty, child.toSeq: _*)

/**
* Returns concatenation of `text(n)` for each child `n`.
Expand Down
2 changes: 1 addition & 1 deletion shared/src/main/scala/scala/xml/Group.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import scala.collection.Seq
*/
// Note: used by the Scala compiler.
final case class Group(nodes: Seq[Node]) extends Node {
override def theSeq: Seq[Node] = nodes
override def theSeq: ScalaVersionSpecific.SeqOfNode = nodes.toSeq

override def canEqual(other: Any): Boolean = other match {
case _: Group => true
Expand Down
17 changes: 9 additions & 8 deletions shared/src/main/scala/scala/xml/MetaData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ abstract class MetaData
with Iterable[MetaData]
with Equality
with Serializable
with ScalaVersionSpecificMetaData
{
private[xml] def isNull: Boolean = this.eq(Null)

Expand All @@ -106,7 +107,7 @@ abstract class MetaData
* @param key
* @return value as Seq[Node] if key is found, null otherwise
*/
def apply(key: String): Seq[Node]
def apply(key: String): ScalaVersionSpecific.SeqOfNode

/**
* convenience method, same as `apply(namespace, owner.scope, key)`.
Expand All @@ -115,7 +116,7 @@ abstract class MetaData
* @param owner the element owning this attribute list
* @param key the attribute key
*/
final def apply(namespace_uri: String, owner: Node, key: String): Seq[Node] =
final def apply(namespace_uri: String, owner: Node, key: String): ScalaVersionSpecific.SeqOfNode =
apply(namespace_uri, owner.scope, key)

/**
Expand All @@ -126,7 +127,7 @@ abstract class MetaData
* @param k to be looked for
* @return value as Seq[Node] if key is found, null otherwise
*/
def apply(namespace_uri: String, scp: NamespaceBinding, k: String): Seq[Node]
def apply(namespace_uri: String, scp: NamespaceBinding, k: String): ScalaVersionSpecific.SeqOfNode

/**
* returns a copy of this MetaData item with next field set to argument.
Expand Down Expand Up @@ -168,7 +169,7 @@ abstract class MetaData
def key: String

/** returns value of this MetaData item */
def value: Seq[Node]
def value: ScalaVersionSpecific.SeqOfNode

/**
* Returns a String containing "prefix:key" if the first key is
Expand All @@ -183,7 +184,7 @@ abstract class MetaData
* Returns a Map containing the attributes stored as key/value pairs.
*/
def asAttrMap: Map[String, String] =
iterator.map(x => (x.prefixedKey, x.value.text)).toMap
iterator.map(x => (x.prefixedKey, NodeSeq.fromSeq(x.value).text)).toMap

/** returns Null or the next MetaData item */
def next: MetaData
Expand All @@ -194,10 +195,10 @@ abstract class MetaData
* @param key
* @return value in Some(Seq[Node]) if key is found, None otherwise
*/
final def get(key: String): Option[Seq[Node]] = Option(apply(key))
final def get(key: String): Option[ScalaVersionSpecific.SeqOfNode] = Option(apply(key))

/** same as get(uri, owner.scope, key) */
final def get(uri: String, owner: Node, key: String): Option[Seq[Node]] =
final def get(uri: String, owner: Node, key: String): Option[ScalaVersionSpecific.SeqOfNode] =
get(uri, owner.scope, key)

/**
Expand All @@ -208,7 +209,7 @@ abstract class MetaData
* @param key to be looked fore
* @return value as `Some[Seq[Node]]` if key is found, None otherwise
*/
final def get(uri: String, scope: NamespaceBinding, key: String): Option[Seq[Node]] =
final def get(uri: String, scope: NamespaceBinding, key: String): Option[ScalaVersionSpecific.SeqOfNode] =
Option(apply(uri, scope, key))

protected def toString1: String = sbToString(toString1)
Expand Down
16 changes: 8 additions & 8 deletions shared/src/main/scala/scala/xml/Node.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ object Node {
/** the empty namespace */
val EmptyNamespace: String = ""

def unapplySeq(n: Node): Some[(String, MetaData, ScalaVersionSpecific.SeqNodeUnapplySeq)] =
Some((n.label, n.attributes, n.child.toSeq))
def unapplySeq(n: Node): Some[(String, MetaData, ScalaVersionSpecific.SeqOfNode)] =
Some((n.label, n.attributes, n.child))
}

/**
Expand All @@ -45,7 +45,7 @@ object Node {
*
* @author Burak Emir and others
*/
abstract class Node extends NodeSeq {
abstract class Node extends NodeSeq with ScalaVersionSpecificNode {

/** prefix of this node */
def prefix: String = null
Expand Down Expand Up @@ -92,7 +92,7 @@ abstract class Node extends NodeSeq {
* @return value of `UnprefixedAttribute` with given key
* in attributes, if it exists, otherwise `null`.
*/
final def attribute(key: String): Option[Seq[Node]] = attributes.get(key)
final def attribute(key: String): Option[ScalaVersionSpecific.SeqOfNode] = attributes.get(key)

/**
* Convenience method, looks up a prefixed attribute in attributes of this node.
Expand All @@ -103,7 +103,7 @@ abstract class Node extends NodeSeq {
* @return value of `PrefixedAttribute` with given namespace
* and given key, otherwise `'''null'''`.
*/
final def attribute(uri: String, key: String): Option[Seq[Node]] =
final def attribute(uri: String, key: String): Option[ScalaVersionSpecific.SeqOfNode] =
attributes.get(uri, this, key)

/**
Expand All @@ -120,12 +120,12 @@ abstract class Node extends NodeSeq {
*
* @return all children of this node
*/
def child: Seq[Node]
def child: ScalaVersionSpecific.SeqOfNode

/**
* Children which do not stringify to "" (needed for equality)
*/
def nonEmptyChildren: Seq[Node] = child.filterNot(_.toString.isEmpty)
def nonEmptyChildren: ScalaVersionSpecific.SeqOfNode = child.filterNot(_.toString.isEmpty)

/**
* Descendant axis (all descendants of this node, not including node itself)
Expand Down Expand Up @@ -166,7 +166,7 @@ abstract class Node extends NodeSeq {
/**
* returns a sequence consisting of only this node
*/
override def theSeq: Seq[Node] = this :: Nil
override def theSeq: ScalaVersionSpecific.SeqOfNode = this :: Nil

/**
* String representation of this node
Expand Down
Loading