cats-parse icon indicating copy to clipboard operation
cats-parse copied to clipboard

Should `Parser` have a typeclass?

Open morgen-peschke opened this issue 3 years ago • 2 comments

One of the things I've noticed while using the library is using Parsers tends to follow this pattern (at least for me):

Foo.parser.parseAll(raw)

This has a couple of minor pain points (notably, "what did I name the parser?"), which should be straightforward to fix. Would you be interested in a PR that adds the needed scaffolding to allow this sort of thing instead:

import cats.parser.syntax._

raw.parseAll[Foo]

I'm planning on implementing this in one of my weekend projects anyway, so if you're interested I'll implement it as a PR instead of just something local to my project.

morgen-peschke avatar Aug 03 '22 22:08 morgen-peschke

This is an interesting question. I personally think typeclasses work best when there is only one reasonable implementation. I don't think that is usually the case for parsers (consider Parser[Int] might not be very uncommon).

That said, you could imagine something similar to the scalacheck Arbitrary/Gen design, where Gen is not a typeclass, but Arbitrary is.

so, you could imagine something like:

trait DefaultParser0[A] {
  def parser0: Parser0[A]
}

object syntax {
  final case class ParserOps(private val str: String) extends AnyVal {
    def parseAll[A](implicit dp: DefaultParser0[A]): Either[Parser.Error, A] = dp.parser0.parseAll(str)
  }
}

Is that the kind of thing you are thinking about? I think something like this may work. Maybe we can hash out a rough design in this issue before we commit too much. What do you think?

johnynek avatar Aug 03 '22 23:08 johnynek

Yep, that's pretty much what I was thinking.

I agree that default parsers for most things don't really make sense, so we wouldn't need much in the way of default instances. Something like this should be enough:

trait Parseable[A] {
  def parser0: Parser0[A]
}
object Parseable {
  def apply[A](implicit P: Parseable[A]): P.type = P

  def instance[A](p: Parser0[A]): Parseable[A] = new Impl[A](p)

  private final class Impl[A](override val parser0: Parser0[A])
      extends Parseable[A]
      with Serializable
}

Parseable.instance and Impl are because the chance that ScalaTest will close over a Parseable instance is pretty good, and it chokes if it grabs anything that doesn't extend Serializable.

For the syntax, it might make sense to include parse as well as parseAll:

object syntax {
  implicit final class ParseableOps(private val raw: String) extends AnyVal {
    def parse[A: Parseable]: Either[Parser.Error, (String, A)] =
      Parseable[A].parser0.parse(raw)

    def parseAll[A: Parseable]: Either[Parser.Error, A] =
      Parseable[A].parser0.parseAll(raw)
  }
}

I did a quick assumptions check in scastie, and the ergonomics look good:

object trial {
  object definitions {
    object Foo extends supertagged.NewType[String] {
      val parser0: Parser0[Type] = Parser.string0("Foo").string.map(apply(_))
      implicit final val FooIsParseable: Parseable[Type] =
        Parseable.instance(parser0)
    }
    type Foo = Foo.Type

    object Bar extends supertagged.NewType[String] {
      val parser: Parser[Type] = Parser.string("Bar").string.map(apply(_))
      implicit final val FooIsParseable: Parseable[Type] =
        Parseable.instance(parser)
    }
    type Bar = Bar.Type
  }

  object use {
    import definitions.{Foo, Bar}
    import syntax._

    def run(): Unit = {
      List("Foo|", "Bar|").foreach { input =>
        println(s"${"-" * 10} $input ${"-" * 10}")
        println(s"parse[Foo]: ${input.parse[Foo]}")
        println(s"parse[Bar]: ${input.parse[Bar]}")
        println()
        println(s"parseAll[Foo]: ${input.parseAll[Foo]}")
        println(s"parseAll[Bar]: ${input.parseAll[Bar]}")
        println()
      }
    }
  }
}

morgen-peschke avatar Aug 04 '22 00:08 morgen-peschke