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.

Sign up for GitHub

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

Jump to bottom

Fix missing constraint that Slice step cannot be 0 #17

Merged
merged 1 commit into from
Aug 26, 2022
Merged

Fix missing constraint that Slice step cannot be 0 #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 35 additions & 16 deletions core/src/peschke/python/SliceParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import cats.syntax.all._
import peschke.python.Slice._
import peschke.python.SliceParser.ParseError._

/** Parse a [[peschke.collections.python.Slice]], using modified Python slice
* syntax.
/** Parse a [[peschke.python.Slice]], using modified Python slice syntax.
*
* The primary modification is that the indices must be integers, rather than
* expressions.
Expand Down Expand Up @@ -46,8 +45,7 @@ object SliceParser {

object ErrorContext extends supertagged.NewType[String] {

private[SliceParser] def extract(start: Int, length: Int, raw: String)
: Type = {
private[python] def extract(start: Int, length: Int, raw: String): Type = {
val len = length.max(3)
val (before, targetAndAfter) = raw.splitAt(start)
val (target, after) = targetAndAfter.splitAt(1)
Expand Down Expand Up @@ -76,6 +74,7 @@ object SliceParser {
val EndSkip = "end-skipped"
val StepGiven = "step-given"
val StepSkip = "step-skipped"
val StepIsZero = "step-zero"
}
}

Expand Down Expand Up @@ -107,6 +106,8 @@ object SliceParser {
extends ParseError
final case class UnexpectedInput(index: Index, context: ErrorContext)
extends ParseError
final case class StepCannotBeZero(index: Index, context: ErrorContext)
extends ParseError

private def priority(pe: ParseError): Int = pe match {
case ExpectedOpenBrace(_, _, _) => 1
Expand All @@ -115,6 +116,7 @@ object SliceParser {
case ExpectedNumber(_, _) => 4
case ExpectedSeparator(_, _) => 5
case UnexpectedInput(_, _) => 6
case StepCannotBeZero(_, _) => 7
}

implicit val order: Order[ParseError] =
Expand All @@ -132,16 +134,21 @@ object SliceParser {
s"$index :: expected ':' at: $context"
case UnexpectedInput(index, context) =>
s"$index :: unexpected input at: $context"
case StepCannotBeZero(index, context) =>
s"$index :: step cannot be zero at: $context"
}
}

def default[F[_]](openBrace: String, closeBrace: String, contextLength: Int)
(implicit AE: ApplicativeError[F, NonEmptyChain[ParseError]])
: SliceParser[F] = new SliceParser[F] {
private val indexParser: Parser[Long] = Numbers
.signedIntString.mapFilter { raw =>
Either.catchOnly[NumberFormatException](raw.toLong).toOption
}.withContext(ErrorContext.Names.Number)
private val indexParser: Parser[Long] =
Numbers
.signedIntString
.mapFilter { raw =>
Either.catchOnly[NumberFormatException](raw.toLong).toOption
}
.withContext(ErrorContext.Names.Number)

private val openBraceParser: Parser[Unit] =
Parser.string(openBrace).withContext(ErrorContext.Names.OpenBrace)
Expand All @@ -162,17 +169,22 @@ object SliceParser {
val separator: Parser[Unit] = Parser.char(':').withContext(Separator)

val startP: Parser0[Option[Long]] = opt(indexParser, StartGiven, StartSkip)
val endP: Parser0[Option[Long]] = opt(indexParser, EndGiven, EndSkip)
val stepP: Parser0[Option[Long]] = opt(indexParser, StepGiven, StepSkip)
val endP: Parser0[Option[Long]] = opt(indexParser, EndGiven, EndSkip)
val stepP: Parser0[Option[Long]] =
opt(indexParser, StepGiven, StepSkip).flatMap {
case Some(0L) => Parser.fail.withContext(StepIsZero)
case nonZero => Parser.pure(nonZero)
}

val stepExp: Parser[Long] = (separator *> stepP).map(_.getOrElse(1))

((startP <* separator) ~ (endP ~ stepExp.?).?).map {
case (Some(index), None) => At(index)
case (startOpt, Some((endOpt, stepOpt))) =>
Slice(startOpt, endOpt, stepOpt)
case (startOpt, None) => Slice(startOpt, None, None)
}
((startP <* separator) ~ (endP ~ stepExp.?).?)
.map {
case (Some(index), None) => At(index)
case (startOpt, Some((endOpt, stepOpt))) =>
Slice(startOpt, endOpt, stepOpt)
case (startOpt, None) => Slice(startOpt, None, None)
}
}

private val bracedSliceParser: Parser0[Slice] =
Expand All @@ -199,6 +211,13 @@ object SliceParser {
ExpectedNumber(index, context) :: Nil
case Separator | StartSkip | EndSkip | StepSkip =>
ExpectedSeparator(index, context) :: Nil
case StepIsZero =>
// Because this parser fails during post-parse validation, the offset needs to be adjusted
// for the error to make sense to the user.
StepCannotBeZero(
Index(expectation.offset - 1),
ErrorContext.extract(expectation.offset - 1, contextLength, input)
) :: Nil
case _ => Nil
}).getOrElse(NonEmptyChain.one {
expectation match {
Expand Down
44 changes: 37 additions & 7 deletions core/test/src-2.13/peschke/python/SliceParserTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package peschke.python

import cats.data.NonEmptyChain
import cats.syntax.all._
import org.scalacheck.Arbitrary
import org.scalacheck.Gen
import org.scalatest.matchers.Matcher
import peschke.PropSpec
import peschke.python.Slice._
import peschke.python.SliceParser.ErrorContext
import peschke.python.SliceParser.Index
import peschke.python.SliceParser.ParseError
import peschke.python.SliceParserTest._
import peschke.scalacheck.syntax._
Expand All @@ -20,13 +23,21 @@ class SliceParserTest extends PropSpec {
val parse: Matcher[Test] = Matcher { test =>
be(test.range.asRight).apply(rangeParser.parse(test.raw))
}

val failToParse: Matcher[(String, ParseError)] = Matcher {
case (input, expected) =>
be(NonEmptyChain(expected).asLeft).apply(rangeParser.parse(input))
}

val parsePrefix: Matcher[(Test, String)] = Matcher {
case (test, suffix) =>
be((test.range, suffix).asRight).apply(rangeParser.parsePrefix(test.raw))
}

val parseUnbraced: Matcher[Test] = Matcher { test =>
be(test.range.asRight).apply(rangeParser.parseUnbraced(test.rawNoBraces))
}

val parseUnbracedPrefix: Matcher[(Test, String)] = Matcher {
case (test, suffix) =>
be((test.range, suffix).asRight)
Expand Down Expand Up @@ -152,6 +163,10 @@ class SliceParserTest extends PropSpec {
}

// endregion

property("Step cannot be zero") {
forAll(slicesWithZeroStep)(_ must failToParse)
}
}

object SliceParserTest {
Expand All @@ -174,13 +189,14 @@ object SliceParserTest {
suffix
)

val stepGen: Gen[Int] =
Gen.oneOf(Gen.chooseNum(Int.MinValue, -1), Gen.chooseNum(1, Int.MaxValue))

val allGen: Gen[Test] =
Gen.oneOf(
Gen.const(Test.passing(":", All(1))),
Gen.const(Test.passing("::", All(1))),
Arbitrary.arbitrary[Int].map { step =>
Test.passing(s"::$step", All(step))
}
stepGen.map { step => Test.passing(s"::$step", All(step)) }
)

val fromStartGen: Gen[Test] =
Expand All @@ -191,7 +207,7 @@ object SliceParserTest {
.arbitrary[Int].map(end => Test.passing(s":$end:", FromStart(end, 1))),
for {
end <- Arbitrary.arbitrary[Int]
step <- Arbitrary.arbitrary[Int]
step <- stepGen
} yield Test.passing(s":$end:$step", FromStart(end, step))
)

Expand All @@ -203,15 +219,15 @@ object SliceParserTest {
.arbitrary[Int].map(start => Test.passing(s"$start::", ToEnd(start, 1))),
for {
start <- Arbitrary.arbitrary[Int]
step <- Arbitrary.arbitrary[Int]
step <- stepGen
} yield Test.passing(s"$start::$step", ToEnd(start, step))
)

val subSliceGen: Gen[Test] =
for {
start <- Arbitrary.arbitrary[Int]
end <- Arbitrary.arbitrary[Int]
step <- Arbitrary.arbitrary[Int]
step <- stepGen
} yield Test.passing(s"$start:$end:$step", SubSlice(start, end, step))

val atGen: Gen[Test] =
Expand All @@ -221,11 +237,25 @@ object SliceParserTest {
for {
start <- Arbitrary.arbitrary[Int].optional.map(_.fold("")(_.show))
end <- Arbitrary.arbitrary[Int].optional.map(_.fold("")(_.show))
step <- Arbitrary.arbitrary[Int].optional.map(_.fold("")(_.show))
step <- stepGen.optional.map(_.fold("")(_.show))
collapse <- Arbitrary.arbitrary[Boolean]
} yield {
val raw = s"$start:$end:$step"
if (collapse) raw.replace("::", ":")
else raw
}

val slicesWithZeroStep: Gen[(String, ParseError)] =
for {
start <- Arbitrary.arbitrary[Int].optional.map(_.fold("")(_.show))
end <- Arbitrary.arbitrary[Int].optional.map(_.fold("")(_.show))
} yield {
val prefix = s"[$start:$end:"
val input = s"${prefix}0]"
val stepIndex = prefix.length
input -> ParseError.StepCannotBeZero(
Index(stepIndex),
ErrorContext.extract(stepIndex, 5, input)
)
}
}