From 6b61e430023ccd829205dbd7612f1e072beef81a Mon Sep 17 00:00:00 2001 From: git-hulk Date: Sun, 5 Jul 2026 15:17:42 +0800 Subject: [PATCH 1/2] Fix traversal of semantic AST children --- parser/ast.go | 29 +++++- parser/format.go | 6 +- parser/traversal_drift_test.go | 179 +++++++++++++++++++++++++++++++++ parser/walk.go | 9 ++ 4 files changed, 219 insertions(+), 4 deletions(-) diff --git a/parser/ast.go b/parser/ast.go index 448f8f6..cda83ec 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -2478,6 +2478,11 @@ func (t *TTLPolicyRule) Accept(visitor ASTVisitor) error { return err } } + if t.Action != nil { + if err := t.Action.Accept(visitor); err != nil { + return err + } + } return visitor.VisitTTLPolicyRule(t) } @@ -3165,6 +3170,11 @@ func (c *ColumnDef) Accept(visitor ASTVisitor) error { return err } } + if c.CompressionCodec != nil { + if err := c.CompressionCodec.Accept(visitor); err != nil { + return err + } + } return visitor.VisitColumnDef(c) } @@ -4504,14 +4514,27 @@ func (w *WithTimeoutClause) Pos() Pos { } func (w *WithTimeoutClause) End() Pos { - return w.Number.End() + if w.Number != nil { + return w.Number.End() + } + if w.Expr != nil { + return w.Expr.End() + } + return w.WithTimeoutPos } func (w *WithTimeoutClause) Accept(visitor ASTVisitor) error { visitor.Enter(w) defer visitor.Leave(w) - if err := w.Number.Accept(visitor); err != nil { - return err + if w.Expr != nil { + if err := w.Expr.Accept(visitor); err != nil { + return err + } + } + if w.Number != nil { + if err := w.Number.Accept(visitor); err != nil { + return err + } } return visitor.VisitWithTimeoutExpr(w) } diff --git a/parser/format.go b/parser/format.go index 7fe0bbd..2f33c4a 100644 --- a/parser/format.go +++ b/parser/format.go @@ -2859,5 +2859,9 @@ func (w *WithClause) FormatSQL(formatter *Formatter) { func (w *WithTimeoutClause) FormatSQL(formatter *Formatter) { formatter.WriteString("WITH TIMEOUT ") - formatter.WriteExpr(w.Number) + if w.Number != nil { + formatter.WriteExpr(w.Number) + return + } + formatter.WriteExpr(w.Expr) } diff --git a/parser/traversal_drift_test.go b/parser/traversal_drift_test.go index 5e47cd5..5cb5a6c 100644 --- a/parser/traversal_drift_test.go +++ b/parser/traversal_drift_test.go @@ -237,6 +237,185 @@ func TestTraversalEnginesVisitSameFields(t *testing.T) { require.Empty(t, problems, "child-field traversal drift between Accept and Walk") } +// TestTraversalEnginesVisitAllSemanticFields guards against a field being +// skipped by both traversal engines. The parity test above cannot catch that +// because both Accept and Walk can agree on the same omission. +func TestTraversalEnginesVisitAllSemanticFields(t *testing.T) { + fset := token.NewFileSet() + astFile, err := goparser.ParseFile(fset, "ast.go", nil, 0) + require.NoError(t, err) + walkFile, err := goparser.ParseFile(fset, "walk.go", nil, 0) + require.NoError(t, err) + + structFields := map[string]map[string]string{} + acceptTypes := map[string]bool{} + for _, decl := range astFile.Decls { + switch d := decl.(type) { + case *ast.GenDecl: + for _, spec := range d.Specs { + ts, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + st, ok := ts.Type.(*ast.StructType) + if !ok { + continue + } + fields := map[string]string{} + for _, field := range st.Fields.List { + for _, name := range field.Names { + fields[name.Name] = baseTypeName(field.Type) + } + } + structFields[ts.Name.Name] = fields + } + case *ast.FuncDecl: + if d.Name.Name != "Accept" || d.Recv == nil || len(d.Recv.List) != 1 { + continue + } + if star, ok := d.Recv.List[0].Type.(*ast.StarExpr); ok { + if ident, ok := star.X.(*ast.Ident); ok { + acceptTypes[ident.Name] = true + } + } + } + } + + semanticInterfaces := map[string]bool{ + "Expr": true, + "DDL": true, + "AlterTableClause": true, + "ColumnType": true, + "Literal": true, + } + semanticHelpers := semanticHelperTypes(structFields, acceptTypes, semanticInterfaces) + acceptFields := receiverFields(astFile, "Accept") + walkFields := walkCaseFields(walkFile) + + var problems []string + for typeName, fields := range structFields { + if !acceptTypes[typeName] { + continue + } + for fieldName, baseType := range fields { + if !acceptTypes[baseType] && !semanticInterfaces[baseType] && !semanticHelpers[baseType] { + continue + } + if !acceptFields[typeName][fieldName] { + problems = append(problems, typeName+"."+fieldName+" is an AST child but is not traversed by Accept") + } + if !walkFields[typeName][fieldName] { + problems = append(problems, typeName+"."+fieldName+" is an AST child but is not traversed by Walk") + } + } + } + + sort.Strings(problems) + require.Empty(t, problems, "semantic AST fields must be reachable by traversal") +} + +func baseTypeName(expr ast.Expr) string { + switch t := expr.(type) { + case *ast.Ident: + return t.Name + case *ast.StarExpr: + return baseTypeName(t.X) + case *ast.ArrayType: + return baseTypeName(t.Elt) + default: + return "" + } +} + +func semanticHelperTypes(structFields map[string]map[string]string, acceptTypes, semanticInterfaces map[string]bool) map[string]bool { + helpers := map[string]bool{} + changed := true + for changed { + changed = false + for typeName, fields := range structFields { + if acceptTypes[typeName] || helpers[typeName] { + continue + } + for _, baseType := range fields { + if acceptTypes[baseType] || semanticInterfaces[baseType] || helpers[baseType] { + helpers[typeName] = true + changed = true + break + } + } + } + } + return helpers +} + +func receiverFields(file *ast.File, methodName string) map[string]map[string]bool { + fieldsByType := map[string]map[string]bool{} + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name != methodName || fn.Recv == nil || len(fn.Recv.List) != 1 { + continue + } + star, ok := fn.Recv.List[0].Type.(*ast.StarExpr) + if !ok { + continue + } + typeIdent, ok := star.X.(*ast.Ident) + if !ok { + continue + } + fieldsByType[typeIdent.Name] = directSelectors(fn.Body, fn.Recv.List[0].Names[0].Name) + } + return fieldsByType +} + +func walkCaseFields(file *ast.File) map[string]map[string]bool { + fieldsByType := map[string]map[string]bool{} + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name != "Walk" || fn.Recv != nil { + continue + } + ast.Inspect(fn.Body, func(node ast.Node) bool { + cc, ok := node.(*ast.CaseClause) + if !ok { + return true + } + fields := map[string]bool{} + for _, stmt := range cc.Body { + for field := range directSelectors(stmt, "n") { + fields[field] = true + } + } + for _, expr := range cc.List { + star, ok := expr.(*ast.StarExpr) + if !ok { + continue + } + if ident, ok := star.X.(*ast.Ident); ok { + fieldsByType[ident.Name] = fields + } + } + return true + }) + } + return fieldsByType +} + +func directSelectors(node ast.Node, baseName string) map[string]bool { + fields := map[string]bool{} + ast.Inspect(node, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + if ident, ok := sel.X.(*ast.Ident); ok && ident.Name == baseName { + fields[sel.Sel.Name] = true + } + return true + }) + return fields +} + // TestInsertStmtEndWithoutValues guards that End() does not panic on // `INSERT INTO t FORMAT CSV`, where the data arrives out of band and the // statement carries neither VALUES nor a SELECT. diff --git a/parser/walk.go b/parser/walk.go index 63f5929..d57bb11 100644 --- a/parser/walk.go +++ b/parser/walk.go @@ -1059,6 +1059,9 @@ func Walk(node Expr, fn WalkFunc) bool { if !Walk(n.Comment, fn) { return false } + if !Walk(n.CompressionCodec, fn) { + return false + } case *ScalarType: if !Walk(n.Name, fn) { return false @@ -1174,6 +1177,9 @@ func Walk(node Expr, fn WalkFunc) bool { if !Walk(n.ToDisk, fn) { return false } + if !Walk(n.Action, fn) { + return false + } case *TTLPolicyRuleAction: if !Walk(n.Codec, fn) { return false @@ -1257,6 +1263,9 @@ func Walk(node Expr, fn WalkFunc) bool { } } case *WithTimeoutClause: + if !Walk(n.Expr, fn) { + return false + } if !Walk(n.Number, fn) { return false } From e174236dc84238e6133461115bde80b1559b3ca7 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Sun, 5 Jul 2026 15:21:46 +0800 Subject: [PATCH 2/2] Revert traversal drift guard test --- parser/traversal_drift_test.go | 179 --------------------------------- 1 file changed, 179 deletions(-) diff --git a/parser/traversal_drift_test.go b/parser/traversal_drift_test.go index 5cb5a6c..5e47cd5 100644 --- a/parser/traversal_drift_test.go +++ b/parser/traversal_drift_test.go @@ -237,185 +237,6 @@ func TestTraversalEnginesVisitSameFields(t *testing.T) { require.Empty(t, problems, "child-field traversal drift between Accept and Walk") } -// TestTraversalEnginesVisitAllSemanticFields guards against a field being -// skipped by both traversal engines. The parity test above cannot catch that -// because both Accept and Walk can agree on the same omission. -func TestTraversalEnginesVisitAllSemanticFields(t *testing.T) { - fset := token.NewFileSet() - astFile, err := goparser.ParseFile(fset, "ast.go", nil, 0) - require.NoError(t, err) - walkFile, err := goparser.ParseFile(fset, "walk.go", nil, 0) - require.NoError(t, err) - - structFields := map[string]map[string]string{} - acceptTypes := map[string]bool{} - for _, decl := range astFile.Decls { - switch d := decl.(type) { - case *ast.GenDecl: - for _, spec := range d.Specs { - ts, ok := spec.(*ast.TypeSpec) - if !ok { - continue - } - st, ok := ts.Type.(*ast.StructType) - if !ok { - continue - } - fields := map[string]string{} - for _, field := range st.Fields.List { - for _, name := range field.Names { - fields[name.Name] = baseTypeName(field.Type) - } - } - structFields[ts.Name.Name] = fields - } - case *ast.FuncDecl: - if d.Name.Name != "Accept" || d.Recv == nil || len(d.Recv.List) != 1 { - continue - } - if star, ok := d.Recv.List[0].Type.(*ast.StarExpr); ok { - if ident, ok := star.X.(*ast.Ident); ok { - acceptTypes[ident.Name] = true - } - } - } - } - - semanticInterfaces := map[string]bool{ - "Expr": true, - "DDL": true, - "AlterTableClause": true, - "ColumnType": true, - "Literal": true, - } - semanticHelpers := semanticHelperTypes(structFields, acceptTypes, semanticInterfaces) - acceptFields := receiverFields(astFile, "Accept") - walkFields := walkCaseFields(walkFile) - - var problems []string - for typeName, fields := range structFields { - if !acceptTypes[typeName] { - continue - } - for fieldName, baseType := range fields { - if !acceptTypes[baseType] && !semanticInterfaces[baseType] && !semanticHelpers[baseType] { - continue - } - if !acceptFields[typeName][fieldName] { - problems = append(problems, typeName+"."+fieldName+" is an AST child but is not traversed by Accept") - } - if !walkFields[typeName][fieldName] { - problems = append(problems, typeName+"."+fieldName+" is an AST child but is not traversed by Walk") - } - } - } - - sort.Strings(problems) - require.Empty(t, problems, "semantic AST fields must be reachable by traversal") -} - -func baseTypeName(expr ast.Expr) string { - switch t := expr.(type) { - case *ast.Ident: - return t.Name - case *ast.StarExpr: - return baseTypeName(t.X) - case *ast.ArrayType: - return baseTypeName(t.Elt) - default: - return "" - } -} - -func semanticHelperTypes(structFields map[string]map[string]string, acceptTypes, semanticInterfaces map[string]bool) map[string]bool { - helpers := map[string]bool{} - changed := true - for changed { - changed = false - for typeName, fields := range structFields { - if acceptTypes[typeName] || helpers[typeName] { - continue - } - for _, baseType := range fields { - if acceptTypes[baseType] || semanticInterfaces[baseType] || helpers[baseType] { - helpers[typeName] = true - changed = true - break - } - } - } - } - return helpers -} - -func receiverFields(file *ast.File, methodName string) map[string]map[string]bool { - fieldsByType := map[string]map[string]bool{} - for _, decl := range file.Decls { - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Name.Name != methodName || fn.Recv == nil || len(fn.Recv.List) != 1 { - continue - } - star, ok := fn.Recv.List[0].Type.(*ast.StarExpr) - if !ok { - continue - } - typeIdent, ok := star.X.(*ast.Ident) - if !ok { - continue - } - fieldsByType[typeIdent.Name] = directSelectors(fn.Body, fn.Recv.List[0].Names[0].Name) - } - return fieldsByType -} - -func walkCaseFields(file *ast.File) map[string]map[string]bool { - fieldsByType := map[string]map[string]bool{} - for _, decl := range file.Decls { - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Name.Name != "Walk" || fn.Recv != nil { - continue - } - ast.Inspect(fn.Body, func(node ast.Node) bool { - cc, ok := node.(*ast.CaseClause) - if !ok { - return true - } - fields := map[string]bool{} - for _, stmt := range cc.Body { - for field := range directSelectors(stmt, "n") { - fields[field] = true - } - } - for _, expr := range cc.List { - star, ok := expr.(*ast.StarExpr) - if !ok { - continue - } - if ident, ok := star.X.(*ast.Ident); ok { - fieldsByType[ident.Name] = fields - } - } - return true - }) - } - return fieldsByType -} - -func directSelectors(node ast.Node, baseName string) map[string]bool { - fields := map[string]bool{} - ast.Inspect(node, func(n ast.Node) bool { - sel, ok := n.(*ast.SelectorExpr) - if !ok { - return true - } - if ident, ok := sel.X.(*ast.Ident); ok && ident.Name == baseName { - fields[sel.Sel.Name] = true - } - return true - }) - return fields -} - // TestInsertStmtEndWithoutValues guards that End() does not panic on // `INSERT INTO t FORMAT CSV`, where the data arrives out of band and the // statement carries neither VALUES nor a SELECT.