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

Improve YamlError ADT #271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve YamlError ADT #271

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/shared/src/main/scala-3/org/virtuslab/yaml/YamlDecoderCrossCompat.scala
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@ private[yaml] trait DecoderMacros {
.map { (k, v) =>
k match {
case ScalarNode(scalarKey, _) => Right((scalarKey, v))
case _ => Left(ConstructError(s"Parameter of a class must be a scalar value"))
case _ => Left(ConstructError.from(s"Parameter of a class must be a scalar value"))
}
}
val (error, valuesSeq) = keyValueMap.partitionMap(identity)
Expand All @@ -40,7 +40,7 @@ private[yaml] trait DecoderMacros {
case Some(value) => c.construct(value)
case None =>
if (isOptional) Right(None)
else Left(ConstructError(s"Key $label doesn't exist in parsed document"))
else Left(ConstructError.from(s"Key $label doesn't exist in parsed document"))
}
val (left, right) = values.partitionMap(identity)
if left.nonEmpty then Left(left.head)
Expand Down Expand Up @@ -68,7 +68,7 @@ private[yaml] trait DecoderMacros {
)
} yield (constructedValues)
case _ =>
Left(ConstructError(s"Expected MappingNode, got ${node.getClass.getSimpleName}"))
Left(ConstructError.from(s"Expected MappingNode, got ${node.getClass.getSimpleName}"))
}

protected inline def sumOf[T](s: Mirror.SumOf[T]) =
Expand All @@ -80,7 +80,7 @@ private[yaml] trait DecoderMacros {
.from(instances)
.map(c => c.construct(node))
.collectFirst { case r @ Right(_) => r }
.getOrElse(Left(ConstructError(s"Cannot parse $node")))
.getOrElse(Left(ConstructError.from(s"Cannot parse $node")))

protected inline def summonSumOf[T <: Tuple]: List[YamlDecoder[_]] = inline erasedValue[T] match
case _: (t *: ts) =>
Expand Down
10 changes: 6 additions & 4 deletions core/shared/src/main/scala/org/virtuslab/yaml/YamlDecoder.scala
View file
Open in desktop
Expand Up @@ -26,9 +26,11 @@ object YamlDecoder extends YamlDecoderCompanionCrossCompat {
if (pf.isDefinedAt(node)) pf(node)
else
Left(
ConstructError(s"""|Could't construct ${classTag.runtimeClass.getName} from ${node.tag}
|${node.pos.map(_.errorMsg).getOrElse("")}
|""".stripMargin)
ConstructError.from(
s"""|Could't construct ${classTag.runtimeClass.getName} from ${node.tag}
|${node.pos.map(_.errorMsg).getOrElse("")}
|""".stripMargin
)
)
}

Expand Down Expand Up @@ -96,7 +98,7 @@ object YamlDecoder extends YamlDecoderCompanionCrossCompat {
case Some(decoder) => decoder.construct(node)
case None =>
Left(
ConstructError(
ConstructError.from(
s"""|Could't construct runtime instance of ${node.tag}
|${node.pos.map(_.errorMsg).getOrElse("")}
|If you're using custom datatype consider using yaml.as[MyType] instead of Any
Expand Down
79 changes: 48 additions & 31 deletions core/shared/src/main/scala/org/virtuslab/yaml/YamlError.scala
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@ import scala.util.control.NoStackTrace

import org.virtuslab.yaml.internal.load.reader.token.Token
import org.virtuslab.yaml.internal.load.reader.token.TokenKind
import org.virtuslab.yaml.internal.load.TagValue

/**
* An ADT representing a decoding failure.
Expand All @@ -13,39 +14,49 @@ sealed trait YamlError {
def msg: String
}

final case class ParseError(msg: String) extends YamlError
sealed trait ParseError extends YamlError
object ParseError {
def from(expected: String, got: Token): ParseError = ParseError(
s"""|Expected
def from(expected: String, got: Token): ParseError = ExpectedTokenKind(expected, got)
def from(expected: TokenKind, got: Token): ParseError = ParseError.from(expected.toString, got)

final case class ExpectedTokenKind(expected: String, got: Token) extends ParseError {
def msg: String =
s"""|Expected
|$expected but instead got ${got.kind}
|${got.range.errorMsg}""".stripMargin
)
def from(expected: TokenKind, got: Token): ParseError = ParseError.from(expected.toString, got)
}

final case class NoRegisteredTagDirective(handleKey: String, tokenTag: Token) extends ParseError {
def msg: String = s"There is no registered tag directive for handle $handleKey"
}
}

final case class ComposerError(msg: String) extends YamlError

final case class ModifyError(msg: String) extends YamlError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for future: ModifyError is shouldn't rather be part of YamlError hierarchy. cc: @lbialy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you quickly enumerate what each of these error types is meant to represent and when can they occur?


final case class ConstructError(msg: String) extends YamlError
final case class ConstructError(
errorMsg: String,
node: Option[Node],
expected: Option[String]
) extends YamlError {
def msg: String = node.flatMap(_.pos) match {
case Some(range) =>
s"""|$errorMsg
|at ${range.start.line}:${range.start.column},${expected
.map(exp => s" expected $exp")
.getOrElse("")}
|${range.errorMsg} """.stripMargin
case None =>
errorMsg
}
}
object ConstructError {
private def from(
errorMsg: String,
node: Option[Node],
expected: Option[String]
): ConstructError = {
val msg = node.flatMap(_.pos) match {
case Some(range) =>
s"""|$errorMsg
|at ${range.start.line}:${range.start.column},${expected
.map(exp => s" expected $exp")
.getOrElse("")}
|${range.errorMsg} """.stripMargin
case None =>
errorMsg
}
ConstructError(msg)
}
): ConstructError = ConstructError(errorMsg, node, expected)
def from(errorMsg: String, node: Node, expected: String): ConstructError =
from(errorMsg, Some(node), Some(expected))
def from(errorMsg: String, expected: String, node: Node): ConstructError =
Expand All @@ -65,18 +76,24 @@ object ConstructError {
def from(t: Throwable): ConstructError = from(t.getMessage, None, None)
}

final case class ScannerError(msg: String) extends Throwable with YamlError with NoStackTrace
sealed trait ScannerError extends Throwable with YamlError with NoStackTrace
object ScannerError {
def from(obtained: String, got: Token): ScannerError = ScannerError(
s"""|Obtained
|$obtained but expected got ${got.kind}
|${got.range.errorMsg}""".stripMargin
)
def from(obtained: String, got: Token): ScannerError = Obtained(obtained, got)

def from(range: Range, msg: String): ScannerError = AtRange(range, msg)

def from(range: Range, msg: String): ScannerError = ScannerError(
s"""|Error at line ${range.start.line}, column ${range.start.column}:
|${range.errorMsg}
|$msg
|""".stripMargin
)
case class Obtained(obtained: String, got: Token) extends ScannerError {
def msg: String =
s"""|Obtained
|$obtained but expected got ${got.kind}
|${got.range.errorMsg}""".stripMargin
}

case class AtRange(range: Range, rawMsg: String) extends ScannerError {
def msg: String =
s"""|Error at line ${range.start.line}, column ${range.start.column}:
|${range.errorMsg}
|$msg
|""".stripMargin
}
}
2 changes: 1 addition & 1 deletion core/shared/src/main/scala/org/virtuslab/yaml/internal/load/parse/ParserImpl.scala
View file
Open in desktop
Expand Up @@ -456,7 +456,7 @@ final class ParserImpl private (in: Tokenizer) extends Parser {
else CustomTag(tagValue)
parseNodeAttributes(in.peekToken(), metadata.withTag(tag))
case None =>
Left(ParseError(s"There is no registered tag directive for handle $handleKey"))
Left(ParseError.NoRegisteredTagDirective(handleKey, token))
}
}
case _ => Right(metadata, token)
Expand Down
2 changes: 1 addition & 1 deletion core/shared/src/test/scala-3/org/virtuslab/yaml/ConstructSuite.scala
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@ class ConstructSuite extends munit.FunSuite:
)

val expectedConstructError =
Left(ConstructError(s"Parameter of a class must be a scalar value"))
Left(ConstructError.from(s"Parameter of a class must be a scalar value"))
assertEquals(node.as[DummyClass], expectedConstructError)
}

Expand Down
2 changes: 2 additions & 0 deletions core/shared/src/test/scala/org/virtuslab/yaml/BaseYamlSuite.scala
View file
Open in desktop
Expand Up @@ -38,6 +38,8 @@ trait BaseYamlSuite extends munit.FunSuite {
loop(Nil)
}

def asNode: Either[YamlError, Node] = new org.virtuslab.yaml.StringOps(yaml).asNode

def debugTokens: Unit = pprint.pprintln(tokens)
}

Expand Down
34 changes: 34 additions & 0 deletions core/shared/src/test/scala/org/virtuslab/yaml/parser/ParserSuite.scala
View file
Open in desktop
Expand Up @@ -3,6 +3,8 @@ package parser

import org.virtuslab.yaml.internal.load.parse.EventKind._
import org.virtuslab.yaml.internal.load.reader.token.ScalarStyle
import org.virtuslab.yaml.internal.load.reader.token.Token
import org.virtuslab.yaml.internal.load.reader.token.TokenKind.MappingKey

class ParserSuite extends BaseYamlSuite {

Expand Down Expand Up @@ -89,4 +91,36 @@ class ParserSuite extends BaseYamlSuite {

assertEquals(yaml.events, Right(expectedEvents))
}

test("Parsing error") {
val errorMessage = """Expected
|BlockEnd but instead got MappingKey
| -- zipcode: 12-345
| ^""".stripMargin

val yaml =
"""name: John Wick
|age: 40
|address:
| - city: Anywhere
| -- zipcode: 12-345
|""".stripMargin

val yamlLines = yaml.split("\n", -1).toVector

assertEquals(
yaml.asNode,
Left(
ParseError.ExpectedTokenKind(
"BlockEnd",
Token(
MappingKey,
Range(Position(65, 4, 13), yamlLines, None)
)
)
)
)

assertEquals(yaml.asNode.left.map(_.msg), Left(errorMessage))
}
}