diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala index 6b5fe3a3..876f9a40 100644 --- a/core/shared/src/main/scala/cats/parse/Parser.scala +++ b/core/shared/src/main/scala/cats/parse/Parser.scala @@ -1282,12 +1282,19 @@ object Parser { /** product with the first argument being a Parser */ def product10[A, B](first: Parser[A], second: Parser0[B]): Parser[(A, B)] = - Impl.Prod(first, second) + first match { + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen + case _ => Impl.Prod(first, second) + } /** product with the second argument being a Parser */ def product01[A, B](first: Parser0[A], second: Parser[B]): Parser[(A, B)] = - Impl.Prod(first, second) + first match { + case p1: Parser[A] => product10(p1, second) + case _ => Impl.Prod(first, second) + } /** softProduct, a variant of product A soft product backtracks if the first succeeds and the * second is an epsilon-failure. By contrast product will be a failure in that case @@ -1312,7 +1319,11 @@ object Parser { * see @Parser.soft */ def softProduct10[A, B](first: Parser[A], second: Parser0[B]): Parser[(A, B)] = - Impl.SoftProd(first, second) + first match { + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen + case _ => Impl.SoftProd(first, second) + } /** softProduct with the second argument being a Parser A soft product backtracks if the first * succeeds and the second is an epsilon-failure. By contrast product will be a failure in that @@ -1321,7 +1332,11 @@ object Parser { * see @Parser.soft */ def softProduct01[A, B](first: Parser0[A], second: Parser[B]): Parser[(A, B)] = - Impl.SoftProd(first, second) + first match { + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen + case _ => Impl.SoftProd(first, second) + } /** transform a Parser0 result */ @@ -1348,10 +1363,8 @@ object Parser { case _ => AndThen(f0).andThen(fn) } Impl.Map(p0, f1) - case Impl.Fail() | Impl.FailWith(_) => - // these are really Parser[Nothing{ - // but scala can't see that, so we cast - p.asInstanceOf[Parser[B]] + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen case _ => Impl.Map(p, fn) } @@ -1382,21 +1395,31 @@ object Parser { * to optimization */ def flatMap0[A, B](pa: Parser0[A])(fn: A => Parser0[B]): Parser0[B] = - Impl.FlatMap0(pa, fn) + pa match { + case p: Parser[A] => flatMap10(p)(fn) + case _ => Impl.FlatMap0(pa, fn) + } /** Standard monadic flatMap where you start with a Parser Avoid this function if possible. If you * can instead use product, ~, *>, or <* use that. flatMap always has to allocate a parser, and * the parser is less amenable to optimization */ def flatMap10[A, B](pa: Parser[A])(fn: A => Parser0[B]): Parser[B] = - Impl.FlatMap(pa, fn) + pa match { + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen + case _ => Impl.FlatMap(pa, fn) + } /** Standard monadic flatMap where you end with a Parser Avoid this function if possible. If you * can instead use product, ~, *>, or <* use that. flatMap always has to allocate a parser, and * the parser is less amenable to optimization */ def flatMap01[A, B](pa: Parser0[A])(fn: A => Parser[B]): Parser[B] = - Impl.FlatMap(pa, fn) + pa match { + case p1: Parser[A] => flatMap10(p1)(fn) + case _ => Impl.FlatMap(pa, fn) + } /** tail recursive monadic flatMaps This is a rarely used function, but needed to implement * cats.FlatMap Avoid this function if possible. If you can instead use product, ~, *>, or <* use @@ -1588,10 +1611,8 @@ object Parser { case v @ Impl.Void(_) => v case _ => Impl.unmap(pa) match { - case f @ (Impl.Fail() | Impl.FailWith(_)) => - // these are really Parser[Nothing] - // but scala can't see that, so we cast - f.asInstanceOf[Parser[Unit]] + case f @ Impl.Fail() => f.widen + case f @ Impl.FailWith(_) => f.widen case p: Impl.Str => p case p: Impl.StringIn => p case p: Impl.IgnoreCase => p @@ -2202,6 +2223,8 @@ object Parser { state.error = Eval.later(Chain.one(Expectation.Fail(offset))) null.asInstanceOf[A] } + + def widen[B]: Parser[B] = this.asInstanceOf[Parser[B]] } case class FailWith[A](message: String) extends Parser[A] { @@ -2210,6 +2233,21 @@ object Parser { state.error = Eval.later(Chain.one(Expectation.FailWith(offset, message))) null.asInstanceOf[A] } + + def widen[B]: Parser[B] = this.asInstanceOf[Parser[B]] + } + + /* + * We don't want to include empty Fail messages inside of a oneOf + * since they are the zeros of the orElse operation + */ + final def filterFails(offset: Int, fs: Chain[Expectation]): Chain[Expectation] = { + val fs1 = fs.filter { + case Expectation.Fail(o) if o == offset => false + case _ => true + } + if (fs1.isEmpty) Chain.one(Expectation.Fail(offset)) + else fs1 } final def oneOf[A](all: Array[Parser0[A]], state: State): A = { @@ -2235,7 +2273,7 @@ object Parser { } // if we got here, all of them failed, but we // never advanced the offset - state.error = errs + state.error = errs.map(filterFails(offset, _)) null.asInstanceOf[A] } diff --git a/core/shared/src/test/scala/cats/parse/ParserTest.scala b/core/shared/src/test/scala/cats/parse/ParserTest.scala index 779b67bf..aa964b7f 100644 --- a/core/shared/src/test/scala/cats/parse/ParserTest.scala +++ b/core/shared/src/test/scala/cats/parse/ParserTest.scala @@ -602,6 +602,12 @@ class ParserTest extends munit.ScalaCheckSuite { .withMinSuccessfulTests(tests) .withMaxDiscardRatio(10) + // override val scalaCheckInitialSeed = + // past regressions that we can check periodically + // "bUQvEfxFZ73bxtVjauK8tJDrEKOFbxUfk6WrGiy3bkH=" + // "PPsKExr4HRlyCXkMrC6Rki5u59V88vwSeVTiGWJFS3G=" + // "Ova1uT18mkE4uTX4RdgQza6z70fxyv6micl4hIZvywP=" + def parseTest[A: Eq](p: Parser0[A], str: String, a: A) = p.parse(str) match { case Right((_, res)) => @@ -811,24 +817,28 @@ class ParserTest extends munit.ScalaCheckSuite { } } - def orElse[A](p1: Parser0[A], p2: Parser0[A], str: String): Either[Parser.Error, (String, A)] = { - if (p1 == Parser.Fail) p2.parse(str) - else if (p2 == Parser.Fail) p1.parse(str) - else - p1.parse(str) match { - case left @ Left(err) => - if (err.failedAtOffset == 0) { - p2.parse(str) - .leftMap { err1 => - if (err1.failedAtOffset == 0) { - val errs = err.expected ::: err1.expected - Parser.Error(err1.failedAtOffset, Parser.Expectation.unify(errs)) - } else err1 - } - } else left - case right => right - } - } + def orElse[A](p1: Parser0[A], p2: Parser0[A], str: String): Either[Parser.Error, (String, A)] = + p1.parse(str) match { + case left @ Left(err) => + if (err.failedAtOffset == 0) { + p2.parse(str) + .leftMap { err1 => + if (err1.failedAtOffset == 0) { + val errs = err.expected ::: err1.expected + NonEmptyList.fromList(errs.filter { + case Parser.Expectation.Fail(0) => false + case _ => true + }) match { + case None => + Parser.Error(0, NonEmptyList(Parser.Expectation.Fail(0), Nil)) + case Some(nel) => + Parser.Error(err1.failedAtOffset, Parser.Expectation.unify(nel)) + } + } else err1 + } + } else left + case right => right + } property("oneOf0 composes as expected") { forAll(ParserGen.gen0, ParserGen.gen0, Arbitrary.arbitrary[String]) { (genP1, genP2, str) => @@ -1429,11 +1439,11 @@ class ParserTest extends munit.ScalaCheckSuite { property("with1 *> and with1 <* work as expected") { forAll(ParserGen.gen0, ParserGen.gen, Arbitrary.arbitrary[String]) { (p1, p2, str) => val rp1 = p1.fa.with1 *> p2.fa - val rp2 = (p1.fa.with1 ~ p2.fa).map(_._2) + val rp2 = (p1.fa.void.with1 ~ p2.fa).map(_._2) assertEquals(rp1.parse(str), rp2.parse(str)) val rp3 = p1.fa.with1 <* p2.fa - val rp4 = (p1.fa.with1 ~ p2.fa).map(_._1) + val rp4 = (p1.fa.with1 ~ p2.fa.void).map(_._1) assertEquals(rp3.parse(str), rp4.parse(str)) } } @@ -2102,6 +2112,15 @@ class ParserTest extends munit.ScalaCheckSuite { } } + property("a Parser never succeeds and does not advance") { + forAll(ParserGen.gen, Arbitrary.arbitrary[String]) { (genP, str) => + genP.fa.parse(str) match { + case Right((rest, _)) => assertNotEquals(rest, str) + case Left(_) => () + } + } + } + property("select on pure values works as expected") { forAll { (left: Option[Either[Int, String]], right: Option[Int => String], str: String) => val pleft = left match {