Skip to content

Commit 72aae81

Browse files
committed
Reject mismatched kinds in multi-kind node factories
Node casts now trust Kind to recover the concrete struct, so a factory that accepts any kind could hand out a header whose cast reinterprets the wrong allocation. Previously the payload type assertion caught this. Restore the guarantee by rejecting kinds the struct cannot represent at construction time.
1 parent 8150f86 commit 72aae81

3 files changed

Lines changed: 90 additions & 8 deletions

File tree

‎tools/scripts/tsc/generate-go-ast.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,27 +374,31 @@ function generateBaseStructDefs(w: CodeWriter) {
374374

375375
// ── Generate As*() cast methods ────────────────────────────────────────────
376376

377-
function generateAsCast(w: CodeWriter, node: NodeType) {
378-
const structName = node.name;
377+
function generateKindCheck(w: CodeWriter, node: NodeType, expression: string, message: string) {
379378
const kinds = canonicalNodesByKind().get(node) ?? [];
380-
w.write(`func (n *Node) As${structName}() *${structName} {`);
381-
w.push();
382379
if (kinds.length === 1) {
383-
w.write(`if n.Kind != ${kinds[0]} {`);
380+
w.write(`if ${expression} != ${kinds[0]} {`);
384381
w.push();
385-
w.write(`panic("Invalid node cast to ${structName}")`);
382+
w.write(`panic("${message}")`);
386383
w.pop();
387384
w.write("}");
388385
}
389386
else {
390-
w.write("switch n.Kind {");
387+
w.write(`switch ${expression} {`);
391388
w.write(`case ${kinds.join(", ")}:`);
392389
w.write("default:");
393390
w.push();
394-
w.write(`panic("Invalid node cast to ${structName}")`);
391+
w.write(`panic("${message}")`);
395392
w.pop();
396393
w.write("}");
397394
}
395+
}
396+
397+
function generateAsCast(w: CodeWriter, node: NodeType) {
398+
const structName = node.name;
399+
w.write(`func (n *Node) As${structName}() *${structName} {`);
400+
w.push();
401+
generateKindCheck(w, node, "n.Kind", `Invalid node cast to ${structName}`);
398402
w.write(`return (*${structName})(unsafe.Pointer(n))`);
399403
w.pop();
400404
w.write("}");
@@ -444,6 +448,10 @@ function emitNewFactory(
444448
w.write(`func (f *NodeFactory) ${funcName}(${params}) *Node {`);
445449
w.push();
446450

451+
if (kindMember) {
452+
generateKindCheck(w, node, kindMember.goParamName(), `Invalid node kind for ${structName}`);
453+
}
454+
447455
if (node.arena) {
448456
w.write(`data := f.${api.uncapitalize(structName)}Arena.New()`);
449457
}

‎tsc/internal/ast/ast_generated.go‎

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tsc/internal/ast/nodeaccessors_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package ast_test
22

33
import (
4+
"reflect"
45
"runtime"
6+
"slices"
57
"testing"
68
"unsafe"
79

@@ -10,6 +12,48 @@ import (
1012
"gotest.tools/v3/assert"
1113
)
1214

15+
func TestNodeFactoryKinds(t *testing.T) {
16+
t.Parallel()
17+
for _, test := range []struct {
18+
name string
19+
kinds []ast.Kind
20+
}{
21+
{"ForInOrOfStatement", []ast.Kind{ast.KindForInStatement, ast.KindForOfStatement}},
22+
{"CaseOrDefaultClause", []ast.Kind{ast.KindCaseClause, ast.KindDefaultClause}},
23+
{"BindingPattern", []ast.Kind{ast.KindArrayBindingPattern, ast.KindObjectBindingPattern}},
24+
{"KeywordExpression", []ast.Kind{ast.KindFalseKeyword, ast.KindImportKeyword, ast.KindNullKeyword, ast.KindSuperKeyword, ast.KindThisKeyword, ast.KindTrueKeyword}},
25+
{"KeywordTypeNode", []ast.Kind{ast.KindVoidKeyword, ast.KindAnyKeyword, ast.KindBooleanKeyword, ast.KindIntrinsicKeyword, ast.KindNeverKeyword, ast.KindNumberKeyword, ast.KindObjectKeyword, ast.KindStringKeyword, ast.KindSymbolKeyword, ast.KindUndefinedKeyword, ast.KindUnknownKeyword, ast.KindBigIntKeyword}},
26+
{"JSDocParameterOrPropertyTag", []ast.Kind{ast.KindJSDocParameterTag, ast.KindJSDocPropertyTag}},
27+
} {
28+
t.Run(test.name, func(t *testing.T) {
29+
t.Parallel()
30+
factory := reflect.ValueOf(ast.NewNodeFactory(ast.NodeFactoryHooks{}))
31+
constructor := factory.MethodByName("New" + test.name)
32+
args := make([]reflect.Value, constructor.Type().NumIn())
33+
for i := range args {
34+
args[i] = reflect.Zero(constructor.Type().In(i))
35+
}
36+
for kind := ast.Kind(-1); kind <= ast.KindCount+2; kind++ {
37+
args[0] = reflect.ValueOf(kind)
38+
var node *ast.Node
39+
panicked := func() (panicked bool) {
40+
defer func() { panicked = recover() != nil }()
41+
node = constructor.Call(args)[0].Interface().(*ast.Node)
42+
return false
43+
}()
44+
valid := slices.Contains(test.kinds, kind)
45+
if panicked == valid {
46+
t.Fatalf("New%s(%v): valid=%v, panicked=%v", test.name, kind, valid, panicked)
47+
}
48+
if valid {
49+
assert.Equal(t, node.Kind, kind)
50+
assert.Assert(t, reflect.ValueOf(node).MethodByName("As" + test.name).Call(nil)[0].Pointer() != 0)
51+
}
52+
}
53+
})
54+
}
55+
}
56+
1357
func TestNodeAccessors(t *testing.T) {
1458
t.Parallel()
1559
factory := ast.NewNodeFactory(ast.NodeFactoryHooks{})

0 commit comments

Comments
 (0)