From 77aa36303a041a696cd50a9e7893677a5bc1027b Mon Sep 17 00:00:00 2001 From: i10416 Date: Wed, 24 Jan 2024 07:00:19 +0900 Subject: [PATCH 1/3] fix(#16458): regression in xml syntax parsing xLiteral mistakenly assumed that the element just after `<` is always non-special element, but it is not true. It could be xCharData, comment, xProcInstr. --- .../dotc/parsing/xml/MarkupParserCommon.scala | 8 ++++ .../dotc/parsing/xml/MarkupParsers.scala | 15 ++++++-- tests/pos/i16458.scala | 37 +++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 tests/pos/i16458.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala index 803470fe85a5..906d104041b2 100644 --- a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala +++ b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala @@ -202,6 +202,14 @@ private[dotty] trait MarkupParserCommon { /** skip optional space S? */ def xSpaceOpt(): Unit = while (isSpace(ch) && !eof) nextch() + /** skip optional space S? and return the number of consumed characters */ + def xSpaceOptN(): Int = + var i = 0 + while (isSpace(ch) && !eof) do + nextch() + i += 1 + i + /** scan [3] S ::= (#x20 | #x9 | #xD | #xA)+ */ def xSpace(): Unit = if (isSpace(ch)) { nextch(); xSpaceOpt() } diff --git a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala index 22ef15b6f497..2a8ae5c64a5b 100644 --- a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala @@ -371,10 +371,17 @@ object MarkupParsers { // parse more XML? if (charComingAfter(xSpaceOpt()) == '<') { while { - xSpaceOpt() - nextch() - ts.append(element) - charComingAfter(xSpaceOpt()) == '<' + if xSpaceOptN() == 0 then + nextch() + if content_LT(ts) then // Is `` valid xml? + xToken("/>") + charComingAfter(xSpaceOpt()) == '<' + else + // this is surely not a special node as any special node + // should start with `<{special symbol}` without space. + nextch() + ts.append(element) + charComingAfter(xSpaceOpt()) == '<' } do () handle.makeXMLseq(Span(start, curOffset, start), ts) } diff --git a/tests/pos/i16458.scala b/tests/pos/i16458.scala new file mode 100644 index 000000000000..fe4c90dcb41c --- /dev/null +++ b/tests/pos/i16458.scala @@ -0,0 +1,37 @@ +def x =
FooBar
+ +package scala.xml { + type MetaData = AnyRef + + trait NamespaceBinding + object TopScope extends NamespaceBinding + object Null + abstract class Node { + def label: String + def child: Seq[Node] + override def toString = label + child.mkString + } + class Comment(commentText: String) extends Node{ + def label = commentText + def child = Nil + } + class Elem(prefix: String, val label: String, attributes1: MetaData, scope: NamespaceBinding, minimizeEmpty: Boolean, val child: Node*) extends Node + class NodeBuffer extends Seq[Node] { + val nodes = scala.collection.mutable.ArrayBuffer.empty[Node] + def &+(o: Any): NodeBuffer = + o match { + case n: Node => nodes.addOne(n) ; this + case t: Text => nodes.addOne(Atom(t)) ; this + } + // Members declared in scala.collection.IterableOnce + def iterator: Iterator[scala.xml.Node] = nodes.iterator + // Members declared in scala.collection.SeqOps + def apply(i: Int): scala.xml.Node = nodes(i) + def length: Int = nodes.length + } + case class Text(text: String) + case class Atom(t: Text) extends Node { + def label = t.text + def child = Nil + } +} \ No newline at end of file From 27046feabeb0f3682c0fc734c9d4e49e7d082a0f Mon Sep 17 00:00:00 2001 From: i10416 Date: Wed, 24 Jan 2024 12:58:26 +0900 Subject: [PATCH 2/3] improve: just use content_LT to fix 16458 Just replacing element with content_LT, it works. See https://github.com/lampepfl/dotty/pull/19522#issuecomment-1907084802 --- .../dotc/parsing/xml/MarkupParserCommon.scala | 8 -------- .../tools/dotc/parsing/xml/MarkupParsers.scala | 15 ++++----------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala index 906d104041b2..803470fe85a5 100644 --- a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala +++ b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParserCommon.scala @@ -202,14 +202,6 @@ private[dotty] trait MarkupParserCommon { /** skip optional space S? */ def xSpaceOpt(): Unit = while (isSpace(ch) && !eof) nextch() - /** skip optional space S? and return the number of consumed characters */ - def xSpaceOptN(): Int = - var i = 0 - while (isSpace(ch) && !eof) do - nextch() - i += 1 - i - /** scan [3] S ::= (#x20 | #x9 | #xD | #xA)+ */ def xSpace(): Unit = if (isSpace(ch)) { nextch(); xSpaceOpt() } diff --git a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala index 2a8ae5c64a5b..06e8645b82c0 100644 --- a/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala @@ -371,17 +371,10 @@ object MarkupParsers { // parse more XML? if (charComingAfter(xSpaceOpt()) == '<') { while { - if xSpaceOptN() == 0 then - nextch() - if content_LT(ts) then // Is `` valid xml? - xToken("/>") - charComingAfter(xSpaceOpt()) == '<' - else - // this is surely not a special node as any special node - // should start with `<{special symbol}` without space. - nextch() - ts.append(element) - charComingAfter(xSpaceOpt()) == '<' + xSpaceOpt() + nextch() + content_LT(ts) + charComingAfter(xSpaceOpt()) == '<' } do () handle.makeXMLseq(Span(start, curOffset, start), ts) } From 9de9d57930b391b8f064f3630af5122d0941c949 Mon Sep 17 00:00:00 2001 From: i10416 Date: Wed, 24 Jan 2024 14:21:55 +0900 Subject: [PATCH 3/3] tidy: use run test instead of compile test To check parsing properly, it is better to run a test and assert parse result. --- tests/{pos => run}/i16458.scala | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) rename tests/{pos => run}/i16458.scala (70%) diff --git a/tests/pos/i16458.scala b/tests/run/i16458.scala similarity index 70% rename from tests/pos/i16458.scala rename to tests/run/i16458.scala index fe4c90dcb41c..1c4b0365e45e 100644 --- a/tests/pos/i16458.scala +++ b/tests/run/i16458.scala @@ -1,4 +1,21 @@ -def x =
FooBar
+ +object Test { + import scala.xml.* + def main(args: Array[String]): Unit = { + val xml =
FooBar
+ assert( + xml match + case Seq(elm: Elem, comment: Comment) if + elm.label == "div" && + elm.child(0) == Atom(Text("FooBar")) && + comment.label == " /.modal-content " + => true + case _ => false + , + xml + ) + } +} package scala.xml { type MetaData = AnyRef @@ -16,7 +33,7 @@ package scala.xml { def child = Nil } class Elem(prefix: String, val label: String, attributes1: MetaData, scope: NamespaceBinding, minimizeEmpty: Boolean, val child: Node*) extends Node - class NodeBuffer extends Seq[Node] { + class NodeBuffer extends Seq[Node] { val nodes = scala.collection.mutable.ArrayBuffer.empty[Node] def &+(o: Any): NodeBuffer = o match {