From 12180ee3f216170384d6fece9728d5e5a9fb1281 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 18:44:04 +0000 Subject: [PATCH] parser: record scanned comments and stamp table/column positions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the comments the lexer skips — '-- ' and '#' line comments and '/* */' block comments — as byte-offset spans on the Scanner, exposed through Scanner.Comments and Parser.Comments. Executable comments and optimizer hints are lexed as SQL, so they are not recorded, and recording is gated on strictly increasing positions because the scanner's lookaheads re-read source text. Stamp OriginTextPosition on FROM-clause table factors, INSERT/REPLACE table references and INSERT column lists, which goyacc left unstamped (it stamped only symbols). Consumers placing comments or errors need these positions; only Accept-reachable nodes are stamped so the restore round-trip tests can keep resetting them. This gives sqlc's MySQL engine what its ParseFile needs to power sqlc fmt: statements and comments from one lexer pass, with enough positions to anchor comments and the author's line breaks. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01C48orUYrL7xE989UGLTVqi --- parser/api.go | 7 ++ parser/comments_test.go | 142 ++++++++++++++++++++++++++++++++++++++++ parser/lexer.go | 33 ++++++++++ parser/parse_dml.go | 32 +++++++-- parser/parse_select.go | 12 ++++ 5 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 parser/comments_test.go diff --git a/parser/api.go b/parser/api.go index 824d8f2..a18993a 100644 --- a/parser/api.go +++ b/parser/api.go @@ -182,6 +182,13 @@ func (parser *Parser) ParseOneStmt(sql, charset, collation string) (ast.StmtNode return stmts[0], nil } +// Comments returns the comments the lexer scanned during the most recent +// Parse, ParseSQL or ParseOneStmt call, ordered by position. Offsets index +// the sql string given to that call. The slice is reused by the next parse. +func (parser *Parser) Comments() []Comment { + return parser.rdScan.Comments() +} + // SetSQLMode sets the SQL mode for parser. func (parser *Parser) SetSQLMode(mode mysql.SQLMode) { parser.lexer.SetSQLMode(mode) diff --git a/parser/comments_test.go b/parser/comments_test.go new file mode 100644 index 0000000..7d1e1e6 --- /dev/null +++ b/parser/comments_test.go @@ -0,0 +1,142 @@ +// Copyright 2026 The sqlc Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package parser + +import ( + "reflect" + "testing" +) + +// TestParserComments pins the comment-recording contract: every skipped +// comment is recorded exactly once with offsets that slice its text back +// out of the source, and constructs the lexer turns into SQL (executable +// comments, optimizer hints) are not comments. +func TestParserComments(t *testing.T) { + tests := []struct { + name string + sql string + want []string // want[i] == sql[Begin:End] of comment i + }{ + { + name: "no comments", + sql: "SELECT 1", + want: nil, + }, + { + name: "dash line comment above statement", + sql: "-- name: GetOne :one\nSELECT 1", + want: []string{"-- name: GetOne :one"}, + }, + { + name: "hash line comment", + sql: "# leading\nSELECT 1", + want: []string{"# leading"}, + }, + { + name: "hash comment at EOF without newline", + sql: "SELECT 1; # trailing", + want: []string{"# trailing"}, + }, + { + name: "block comment inside statement", + sql: "SELECT /* inline note */ 1", + want: []string{"/* inline note */"}, + }, + { + name: "multi-line block comment", + sql: "SELECT 1 /* one\n two */ + 2", + want: []string{"/* one\n two */"}, + }, + { + name: "star-heavy block comment", + sql: "SELECT /** starry **/ 1", + want: []string{"/** starry **/"}, + }, + { + name: "comments between statements", + sql: "SELECT 1; -- after one\n-- before two\nSELECT 2;", + want: []string{"-- after one", "-- before two"}, + }, + { + name: "every syntax in one file", + sql: "-- dash\n# hash\nSELECT /* block */ 1;", + want: []string{"-- dash", "# hash", "/* block */"}, + }, + { + // `AS` makes Lex look ahead for `OF` with a saved-and-restored + // reader, scanning the comment twice; it must be recorded once. + name: "comment re-scanned by lookahead", + sql: "SELECT 1 AS /* dup */ x", + want: []string{"/* dup */"}, + }, + { + name: "double dash without space is an operator", + sql: "SELECT 1 --2", + want: nil, + }, + { + name: "executable comment is SQL", + sql: "SELECT /*!80000 1 */", + want: nil, + }, + { + name: "optimizer hint is a token", + sql: "SELECT /*+ MAX_EXECUTION_TIME(1000) */ 1", + want: nil, + }, + { + name: "misplaced optimizer hint is a comment", + sql: "SELECT 1 + /*+ MAX_EXECUTION_TIME(1000) */ 2", + want: []string{"/*+ MAX_EXECUTION_TIME(1000) */"}, + }, + } + p := New() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, _, err := p.Parse(tt.sql, "", ""); err != nil { + t.Fatalf("Parse(%q): %v", tt.sql, err) + } + var got []string + last := -1 + for _, c := range p.Comments() { + if c.Begin <= last { + t.Fatalf("comments out of order: Begin %d after %d", c.Begin, last) + } + last = c.Begin + got = append(got, tt.sql[c.Begin:c.End]) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Parse(%q) comments = %q, want %q", tt.sql, got, tt.want) + } + }) + } +} + +// TestParserCommentsReset checks that a parse clears the comments of the +// one before it. +func TestParserCommentsReset(t *testing.T) { + p := New() + if _, _, err := p.Parse("SELECT 1 -- one", "", ""); err != nil { + t.Fatal(err) + } + if got := len(p.Comments()); got != 1 { + t.Fatalf("first parse recorded %d comments, want 1", got) + } + if _, _, err := p.Parse("SELECT 2", "", ""); err != nil { + t.Fatal(err) + } + if got := len(p.Comments()); got != 0 { + t.Fatalf("second parse kept %d stale comments, want 0", got) + } +} diff --git a/parser/lexer.go b/parser/lexer.go index d13cc8e..ab0533b 100644 --- a/parser/lexer.go +++ b/parser/lexer.go @@ -45,6 +45,7 @@ type Scanner struct { errs []error warns []error + comments []Comment stmtStartPos int // inBangComment is true if we are inside a `/*! ... */` block. @@ -91,6 +92,33 @@ func (s *Scanner) Errors() (warns []error, errs []error) { return s.warns, s.errs } +// Comment is one comment the scanner skipped while lexing: a `-- ` or `#` +// line comment (End excludes the terminating newline) or a `/* ... */` +// block comment (End includes the closing `*/`). Begin and End are byte +// offsets into the source. Executable comments (`/*! ... */` and +// recognized `/*T![...] ... */`) and optimizer hints (`/*+ ... */`) are +// lexed as SQL rather than skipped, so they are not recorded. +type Comment struct { + Begin int + End int +} + +// Comments returns the comments scanned so far, ordered by position. +func (s *Scanner) Comments() []Comment { + return s.comments +} + +// recordComment appends a scanned comment. The scanner re-reads source +// text when it looks ahead (getNextToken and friends save and restore the +// reader), so the same comment can be scanned more than once; recording is +// gated on strictly increasing positions to keep one entry per comment. +func (s *Scanner) recordComment(begin, end int) { + if n := len(s.comments); n > 0 && s.comments[n-1].Begin >= begin { + return + } + s.comments = append(s.comments, Comment{Begin: begin, End: end}) +} + // reset resets the sql string to be scanned. func (s *Scanner) reset(sql string) { s.client = charset.FindEncoding(mysql.DefaultCharset) @@ -99,6 +127,7 @@ func (s *Scanner) reset(sql string) { s.buf.Reset() s.errs = s.errs[:0] s.warns = s.warns[:0] + s.comments = s.comments[:0] s.stmtStartPos = 0 s.inBangComment = false s.lastKeyword = 0 @@ -497,9 +526,11 @@ func startWithBb(s *Scanner) (tok int, pos Pos, lit string) { } func startWithSharp(s *Scanner) (tok int, pos Pos, lit string) { + begin := s.r.pos().Offset s.r.incAsLongAs(func(ch byte) bool { return ch != '\n' }) + s.recordComment(begin, s.r.pos().Offset) return s.scan() } @@ -511,6 +542,7 @@ func startWithDash(s *Scanner) (tok int, pos Pos, lit string) { s.r.incAsLongAs(func(ch byte) bool { return ch != '\n' }) + s.recordComment(pos.Offset, s.r.pos().Offset) return s.scan() } } @@ -602,6 +634,7 @@ func startWithSlash(s *Scanner) (tok int, pos Pos, lit string) { s.lastHintPos = pos return hintComment, pos, s.r.data(&pos) } + s.recordComment(pos.Offset, s.r.pos().Offset) return s.scan() case '*': currentCharIsStar = true diff --git a/parser/parse_dml.go b/parser/parse_dml.go index 29c1c66..3d88f27 100644 --- a/parser/parse_dml.go +++ b/parser/parse_dml.go @@ -57,7 +57,14 @@ func (r *rdParser) parseInsertIntoStmt() ast.StmtNode { priority := r.parsePriorityOpt() ignoreErr := r.accept(ignore) r.accept(into) + // The table reference's start offset is recorded the way + // parseTableFactor stamps FROM-clause tables; InsertStmt.Accept + // visits the TableName, so the restore round-trip test resets it. + offset := r.cur().offset tn := r.parseTableName() + if !r.sc.skipPositionRecording { + tn.SetOriginTextPosition(offset) + } partitions := r.parsePartitionNameListOpt() x := r.parseInsertValues() x.Priority = priority @@ -87,7 +94,12 @@ func (r *rdParser) parseReplaceIntoStmt() ast.StmtNode { hints := r.parseTableOptimizerHintsOpt() priority := r.parsePriorityOpt() r.accept(into) + // Stamped like the INSERT INTO table reference above. + offset := r.cur().offset tn := r.parseTableName() + if !r.sc.skipPositionRecording { + tn.SetOriginTextPosition(offset) + } partitions := r.parsePartitionNameListOpt() x := r.parseInsertValues() if hints != nil { @@ -107,11 +119,23 @@ func (r *rdParser) parseInsertValues() *ast.InsertStmt { case r.tok() == int('(') && !r.subSelectFollows(): // '(' ColumnNameListOpt ')' followed by values or a query. r.advance() - var cols []*ast.ColumnName + cols := []*ast.ColumnName{} if r.tok() != int(')') { - cols = r.parseColumnNameList() - } else { - cols = []*ast.ColumnName{} + // Each column records its start offset (InsertStmt.Accept + // visits Columns, so the restore round-trip test resets + // them); parseColumnNameList stays unstamped because other + // call sites hang its columns off nodes Accept skips. + for { + offset := r.cur().offset + col := r.parseColumnName() + if !r.sc.skipPositionRecording { + col.SetOriginTextPosition(offset) + } + cols = append(cols, col) + if !r.accept(int(',')) { + break + } + } } r.expect(int(')')) if r.tok() == value || (r.tok() == values && r.la(1) != row) { diff --git a/parser/parse_select.go b/parser/parse_select.go index aaa00c3..ef28d90 100644 --- a/parser/parse_select.go +++ b/parser/parse_select.go @@ -1110,7 +1110,19 @@ func (r *rdParser) parseTableFactor() ast.ResultSetNode { default: // TableFactor: TableName PartitionNameListOpt TableAsNameOpt // AsOfClauseOpt IndexHintListOpt TableSampleOpt + // + // Unlike goyacc, which stamped only symbols, the + // production's start offset is recorded on the table reference: + // consumers placing comments or errors need its position, and 0 + // (the unstamped value) is indistinguishable from the start of + // the input. Only this production stamps a TableName — every + // TableName here is reachable by Accept, which the restore + // round-trip test relies on to reset positions. + offset := r.cur().offset tn := r.parseTableName() + if !r.sc.skipPositionRecording { + tn.SetOriginTextPosition(offset) + } tn.PartitionNames = r.parsePartitionNameListOpt() asName := r.parseTableAsNameOpt() if r.tok() == asof {