Skip to content

Commit

Permalink
Add more context to schema parse errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@josephschorr
josephschorr committed Feb 7, 2022
1 parent ba86e59 commit 0382172
Show file tree
Hide file tree
Showing 13 changed files with 115 additions and 81 deletions.
5 changes: 3 additions & 2 deletions internal/services/v0/developer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ func TestEditCheck(t *testing.T) {
[]*v0.RelationTuple{},
[]*v0.RelationTuple{},
&v0.DeveloperError{
Message: "parse error in `schema`, line 3, column 4: Expected identifier, found token TokenTypeRightBrace",
Message: "Expected identifier, found token TokenTypeRightBrace",
Kind: v0.DeveloperError_SCHEMA_ISSUE,
Source: v0.DeveloperError_SCHEMA,
Line: 3,
Column: 4,
Context: "}",
},
nil,
},
Expand All @@ -112,7 +113,7 @@ func TestEditCheck(t *testing.T) {
[]*v0.RelationTuple{},
[]*v0.RelationTuple{},
&v0.DeveloperError{
Message: "parse error in `schema`, line 1, column 1: error in object definition fo: invalid NamespaceDefinition.Name: value does not match regex pattern \"^([a-z][a-z0-9_]{1,62}[a-z0-9]/)?[a-z][a-z0-9_]{1,62}[a-z0-9]$\"",
Message: "error in object definition fo: invalid NamespaceDefinition.Name: value does not match regex pattern \"^([a-z][a-z0-9_]{1,62}[a-z0-9]/)?[a-z][a-z0-9_]{1,62}[a-z0-9]$\"",
Kind: v0.DeveloperError_SCHEMA_ISSUE,
Source: v0.DeveloperError_SCHEMA,
Line: 1,
Expand Down
3 changes: 2 additions & 1 deletion pkg/development/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ func CompileSchema(schema string) ([]*v0.NamespaceDefinition, *v0.DeveloperError
}

return []*v0.NamespaceDefinition{}, &v0.DeveloperError{
Message: contextError.Error(),
Message: contextError.BaseErrorMessage,
Kind: v0.DeveloperError_SCHEMA_ISSUE,
Source: v0.DeveloperError_SCHEMA,
Line: uint32(line) + 1, // 0-indexed in parser.
Column: uint32(col) + 1, // 0-indexed in parser.
Context: contextError.ErrorSourceCode,
}, nil
}

Expand Down
30 changes: 22 additions & 8 deletions pkg/schemadsl/compiler/compiler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ type InputSchema struct {
// ErrorWithContext defines an error which contains contextual information.
type ErrorWithContext struct {
error
SourceRange input.SourceRange
Source input.Source
BaseErrorMessage string
SourceRange input.SourceRange
Source input.Source
ErrorSourceCode string
}

type errorWithNode struct {
Expand Down Expand Up @@ -52,7 +54,7 @@ func Compile(schemas []InputSchema, objectTypePrefix *string) ([]*v0.NamespaceDe
if err != nil {
var errorWithNode errorWithNode
if errors.As(err, &errorWithNode) {
err = toContextError(errorWithNode.error.Error(), errorWithNode.node, mapper)
err = toContextError(errorWithNode.error.Error(), "", errorWithNode.node, mapper)
}

return []*v0.NamespaceDefinition{}, err
Expand All @@ -74,10 +76,20 @@ func errorNodeToError(node *dslNode, mapper input.PositionMapper) error {
return fmt.Errorf("could not get error message for error node: %w", err)
}

return toContextError(errMessage, node, mapper)
errorSourceCode := ""
if node.Has(dslshape.NodePredicateErrorSource) {
es, err := node.GetString(dslshape.NodePredicateErrorSource)
if err != nil {
return fmt.Errorf("could not get error source for error node: %w", err)
}

errorSourceCode = es
}

return toContextError(errMessage, errorSourceCode, node, mapper)
}

func toContextError(errMessage string, node *dslNode, mapper input.PositionMapper) error {
func toContextError(errMessage string, errorSourceCode string, node *dslNode, mapper input.PositionMapper) error {
sourceRange, err := node.Range(mapper)
if err != nil {
return fmt.Errorf("could not get range for error node: %w", err)
Expand All @@ -94,9 +106,11 @@ func toContextError(errMessage string, node *dslNode, mapper input.PositionMappe
}

return ErrorWithContext{
error: fmt.Errorf("parse error in %s: %s", formattedRange, errMessage),
SourceRange: sourceRange,
Source: input.Source(source),
error: fmt.Errorf("parse error in %s: %s", formattedRange, errMessage),
BaseErrorMessage: errMessage,
SourceRange: sourceRange,
Source: input.Source(source),
ErrorSourceCode: errorSourceCode,
}
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/schemadsl/dslshape/dslshape.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const (
// The message for the parsing error.
NodePredicateErrorMessage = "error-message"

// The (optional) source to highlight for the parsing error.
NodePredicateErrorSource = "error-source"

//
// NodeTypeComment
//
Expand Down
9 changes: 5 additions & 4 deletions pkg/schemadsl/lexer/lex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type Lexeme struct {
Kind TokenType // The type of this lexeme.
Position input.BytePosition // The starting position of this token in the input string.
Value string // The textual value of this token.
Error string // The error associated with the lexeme, if any.
}

// stateFn represents the state of the scanner as a function that returns the next state.
Expand Down Expand Up @@ -136,7 +137,7 @@ func (l *Lexer) value() string {

// emit passes an token back to the client.
func (l *Lexer) emit(t TokenType) {
currentToken := Lexeme{t, l.start, l.value()}
currentToken := Lexeme{t, l.start, l.value(), ""}

if t == TokenTypeWhitespace {
l.lastNonWhitespaceToken = currentToken
Expand All @@ -158,8 +159,8 @@ func (l *Lexer) emit(t TokenType) {

// errorf returns an error token and terminates the scan by passing
// back a nil pointer that will be the next state, terminating l.nexttoken.
func (l *Lexer) errorf(format string, args ...interface{}) stateFn {
l.tokens <- Lexeme{TokenTypeError, l.start, fmt.Sprintf(format, args...)}
func (l *Lexer) errorf(currentRune rune, format string, args ...interface{}) stateFn {
l.tokens <- Lexeme{TokenTypeError, l.start, string(currentRune), fmt.Sprintf(format, args...)}
return nil
}

Expand Down Expand Up @@ -220,7 +221,7 @@ func buildLexUntil(findType TokenType, checker checkFn) stateFn {
r := l.next()
isValid, err := checker(r)
if err != nil {
return l.errorf("%v", err)
return l.errorf(r, "%v", err)
}
if !isValid {
l.backup()
Expand Down
6 changes: 3 additions & 3 deletions pkg/schemadsl/lexer/lex_def.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ Loop:
if l.acceptString("..") {
l.emit(TokenTypeEllipsis)
} else {
return l.errorf("unrecognized character at this location: %#U", r)
return l.errorf(r, "unrecognized character at this location: %#U", r)
}

case r == '-':
Expand Down Expand Up @@ -159,7 +159,7 @@ Loop:

l.emit(TokenTypeDiv)
default:
return l.errorf("unrecognized character at this location: %#U", r)
return l.errorf(r, "unrecognized character at this location: %#U", r)
}
}

Expand Down Expand Up @@ -192,7 +192,7 @@ func lexMultilineComment(l *Lexer) stateFn {
// Otherwise, consume until we hit EOFRUNE.
r := l.next()
if r == EOFRUNE {
return l.errorf("Unterminated multiline comment")
return l.errorf(r, "Unterminated multiline comment")
}
}
}
Expand Down
126 changes: 63 additions & 63 deletions pkg/schemadsl/lexer/lex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,125 +13,125 @@ type lexerTest struct {
}

var (
tEOF = Lexeme{TokenTypeEOF, 0, ""}
tWhitespace = Lexeme{TokenTypeWhitespace, 0, " "}
tEOF = Lexeme{TokenTypeEOF, 0, "", ""}
tWhitespace = Lexeme{TokenTypeWhitespace, 0, " ", ""}
)

var lexerTests = []lexerTest{
// Simple tests.
{"empty", "", []Lexeme{tEOF}},

{"single whitespace", " ", []Lexeme{tWhitespace, tEOF}},
{"single tab", "\t", []Lexeme{{TokenTypeWhitespace, 0, "\t"}, tEOF}},
{"single tab", "\t", []Lexeme{{TokenTypeWhitespace, 0, "\t", ""}, tEOF}},
{"multiple whitespace", " ", []Lexeme{tWhitespace, tWhitespace, tWhitespace, tEOF}},

{"newline r", "\r", []Lexeme{{TokenTypeNewline, 0, "\r"}, tEOF}},
{"newline n", "\n", []Lexeme{{TokenTypeNewline, 0, "\n"}, tEOF}},
{"newline rn", "\r\n", []Lexeme{{TokenTypeNewline, 0, "\r"}, {TokenTypeNewline, 0, "\n"}, tEOF}},
{"newline r", "\r", []Lexeme{{TokenTypeNewline, 0, "\r", ""}, tEOF}},
{"newline n", "\n", []Lexeme{{TokenTypeNewline, 0, "\n", ""}, tEOF}},
{"newline rn", "\r\n", []Lexeme{{TokenTypeNewline, 0, "\r", ""}, {TokenTypeNewline, 0, "\n", ""}, tEOF}},

{"comment", "// a comment", []Lexeme{{TokenTypeSinglelineComment, 0, "// a comment"}, tEOF}},
{"multiline comment", "/* a comment\n foo*/", []Lexeme{{TokenTypeMultilineComment, 0, "/* a comment\n foo*/"}, tEOF}},
{"comment", "// a comment", []Lexeme{{TokenTypeSinglelineComment, 0, "// a comment", ""}, tEOF}},
{"multiline comment", "/* a comment\n foo*/", []Lexeme{{TokenTypeMultilineComment, 0, "/* a comment\n foo*/", ""}, tEOF}},

{"left brace", "{", []Lexeme{{TokenTypeLeftBrace, 0, "{"}, tEOF}},
{"right brace", "}", []Lexeme{{TokenTypeRightBrace, 0, "}"}, tEOF}},
{"left brace", "{", []Lexeme{{TokenTypeLeftBrace, 0, "{", ""}, tEOF}},
{"right brace", "}", []Lexeme{{TokenTypeRightBrace, 0, "}", ""}, tEOF}},

{"left paren", "(", []Lexeme{{TokenTypeLeftParen, 0, "("}, tEOF}},
{"right paren", ")", []Lexeme{{TokenTypeRightParen, 0, ")"}, tEOF}},
{"left paren", "(", []Lexeme{{TokenTypeLeftParen, 0, "(", ""}, tEOF}},
{"right paren", ")", []Lexeme{{TokenTypeRightParen, 0, ")", ""}, tEOF}},

{"semicolon", ";", []Lexeme{{TokenTypeSemicolon, 0, ";"}, tEOF}},
{"star", "*", []Lexeme{{TokenTypeStar, 0, "*"}, tEOF}},
{"semicolon", ";", []Lexeme{{TokenTypeSemicolon, 0, ";", ""}, tEOF}},
{"star", "*", []Lexeme{{TokenTypeStar, 0, "*", ""}, tEOF}},

{"right arrow", "->", []Lexeme{{TokenTypeRightArrow, 0, "->"}, tEOF}},
{"right arrow", "->", []Lexeme{{TokenTypeRightArrow, 0, "->", ""}, tEOF}},

{"hash", "#", []Lexeme{{TokenTypeHash, 0, "#"}, tEOF}},
{"ellipsis", "...", []Lexeme{{TokenTypeEllipsis, 0, "..."}, tEOF}},
{"hash", "#", []Lexeme{{TokenTypeHash, 0, "#", ""}, tEOF}},
{"ellipsis", "...", []Lexeme{{TokenTypeEllipsis, 0, "...", ""}, tEOF}},

{"relation reference", "foo#...", []Lexeme{
{TokenTypeIdentifier, 0, "foo"},
{TokenTypeHash, 0, "#"},
{TokenTypeEllipsis, 0, "..."},
{TokenTypeIdentifier, 0, "foo", ""},
{TokenTypeHash, 0, "#", ""},
{TokenTypeEllipsis, 0, "...", ""},
tEOF,
}},

{"plus", "+", []Lexeme{{TokenTypePlus, 0, "+"}, tEOF}},
{"minus", "-", []Lexeme{{TokenTypeMinus, 0, "-"}, tEOF}},
{"plus", "+", []Lexeme{{TokenTypePlus, 0, "+", ""}, tEOF}},
{"minus", "-", []Lexeme{{TokenTypeMinus, 0, "-", ""}, tEOF}},

{"keyword", "definition", []Lexeme{{TokenTypeKeyword, 0, "definition"}, tEOF}},
{"identifier", "define", []Lexeme{{TokenTypeIdentifier, 0, "define"}, tEOF}},
{"keyword", "definition", []Lexeme{{TokenTypeKeyword, 0, "definition", ""}, tEOF}},
{"identifier", "define", []Lexeme{{TokenTypeIdentifier, 0, "define", ""}, tEOF}},
{"typepath", "foo/bar", []Lexeme{
{TokenTypeIdentifier, 0, "foo"},
{TokenTypeDiv, 0, "/"},
{TokenTypeIdentifier, 0, "bar"},
{TokenTypeIdentifier, 0, "foo", ""},
{TokenTypeDiv, 0, "/", ""},
{TokenTypeIdentifier, 0, "bar", ""},
tEOF,
}},

{"type star", "foo:*", []Lexeme{
{TokenTypeIdentifier, 0, "foo"},
{TokenTypeColon, 0, ":"},
{TokenTypeStar, 0, "*"},
{TokenTypeIdentifier, 0, "foo", ""},
{TokenTypeColon, 0, ":", ""},
{TokenTypeStar, 0, "*", ""},
tEOF,
}},

{"expression", "foo->bar", []Lexeme{
{TokenTypeIdentifier, 0, "foo"},
{TokenTypeRightArrow, 0, "->"},
{TokenTypeIdentifier, 0, "bar"},
{TokenTypeIdentifier, 0, "foo", ""},
{TokenTypeRightArrow, 0, "->", ""},
{TokenTypeIdentifier, 0, "bar", ""},
tEOF,
}},

{"relation", "/* foo */relation parent: namespace | organization\n", []Lexeme{
{TokenTypeMultilineComment, 0, "/* foo */"},
{TokenTypeKeyword, 0, "relation"},
{TokenTypeMultilineComment, 0, "/* foo */", ""},
{TokenTypeKeyword, 0, "relation", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "parent"},
{TokenTypeColon, 0, ":"},
{TokenTypeIdentifier, 0, "parent", ""},
{TokenTypeColon, 0, ":", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "namespace"},
{TokenTypeIdentifier, 0, "namespace", ""},
tWhitespace,
{TokenTypePipe, 0, "|"},
{TokenTypePipe, 0, "|", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "organization"},
{TokenTypeSyntheticSemicolon, 0, "\n"},
{TokenTypeIdentifier, 0, "organization", ""},
{TokenTypeSyntheticSemicolon, 0, "\n", ""},
tEOF,
}},

{"relation", "/* foo */relation parent: namespace | organization;", []Lexeme{
{TokenTypeMultilineComment, 0, "/* foo */"},
{TokenTypeKeyword, 0, "relation"},
{TokenTypeMultilineComment, 0, "/* foo */", ""},
{TokenTypeKeyword, 0, "relation", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "parent"},
{TokenTypeColon, 0, ":"},
{TokenTypeIdentifier, 0, "parent", ""},
{TokenTypeColon, 0, ":", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "namespace"},
{TokenTypeIdentifier, 0, "namespace", ""},
tWhitespace,
{TokenTypePipe, 0, "|"},
{TokenTypePipe, 0, "|", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "organization"},
{TokenTypeSemicolon, 0, ";"},
{TokenTypeIdentifier, 0, "organization", ""},
{TokenTypeSemicolon, 0, ";", ""},
tEOF,
}},

{"relation", "/* foo */relation parent: namespace:*\n", []Lexeme{
{TokenTypeMultilineComment, 0, "/* foo */"},
{TokenTypeKeyword, 0, "relation"},
{TokenTypeMultilineComment, 0, "/* foo */", ""},
{TokenTypeKeyword, 0, "relation", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "parent"},
{TokenTypeColon, 0, ":"},
{TokenTypeIdentifier, 0, "parent", ""},
{TokenTypeColon, 0, ":", ""},
tWhitespace,
{TokenTypeIdentifier, 0, "namespace"},
{TokenTypeColon, 0, ":"},
{TokenTypeStar, 0, "*"},
{TokenTypeSyntheticSemicolon, 0, "\n"},
{TokenTypeIdentifier, 0, "namespace", ""},
{TokenTypeColon, 0, ":", ""},
{TokenTypeStar, 0, "*", ""},
{TokenTypeSyntheticSemicolon, 0, "\n", ""},
tEOF,
}},

{"expression with parens", "(foo->bar)\n", []Lexeme{
{TokenTypeLeftParen, 0, "("},
{TokenTypeIdentifier, 0, "foo"},
{TokenTypeRightArrow, 0, "->"},
{TokenTypeIdentifier, 0, "bar"},
{TokenTypeRightParen, 0, ")"},
{TokenTypeSyntheticSemicolon, 0, "\n"},
{TokenTypeLeftParen, 0, "(", ""},
{TokenTypeIdentifier, 0, "foo", ""},
{TokenTypeRightArrow, 0, "->", ""},
{TokenTypeIdentifier, 0, "bar", ""},
{TokenTypeRightParen, 0, ")", ""},
{TokenTypeSyntheticSemicolon, 0, "\n", ""},
tEOF,
}},
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/schemadsl/parser/parser_impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ func (p *sourceParser) isKeyword(keyword string) bool {
// node.
func (p *sourceParser) emitErrorf(format string, args ...interface{}) {
errorNode := p.createErrorNodef(format, args...)
if len(p.currentToken.Value) > 0 {
errorNode.Decorate(dslshape.NodePredicateErrorSource, p.currentToken.Value)
}
p.currentNode().Connect(dslshape.NodePredicateChild, errorNode)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/schemadsl/parser/tests/brokenwildcard.zed.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ NodeTypeFile
NodeTypeError
end-rune = 74
error-message = Expected one of: [TokenTypeStar], found: TokenTypePipe
error-source = |
input-source = broken wildcard test
start-rune = 76
NodeTypeSpecificTypeReference
Expand Down
2 changes: 2 additions & 0 deletions pkg/schemadsl/parser/tests/permission_invalid_expression.zed.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ NodeTypeFile
NodeTypeError
end-rune = 36
error-message = Expected end of statement or definition, found: TokenTypeMinus
error-source = -
input-source = permission invalid expression test
start-rune = 38
NodeTypeError
end-rune = 36
error-message = Unexpected token at root level: TokenTypeMinus
error-source = -
input-source = permission invalid expression test
start-rune = 38
Loading

0 comments on commit 0382172

Please sign in to comment.