Fix Parser.parseDocuments (#346)

Co-authored-by: Luca Violanti <luca.violanti@kaluza.com> fixes #343 closes #342
circe · Nov 20, 2022 · e6cdde3 · e6cdde3
1 parent c0a1f8d
commit e6cdde3
Show file tree

Hide file tree

Showing 6 changed files with 211 additions and 8 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -26,6 +26,7 @@ jobs:
   build:
     name: Build and Test
     strategy:
+      fail-fast: false
       matrix:
         os: [ubuntu-latest]
         scala: [2.12.17, 2.13.10, 3.2.1]

diff --git a/build.sbt b/build.sbt
@@ -3,6 +3,7 @@ ThisBuild / circeRootOfCodeCoverage := None
 ThisBuild / startYear := Some(2016)
 ThisBuild / scalafixScalaBinaryVersion := "2.12"
 ThisBuild / tlFatalWarningsInCi := false //TODO: ... fix this someday
+ThisBuild / githubWorkflowBuildMatrixFailFast := Some(false)
 
 val Versions = new {
   val circe = "0.14.3"

diff --git a/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala b/circe-yaml-v12/src/main/scala/io/circe/yaml/v12/ParserImpl.scala
@@ -46,7 +46,10 @@ class ParserImpl(settings: LoadSettings) extends common.Parser {
   def parse(yaml: String): Either[ParsingFailure, Json] =
     parse(new StringReader(yaml))
 
-  def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml).map(yamlToJson)
+  def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml) match {
+    case Left(error)   => Stream(Left(error))
+    case Right(stream) => stream.map(yamlToJson)
+  }
   def parseDocuments(yaml: String): Stream[Either[ParsingFailure, Json]] = parseDocuments(new StringReader(yaml))
 
   private[this] def asScala[T](ot: Optional[T]): Option[T] =
@@ -65,8 +68,8 @@ class ParserImpl(settings: LoadSettings) extends common.Parser {
       case Right(Some(value)) => Right(value)
     }
 
-  private[this] def parseStream(reader: Reader) =
-    createComposer(reader).asScala.toStream
+  private[this] def parseStream(reader: Reader): Either[ParsingFailure, Stream[Node]] =
+    Either.catchNonFatal(createComposer(reader).asScala.toStream).leftMap(err => ParsingFailure(err.getMessage, err))
 
   final def decode[A: Decoder](input: Reader): Either[Error, A] =
     finishDecode(parse(input))

diff --git a/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala b/circe-yaml-v12/src/test/scala/io/circe/yaml/v12/ParserTests.scala
@@ -16,15 +16,18 @@
 
 package io.circe.yaml.v12
 
+import io.circe.Json
 import io.circe.syntax._
 import org.scalatest.EitherValues
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
+import java.io.StringReader
+
 class ParserTests extends AnyFlatSpec with Matchers with EitherValues {
   // the laws should do a pretty good job of surfacing errors; these are mainly to ensure test coverage
 
-  "Parser" should "fail on invalid tagged numbers" in {
+  "Parser.parse" should "fail on invalid tagged numbers" in {
     assert(parser.parse("!!int 12foo").isLeft)
   }
 
@@ -137,4 +140,98 @@ class ParserTests extends AnyFlatSpec with Matchers with EitherValues {
         .isLeft
     )
   }
+
+  "Parser.parseDocuments" should "fail on invalid tagged numbers" in {
+    val result = parser.parseDocuments(new StringReader("!!int 12foo")).toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+  }
+
+  it should "fail to parse complex keys" in {
+    val result = parser
+      .parseDocuments(new StringReader("""
+          |? - foo
+          |  - bar
+          |: 1""".stripMargin))
+      .toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+  }
+
+  it should "fail to parse invalid YAML" in {
+    val result = parser.parseDocuments(new StringReader("""foo: - bar""")).toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+    assert(result.head.isInstanceOf[Either[io.circe.ParsingFailure, Json]])
+  }
+
+  it should "parse yes as true" in {
+    val result = parser.parseDocuments(new StringReader("""foo: yes""")).toList
+    assert(result.size == 1)
+    assert(result.head.isRight)
+  }
+
+  it should "parse hexadecimal as strings" in {
+    val result = parser.parseDocuments(new StringReader("""[0xFF, 0xff, 0xab_cd]""")).toList
+    assert(result.size == 1)
+    assert(result.head.contains(Seq("0xFF", "0xff", "0xab_cd").asJson.asJson))
+  }
+
+  it should "parse decimal with underscore breaks as strings" in {
+    val result = parser.parseDocuments(new StringReader("""foo: 1_000_000""")).toList
+    assert(result.size == 1)
+    assert(result.head.contains(Map("foo" -> "1_000_000").asJson))
+  }
+
+  it should "parse empty string as 0 documents" in {
+    val result = parser.parseDocuments(new StringReader("")).toList
+    assert(result.isEmpty)
+  }
+
+  it should "parse blank string as 0 documents" in {
+    val result = parser.parseDocuments(new StringReader("   ")).toList
+    assert(result.isEmpty)
+  }
+
+  it should "parse aliases" in {
+    val result = parser
+      .parseDocuments(
+        new StringReader(
+          """
+            | aliases:
+            |   - &alias1
+            |     foo:
+            |       bar
+            | baz:
+            |  - *alias1
+            |  - *alias1
+            |""".stripMargin
+        )
+      )
+      .toList
+    assert(result.size == 1)
+    assert(result.head.isRight)
+  }
+
+  it should "fail to parse too many aliases" in {
+    val result =
+      Parser
+        .make(Parser.Config(maxAliasesForCollections = 1))
+        .parseDocuments(
+          new StringReader(
+            """
+          | aliases:
+          |   - &alias1
+          |     foo:
+          |       bar
+          | baz:
+          |  - *alias1
+          |  - *alias1
+          |""".stripMargin
+          )
+        )
+        .toList
+    assertResult(1)(result.size)
+    assert(result.head.isLeft)
+  }
 }
diff --git a/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala b/circe-yaml/src/main/scala/io/circe/yaml/Parser.scala
@@ -52,14 +52,20 @@ final case class Parser(
 
   def parse(yaml: String): Either[ParsingFailure, Json] = parse(new StringReader(yaml))
 
-  def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml).map(yamlToJson)
+  def parseDocuments(yaml: Reader): Stream[Either[ParsingFailure, Json]] = parseStream(yaml) match {
+    case Left(error)   => Stream(Left(error))
+    case Right(stream) => stream.map(yamlToJson)
+  }
+
   def parseDocuments(yaml: String): Stream[Either[ParsingFailure, Json]] = parseDocuments(new StringReader(yaml))
 
   private[this] def parseSingle(reader: Reader): Either[ParsingFailure, Node] =
     Either.catchNonFatal(new Yaml(loaderOptions).compose(reader)).leftMap(err => ParsingFailure(err.getMessage, err))
 
-  private[this] def parseStream(reader: Reader): Stream[Node] =
-    new Yaml(loaderOptions).composeAll(reader).asScala.toStream
+  private[this] def parseStream(reader: Reader): Either[ParsingFailure, Stream[Node]] =
+    Either
+      .catchNonFatal(new Yaml(loaderOptions).composeAll(reader).asScala.toStream)
+      .leftMap(err => ParsingFailure(err.getMessage, err))
 
   final def decode[A: Decoder](input: Reader): Either[Error, A] =
     finishDecode(parse(input))

diff --git a/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala b/circe-yaml/src/test/scala/io/circe/yaml/ParserTests.scala
@@ -22,10 +22,12 @@ import org.scalatest.EitherValues
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 
+import java.io.StringReader
+
 class ParserTests extends AnyFlatSpec with Matchers with EitherValues {
   // the laws should do a pretty good job of surfacing errors; these are mainly to ensure test coverage
 
-  "Parser" should "fail on invalid tagged numbers" in {
+  "Parser.parse" should "fail on invalid tagged numbers" in {
     assert(parser.parse("!!int 12foo").isLeft)
   }
 
@@ -138,4 +140,97 @@ class ParserTests extends AnyFlatSpec with Matchers with EitherValues {
         .isLeft
     )
   }
+
+  "Parser.parseDocuments" should "fail on invalid tagged numbers" in {
+    val result = parser.parseDocuments(new StringReader("!!int 12foo")).toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+  }
+
+  it should "fail to parse complex keys" in {
+    val result = parser
+      .parseDocuments(new StringReader("""
+        |? - foo
+        |  - bar
+        |: 1""".stripMargin))
+      .toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+  }
+
+  it should "fail to parse invalid YAML" in {
+    val result = parser.parseDocuments(new StringReader("""foo: - bar""")).toList
+    assert(result.size == 1)
+    assert(result.head.isLeft)
+    assert(result.head.isInstanceOf[Either[io.circe.ParsingFailure, Json]])
+  }
+
+  it should "parse yes as true" in {
+    val result = parser.parseDocuments(new StringReader("""foo: yes""")).toList
+    assert(result.size == 1)
+    assert(result.head.isRight)
+  }
+
+  it should "parse hexadecimal" in {
+    val result = parser.parseDocuments(new StringReader("""[0xFF, 0xff, 0xab_cd]""")).toList
+    assert(result.size == 1)
+    assert(result.head.contains(Seq(0xff, 0xff, 0xabcd).asJson))
+  }
+
+  it should "parse decimal with underscore breaks" in {
+    val result = parser.parseDocuments(new StringReader("""foo: 1_000_000""")).toList
+    assert(result.size == 1)
+    assert(result.head.contains(Map("foo" -> 1000000).asJson))
+  }
+
+  it should "parseDocuments empty string as 0 documents" in {
+    val result = parser.parseDocuments(new StringReader("")).toList
+    assert(result.isEmpty)
+  }
+
+  it should "parseDocuments blank string as 0 documents" in {
+    val result = parser.parseDocuments(new StringReader("   ")).toList
+    assert(result.isEmpty)
+  }
+
+  it should "parse aliases" in {
+    val result = parser
+      .parseDocuments(
+        new StringReader(
+          """
+            | aliases:
+            |   - &alias1
+            |     foo:
+            |       bar
+            | baz:
+            |  - *alias1
+            |  - *alias1
+            |""".stripMargin
+        )
+      )
+      .toList
+    assert(result.size == 1)
+    assert(result.head.isRight)
+  }
+
+  it should "fail to parse too many aliases" in {
+    val result =
+      Parser(maxAliasesForCollections = 1)
+        .parseDocuments(
+          new StringReader(
+            """
+            | aliases:
+            |   - &alias1
+            |     foo:
+            |       bar
+            | baz:
+            |  - *alias1
+            |  - *alias1
+            |""".stripMargin
+          )
+        )
+        .toList
+    assertResult(1)(result.size)
+    assert(result.head.isLeft)
+  }
 }