Skip to content

Make SourceFileAffectingCompilerOptions hold fully computed values, use in binder #788

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.

Sign up for GitHub

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 2 commits into from
Apr 10, 2025
Merged

Make SourceFileAffectingCompilerOptions hold fully computed values, use in binder #788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52892a2
Compute SourceFileAffectingCompilerOptions and use in binder
jakebailey Apr 10, 2025
25b196d
Update internal/project/service_test.go
jakebailey Apr 10, 2025
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
22 changes: 11 additions & 11 deletions internal/binder/binder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const (

type Binder struct {
file *ast.SourceFile
options *core.CompilerOptions
options *core.SourceFileAffectingCompilerOptions
languageVersion core.ScriptTarget
bindFunc func(*ast.Node) bool
unreachableFlow *ast.FlowNode
Expand Down Expand Up @@ -86,7 +86,7 @@ type ActiveLabel struct {
func (label *ActiveLabel) BreakTarget() *ast.FlowNode { return label.breakTarget }
func (label *ActiveLabel) ContinueTarget() *ast.FlowNode { return label.continueTarget }

func BindSourceFile(file *ast.SourceFile, options *core.CompilerOptions) {
func BindSourceFile(file *ast.SourceFile, options *core.SourceFileAffectingCompilerOptions) {
// This is constructed this way to make the compiler "out-line" the function,
// avoiding most work in the common case where the file has already been bound.
if !file.IsBound() {
Expand All @@ -111,14 +111,14 @@ func putBinder(b *Binder) {
binderPool.Put(b)
}

func bindSourceFile(file *ast.SourceFile, options *core.CompilerOptions) {
func bindSourceFile(file *ast.SourceFile, options *core.SourceFileAffectingCompilerOptions) {
file.BindOnce(func() {
b := getBinder()
defer putBinder(b)
b.file = file
b.options = options
b.languageVersion = options.GetEmitScriptTarget()
b.inStrictMode = (options.AlwaysStrict.IsTrue() || options.Strict.IsTrue()) && !file.IsDeclarationFile || ast.IsExternalModule(file)
b.languageVersion = options.EmitScriptTarget
b.inStrictMode = options.BindInStrictMode && !file.IsDeclarationFile || ast.IsExternalModule(file)
b.unreachableFlow = b.newFlowNode(ast.FlowFlagsUnreachable)
b.reportedUnreachableFlow = b.newFlowNode(ast.FlowFlagsUnreachable)
b.bind(file.AsNode())
Expand Down Expand Up @@ -1331,7 +1331,7 @@ func (b *Binder) checkStrictModeWithStatement(node *ast.Node) {

func (b *Binder) checkStrictModeLabeledStatement(node *ast.Node) {
// Grammar checking for labeledStatement
if b.inStrictMode && b.options.Target >= core.ScriptTargetES2015 {
if b.inStrictMode && b.options.EmitScriptTarget >= core.ScriptTargetES2015 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a porting bug I'm fixing, FWIW.

data := node.AsLabeledStatement()
if ast.IsDeclarationStatement(data.Statement) || ast.IsVariableStatement(data.Statement) {
b.errorOnFirstToken(data.Label, diagnostics.A_label_is_not_allowed_here)
Expand Down Expand Up @@ -1665,7 +1665,7 @@ func (b *Binder) checkUnreachable(node *ast.Node) bool {

func (b *Binder) shouldReportErrorOnModuleDeclaration(node *ast.Node) bool {
instanceState := ast.GetModuleInstanceState(node)
return instanceState == ast.ModuleInstanceStateInstantiated || (instanceState == ast.ModuleInstanceStateConstEnumOnly && b.options.ShouldPreserveConstEnums())
return instanceState == ast.ModuleInstanceStateInstantiated || (instanceState == ast.ModuleInstanceStateConstEnumOnly && b.options.ShouldPreserveConstEnums)
}

func (b *Binder) errorOnEachUnreachableRange(node *ast.Node, isError bool) {
Expand Down Expand Up @@ -2421,8 +2421,8 @@ func (b *Binder) bindInitializer(node *ast.Node) {
b.currentFlow = b.finishFlowLabel(exitFlow)
}

func isEnumDeclarationWithPreservedEmit(node *ast.Node, options *core.CompilerOptions) bool {
return node.Kind == ast.KindEnumDeclaration && (!ast.IsEnumConst(node) || options.ShouldPreserveConstEnums())
func isEnumDeclarationWithPreservedEmit(node *ast.Node, options *core.SourceFileAffectingCompilerOptions) bool {
return node.Kind == ast.KindEnumDeclaration && (!ast.IsEnumConst(node) || options.ShouldPreserveConstEnums)
}

func setFlowNode(node *ast.Node, flowNode *ast.FlowNode) {
Expand Down Expand Up @@ -2746,11 +2746,11 @@ func isFunctionSymbol(symbol *ast.Symbol) bool {
return false
}

func unreachableCodeIsError(options *core.CompilerOptions) bool {
func unreachableCodeIsError(options *core.SourceFileAffectingCompilerOptions) bool {
return options.AllowUnreachableCode == core.TSFalse
}

func unusedLabelIsError(options *core.CompilerOptions) bool {
func unusedLabelIsError(options *core.SourceFileAffectingCompilerOptions) bool {
return options.AllowUnusedLabels == core.TSFalse
}

Expand Down
3 changes: 2 additions & 1 deletion internal/binder/binder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func BenchmarkBind(b *testing.B) {
}

compilerOptions := &core.CompilerOptions{Target: core.ScriptTargetESNext, ModuleKind: core.ModuleKindNodeNext}
sourceAffecting := compilerOptions.SourceFileAffecting()

// The above parses do a lot of work; ensure GC is finished before we start collecting performance data.
// GC must be called twice to allow things to settle.
Expand All @@ -36,7 +37,7 @@ func BenchmarkBind(b *testing.B) {

b.ResetTimer()
for i := range b.N {
BindSourceFile(sourceFiles[i], compilerOptions)
BindSourceFile(sourceFiles[i], sourceAffecting)
}
})
}
Expand Down
14 changes: 12 additions & 2 deletions internal/compiler/program.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ type Program struct {
currentDirectory string
configFileParsingDiagnostics []*ast.Diagnostic

sourceAffectingCompilerOptionsOnce sync.Once
sourceAffectingCompilerOptions *core.SourceFileAffectingCompilerOptions

resolver *module.Resolver

comparePathsOptions tspath.ComparePathsOptions
Expand Down Expand Up @@ -184,12 +187,19 @@ func (p *Program) GetConfigFileParsingDiagnostics() []*ast.Diagnostic {
return slices.Clip(p.configFileParsingDiagnostics)
}

func (p *Program) getSourceAffectingCompilerOptions() *core.SourceFileAffectingCompilerOptions {
p.sourceAffectingCompilerOptionsOnce.Do(func() {
p.sourceAffectingCompilerOptions = p.compilerOptions.SourceFileAffecting()
})
return p.sourceAffectingCompilerOptions
}

func (p *Program) BindSourceFiles() {
wg := core.NewWorkGroup(p.programOptions.SingleThreaded)
for _, file := range p.files {
if !file.IsBound() {
wg.Queue(func() {
binder.BindSourceFile(file, p.compilerOptions)
binder.BindSourceFile(file, p.getSourceAffectingCompilerOptions())
})
}
}
Expand Down Expand Up @@ -388,7 +398,7 @@ func SortAndDeduplicateDiagnostics(diagnostics []*ast.Diagnostic) []*ast.Diagnos
func (p *Program) getDiagnosticsHelper(sourceFile *ast.SourceFile, ensureBound bool, ensureChecked bool, getDiagnostics func(*ast.SourceFile) []*ast.Diagnostic) []*ast.Diagnostic {
if sourceFile != nil {
if ensureBound {
binder.BindSourceFile(sourceFile, p.compilerOptions)
binder.BindSourceFile(sourceFile, p.getSourceAffectingCompilerOptions())
}
return SortAndDeduplicateDiagnostics(getDiagnostics(sourceFile))
}
Expand Down
41 changes: 16 additions & 25 deletions internal/core/compileroptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,34 +269,25 @@ func (options *CompilerOptions) HasJsonModuleEmitEnabled() bool {
return true
}

// SourceFileAffectingCompilerOptions are the CompilerOptions values that when
// changed require a new SourceFile be created.
// SourceFileAffectingCompilerOptions are the precomputed CompilerOptions values which
// affect the parse and bind of a source file.
type SourceFileAffectingCompilerOptions struct {
// !!! generate this
Target ScriptTarget
Jsx JsxEmit
JsxImportSource string
ImportHelpers Tristate
AlwaysStrict Tristate
ModuleDetection ModuleDetectionKind
AllowUnreachableCode Tristate
AllowUnusedLabels Tristate
PreserveConstEnums Tristate
IsolatedModules Tristate
AllowUnreachableCode Tristate
AllowUnusedLabels Tristate
BindInStrictMode bool
EmitScriptTarget ScriptTarget
NoFallthroughCasesInSwitch Tristate
ShouldPreserveConstEnums bool
}

func (options *CompilerOptions) SourceFileAffecting() SourceFileAffectingCompilerOptions {
return SourceFileAffectingCompilerOptions{
Target: options.Target,
Jsx: options.Jsx,
JsxImportSource: options.JsxImportSource,
ImportHelpers: options.ImportHelpers,
AlwaysStrict: options.AlwaysStrict,
ModuleDetection: options.ModuleDetection,
AllowUnreachableCode: options.AllowUnreachableCode,
AllowUnusedLabels: options.AllowUnusedLabels,
PreserveConstEnums: options.PreserveConstEnums,
IsolatedModules: options.IsolatedModules,
func (options *CompilerOptions) SourceFileAffecting() *SourceFileAffectingCompilerOptions {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not cached anywhere since we'd have to put it on CompilerOptions, which means sync.Once, which means no more DeepEqual.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this should continue to be a value type or not, since we pass it around and so on. But it's not that large, actually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and it keeping it a value doesn't actually hurt or help perf, so I could do that.

return &SourceFileAffectingCompilerOptions{
AllowUnreachableCode: options.AllowUnreachableCode,
AllowUnusedLabels: options.AllowUnusedLabels,
BindInStrictMode: options.AlwaysStrict.IsTrue() || options.Strict.IsTrue(),
EmitScriptTarget: options.GetEmitScriptTarget(),
NoFallthroughCasesInSwitch: options.NoFallthroughCasesInSwitch,
ShouldPreserveConstEnums: options.ShouldPreserveConstEnums(),
}
}

Expand Down
44 changes: 23 additions & 21 deletions internal/printer/namegenerator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"gotest.tools/v3/assert"
)

var defaultSourceFileAffectingOptions = (&core.CompilerOptions{}).SourceFileAffecting()

func TestTempVariable1(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -257,7 +259,7 @@ func TestGeneratedNameForIdentifier1(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("function f() {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].Name()
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -274,7 +276,7 @@ func TestGeneratedNameForIdentifier2(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("function f() {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].Name()
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{
Expand All @@ -294,7 +296,7 @@ func TestGeneratedNameForIdentifier3(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("function f() {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].Name()
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{
Expand All @@ -316,7 +318,7 @@ func TestGeneratedNameForNamespace1(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("namespace foo { }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

ns1 := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(ns1, printer.AutoGenerateOptions{})
Expand All @@ -334,7 +336,7 @@ func TestGeneratedNameForNamespace2(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("namespace foo { var foo; }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

ns1 := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(ns1, printer.AutoGenerateOptions{})
Expand All @@ -352,7 +354,7 @@ func TestGeneratedNameForNamespace3(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("namespace ns1 { namespace foo { var foo; } } namespace ns2 { namespace foo { var foo; } }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

ns1 := file.Statements.Nodes[0].AsModuleDeclaration().Body.AsModuleBlock().Statements.Nodes[0]
ns2 := file.Statements.Nodes[1].AsModuleDeclaration().Body.AsModuleBlock().Statements.Nodes[0]
Expand All @@ -374,7 +376,7 @@ func TestGeneratedNameForNamespace4(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("namespace ns1 { namespace foo { var foo; } } namespace ns2 { namespace foo { var foo; } }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

ns1 := file.Statements.Nodes[0].AsModuleDeclaration().Body.AsModuleBlock().Statements.Nodes[0]
ns2 := file.Statements.Nodes[1].AsModuleDeclaration().Body.AsModuleBlock().Statements.Nodes[0]
Expand All @@ -400,7 +402,7 @@ func TestGeneratedNameForNodeCached(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("namespace foo { var foo; }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

ns1 := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(ns1, printer.AutoGenerateOptions{})
Expand All @@ -420,7 +422,7 @@ func TestGeneratedNameForImport(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("import * as foo from 'foo'", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -437,7 +439,7 @@ func TestGeneratedNameForExport(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export * as foo from 'foo'", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -454,7 +456,7 @@ func TestGeneratedNameForFunctionDeclaration1(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export function f() {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -471,7 +473,7 @@ func TestGeneratedNameForFunctionDeclaration2(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export default function () {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -488,7 +490,7 @@ func TestGeneratedNameForClassDeclaration1(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export class C {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -505,7 +507,7 @@ func TestGeneratedNameForClassDeclaration2(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export default class {}", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -522,7 +524,7 @@ func TestGeneratedNameForExportAssignment(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("export default 0", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -539,7 +541,7 @@ func TestGeneratedNameForClassExpression(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("(class {})", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].AsExpressionStatement().Expression.AsParenthesizedExpression().Expression
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -556,7 +558,7 @@ func TestGeneratedNameForMethod1(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("class C { m() {} }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].AsClassDeclaration().Members.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -573,7 +575,7 @@ func TestGeneratedNameForMethod2(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("class C { 0() {} }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].AsClassDeclaration().Members.Nodes[0]
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -590,7 +592,7 @@ func TestGeneratedPrivateNameForMethod(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("class C { m() {} }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].AsClassDeclaration().Members.Nodes[0]
name1 := ec.NewGeneratedPrivateNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -607,7 +609,7 @@ func TestGeneratedNameForComputedPropertyName(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("class C { [x] }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := file.Statements.Nodes[0].AsClassDeclaration().Members.Nodes[0].Name()
name1 := ec.NewGeneratedNameForNode(n, printer.AutoGenerateOptions{})
Expand All @@ -624,7 +626,7 @@ func TestGeneratedNameForOther(t *testing.T) {
ec := printer.NewEmitContext()

file := parsetestutil.ParseTypeScript("class C { [x] }", false /*jsx*/)
binder.BindSourceFile(file, &core.CompilerOptions{})
binder.BindSourceFile(file, defaultSourceFileAffectingOptions)

n := ec.Factory.NewObjectLiteralExpression(
ec.Factory.NewNodeList([]*ast.Node{}),
Expand Down
2 changes: 1 addition & 1 deletion internal/project/documentregistry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type registryKey struct {

func newRegistryKey(options *core.CompilerOptions, path tspath.Path, scriptKind core.ScriptKind) registryKey {
return registryKey{
SourceFileAffectingCompilerOptions: options.SourceFileAffecting(),
SourceFileAffectingCompilerOptions: *options.SourceFileAffecting(),
path: path,
scriptKind: scriptKind,
}
Expand Down
Loading