diff --git a/parser/error_test.go b/parser/error_test.go index 9d185cc..c888677 100644 --- a/parser/error_test.go +++ b/parser/error_test.go @@ -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: " — 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) +} diff --git a/parser/lexer.go b/parser/lexer.go index 3cdea6a..a6205de 100644 --- a/parser/lexer.go +++ b/parser/lexer.go @@ -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 { @@ -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 } @@ -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 diff --git a/parser/parser_common.go b/parser/parser_common.go index 18b927b..dda9cfa 100644 --- a/parser/parser_common.go +++ b/parser/parser_common.go @@ -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 { + // 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(), diff --git a/parser/parser_table.go b/parser/parser_table.go index 0380b75..cc3f709 100644 --- a/parser/parser_table.go +++ b/parser/parser_table.go @@ -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 }