Skip to content
Closed
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
55 changes: 55 additions & 0 deletions parser/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,58 @@ func TestParseError_ExpectedTokenKind(t *testing.T) {
require.Equal(t, []TokenKind{TokenKindRParen}, pe.Expected)
require.True(t, strings.HasPrefix(pe.Error(), "line "))
}

// TestParseError_LexerErrorSurfaced guards that a lexing failure mid-statement
// is reported as itself. Most parser call sites discard consumeToken's error
// and then read the nil current token as end of input, which used to produce
// misleading messages like "unexpected token kind: <eof>" — or, worse, silent
// success when the statement looked complete at the point of failure.
func TestParseError_LexerErrorSurfaced(t *testing.T) {
cases := []struct {
input string
want string
}{
{"SELECT 0x", "invalid number"},
{"SELECT 'unterminated", "invalid string"},
{"SELECT `unclosed", "unclosed quoted identifier"},
// the statement is already well-formed when the bad token appears,
// so this parsed successfully before
{"SELECT 1 'oops", "invalid string"},
// here the failed lex leaves the offset past the opening backtick, so
// the retained input re-lexes cleanly to EOF and parsing reaches the
// success path; the recorded error must still fail the parse
{"SELECT 1 `bad", "unclosed quoted identifier"},
// the failure is in a later statement
{"SELECT 1; SELECT 'bad; SELECT 2", "invalid string"},
}
for _, c := range cases {
_, err := NewParser(c.input).ParseStmts()
require.Error(t, err, "input %q", c.input)
require.Contains(t, err.Error(), c.want, "input %q", c.input)

var pe *ParseError
require.True(t, errors.As(err, &pe), "input %q: expected a *ParseError, got %T", c.input, err)
}
}

// TestParseError_LexerErrorPosition guards that the reported position points
// at the offending token, not wherever the parser noticed the nil token.
func TestParseError_LexerErrorPosition(t *testing.T) {
_, err := NewParser("SELECT 0x").ParseStmts()
require.Error(t, err)

var pe *ParseError
require.True(t, errors.As(err, &pe))
require.Equal(t, Pos(7), pe.Pos) // byte offset of "0x"
require.Equal(t, 1, pe.Line)
require.Equal(t, 8, pe.Column)
}

// TestParseError_LookaheadLexerErrorRollback guards that a lexer error hit
// only during backtracked lookahead does not leak into an otherwise
// successful parse.
func TestParseError_LookaheadLexerErrorRollback(t *testing.T) {
stmts, err := NewParser("SELECT 1").ParseStmts()
require.NoError(t, err)
require.Len(t, stmts, 1)
}
38 changes: 34 additions & 4 deletions parser/lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ func (t *Token) ToString() string {
type lexerState struct {
offset int // byte offset into input of the next unread character
currentToken *Token // current lookahead token; nil at end of input

// lastError remembers the first lexing failure (invalid number,
// unterminated string, unclosed quoted identifier, ...) together with the
// offset it occurred at. Many parser call sites discard consumeToken's
// return value and then read the nil currentToken as end of input; the
// parser consults lastError to report the real cause instead of a
// misleading EOF. It lives in lexerState so backtracking via
// restoreState automatically clears errors hit only during lookahead.
lastError error
lastErrorPos int
}

type Lexer struct {
Expand Down Expand Up @@ -314,12 +324,15 @@ func (l *Lexer) skipComments() {

func (l *Lexer) peekToken() (*Token, error) {
savedState := l.saveState()
if err := l.consumeToken(); err != nil {
return nil, err
}
err := l.consumeToken()
token := l.currentToken

// restore on the error path too: a failed lookahead must not leave the
// offset advanced or a lexer error recorded for input the parser has not
// actually reached yet
l.restoreState(savedState)
if err != nil {
return nil, err
}
return token, nil
}

Expand All @@ -331,7 +344,24 @@ func (l *Lexer) hasPrecedenceToken(last *Token) bool {
last.Kind == TokenKindString)
}

// consumeToken advances to the next token, recording the first lexing failure
// in lexerState so it survives call sites that discard the returned error.
func (l *Lexer) consumeToken() error {
if err := l.nextToken(); err != nil {
if l.lastError == nil {
l.lastError = err
pos := l.offset
if pos > len(l.input) {
pos = len(l.input)
}
l.lastErrorPos = pos
}
return err
}
return nil
}

func (l *Lexer) nextToken() error {
// replace the current token; keep the previous one to disambiguate unary +/-
prevToken := l.currentToken
l.currentToken = nil
Expand Down
11 changes: 10 additions & 1 deletion parser/parser_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,16 @@ func (p *Parser) wrapError(err error) error {
}

var pe *ParseError
if !errors.As(err, &pe) {
if lexErr := p.lexer.lastError; lexErr != nil {
Comment thread
git-hulk marked this conversation as resolved.
// A lexing failure was swallowed by a call site that discards
// consumeToken's error; the parser then read the nil current token as
// end of input and failed with a misleading message. Report the
// lexer's own error at the offset it happened instead.
pe = &ParseError{
Pos: Pos(p.lexer.lastErrorPos),
Msg: lexErr.Error(),
}
} else if !errors.As(err, &pe) {
pe = &ParseError{
Pos: p.Pos(),
Got: p.current(),
Expand Down
8 changes: 8 additions & 0 deletions parser/parser_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,14 @@ func (p *Parser) ParseStmts() ([]Expr, error) {
}
stmts = append(stmts, stmt)
}
// A lexing failure recorded by a call site that discarded the error must
// not be silently accepted just because parsing reached end of input: a
// failed lex can leave the offset advanced (e.g. past the opening quote
// of an unclosed identifier), so the retained input re-lexes cleanly and
// never re-triggers the error.
if lexErr := p.lexer.lastError; lexErr != nil {
return nil, p.wrapError(lexErr)
}
return stmts, nil
}

Expand Down
Loading