Improve error reporting for rep0, repAs0 (#396)

typelevel · Mar 28, 2022 · bfa5224 · bfa5224
1 parent 7856e98
commit bfa5224
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 31 deletions.
diff --git a/core/shared/src/main/scala/cats/parse/Parser.scala b/core/shared/src/main/scala/cats/parse/Parser.scala
@@ -1099,7 +1099,11 @@ object Parser {
     *   this can wind up parsing nothing
     */
   def repAs0[A, B](p1: Parser[A])(implicit acc: Accumulator0[A, B]): Parser0[B] =
-    Impl.Rep0(p1, Int.MaxValue, acc)
+    Impl.OneOf0(
+      Impl.Rep(p1, 1, Int.MaxValue, acc) ::
+        pure(acc.newAppender().finish()) ::
+        Nil
+    )
 
   /** Repeat the parser 0 or more times, but no more than `max`
     *
@@ -1114,12 +1118,17 @@ object Parser {
     */
   def repAs0[A, B](p1: Parser[A], max: Int)(implicit acc: Accumulator0[A, B]): Parser0[B] = {
     require(max >= 0, s"max should be >= 0, was $max")
+    val empty = acc.newAppender().finish()
     if (max == 0) {
       // exactly 0 times
-      pure(acc.newAppender().finish())
+      pure(empty)
     } else {
       // 0 or more times
-      Impl.Rep0(p1, max - 1, acc)
+      Impl.OneOf0(
+        Impl.Rep(p1, 1, max - 1, acc) ::
+          pure(empty) ::
+          Nil
+      )
     }
   }
 
@@ -1730,7 +1739,7 @@ object Parser {
       case peek @ Impl.Peek(_) => peek
       case s if Impl.alwaysSucceeds(s) => unit
       case notPeek =>
-        // TODO: we can adjust Rep0/Rep to do minimal
+        // TODO: we can adjust Rep to do minimal
         // work since we rewind after we are sure there is
         // a match
         Impl.Peek(void0(notPeek))
@@ -2257,9 +2266,9 @@ object Parser {
             ) =>
           // these are always unit
           someUnit.asInstanceOf[Option[A]]
-        case Rep0(_, _, _) | Rep(_, _, _, _) | FlatMap0(_, _) | FlatMap(_, _) | TailRecM(_, _) |
-            TailRecM0(_, _) | Defer(_) | Defer0(_) | GetCaret | Index | Length(_) | Fail() |
-            FailWith(_) | CharIn(_, _, _) | AnyChar | StringP(
+        case Rep(_, _, _, _) | FlatMap0(_, _) | FlatMap(_, _) | TailRecM(_, _) | TailRecM0(_, _) |
+            Defer(_) | Defer0(_) | GetCaret | Index | Length(_) | Fail() | FailWith(_) |
+            CharIn(_, _, _) | AnyChar | StringP(
               _
             ) | OneOf(Nil) | OneOf0(Nil) | StringP0(_) | Select(_, _) | Select0(_, _) | StringIn(
               _
@@ -2290,8 +2299,7 @@ object Parser {
         case Length(_) | StringP(_) | StringIn(_) | Prod(_, _) | SoftProd(_, _) | Map(_, _) |
             Select(_, _) | FlatMap(_, _) | TailRecM(_, _) | Defer(_) | Rep(_, _, _, _) | AnyChar |
             CharIn(_, _, _) | StringP0(_) | Index | GetCaret | Prod0(_, _) | SoftProd0(_, _) |
-            Map0(_, _) | Select0(_, _) | FlatMap0(_, _) | TailRecM0(_, _) | Defer0(_) |
-            Rep0(_, _, _) =>
+            Map0(_, _) | Select0(_, _) | FlatMap0(_, _) | TailRecM0(_, _) | Defer0(_) =>
           false
       }
 
@@ -2365,7 +2373,6 @@ object Parser {
             case UnmapDefer0(_) => pa // already unmapped
             case _ => Defer0(UnmapDefer0(fn))
           }
-        case Rep0(p, max, _) => Rep0(unmap(p), max, Accumulator0.unitAccumulator0)
         case WithContextP0(ctx, p0) => WithContextP0(ctx, unmap0(p0))
         case StartParser | EndParser | TailRecM0(_, _) | FlatMap0(_, _) =>
           // we can't transform this significantly
@@ -2962,22 +2969,6 @@ object Parser {
       }
     }
 
-    case class Rep0[A, B](p1: Parser[A], maxMinusOne: Int, acc: Accumulator0[A, B])
-        extends Parser0[B] {
-      private[this] val ignore: B = null.asInstanceOf[B]
-
-      override def parseMut(state: State): B = {
-        if (state.capture) {
-          val app = acc.newAppender()
-          if (repCapture(p1, 0, maxMinusOne, state, app)) app.finish()
-          else ignore
-        } else {
-          repNoCapture(p1, 0, maxMinusOne, state)
-          ignore
-        }
-      }
-    }
-
     /** A parser that can repeats the underlying parser multiple times
       */
     case class Rep[A, B](p1: Parser[A], min: Int, maxMinusOne: Int, acc1: Accumulator[A, B])

diff --git a/core/shared/src/test/scala/cats/parse/ParserTest.scala b/core/shared/src/test/scala/cats/parse/ParserTest.scala
@@ -3012,4 +3012,13 @@ class ParserTest extends munit.ScalaCheckSuite {
         assertEquals(left.parse(str).void, right.parse(str).void)
     }
   }
+
+  property("a.rep0 ~ b is the same as (a.repAs ~ b) | (pure(Nil).with1 ~ b)") {
+    forAll(ParserGen.gen, ParserGen.gen, arbitrary[String]) { (a, b, str) =>
+      val left = a.fa.rep0 ~ b.fa
+      val right = (a.fa.repAs[List[a.A]] ~ b.fa) | (Parser.pure(List.empty[a.A]).with1 ~ b.fa)
+
+      assertEquals(left.parse(str), right.parse(str))
+    }
+  }
 }
diff --git a/project/MimaExclusionRules.scala b/project/MimaExclusionRules.scala
@@ -1,7 +1,11 @@
-import com.typesafe.tools.mima.core.ProblemFilters
-import com.typesafe.tools.mima.core.IncompatibleMethTypeProblem
-import com.typesafe.tools.mima.core.IncompatibleResultTypeProblem
-import com.typesafe.tools.mima.core.DirectMissingMethodProblem
+import com.typesafe.tools.mima.core.{
+  ProblemFilters,
+  IncompatibleMethTypeProblem,
+  IncompatibleResultTypeProblem,
+  DirectMissingMethodProblem,
+  MissingClassProblem
+}
+
 object MimaExclusionRules {
   val parserImpl = Seq(
     "cats.parse.Parser#Impl.mergeCharIn",
@@ -18,7 +22,11 @@ object MimaExclusionRules {
   ).map(ProblemFilters.exclude[IncompatibleResultTypeProblem](_)) ++ Seq(
     "cats.parse.Parser#Impl.mergeCharIn",
     "cats.parse.Parser#Impl.mergeStrIn"
-  ).map(ProblemFilters.exclude[DirectMissingMethodProblem](_))
+  ).map(ProblemFilters.exclude[DirectMissingMethodProblem](_)) ++ Seq(
+    "cats.parse.Parser$Impl$Rep0",
+    "cats.parse.Parser$Impl$Rep0$"
+  ).map(ProblemFilters.exclude[MissingClassProblem](_))
+
   // TODO: Remove these rules in future release.
   val bitSetUtil = Seq(
     "cats.parse.BitSetUtil.isSingleton",