Skip to content

Commit

Permalink
feat(parser): Strict indentation [fixes LNG-135] (#714)
Browse files Browse the repository at this point in the history 
* Implemented indentation consistency checking

* Added comments

---------

Co-authored-by: Dima <[email protected]>
  • Loading branch information
@InversionSpaces @DieMyst
InversionSpaces and DieMyst authored May 29, 2023
1 parent 32430f4 commit ae2a433
Show file tree
Hide file tree
Showing 2 changed files with 176 additions and 37 deletions.
114 changes: 87 additions & 27 deletions parser/src/main/scala/aqua/parser/ListToTreeConverter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ final case class ListToTreeConverter[F[_]](
stack: List[ListToTreeConverter.Block[F]] = Nil, // Stack of opened blocks
errors: Chain[ParserError[F]] = Chain.empty[ParserError[F]] // Errors
)(using Comonad[F]) {

// Import helper functions
import ListToTreeConverter.*

Expand All @@ -30,43 +31,60 @@ final case class ListToTreeConverter[F[_]](
private def pushBlock(indent: F[String], line: Tree[F]): ListToTreeConverter[F] =
copy(currentBlock = Block(indent, line), stack = currentBlock :: stack)

private def addToCurrentBlock(line: Tree[F]): ListToTreeConverter[F] =
copy(currentBlock = currentBlock.add(line))
private def addToCurrentBlock(indent: F[String], line: Tree[F]): ListToTreeConverter[F] =
copy(currentBlock = currentBlock.add(indent, line))

private def popBlock: Option[ListToTreeConverter[F]] =
stack match {
case Nil => None
case prevBlock :: tail =>
Some(copy(currentBlock = prevBlock.add(currentBlock.close), stack = tail))
Some(
copy(
currentBlock = prevBlock.add(
currentBlock.indent,
currentBlock.close
),
stack = tail
)
)
}

/**
* Method to call on each new line
*/
@scala.annotation.tailrec
def next(indent: F[String], line: Tree[F]): ListToTreeConverter[F] =
if (indentValue(indent) > indentValue(currentBlock.indent)) {
if (isBlock(line)) {
pushBlock(indent, line)
} else {
val expr = lastExpr(line)

if (currentBlock.canAdd(expr)) {
addToCurrentBlock(line)
currentBlock.classifyIndent(indent) match {
case IndentRelation.Child(consistent) =>
val consistentChecked = if (!consistent) {
addError(inconsistentIndentError(indent))
} else this

if (isBlock(line)) {
consistentChecked.pushBlock(indent, line)
} else {
addError(wrongChildError(indent, expr))
val expr = lastExpr(line)

if (currentBlock.isValidChild(expr)) {
consistentChecked.addToCurrentBlock(indent, line)
} else {
consistentChecked.addError(wrongChildError(indent, expr))
}
}
}
} else {
val emptyChecked = if (currentBlock.isEmpty) {
addError(emptyBlockError(currentBlock.indent))
} else this

emptyChecked.popBlock match {
case Some(blockPopped) => blockPopped.next(indent, line)
// This should not happen because of the way of parsing
case _ => emptyChecked.addError(unexpectedIndentError(indent))
}
// Note: this doesn't necessarly mean that indentation is correct
// next block will check it
case IndentRelation.Sibling =>
val emptyChecked = if (currentBlock.isEmpty) {
addError(emptyBlockError(currentBlock.indent))
} else this

emptyChecked.popBlock match {
case Some(blockPopped) => blockPopped.next(indent, line)
// This should not happen because of the way of parsing
case _ => emptyChecked.addError(unexpectedIndentError(indent))
}
case IndentRelation.Unexpected =>
addError(unexpectedIndentError(indent))
}

/**
Expand Down Expand Up @@ -94,19 +112,55 @@ object ListToTreeConverter {
def apply[F[_]](open: Tree[F])(using Comonad[F]): ListToTreeConverter[F] =
ListToTreeConverter(Block(open.head.token.as(""), open))

/**
* Describes the realtion of next line to the current block
*/
enum IndentRelation {
case Child(consistent: Boolean)
case Sibling
case Unexpected
}

/**
* Data associated with a block
*/
final case class Block[F[_]](
indent: F[String], // Indentation of the block opening line
block: Tree[F], // Block opening line
childIndent: Option[F[String]] = None, // Indentation of the first child
content: Chain[Tree[F]] = Chain.empty[Tree[F]] // Children of the block
) {

/**
* Classify the next line relative to the block
*/
def classifyIndent(lineIndent: F[String])(using Comonad[F]): IndentRelation = {
val blockIndentStr = indent.extract
val lineIndentStr = lineIndent.extract

if (lineIndentStr.startsWith(blockIndentStr)) {
lazy val consistentChild = childIndent
.map(_.extract)
.fold(
lineIndentStr.length > blockIndentStr.length
)(_ == lineIndentStr)

if (lineIndentStr.length == blockIndentStr.length) {
IndentRelation.Sibling
} else {
IndentRelation.Child(consistentChild)
}
} else if (blockIndentStr.startsWith(lineIndentStr)) {
IndentRelation.Sibling
} else {
IndentRelation.Unexpected
}
}

/**
* Check if expr can be added to this block
*/
def canAdd(expr: Expr[F]): Boolean = {
def isValidChild(expr: Expr[F]): Boolean = {
def checkFor(tree: Tree[F]): Boolean =
tree.head.companion match {
case indented: AndIndented =>
Expand All @@ -123,10 +177,13 @@ object ListToTreeConverter {
}

/**
* Add child to the block
* Add line to the block
*/
def add(child: Tree[F]): Block[F] =
copy(content = content :+ child)
def add(indent: F[String], line: Tree[F]): Block[F] =
copy(
content = content :+ line,
childIndent = childIndent.orElse(Some(indent))
)

/**
* Check if the block has no children
Expand Down Expand Up @@ -182,6 +239,9 @@ object ListToTreeConverter {
def emptyBlockError[F[_]](indent: F[String]): ParserError[F] =
BlockIndentError(indent, "Block expression has no body")

def inconsistentIndentError[F[_]](indent: F[String]): ParserError[F] =
BlockIndentError(indent, "Inconsistent indentation in the block")

def unexpectedIndentError[F[_]](indent: F[String]): ParserError[F] =
BlockIndentError(indent, "Unexpected indentation")

Expand Down
99 changes: 89 additions & 10 deletions parser/src/test/scala/aqua/parser/FuncExprSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import cats.Id
import cats.data.{Chain, NonEmptyList}
import cats.free.Cofree
import cats.syntax.foldable.*
import cats.data.Validated.{Invalid, Valid}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.{Inside, Inspectors}
import org.scalatest.matchers.should.Matchers
import cats.~>

Expand All @@ -27,7 +29,7 @@ import scala.language.implicitConversions
import aqua.parser.lift.Span
import aqua.parser.lift.Span.{P0ToSpan, PToSpan}

class FuncExprSpec extends AnyFlatSpec with Matchers with AquaSpec {
class FuncExprSpec extends AnyFlatSpec with Matchers with Inside with Inspectors with AquaSpec {
import AquaSpec._

private val parser = Parser.spanParser
Expand Down Expand Up @@ -116,16 +118,87 @@ class FuncExprSpec extends AnyFlatSpec with Matchers with AquaSpec {

}

// TODO: unignore in LNG-135
"function with wrong indent" should "parse with error" ignore {
val script =
"""func tryGen() -> bool:
| on "deeper" via "deep":
| v <- Local.gt()
"function with mixed blocks indent" should "parse without error" in {
val scriptFor =
"""func try():
| v <- Local.call()
| for x <- v:
| foo(x)
| for y <- x:
| bar(y)
| for z <- v:
| baz(z)
|""".stripMargin

val scriptIf =
"""func try(v: bool):
| if v:
| foo()
| else:
| if v:
| bar()
| else:
| baz()
|""".stripMargin

val scriptOn =
"""func try():
| on "some" via "some":
| foo()
| on "some" via "some":
| bar()
| on "some" via "some":
| bar()
|""".stripMargin

forAll(List(scriptFor, scriptIf, scriptOn)) { script =>
parser.parseAll(script).value.toEither.isRight shouldBe true
}
}

"function with wrong indent" should "parse with error" in {
val scriptSimple =
"""func try():
| v <- Local.call()
| x = v
|""".stripMargin

val scriptReturn =
"""func try() -> bool:
| v <- Local.call()
| <- v
|""".stripMargin

parser.parseAll(script).value.toEither.isLeft shouldBe true
val scriptFor =
"""func try():
| v <- call()
| for x <- v:
| foo(x)
|""".stripMargin

val scriptIf =
"""func try(v: bool):
| if v:
| foo()
| call()
|""".stripMargin

val scriptOn =
"""func try():
| call()
| on "some" via "some":
| foo()
|""".stripMargin

forAll(List(scriptSimple, scriptReturn, scriptFor, scriptIf, scriptOn)) { script =>
inside(parser.parseAll(script).value) { case Invalid(errors) =>
forAll(errors.toList) { error =>
inside(error) { case BlockIndentError(_, message) =>
message shouldEqual "Inconsistent indentation in the block"
}
}
}
}
}

"function with multiline definitions" should "parse without error" in {
Expand Down Expand Up @@ -179,7 +252,10 @@ class FuncExprSpec extends AnyFlatSpec with Matchers with AquaSpec {
)
qTree.d() shouldBe ArrowExpr(toArrowType(Nil, Some(scToBt(bool))))
qTree.d() shouldBe OnExpr(toStr("deeper"), List(toStr("deep")))
qTree.d() shouldBe CallArrowExpr(List("v"), CallArrowToken(Some(toNamedType("Local")), "gt", Nil))
qTree.d() shouldBe CallArrowExpr(
List("v"),
CallArrowToken(Some(toNamedType("Local")), "gt", Nil)
)
qTree.d() shouldBe ReturnExpr(NonEmptyList.one(toVar("v")))
// genC function
qTree.d() shouldBe FuncExpr(
Expand All @@ -188,7 +264,10 @@ class FuncExprSpec extends AnyFlatSpec with Matchers with AquaSpec {
// List("two": VarLambda[Id])
)
qTree.d() shouldBe ArrowExpr(toNamedArrow(("val" -> string) :: Nil, boolSc :: Nil))
qTree.d() shouldBe CallArrowExpr(List("one"), CallArrowToken(Some(toNamedType("Local")), "gt", List()))
qTree.d() shouldBe CallArrowExpr(
List("one"),
CallArrowToken(Some(toNamedType("Local")), "gt", List())
)
qTree.d() shouldBe OnExpr(toStr("smth"), List(toStr("else")))
qTree.d() shouldBe CallArrowExpr(List("two"), CallArrowToken(None, "tryGen", List()))
qTree.d() shouldBe CallArrowExpr(
Expand Down

0 comments on commit ae2a433

Please sign in to comment.