Add drift test keeping ASTVisitor and Walk traversals in sync#276
Merged
Conversation
Accept/ASTVisitor and Walk/WalkFunc are two independent traversal engines that encode every node's children separately, so a new AST node type added to one can silently be missing from the other. Add a static test that parses the package source and asserts every type with an Accept method has both a Visit method on the ASTVisitor interface and a case in Walk's type switch, and vice versa. The test immediately caught four types missing from Walk - BoolLiteral, CreateNamedCollection, NamedCollectionParam and WindowFrameParam - and their cases are added. Also fix a confirmed traversal bug: JoinTableExpr.Accept returned early after visiting the sample ratio, so VisitJoinTableExpr was never called for table expressions carrying a SAMPLE clause. Add a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Accept/ASTVisitorandWalk/WalkFuncare two independent traversal engines that each encode every node's children separately, so they inevitably drift as node types are added. This PR adds a static drift test and fixes the drift it found.Changes
TestTraversalEnginesCoverSameNodeTypes: parses the package source withgo/astand asserts set-equality between (a) types with anAcceptmethod, (b) parameter types ofASTVisitor's Visit methods, and (c)case *Tentries inWalk's type switch. Any future node type added to one engine but not the other fails CI with the exact type names.BoolLiteral,CreateNamedCollection,NamedCollectionParam, andWindowFrameParamwere missing fromWalk's switch —Walksilently skipped their subtrees (e.g. named-collection params and their values were never visited).JoinTableExpr.Acceptreturned early whenSampleRatio != nil, soVisitJoinTableExprwas never called for table expressions with a SAMPLE clause. AddedTestVisitJoinTableExprWithSampleRatioas regression coverage.Testing
Full suite passes including
-race -compatible.🤖 Generated with Claude Code