Skip to content

Set parents during parse, not bind #1251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions internal/ast/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,8 +904,7 @@ func newParentInChildrenSetter() func(node *Node) bool {
}

state.visit = func(node *Node) bool {
if state.parent != nil && node.Parent != state.parent {
// Avoid data races on no-ops
if state.parent != nil {
node.Parent = state.parent
}
saveParent := state.parent
Expand Down
42 changes: 0 additions & 42 deletions internal/binder/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ type Binder struct {
unreachableFlow *ast.FlowNode
reportedUnreachableFlow *ast.FlowNode

parent *ast.Node
container *ast.Node
thisContainer *ast.Node
blockScopeContainer *ast.Node
Expand Down Expand Up @@ -211,9 +210,6 @@ func (b *Binder) declareSymbolEx(symbolTable ast.SymbolTable, parent *ast.Symbol
} else if !(includes&ast.SymbolFlagsVariable != 0 && symbol.Flags&ast.SymbolFlagsAssignment != 0 ||
includes&ast.SymbolFlagsAssignment != 0 && symbol.Flags&ast.SymbolFlagsVariable != 0) {
// Assignment declarations are allowed to merge with variables, no matter what other flags they have.
if node.Name() != nil {
setParent(node.Name(), node)
}
// Report errors every position with duplicate declaration
// Report errors on previous encountered declarations
var message *diagnostics.Message
Expand Down Expand Up @@ -584,9 +580,6 @@ func (b *Binder) bind(node *ast.Node) bool {
if node == nil {
return false
}
if node.Parent == nil || node.Parent.Flags&ast.NodeFlagsReparsed != 0 {
node.Parent = b.parent
}
saveInStrictMode := b.inStrictMode
// Even though in the AST the jsdoc @typedef node belongs to the current node,
// its symbol might be in the same scope with the current node's symbol. Consider:
Expand Down Expand Up @@ -731,9 +724,7 @@ func (b *Binder) bind(node *ast.Node) bool {
// children, as an optimization we don't process those.
thisNodeOrAnySubnodesHasError := node.Flags&ast.NodeFlagsThisNodeHasError != 0
if node.Kind > ast.KindLastToken {
saveParent := b.parent
saveSeenParseError := b.seenParseError
b.parent = node
b.seenParseError = false
containerFlags := GetContainerFlags(node)
if containerFlags == ContainerFlagsNone {
Expand All @@ -744,15 +735,7 @@ func (b *Binder) bind(node *ast.Node) bool {
if b.seenParseError {
thisNodeOrAnySubnodesHasError = true
}
b.parent = saveParent
b.seenParseError = saveSeenParseError
} else {
saveParent := b.parent
if node.Kind == ast.KindEndOfFile {
b.parent = node
}
b.setJSDocParents(node)
b.parent = saveParent
}
if thisNodeOrAnySubnodesHasError {
node.Flags |= ast.NodeFlagsThisNodeOrAnySubNodesHasError
Expand All @@ -762,16 +745,6 @@ func (b *Binder) bind(node *ast.Node) bool {
return false
}

func (b *Binder) setJSDocParents(node *ast.Node) {
for _, jsdoc := range node.JSDoc(b.file) {
setParent(jsdoc, node)
if jsdoc.Kind != ast.KindJSDocImportTag {
// JSDocImportTag children have parents set during parsing for module resolution purposes.
ast.SetParentInChildren(jsdoc)
}
}
}

func (b *Binder) bindPropertyWorker(node *ast.Node) {
isAutoAccessor := ast.IsAutoAccessorPropertyDeclaration(node)
includes := core.IfElse(isAutoAccessor, ast.SymbolFlagsAccessor, ast.SymbolFlagsProperty)
Expand Down Expand Up @@ -868,9 +841,6 @@ func (b *Binder) bindExportDeclaration(node *ast.Node) {
// All export * declarations are collected in an __export symbol
b.declareSymbol(ast.GetExports(b.container.Symbol()), b.container.Symbol(), node, ast.SymbolFlagsExportStar, ast.SymbolFlagsNone)
} else if ast.IsNamespaceExport(decl.ExportClause) {
// declareSymbol walks up parents to find name text, parent _must_ be set
// but won't be set by the normal binder walk until `bindChildren` later on.
setParent(decl.ExportClause, node)
b.declareSymbol(ast.GetExports(b.container.Symbol()), b.container.Symbol(), decl.ExportClause, ast.SymbolFlagsAlias, ast.SymbolFlagsAliasExcludes)
}
}
Expand Down Expand Up @@ -981,7 +951,6 @@ func (b *Binder) bindClassLikeDeclaration(node *ast.Node) {
prototypeSymbol := b.newSymbol(ast.SymbolFlagsProperty|ast.SymbolFlagsPrototype, "prototype")
symbolExport := ast.GetExports(symbol)[prototypeSymbol.Name]
if symbolExport != nil {
setParent(name, node)
b.errorOnNode(symbolExport.Declarations[0], diagnostics.Duplicate_identifier_0, ast.SymbolName(prototypeSymbol))
}
ast.GetExports(symbol)[prototypeSymbol.Name] = prototypeSymbol
Expand Down Expand Up @@ -1026,9 +995,6 @@ func (b *Binder) bindExpandoPropertyAssignment(node *ast.Node) {
symbol = b.lookupEntity(parent, b.container)
}
if symbol = getInitializerSymbol(symbol); symbol != nil {
// Fix up parent pointers since we're going to use these nodes before we bind into them
setParent(expr.Left, node)
setParent(expr.Right, node)
if ast.HasDynamicName(node) {
b.bindAnonymousDeclaration(node, ast.SymbolFlagsProperty|ast.SymbolFlagsAssignment, ast.InternalSymbolNameComputed)
addLateBoundAssignmentDeclarationToSymbol(node, symbol)
Expand Down Expand Up @@ -1599,7 +1565,6 @@ func (b *Binder) bindChildren(node *ast.Node) {
// and set it before we descend into nodes that could actually be part of an assignment pattern.
b.inAssignmentPattern = false
if b.checkUnreachable(node) {
b.setJSDocParents(node)
b.bindEachChild(node)
b.inAssignmentPattern = saveInAssignmentPattern
return
Expand All @@ -1611,7 +1576,6 @@ func (b *Binder) bindChildren(node *ast.Node) {
hasFlowNodeData.FlowNode = b.currentFlow
}
}
b.setJSDocParents(node)
switch node.Kind {
case ast.KindWhileStatement:
b.bindWhileStatement(node)
Expand Down Expand Up @@ -2800,12 +2764,6 @@ func (b *Binder) addDiagnostic(diagnostic *ast.Diagnostic) {
b.file.SetBindDiagnostics(append(b.file.BindDiagnostics(), diagnostic))
}

func setParent(child *ast.Node, parent *ast.Node) {
if child != nil {
child.Parent = parent
}
}

func isSignedNumericLiteral(node *ast.Node) bool {
if node.Kind == ast.KindPrefixUnaryExpression {
node := node.AsPrefixUnaryExpression()
Expand Down
1 change: 0 additions & 1 deletion internal/execute/tsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func fmtMain(sys System, input, output string) ExitStatus {
Path: pathified,
JSDocParsingMode: ast.JSDocParsingModeParseAll,
}, text, core.GetScriptKindFromFileName(string(pathified)))
ast.SetParentInChildren(sourceFile.AsNode())
edits := format.FormatDocument(ctx, sourceFile)
newText := applyBulkEdits(text, edits)

Expand Down
2 changes: 0 additions & 2 deletions internal/format/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func TestFormat(t *testing.T) {
FileName: "/checker.ts",
Path: "/checker.ts",
}, text, core.ScriptKindTS)
ast.SetParentInChildren(sourceFile.AsNode())
edits := format.FormatDocument(ctx, sourceFile)
newText := applyBulkEdits(text, edits)
assert.Assert(t, len(newText) > 0)
Expand Down Expand Up @@ -89,7 +88,6 @@ func BenchmarkFormat(b *testing.B) {
FileName: "/checker.ts",
Path: "/checker.ts",
}, text, core.ScriptKindTS)
ast.SetParentInChildren(sourceFile.AsNode())

b.Run("format checker.ts", func(b *testing.B) {
for b.Loop() {
Expand Down
8 changes: 0 additions & 8 deletions internal/parser/jsdoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ func (p *Parser) parseJSDocTypeExpression(mayOmitBraces bool) *ast.Node {
}

result := p.factory.NewJSDocTypeExpression(t)
// normally parent references are set during binding. However, for clients that only need
// a syntax tree, and no semantic features, then the binding process is an unnecessary
// overhead. This functions allows us to set all the parents, without all the expense of
// binding.
ast.SetParentInChildren(result)
p.finishNode(result, pos)
return result
}
Expand All @@ -107,7 +102,6 @@ func (p *Parser) parseJSDocNameReference() *ast.Node {
}

result := p.factory.NewJSDocNameReference(entityName)
ast.SetParentInChildren(result)
p.finishNode(result, pos)
return result
}
Expand Down Expand Up @@ -145,7 +139,6 @@ func (p *Parser) parseJSDocComment(parent *ast.Node, start int, end int, fullSta
p.parsingContexts = p.parsingContexts | ParsingContexts(PCJSDocComment)

comment := p.parseJSDocCommentWorker(start, end, fullStart, initialIndent)
comment.Parent = parent
// move jsdoc diagnostics to jsdocDiagnostics -- for JS files only
if p.contextFlags&ast.NodeFlagsJavaScriptFile != 0 {
p.jsdocDiagnostics = append(p.jsdocDiagnostics, p.diagnostics[saveDiagnosticsLength:]...)
Expand Down Expand Up @@ -457,7 +450,6 @@ func (p *Parser) parseTag(tags []*ast.Node, margin int) *ast.Node {
tag = p.parseSeeTag(start, tagName, margin, indentText)
case "import":
tag = p.parseImportTag(start, tagName, margin, indentText)
ast.SetParentInChildren(tag)
default:
tag = p.parseUnknownTag(start, tagName, margin, indentText)
}
Expand Down
9 changes: 9 additions & 0 deletions internal/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,15 @@ func (p *Parser) finishSourceFile(result *ast.SourceFile, isDeclarationFile bool
result.TextCount = p.factory.TextCount()
result.IdentifierCount = p.identifierCount
result.SetJSDocCache(p.jsdocCache)

ast.SetParentInChildren(result.AsNode())
for parent, children := range p.jsdocCache {
for _, child := range children {
child.Parent = parent
ast.SetParentInChildren(child)
}
}

ast.SetExternalModuleIndicator(result, p.opts.ExternalModuleIndicatorOptions)
}

Expand Down
5 changes: 0 additions & 5 deletions internal/parser/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func collectExternalModuleReferences(file *ast.SourceFile) {

if file.Flags&ast.NodeFlagsPossiblyContainsDynamicImport != 0 || ast.IsInJSFile(file.AsNode()) {
ast.ForEachDynamicImportOrRequireCall(file /*includeTypeSpaceImports*/, true /*requireStringLiteralLikeArgument*/, true, func(node *ast.Node, moduleSpecifier *ast.Expression) bool {
ast.SetParentInChildren(node) // we need parent data on imports before the program is fully bound, so we ensure it's set here
ast.SetImportsOfSourceFile(file, append(file.Imports(), moduleSpecifier))
return false
})
Expand All @@ -31,9 +30,6 @@ func collectModuleReferences(file *ast.SourceFile, node *ast.Statement, inAmbien
if moduleNameExpr != nil && ast.IsStringLiteral(moduleNameExpr) {
moduleName := moduleNameExpr.AsStringLiteral().Text
if moduleName != "" && (!inAmbientModule || !tspath.IsExternalModuleNameRelative(moduleName)) {
if node.Kind != ast.KindJSImportDeclaration {
ast.SetParentInChildren(node) // we need parent data on imports before the program is fully bound, so we ensure it's set here
}
ast.SetImportsOfSourceFile(file, append(file.Imports(), moduleNameExpr))
// !!! removed `&& p.currentNodeModulesDepth == 0`
if file.UsesUriStyleNodeCoreModules != core.TSTrue && !file.IsDeclarationFile {
Expand All @@ -50,7 +46,6 @@ func collectModuleReferences(file *ast.SourceFile, node *ast.Statement, inAmbien
return
}
if ast.IsModuleDeclaration(node) && ast.IsAmbientModule(node) && (inAmbientModule || ast.HasSyntacticModifier(node, ast.ModifierFlagsAmbient) || file.IsDeclarationFile) {
ast.SetParentInChildren(node)
nameText := node.AsModuleDeclaration().Name().Text()
// Ambient module declarations can be interpreted as augmentations for some existing external modules.
// This will happen in two cases:
Expand Down
Loading