Fix issue 263

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]
     }
 

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 {