Fix exception thrown when expression is too short (#335)

* Fix exception thrown when expression is too short

* Format code

build.sbt

@@ -68,8 +68,7 @@ val commonSettings = Def.settings(
   consoleImports := Seq("cron4s._"),
   initialCommands in console := consoleImports.value
     .map(s => s"import $s")
-    .mkString("\n"),
-  scalafmtOnCompile := true
+    .mkString("\n")
 ) ++ CompilerPlugins.All
 
 lazy val commonJvmSettings = Seq(

modules/core/shared/src/main/scala/cron4s/errors.scala

@@ -29,8 +29,10 @@ object Error {
   implicit val errorShow: Show[Error] = Show.show(_.getMessage)
 }
 
-final case class ParseFailed(msg: String, found: String, position: Int)
-    extends Error(s"$msg at position $position but found '$found'")
+case object ExprTooShort extends Error("The provided expression was too short")
+
+final case class ParseFailed(expected: String, position: Int, found: Option[String] = None)
+    extends Error(s"$expected at position ${position}${found.fold("")(f => s" but found '$f'")}")
 
 sealed trait ValidationError
 object ValidationError {

modules/core/shared/src/main/scala/cron4s/parsing/base.scala

@@ -18,8 +18,18 @@ package cron4s
 package parsing
 
 import scala.util.parsing.combinator.Parsers
+import scala.util.parsing.input.{Position, NoPosition}
 
 private[parsing] trait BaseParser extends Parsers {
-  protected def handleError(err: NoSuccess): ParseFailed =
-    ParseFailed(err.msg, err.next.first.toString, err.next.pos.column)
+  protected def handleError(err: NoSuccess): _root_.cron4s.Error =
+    err.next.pos match {
+      case NoPosition => ExprTooShort
+      case pos: Position =>
+        val position = err.next.pos.column
+        val found = {
+          if (err.next.atEnd) None
+          else Option(err.next.first.toString).filter(_.nonEmpty)
+        }
+        ParseFailed(err.msg, position, found)
+    }
 }

modules/core/shared/src/main/scala/cron4s/parsing/lexer.scala

@@ -87,13 +87,11 @@ object CronLexer extends RegexParsers with BaseParser {
   }
 
   private lazy val tokens: Parser[List[CronToken]] =
-    phrase(
-      rep1(
-        number | text | hyphen | slash | comma | asterisk | questionMark | blank
-      )
+    rep1(
+      number | text | hyphen | slash | comma | asterisk | questionMark | blank
     )
 
-  def tokenize(expr: String): Either[ParseFailed, List[CronToken]] =
+  def tokenize(expr: String): Either[_root_.cron4s.Error, List[CronToken]] =
     parse(tokens, expr) match {
       case err: NoSuccess     => Left(handleError(err))
       case Success(result, _) => Right(result)

modules/core/shared/src/main/scala/cron4s/parsing/parser.scala

@@ -165,12 +165,14 @@ object CronParser extends Parsers with BaseParser {
       any[F].map(any2FieldWithAny)
 
   val cron: Parser[CronExpr] = {
-    (field(seconds) <~ blank) ~
-      (field(minutes) <~ blank) ~
-      (field(hours) <~ blank) ~
-      (fieldWithAny(daysOfMonth) <~ blank) ~
-      (field(months) <~ blank) ~
-      fieldWithAny(daysOfWeek) ^^ {
+    phrase(
+      (field(seconds) <~ blank) ~!
+        (field(minutes) <~ blank) ~!
+        (field(hours) <~ blank) ~!
+        (fieldWithAny(daysOfMonth) <~ blank) ~!
+        (field(months) <~ blank) ~!
+        fieldWithAny(daysOfWeek)
+    ) ^^ {
       case sec ~ min ~ hour ~ day ~ month ~ weekDay =>
         CronExpr(sec, min, hour, day, month, weekDay)
     }

tests/shared/src/test/scala/cron4s/CronSpec.scala

@@ -23,19 +23,57 @@ import cron4s.syntax.all._
 
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.prop.TableDrivenPropertyChecks
 
 import scala.util.{Failure, Success}
 
 /**
   * Created by alonsodomin on 12/04/2017.
   */
-object CronSpec {
+object CronSpec extends TableDrivenPropertyChecks {
   import CronField._
 
-  final val AllEachExpr = "* * * * * *"
-  final val AnyDaysExpr = "* * * ? * ?"
-
-  final val InvalidExprs = List(AllEachExpr, AnyDaysExpr)
+  final val AllEachExpr  = "* * * * * *"
+  final val AnyDaysExpr  = "* * * ? * ?"
+  final val TooLongExpr  = "* * * ? * * *"
+  final val TooShortExpr = "* * * * *"
+
+  final val InvalidExprs =
+    Table(
+      ("description", "expression", "expected error"),
+      (
+        "all stars",
+        AllEachExpr,
+        InvalidCron(
+          NonEmptyList.of(
+            InvalidFieldCombination(
+              "Fields DayOfMonth and DayOfWeek can't both have the expression: *"
+            )
+          )
+        )
+      ),
+      (
+        "symbol ? at two positions",
+        AnyDaysExpr,
+        InvalidCron(
+          NonEmptyList.of(
+            InvalidFieldCombination(
+              "Fields DayOfMonth and DayOfWeek can't both have the expression: ?"
+            )
+          )
+        )
+      ),
+      (
+        "too long expression",
+        TooLongExpr,
+        ParseFailed("end of input expected", 2, Some(" "))
+      ),
+      (
+        "too short expression",
+        TooShortExpr,
+        ExprTooShort
+      )
+    )
 
   final val ValidExpr = CronExpr(
     SeveralNode(BetweenNode[Second](ConstNode(17), ConstNode(30)), ConstNode[Second](5)),
@@ -45,38 +83,26 @@ object CronSpec {
     EachNode[Month],
     AnyNode[DayOfWeek]
   )
+
 }
 
 class CronSpec extends AnyFlatSpec with Matchers {
   import CronSpec._
 
-  "Cron" should "not parse an expression with all *" in {
+  "Cron" should "not parse an invalid expression" in {
     val expectedError =
       InvalidFieldCombination("Fields DayOfMonth and DayOfWeek can't both have the expression: *")
 
-    val parsed = Cron(AllEachExpr)
-    parsed shouldBe Left(InvalidCron(NonEmptyList.of(expectedError)))
-
-    val parsedTry = Cron.tryParse(AllEachExpr)
-    parsedTry should matchPattern { case Failure(InvalidCron(_)) => }
-
-    intercept[InvalidCron] {
-      Cron.unsafeParse(AllEachExpr)
-    }
-  }
-
-  it should "not parse an expression with ? in both DayOfMonth and DayOfWeek" in {
-    val expectedError =
-      InvalidFieldCombination("Fields DayOfMonth and DayOfWeek can't both have the expression: ?")
-
-    val parsed = Cron(AnyDaysExpr)
-    parsed shouldBe Left(InvalidCron(NonEmptyList.of(expectedError)))
+    forAll(InvalidExprs) { (desc: String, expr: String, err: Error) =>
+      val parsed = Cron(expr)
+      parsed shouldBe Left(err)
 
-    val parsedTry = Cron.tryParse(AnyDaysExpr)
-    parsedTry should matchPattern { case Failure(InvalidCron(_)) => }
+      val parsedTry = Cron.tryParse(expr)
+      parsedTry should matchPattern { case Failure(`err`) => }
 
-    intercept[InvalidCron] {
-      Cron.unsafeParse(AnyDaysExpr)
+      intercept[Error] {
+        Cron.unsafeParse(expr)
+      }
     }
   }