Clean up TODOs

ceedubs · Jan 22, 2020 · a7231cb · a7231cb
1 parent 3fb3fc4
commit a7231cb
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 50 deletions.
diff --git a/benchmarks/src/main/scala/AndThenAndOrRegexBenchmarks.scala b/benchmarks/src/main/scala/AndThenAndOrRegexBenchmarks.scala
@@ -2,7 +2,7 @@ package ceedubs.irrec
 package bench
 
 import cats.implicits._
-import regex._, Combinator._
+import regex._, combinator._
 import java.util.regex.Pattern
 import org.openjdk.jmh.annotations.{Benchmark, Scope, State}
 

diff --git a/benchmarks/src/main/scala/RepeatCountRegexBenchmarks.scala b/benchmarks/src/main/scala/RepeatCountRegexBenchmarks.scala
@@ -1,7 +1,7 @@
 package ceedubs.irrec
 package bench
 
-import regex._, Combinator._
+import regex._, combinator._
 import Greediness.Greedy
 
 import cats.data.Chain

diff --git a/benchmarks/src/main/scala/ZeroStarStarABenchmarks.scala b/benchmarks/src/main/scala/ZeroStarStarABenchmarks.scala
@@ -1,7 +1,7 @@
 package ceedubs.irrec
 package bench
 
-import regex._, Combinator._
+import regex._, combinator._
 import Greediness.Greedy
 
 import cats.implicits._

diff --git a/docs/overview.md b/docs/overview.md
@@ -11,7 +11,7 @@ The name is a shameless rip-off of [irreg](https://github.com/non/irreg), which
 ## creating regular expressions
 
 ```scala mdoc:silent
-import ceedubs.irrec.regex._, Combinator._, char._
+import ceedubs.irrec.regex._, combinator._, char._
 import ceedubs.irrec.parse.{regex => r}
 import ceedubs.irrec.regex.Greediness._
 import cats.implicits._
@@ -39,7 +39,6 @@ animalsR.pprint
 ## parsing with a regular expression
 
 ```scala mdoc:silent
-// TODO help with inference
 val animals: ParseState[Char, Animals] = animalsR.compile
 ```
 

diff --git a/docs/usage.md b/docs/usage.md
@@ -11,7 +11,7 @@ sidebar_label: examples
 You can create a regular expression via a `String` literal:
 
 ```scala mdoc:silent
-import ceedubs.irrec.regex._, Combinator._
+import ceedubs.irrec.regex._, combinator._
 import ceedubs.irrec.parse.{regex => r}
 
 val animalLit: RegexC[String] = r("(b|c|r|gn)at")

diff --git a/parser/src/main/scala/Parser.scala b/parser/src/main/scala/Parser.scala
@@ -8,7 +8,7 @@ import ceedubs.irrec.regex.RegexPrettyPrinter.{
   nonCharClassCharsToEscape,
   specialNonCharClassCharToLit
 }
-import Combinator._
+import combinator._
 
 import fastparse._, NoWhitespace._
 import fastparse.Parsed.{Failure, Success}
@@ -84,7 +84,7 @@ object Parser {
   /**
    * Matches the wildcard character `.`.
    */
-  def wildcard[_: P]: P[RegexC[Char]] = P(".").map(_ => Combinator.wildcard)
+  def wildcard[_: P]: P[RegexC[Char]] = P(".").map(_ => combinator.wildcard)
 
   /**
    * Positive integers within the max range of Scala's `Int`.

diff --git a/regex-gen/src/main/scala/RegexGen.scala b/regex-gen/src/main/scala/RegexGen.scala
@@ -166,10 +166,10 @@ object RegexGen {
           for {
             m <- cfg.genMatch
             f <- arbitrary[In => Out]
-          } yield Combinator.mapMatch(m, f)
+          } yield combinator.mapMatch(m, f)
         ),
         // Fail
-        (if (cfg.includeZero) 1 else 0) -> Gen.const(Combinator.fail)
+        (if (cfg.includeZero) 1 else 0) -> Gen.const(combinator.fail)
       )
     else
       Gen.frequency(
@@ -239,7 +239,7 @@ object RegexGen {
       },
       // Eps
       (if (cfg.includeOne) 2 else 0) -> Gen.const(
-        TypeWith(RegexWithEv.fromRegexGen(Gen.const(Combinator.empty[In, Match[In]]))))
+        TypeWith(RegexWithEv.fromRegexGen(Gen.const(combinator.empty[In, Match[In]]))))
     )
     def go(depth: Int): Gen[TypeWith[RegexWithEv[In, Match[In], ?]]] =
       if (depth <= 1) leafGen

diff --git a/regex/src/main/scala/Combinator.scala b/regex/src/main/scala/Combinator.scala
@@ -12,7 +12,7 @@ import cats.implicits._
 // TODO just change Regex to Regex and add stuff there?
 // TODO what should go in here vs on the class? Both? Separate combinator object?
 // TODO star-like method that allows z/fold params
-object Combinator {
+object combinator {
   def matching[A: Order](m: Match[A]): RegexM[A, A] =
     mapMatch(m, identity)
 
@@ -28,7 +28,15 @@ object Combinator {
 
   def wildcard[A: Order]: RegexM[A, A] = matching(Match.Wildcard())
 
-  def or[In, M, Out](l: Regex[In, M, Out], r: Regex[In, M, Out]): Regex[In, M, Out] = l | r
+  // TODO add in optimizations during constructions like this or have a separate method to optimize?
+  def or[In, M, Out](l: Regex[In, M, Out], r: Regex[In, M, Out]): Regex[In, M, Out] = (l, r) match {
+    case (Regex.Or(xs), Regex.Or(ys)) => Regex.Or(xs ::: ys)
+    case (_, Regex.Fail()) => l
+    case (Regex.Fail(), _) => r
+    case (Regex.Or(xs), _) => Regex.Or(r :: xs)
+    case (_, Regex.Or(ys)) => Regex.Or(l :: ys)
+    case _ => Regex.Or(NonEmptyList(l, r :: Nil))
+  }
 
   // TODO test
   def either[In, M, Out1, Out2](

diff --git a/regex/src/main/scala/Regex.scala b/regex/src/main/scala/Regex.scala
@@ -8,20 +8,10 @@ import cats.implicits._
 
 // This code was ported (with minor modifications) from https://hackage.haskell.org/package/regex-applicative
 // TODO document type parameters (especially M)
+// TODO document syntax helper classes
 sealed abstract class Regex[-In, +M, Out] extends Serializable {
   import Regex._
 
-  // TODO add ops on to avoid variance shenanigans?
-  // TODO add in optimizations during constructions like this or have a separate method to optimize?
-  def |[In2 <: In, M2 >: M](o: Regex[In2, M2, Out]): Regex[In2, M2, Out] = (this, o) match {
-    case (Or(xs), Or(ys)) => Or(xs ::: ys)
-    case (_, Fail()) => this
-    case (Fail(), _) => o
-    case (Or(xs), _) => Or(o :: xs)
-    case (_, Or(ys)) => Or(this :: ys)
-    case _ => Or(NonEmptyList(this, o :: Nil))
-  }
-
   def star(greediness: Greediness): Regex[In, M, Chain[Out]] =
     Regex.Star(this, greediness, Chain.empty[Out], (as: Chain[Out], a: Out) => as.append(a))
 
@@ -58,17 +48,7 @@ sealed abstract class Regex[-In, +M, Out] extends Serializable {
     tail.fold(head)(tail => head.map2(tail)(_ concat _))
   }
 
-  def optional[In2 <: In, M2 >: M]: Regex[In2, M2, Option[Out]] =
-    this.map[Option[Out]](Some(_)) | none[Out].pure[Regex[In2, M2, ?]]
-
   def map[B](f: Out => B): Regex[In, M, B] = FMap(this, f)
-
-  def compile[In2 <: In]: ParseState[In2, Out] = Regex.compile(this)
-
-  def matched[In2 <: In]: Regex[In2, M, Chain[In2]] = withMatched.map(_._1)
-
-  def withMatched[In2 <: In]: Regex[In2, M, (Chain[In2], Out)] =
-    Regex.withMatched(this)
 }
 
 object Regex {
@@ -277,7 +257,7 @@ object Regex {
   // TODO naming/documentation
   // TODO ops class
   def matcher[F[_]: Foldable, In, M, Out](r: Regex[In, M, Out]): F[In] => Boolean = {
-    val rc = r.void.compile[In]
+    val rc = r.void.compile
     fin => rc.parseOnly(fin).isDefined
   }
 

diff --git a/regex/src/main/scala/RegexOps.scala b/regex/src/main/scala/RegexOps.scala
@@ -2,13 +2,24 @@ package ceedubs.irrec
 package regex
 
 import cats.Foldable
+import cats.data.Chain
 import cats.implicits._
 import java.util.regex.Pattern
 
-// TODO package
-
 final class RegexOps[In, M, Out](private val r: Regex[In, M, Out]) extends AnyVal {
+  def |(o: Regex[In, M, Out]): Regex[In, M, Out] = combinator.or(r, o)
+
   def matcher[F[_]: Foldable]: F[In] => Boolean = Regex.matcher(r)
+
+  def compile: ParseState[In, Out] = Regex.compile(r)
+
+  def optional: Regex[In, M, Option[Out]] =
+    r.map[Option[Out]](Some(_)) | none[Out].pure[Regex[In, M, ?]]
+
+  def withMatched: Regex[In, M, (Chain[In], Out)] =
+    Regex.withMatched(r)
+
+  def matched: Regex[In, M, Chain[In]] = withMatched.map(_._1)
 }
 
 final class RegexCOps[Out](private val r: RegexC[Out]) extends AnyVal {

diff --git a/regex/src/main/scala/RegexPrettyPrinter.scala b/regex/src/main/scala/RegexPrettyPrinter.scala
@@ -66,7 +66,7 @@ object RegexPrettyPrinter {
     _ match {
       case Literal(a) => f(false, a)
       case MatchSet.Allow(allowed) =>
-        if (allowed.isEmpty) pprint(Combinator.fail) else s"[${showDiet(allowed)}]"
+        if (allowed.isEmpty) pprint(combinator.fail) else s"[${showDiet(allowed)}]"
       case MatchSet.Forbid(forbidden) =>
         if (forbidden.isEmpty) "." else s"[^${showDiet(forbidden)}]"
       case Match.Wildcard() => "."

diff --git a/regex/src/main/scala/char.scala b/regex/src/main/scala/char.scala
@@ -1,7 +1,7 @@
 package ceedubs.irrec
 package regex
 
-import Combinator._
+import combinator._
 
 import cats.implicits._
 

diff --git a/tests/src/test/scala/parse/ParserTests.scala b/tests/src/test/scala/parse/ParserTests.scala
@@ -1,8 +1,8 @@
 package ceedubs.irrec
 package parse
 
-import ceedubs.irrec.regex._, char._, Combinator._
-import ceedubs.irrec.regex.{Combinator => C}
+import ceedubs.irrec.regex._, char._, combinator._
+import ceedubs.irrec.regex.{combinator => C}
 import ceedubs.irrec.regex.CharacterClasses
 import ceedubs.irrec.regex.Match
 import ceedubs.irrec.regex.Match.MatchSet
@@ -470,8 +470,6 @@ class ParserTests extends IrrecSuite {
     assert(parseRegex("a{1,").isLeft)
   }
 
-  // TODO do we want this to just be unit?
-  // TODO clean up pprint calls
   def sameRegex(actual: RegexM[Char, _], expected: RegexM[Char, _]): Assertion = {
     val clue =
       s"""(pprint not optimized):

diff --git a/tests/src/test/scala/regex-gen/RegexMatchGenTests.scala b/tests/src/test/scala/regex-gen/RegexMatchGenTests.scala
@@ -6,7 +6,7 @@ import org.scalacheck.Gen
 
 class RegexMatchGenTests extends IrrecSuite {
   test("regexMatchingStreamGen generates a failing stream for Zero") {
-    val r: RegexC[Long] = Combinator.fail
+    val r: RegexC[Long] = combinator.fail
     val gen = regexMatchingStreamGen[Char](_ => Gen.const('a')).apply(r)
     gen.sample should ===(None)
   }

diff --git a/tests/src/test/scala/regex/RegexMatchTests.scala b/tests/src/test/scala/regex/RegexMatchTests.scala
@@ -1,8 +1,8 @@
 package ceedubs.irrec
 package regex
 
-import Combinator._
-import ceedubs.irrec.regex.{Combinator => C}
+import combinator._
+import ceedubs.irrec.regex.{combinator => C}
 import char._
 import RegexGen._
 import ceedubs.irrec.parse.{regex => parse}
@@ -261,16 +261,15 @@ class RegexMatchTests extends IrrecSuite {
 
   test("word character match") {
     val gen = Gen.oneOf(Gen.alphaNumChar, Gen.const('_'))
-    // TODO needing to specify Char here is annoying. Look into how to make this not so painful
-    val rc = wordChar.compile[Char]
+    val rc = wordChar.compile
     forAll(gen) { c =>
       rc.parseOnlyS(c.toString) should ===(Some(c))
     }
   }
 
   test("whitespace character match") {
     val gen = Gen.oneOf('\t', '\n', '\f', '\r', ' ')
-    val rc = whitespaceChar.compile[Char]
+    val rc = whitespaceChar.compile
     forAll(gen) { c =>
       rc.parseOnlyS(c.toString) should ===(Some(c))
     }
@@ -319,7 +318,7 @@ class RegexMatchTests extends IrrecSuite {
   test("repeat(0, n) matches empty") {
     forAll(arbitrary[RegexM[Int, Unit]], Gen.option(Gen.chooseNum(0, 20)), arbitrary[Greediness]) {
       (r, max, g) =>
-        r.repeat(0, max, g).void.compile.parseOnly(List.empty) should ===(Some(()))
+        r.repeat(0, max, g).void.compile.parseOnly(List.empty[Int]) should ===(Some(()))
     }
   }