If we go ahead with this PR we'll need a new minor release (2.4.0).

There is more API with return typecollection.Seq that probably always returns animmutable.Seq, e.g.

• Metadata.value
• NodeSeq.theSeq
• MetaData.apply /MetaData.get /Node.attribute
• TextBuffer.toText
• BasicTransformer.transform

I'm happy take a look at those as well; I only didchild for now because that's what is affecting the 2.13 migration I'm working on.

For the record, there is animplicit conversioncollection.Seq[Node] => NodeSeq andNodeSeq is a subclass ofimmutable.Seq on 2.13.

So the following works already without this PR

scala>valn:Node= <a><b/></a>valn: scala.xml.Node= <a><b/></a>scala>valc: collection.immutable.Seq[Node]= n.childvalc:Seq[scala.xml.Node]=Seq(<b/>)

But this fails:

scala>valc: collection.immutable.Seq[String]= n.child.map(_.toString)^error:typemismatch;found   :Seq[String] (in scala.collection)required:Seq[String] (in scala.collection.immutable)

probably always returns animmutable.Seq

I took a closer look.

MarkupParser passes aNodeBuffer – I'll change that.

The compiler uses aNodeBuffer for XML literals and passes it directly toElem($buf). That worked on 2.12, sochild is actually a mutable collection. On 2.13 the implicitseqToNodeSeq makes it compile. Details here:scala/scala#11065

I also changedNode.attribute and the relatedMetaData.apply /MetaData.value /MetaData.get methods to returnimmutable.Seq.

And I changedNodeSeq.theSeq, even though this method is probably not used often as part of the public API. The reason is thatNodeSeq extendsimmutable.Seq, and iftheSeq is not immutable, it is essentially an unsafe immutable wrapper around a generic collection:

scala>valb=newNodeBuffer()+= <b/>scala>valx:NodeSeq=Elem(null,"a",Null,TopScope,true,NodeSeq.fromSeq(b):_*)valx: scala.xml.NodeSeq= <a><b/></a>scala> b+= <c/>scala> xvalres0: scala.xml.NodeSeq= <a><b/><c/></a>

I didn't changeBasicTransformer.transform because that API is expected to be subclassed, changes would likely cause source incompatibilities.

This is ready, if you'd like to take a look@dubinsky.

Besides mima, I also usedjardiff tocompare the changes and it looks as expected.

One minor comment -- it might be more efficient to useVectorBuilder rather thanNodeBuffer (which extendsArrayBuffer adding the utility method&+).

Right.. We have to keepNodeBuffer extends ArrayBuffer though, because of source and binary compatibility. So the code emitted by the compiler for literals will keep usingArrayBuffer.

We could use aVectorBuilder within scala-xml on 2.13, but it would involve changing public signatures, e.g.MarkupParser.content1(NamespaceBinding, NodeBuffer): Unit.

So it's probably not worth the effort...

Agreed. If we were maximising performance from XML literals we'd be better to use an immutable ArraySeq builder with a size hint as the compiler knows how many elements there are. But maximising performance isn't really a design goal of scala-xml IMO, so the small overhead ofArrayBuffer =>Vector is okay.