Check span invariants over the corpus, and fix named constraints

Nothing verified byte spans: a wrong one survives the corpus and the
round trip alike, and sqlc slices the source with them. TestSpans walks
every parsed corpus case and asserts that a span is a valid range, that
a child lies within its parent, and that statements are disjoint and in
source order — the last is what makes the text between two statements,
where "-- name:" comments live, unambiguous.

It found that a named constraint's span started after its own CONSTRAINT
clause, so the name was outside the node holding it. That is 20,815
cases checked, and one class of bug.

difftest's split filter also needed two rounds: sqlite3_complete's
scanner is much cruder than the tokenizer, so a ";" ends a statement for
it inside brackets, inside a bind parameter's TCL-style suffix, and
inside a string it pairs differently. Those are all incomparable, not
wrong. 22M mutations at depth 2 now run clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzBeCg7rjweVW3uGPg5G7T

Author: claude

cmd/difftest/main.go

@@ -141,12 +141,7 @@ func main() {
 
 // check asks both parsers about one input and records any disagreement.
 func check(r *sqlitesrc.Runner, sql string, rep *report) {
-	// The oracle splits a script into statements the way the sqlite3 shell
-	// does, with sqlite3_complete, which counts no parentheses: a ";" inside
-	// brackets ends a statement for it. meyer parses the script as a whole.
-	// Where the two disagree about the boundaries there is nothing to
-	// compare, and the disagreement is the harness's, not the parser's.
-	if semicolonInsideParens(sql) {
+	if splitDiffers(sql) {
 		rep.skipped.Add(1)
 		return
 	}
@@ -223,9 +218,16 @@ func checkRoundTrip(sql string, rep *report) {
 	}
 }
 
-// semicolonInsideParens reports whether any ";" appears within brackets,
-// skipping the quoting and comment forms the tokenizer knows about.
-func semicolonInsideParens(sql string) bool {
+// splitDiffers reports whether the oracle would cut the script into
+// statements somewhere meyer would not, which makes the two incomparable.
+//
+// The oracle splits the way the sqlite3 shell does, with sqlite3_complete,
+// whose scanner is much cruder than the tokenizer: it understands strings,
+// the four quoting styles and comments, but nothing else. So a ";" ends a
+// statement for it even inside brackets, and even inside a token the real
+// tokenizer would have swallowed whole -- ":v(a;b)" is one bind parameter
+// to the tokenizer and two statements to sqlite3_complete.
+func splitDiffers(sql string) bool {
 	depth := 0
 	for _, t := range lexer.Lex(sql) {
 		switch t.Kind {
@@ -239,6 +241,29 @@ func semicolonInsideParens(sql string) bool {
 			if depth > 0 {
 				return true
 			}
+		default:
+			if strings.Contains(t.Text, ";") && !completeUnderstands(t) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// completeUnderstands reports whether sqlite3_complete skips over a token
+// the same way the tokenizer does. The closed quoted forms it does, blob
+// literals included, since it reads their body as a single-quoted string.
+// An unterminated one it does not: in ":v('(%d)',changes());" the tokenizer
+// swallows the first quote into the bind parameter and finds a string
+// starting at the second, while sqlite3_complete pairs the two.
+func completeUnderstands(t token.Token) bool {
+	switch t.Kind {
+	case token.STRING, token.BLOB:
+		return true
+	case token.ID:
+		switch t.Text[0] {
+		case '"', '`', '[':
+			return true
 		}
 	}
 	return false

parser/parse_ddl.go

@@ -105,16 +105,22 @@ func (p *parser) parseColumnAndConstraintList(n *ast.CreateTableStmt) {
 		}
 	}
 	var pending *ast.Ident
+	pendingStart := 0
 	for {
 		start := p.cur().Pos
 		if p.at(token.CONSTRAINT) {
 			// tcons ::= CONSTRAINT nm names whatever constraint follows,
 			// and is itself a complete tcons: a list may end with one.
-			p.advance()
+			pendingStart = p.advance().Pos
 			pending = p.expectName()
 		} else {
 			c := p.parseTableConstraint(start)
-			c.Name, pending = pending, nil
+			if pending != nil {
+				// The name is part of the constraint it introduces, so the
+				// constraint's span has to start at its CONSTRAINT keyword.
+				c.Name, c.Span.Start = pending, pendingStart
+				pending = nil
+			}
 			n.Constraints = append(n.Constraints, c)
 		}
 		// tconscomma ::= COMMA. / tconscomma ::= . The comma is optional
@@ -145,18 +151,24 @@ func (p *parser) parseColumnDef() *ast.ColumnDef {
 	n := &ast.ColumnDef{Name: p.expectName()}
 	n.Type = p.parseTypeToken()
 	var pending *ast.Ident
+	pendingStart := 0
 	for {
 		cStart := p.cur().Pos
 		if p.at(token.CONSTRAINT) { // ccons ::= CONSTRAINT nm.
-			p.advance()
+			pendingStart = p.advance().Pos
 			pending = p.expectName()
 			continue
 		}
 		c := p.parseColumnConstraint(cStart)
 		if c == nil {
 			break
 		}
-		c.Name, pending = pending, nil
+		if pending != nil {
+			// As for table constraints, the CONSTRAINT clause belongs to
+			// the constraint that follows it.
+			c.Name, c.Span.Start = pending, pendingStart
+			pending = nil
+		}
 		n.Constraints = append(n.Constraints, c)
 	}
 	n.Span = p.span(start)

parser/span_test.go

@@ -0,0 +1,85 @@
+package parser_test
+
+import (
+	"path/filepath"
+	"strings"
+	"sync/atomic"
+	"testing"
+
+	"github.com/sqlc-dev/meyer/ast"
+	"github.com/sqlc-dev/meyer/internal/testfile"
+	"github.com/sqlc-dev/meyer/parser"
+)
+
+// TestSpans checks the invariants sqlc relies on when it slices the source
+// with a node's byte offsets. Nothing else verifies them: a span can be
+// wrong in a tree that is otherwise perfectly shaped, and both the corpus
+// and the round-trip property would still pass.
+//
+// The invariants are:
+//
+//   - a span is a valid range within the input;
+//   - a child lies within its parent;
+//   - statements are disjoint and in source order, so the text between two
+//     of them (where the "-- name:" comments live) is unambiguous.
+func TestSpans(t *testing.T) {
+	paths, err := filepath.Glob(filepath.Join("testdata", "*.test"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(paths) == 0 {
+		t.Fatal("no corpus files; run go run ./cmd/regenerate-parse")
+	}
+	var checked atomic.Int64
+	t.Cleanup(func() { t.Logf("checked spans of %d cases", checked.Load()) })
+	for _, path := range paths {
+		t.Run(strings.TrimSuffix(filepath.Base(path), ".test"), func(t *testing.T) {
+			t.Parallel()
+			cases, err := testfile.Read(path)
+			if err != nil {
+				t.Fatal(err)
+			}
+			for _, c := range cases {
+				stmts, err := parser.ParseString(c.SQL)
+				if err != nil {
+					continue
+				}
+				checked.Add(1)
+				checkStmtSpans(t, c, stmts)
+			}
+		})
+	}
+}
+
+func checkStmtSpans(t *testing.T, c testfile.Case, stmts []ast.Stmt) {
+	t.Helper()
+	prevEnd := 0
+	for _, s := range stmts {
+		if s.Pos() < prevEnd {
+			t.Errorf("%s: statement at %d starts before the previous one ended at %d\n  %q",
+				c.Name, s.Pos(), prevEnd, c.SQL)
+			return
+		}
+		prevEnd = s.End()
+		if !checkNodeSpans(t, c, s, 0, len(c.SQL)) {
+			return
+		}
+	}
+}
+
+// checkNodeSpans walks a subtree, returning false once something is wrong so
+// that one bad node does not produce a page of consequences.
+func checkNodeSpans(t *testing.T, c testfile.Case, n ast.Node, lo, hi int) bool {
+	t.Helper()
+	if n.Pos() > n.End() || n.Pos() < lo || n.End() > hi {
+		t.Errorf("%s: %T span %d:%d is not within %d:%d\n  %q",
+			c.Name, n, n.Pos(), n.End(), lo, hi, c.SQL)
+		return false
+	}
+	for _, child := range n.Children() {
+		if !checkNodeSpans(t, c, child, n.Pos(), n.End()) {
+			return false
+		}
+	}
+	return true
+}

parser/testdata/ast/ddl.tree

@@ -289,7 +289,7 @@ CreateTableStmt{
   ]
   Constraints: [
     TableConstraint{
-      Span: 434:475
+      Span: 420:475
       Name: Ident{
         Span: 431:433
         Name: "pk"