From 82319d4a9c0ee79fb6e8646e685cb73927c1e192 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Sun, 5 Jul 2026 10:38:47 +0800 Subject: [PATCH 1/3] Surface lexer errors instead of misreporting them as EOF Dozens of parser call sites discard consumeToken's error (`_ = p.lexer.consumeToken()`). When the lexer failed mid-statement (invalid number, unterminated string, unclosed quoted identifier), the current token stayed nil and the parser read that as end of input, producing misleading errors: SELECT 0x => line 1:8 unexpected token kind: or, worse, silent success when the statement happened to look complete at the point of failure: `SELECT 1 'oops` parsed cleanly as SELECT 1. Rather than threading error returns through every call site, the lexer now records its first failure (error + offset) in lexerState: - consumeToken stashes the error; the existing save/restore backtracking clears failures that were only hit during rolled-back lookahead, since the fields live in lexerState. - wrapError prefers the stashed lexer error over whatever downstream symptom the parser tripped on, reporting the real cause at the offset it happened: SELECT 0x => line 1:8 invalid number - Statements that previously succeeded despite a trailing lexer error now fail via the re-lex at the top of the ParseStmts loop. Co-Authored-By: Claude Fable 5 --- parser/error_test.go | 51 +++++++++++++++++++++++++++++++++++++++++ parser/lexer.go | 23 +++++++++++++++++++ parser/parser_common.go | 11 ++++++++- 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/parser/error_test.go b/parser/error_test.go index 9d185cc..0f9229e 100644 --- a/parser/error_test.go +++ b/parser/error_test.go @@ -47,3 +47,54 @@ 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"}, + // 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..e5a304b 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 { @@ -331,7 +341,20 @@ 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 + l.lastErrorPos = min(l.offset, len(l.input)) + } + 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(), From e7398e5eb9a4c9885d14d513a00f1391408fb20a Mon Sep 17 00:00:00 2001 From: git-hulk Date: Sun, 5 Jul 2026 11:08:40 +0800 Subject: [PATCH 2/3] Avoid the min builtin for the pinned golangci-lint typechecker CI's golangci-lint predates Go 1.21 builtins and fails typecheck on min() even though go.mod declares go 1.21. Clamp with a plain comparison instead. Co-Authored-By: Claude Fable 5 --- parser/lexer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/parser/lexer.go b/parser/lexer.go index e5a304b..11cfd6f 100644 --- a/parser/lexer.go +++ b/parser/lexer.go @@ -347,7 +347,11 @@ func (l *Lexer) consumeToken() error { if err := l.nextToken(); err != nil { if l.lastError == nil { l.lastError = err - l.lastErrorPos = min(l.offset, len(l.input)) + pos := l.offset + if pos > len(l.input) { + pos = len(l.input) + } + l.lastErrorPos = pos } return err } From af0ae40f1905b289f82137c160193cc4529652fe Mon Sep 17 00:00:00 2001 From: git-hulk Date: Mon, 6 Jul 2026 15:42:09 +0800 Subject: [PATCH 3/3] Fail ParseStmts when a recorded lexer error survives to the success path Review follow-up: lastError was only consulted through wrapError, which only runs when the parser itself fails. A failed lex can leave the offset advanced - consumeIdent skips the opening backtick before discovering the quote is unclosed - so with `SELECT 1 `bad` the retained input re-lexes cleanly to EOF, parsing reaches the success path, and the recorded error was silently dropped. - ParseStmts now fails if lexerState still carries an error when parsing reaches end of input. - peekToken restores the saved state on the error path too, so a failed lookahead neither leaves the offset advanced nor records an error for input the parser has not actually reached. Co-Authored-By: Claude Fable 5 --- parser/error_test.go | 4 ++++ parser/lexer.go | 11 +++++++---- parser/parser_table.go | 8 ++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/parser/error_test.go b/parser/error_test.go index 0f9229e..c888677 100644 --- a/parser/error_test.go +++ b/parser/error_test.go @@ -64,6 +64,10 @@ func TestParseError_LexerErrorSurfaced(t *testing.T) { // 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"}, } diff --git a/parser/lexer.go b/parser/lexer.go index 11cfd6f..a6205de 100644 --- a/parser/lexer.go +++ b/parser/lexer.go @@ -324,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 } 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 }