Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exception thrown when expression is too short #335

Merged
merged 2 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions modules/core/shared/src/main/scala/cron4s/errors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 12 additions & 2 deletions modules/core/shared/src/main/scala/cron4s/parsing/base.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
8 changes: 3 additions & 5 deletions modules/core/shared/src/main/scala/cron4s/parsing/lexer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 8 additions & 6 deletions modules/core/shared/src/main/scala/cron4s/parsing/parser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
80 changes: 53 additions & 27 deletions tests/shared/src/test/scala/cron4s/CronSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand All @@ -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)
}
}
}

Expand Down