Skip to content

Commit 66a57e8

Browse files
EmyrkEmyrk
committed
fix(bindings/walk): handle the new expression and import nodes
Add Walk cases for the value-expression nodes introduced in #84 (IdentifierExpression, PropertyAccessExpression, CallExpression, ObjectLiteralExpression, PropertyAssignment, Parameter, ArrowFunction, TypeQuery) and the import nodes introduced in #83 (ImportDeclaration, ImportSpecifier). Also add the missing ExpressionWithTypeArguments case that pre-dated those PRs. Without these cases, any Walk over a tree that uses one of these nodes hits the default branch and panics 'convert.Walk: unexpected node type'. Add walk_test.go that builds a synthetic tree containing every concrete Node implementation and asserts Walk visits each one, so any future Node type that forgets a case here fails loudly. Generated with Coder Agents.
1 parent 2f30faf commit 66a57e8

2 files changed

Lines changed: 266 additions & 0 deletions

File tree

‎bindings/walk/walk.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,32 @@ func Walk(v Visitor, node bindings.Node) {
7070
walkList(v, n.Members)
7171
case *bindings.TypeIntersection:
7272
walkList(v, n.Types)
73+
case *bindings.ExpressionWithTypeArguments:
74+
Walk(v, n.Expression)
75+
walkList(v, n.Arguments)
76+
case *bindings.ImportDeclaration:
77+
walkList(v, n.Named)
78+
case *bindings.ImportSpecifier:
79+
// noop
80+
case *bindings.IdentifierExpression:
81+
// noop
82+
case *bindings.PropertyAccessExpression:
83+
Walk(v, n.Expression)
84+
case *bindings.CallExpression:
85+
Walk(v, n.Expression)
86+
walkList(v, n.Arguments)
87+
case *bindings.ObjectLiteralExpression:
88+
walkList(v, n.Properties)
89+
case *bindings.PropertyAssignment:
90+
Walk(v, n.Initializer)
91+
case *bindings.Parameter:
92+
Walk(v, n.Type)
93+
case *bindings.ArrowFunction:
94+
walkList(v, n.Parameters)
95+
Walk(v, n.ReturnType)
96+
Walk(v, n.Body)
97+
case *bindings.TypeQuery:
98+
// noop
7399
default:
74100
panic(fmt.Sprintf("convert.Walk: unexpected node type %T", n))
75101
}

‎bindings/walk/walk_test.go‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
package walk_test
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/coder/guts/bindings"
8+
"github.com/coder/guts/bindings/walk"
9+
)
10+
11+
// recordingVisitor stores every node it visits so tests can assert that
12+
// Walk descends into expected child slots without panicking.
13+
type recordingVisitor struct {
14+
visited []bindings.Node
15+
}
16+
17+
func (r *recordingVisitor) Visit(node bindings.Node) walk.Visitor {
18+
r.visited = append(r.visited, node)
19+
return r
20+
}
21+
22+
// keyword returns a fresh pointer to a LiteralKeyword. LiteralKeyword is a
23+
// string-typed Node, so callers need a pointer-to-string-typed-value, not
24+
// a composite literal.
25+
func keyword(k bindings.LiteralKeyword) *bindings.LiteralKeyword {
26+
return &k
27+
}
28+
29+
// TestWalkCoversAllNodeTypes builds a synthetic tree that contains every
30+
// concrete bindings.Node implementation and walks it. The default branch
31+
// in Walk panics on unhandled types, so this test fails loudly if a new
32+
// node is added to the bindings package without a matching case here.
33+
func TestWalkCoversAllNodeTypes(t *testing.T) {
34+
t.Parallel()
35+
36+
// Construct one of each Node. Some nodes (PropertyAssignment, Parameter,
37+
// HeritageClause, ImportSpecifier, EnumMember) only appear as children
38+
// of a parent, so the parent's slot is the way Walk reaches them.
39+
tree := &bindings.Interface{
40+
Name: bindings.Identifier{Name: "Root"},
41+
Parameters: []*bindings.TypeParameter{
42+
{Name: bindings.Identifier{Name: "T"}, Type: keyword(bindings.KeywordString)},
43+
},
44+
Heritage: []*bindings.HeritageClause{
45+
{
46+
Args: []bindings.ExpressionType{
47+
&bindings.ExpressionWithTypeArguments{
48+
Expression: &bindings.IdentifierExpression{
49+
Name: bindings.Identifier{Name: "Base"},
50+
},
51+
Arguments: []bindings.ExpressionType{
52+
&bindings.ReferenceType{Name: bindings.Identifier{Name: "X"}},
53+
},
54+
},
55+
},
56+
},
57+
},
58+
Fields: []*bindings.PropertySignature{
59+
{
60+
Name: "everything",
61+
Type: &bindings.TypeIntersection{
62+
Types: []bindings.ExpressionType{
63+
&bindings.UnionType{
64+
Types: []bindings.ExpressionType{
65+
&bindings.LiteralType{Value: "a"},
66+
&bindings.Null{},
67+
keyword(bindings.KeywordString),
68+
},
69+
},
70+
&bindings.TypeLiteralNode{
71+
Members: []*bindings.PropertySignature{
72+
{Name: "nested", Type: keyword(bindings.KeywordString)},
73+
},
74+
},
75+
&bindings.ArrayType{Node: keyword(bindings.KeywordString)},
76+
&bindings.TupleType{Node: keyword(bindings.KeywordString)},
77+
&bindings.ArrayLiteralType{
78+
Elements: []bindings.ExpressionType{&bindings.LiteralType{Value: "elt"}},
79+
},
80+
bindings.OperatorNode(bindings.KeywordReadonly, keyword(bindings.KeywordString)),
81+
&bindings.TypeQuery{Name: bindings.Identifier{Name: "Other"}},
82+
},
83+
},
84+
},
85+
},
86+
}
87+
88+
// Independent node group exercising the value-expression and import
89+
// nodes added in PR 83 and PR 84. Walking each top-level node here
90+
// touches the remaining bindings.Node implementations.
91+
nodes := []bindings.Node{
92+
tree,
93+
&bindings.Alias{
94+
Name: bindings.Identifier{Name: "MyAlias"},
95+
Type: keyword(bindings.KeywordString),
96+
},
97+
&bindings.Enum{
98+
Name: bindings.Identifier{Name: "Color"},
99+
Members: []*bindings.EnumMember{
100+
{Name: "Red", Value: &bindings.LiteralType{Value: "red"}},
101+
},
102+
},
103+
&bindings.VariableStatement{
104+
Declarations: &bindings.VariableDeclarationList{
105+
Flags: bindings.NodeFlagsConstant,
106+
Declarations: []*bindings.VariableDeclaration{
107+
{
108+
Name: bindings.Identifier{Name: "Schema"},
109+
Type: &bindings.ReferenceType{Name: bindings.Identifier{Name: "Z"}},
110+
Initializer: &bindings.CallExpression{
111+
Expression: &bindings.PropertyAccessExpression{
112+
Expression: &bindings.IdentifierExpression{
113+
Name: bindings.Identifier{Name: "z"},
114+
},
115+
Name: "object",
116+
},
117+
Arguments: []bindings.ExpressionType{
118+
&bindings.ObjectLiteralExpression{
119+
Properties: []*bindings.PropertyAssignment{
120+
{
121+
Name: "id",
122+
Initializer: &bindings.CallExpression{
123+
Expression: &bindings.PropertyAccessExpression{
124+
Expression: &bindings.IdentifierExpression{
125+
Name: bindings.Identifier{Name: "z"},
126+
},
127+
Name: "string",
128+
},
129+
},
130+
},
131+
},
132+
},
133+
&bindings.ArrowFunction{
134+
Parameters: []*bindings.Parameter{
135+
{Name: "x", Type: keyword(bindings.KeywordString)},
136+
},
137+
ReturnType: keyword(bindings.KeywordString),
138+
Body: &bindings.IdentifierExpression{
139+
Name: bindings.Identifier{Name: "x"},
140+
},
141+
},
142+
},
143+
},
144+
},
145+
},
146+
},
147+
},
148+
&bindings.ImportDeclaration{
149+
Module: "zod",
150+
Named: []*bindings.ImportSpecifier{
151+
{Name: "z"},
152+
},
153+
},
154+
}
155+
156+
v := &recordingVisitor{}
157+
for _, node := range nodes {
158+
walk.Walk(v, node)
159+
}
160+
161+
// Verify that Walk reached every new node type. The set of expected
162+
// node types here is the union of nodes constructed above. If any are
163+
// missing, Walk lost a child slot somewhere along the way.
164+
seen := map[string]bool{}
165+
for _, n := range v.visited {
166+
seen[fmt.Sprintf("%T", n)] = true
167+
}
168+
169+
want := []string{
170+
"*bindings.Interface",
171+
"*bindings.TypeParameter",
172+
"*bindings.HeritageClause",
173+
"*bindings.ExpressionWithTypeArguments",
174+
"*bindings.IdentifierExpression",
175+
"*bindings.ReferenceType",
176+
"*bindings.PropertySignature",
177+
"*bindings.TypeIntersection",
178+
"*bindings.UnionType",
179+
"*bindings.LiteralType",
180+
"*bindings.Null",
181+
"*bindings.LiteralKeyword",
182+
"*bindings.TypeLiteralNode",
183+
"*bindings.ArrayType",
184+
"*bindings.TupleType",
185+
"*bindings.ArrayLiteralType",
186+
"*bindings.OperatorNodeType",
187+
"*bindings.TypeQuery",
188+
"*bindings.Alias",
189+
"*bindings.Enum",
190+
"*bindings.EnumMember",
191+
"*bindings.VariableStatement",
192+
"*bindings.VariableDeclarationList",
193+
"*bindings.VariableDeclaration",
194+
"*bindings.CallExpression",
195+
"*bindings.PropertyAccessExpression",
196+
"*bindings.ObjectLiteralExpression",
197+
"*bindings.PropertyAssignment",
198+
"*bindings.ArrowFunction",
199+
"*bindings.Parameter",
200+
"*bindings.ImportDeclaration",
201+
"*bindings.ImportSpecifier",
202+
}
203+
for _, name := range want {
204+
if !seen[name] {
205+
t.Errorf("Walk did not visit %s", name)
206+
}
207+
}
208+
}
209+
210+
// TestWalkStopsWhenVisitReturnsNil verifies that Walk honours the Visitor
211+
// contract: returning nil from Visit halts descent into that subtree.
212+
func TestWalkStopsWhenVisitReturnsNil(t *testing.T) {
213+
t.Parallel()
214+
215+
leaf := keyword(bindings.KeywordString)
216+
parent := &bindings.ArrayType{Node: leaf}
217+
218+
v := &stoppingVisitor{stopAt: parent}
219+
walk.Walk(v, parent)
220+
221+
if len(v.visited) != 1 {
222+
t.Fatalf("expected 1 visit, got %d", len(v.visited))
223+
}
224+
if v.visited[0] != bindings.Node(parent) {
225+
t.Fatalf("expected to visit parent, got %T", v.visited[0])
226+
}
227+
}
228+
229+
type stoppingVisitor struct {
230+
stopAt bindings.Node
231+
visited []bindings.Node
232+
}
233+
234+
func (s *stoppingVisitor) Visit(node bindings.Node) walk.Visitor {
235+
s.visited = append(s.visited, node)
236+
if node == s.stopAt {
237+
return nil
238+
}
239+
return s
240+
}

0 commit comments

Comments
 (0)