From 1b7822a68822e2814437a0d17f72ebd99f024a86 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:09:19 -0700 Subject: [PATCH 01/10] Add the workspace diagnostics settings The settings a workspace pull reads, ahead of anything that reads them. experimental.workspaceDiagnostics.scope decides which projects a pull reports on, and is off by default: the pull checks projects the editor would otherwise never load, so it is opt-in. serverDiagnosticsDeDuplication lets a client that does not pull documents separately ask for open documents to be included, which a client that pulls both would otherwise show twice. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/vscode-typescript/package.json | 30 ++++++++++++++ packages/vscode-typescript/package.nls.json | 6 +++ .../diagnosticMessages.generated.json | 1 + .../diagnostics/diagnosticMessages.json | 4 ++ .../diagnostics/diagnostics_generated.go | 3 ++ tsc/internal/ls/lsutil/userpreferences.go | 40 +++++++++++++++++++ tsc/internal/lsp/lsproto/lsp.go | 9 +++++ .../state/codeLensAcrossProjects.baseline | 5 +++ 8 files changed, 98 insertions(+) diff --git a/packages/vscode-typescript/package.json b/packages/vscode-typescript/package.json index c14d0a09749ba..78d69c9b8efa1 100644 --- a/packages/vscode-typescript/package.json +++ b/packages/vscode-typescript/package.json @@ -220,6 +220,36 @@ ], "description": "%native-preview.trackFlakyDiagnostics.description%", "scope": "window" + }, + "js/ts.experimental.workspaceDiagnostics.scope": { + "type": "string", + "enum": [ + "off", + "openProjects", + "openProjectsAndDependents", + "allProjects" + ], + "enumDescriptions": [ + "%native-preview.workspaceDiagnostics.off%", + "%native-preview.workspaceDiagnostics.openProjects%", + "%native-preview.workspaceDiagnostics.openProjectsAndDependents%", + "%native-preview.workspaceDiagnostics.allProjects%" + ], + "default": "off", + "tags": [ + "experimental" + ], + "description": "%native-preview.workspaceDiagnostics.description%", + "scope": "window" + }, + "js/ts.experimental.workspaceDiagnostics.serverDiagnosticsDeDuplication": { + "type": "boolean", + "default": true, + "tags": [ + "experimental" + ], + "description": "%native-preview.workspaceDiagnostics.serverDiagnosticsDeDuplication.description%", + "scope": "window" } } } diff --git a/packages/vscode-typescript/package.nls.json b/packages/vscode-typescript/package.nls.json index 437a36410004f..8736f8bca2219 100644 --- a/packages/vscode-typescript/package.nls.json +++ b/packages/vscode-typescript/package.nls.json @@ -41,5 +41,11 @@ "native-preview.trackFlakyDiagnostics.log": "Log an error when a flaky diagnostic is detected.", "native-preview.trackFlakyDiagnostics.never": "Never perform flaky diagnostic checking and logging.", "native-preview.trackFlakyDiagnostics.auto": "Perform flaky diagnostic logging only on VS Code Insiders.", + "native-preview.workspaceDiagnostics.description": "Controls how much of the workspace is checked for errors, including files that are not open. Checking whole projects is expensive.", + "native-preview.workspaceDiagnostics.serverDiagnosticsDeDuplication.description": "Leave a file out of workspace diagnostics while it is open, because the editor reports open files separately and would otherwise show every problem in them twice. Turn this off only for a client that does not request diagnostics per document.", + "native-preview.workspaceDiagnostics.off": "Only report errors in open files.", + "native-preview.workspaceDiagnostics.openProjects": "Report errors in every file of the projects that contain an open file.", + "native-preview.workspaceDiagnostics.openProjectsAndDependents": "Also report errors in the projects that reference those projects.", + "native-preview.workspaceDiagnostics.allProjects": "Report errors in every project in the workspace.", "developer": "Developer" } diff --git a/tsc/internal/diagnostics/diagnosticMessages.generated.json b/tsc/internal/diagnostics/diagnosticMessages.generated.json index 477270535471d..aa53029ffa553 100644 --- a/tsc/internal/diagnostics/diagnosticMessages.generated.json +++ b/tsc/internal/diagnostics/diagnosticMessages.generated.json @@ -1942,6 +1942,7 @@ "The_content_mapper_returned_diagnostic_directives_with_overlapping_virtual_ranges_18108": "The content mapper returned diagnostic directives with overlapping virtual ranges.", "The_invalid_diagnostic_directive_is_in_supplemental_output_0_returned_by_the_content_mapper_18109": "The invalid diagnostic directive is in supplemental output {0} returned by the content mapper.", "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_18110": "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'.", + "Checking_workspace_18111": "Checking workspace", "nodenext_if_module_is_nodenext_node16_if_module_is_node16_or_node18_otherwise_bundler_69010": "`nodenext` if `module` is `nodenext`; `node16` if `module` is `node16` or `node18`; otherwise, `bundler`.", "File_is_a_CommonJS_module_it_may_be_converted_to_an_ES_module_80001": "File is a CommonJS module; it may be converted to an ES module.", "This_constructor_function_may_be_converted_to_a_class_declaration_80002": "This constructor function may be converted to a class declaration.", diff --git a/tsc/internal/diagnostics/diagnosticMessages.json b/tsc/internal/diagnostics/diagnosticMessages.json index 44535fc9b2b6e..f2826c8a61895 100644 --- a/tsc/internal/diagnostics/diagnosticMessages.json +++ b/tsc/internal/diagnostics/diagnosticMessages.json @@ -8887,5 +8887,9 @@ "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'.": { "category": "Message", "code": 18110 + }, + "Checking workspace": { + "category": "Message", + "code": 18111 } } diff --git a/tsc/internal/diagnostics/diagnostics_generated.go b/tsc/internal/diagnostics/diagnostics_generated.go index 539eb35d05c99..6fa1d20ece696 100644 --- a/tsc/internal/diagnostics/diagnostics_generated.go +++ b/tsc/internal/diagnostics/diagnostics_generated.go @@ -3888,6 +3888,8 @@ var The_invalid_diagnostic_directive_is_in_supplemental_output_0_returned_by_the var Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex = &Message{code: 18110, category: CategoryMessage, key: "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_18110", text: "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'."} +var Checking_workspace = &Message{code: 18111, category: CategoryMessage, key: "Checking_workspace_18111", text: "Checking workspace"} + var X_nodenext_if_module_is_nodenext_node16_if_module_is_node16_or_node18_otherwise_bundler = &Message{code: 69010, category: CategoryMessage, key: "nodenext_if_module_is_nodenext_node16_if_module_is_node16_or_node18_otherwise_bundler_69010", text: "`nodenext` if `module` is `nodenext`; `node16` if `module` is `node16` or `node18`; otherwise, `bundler`."} var File_is_a_CommonJS_module_it_may_be_converted_to_an_ES_module = &Message{code: 80001, category: CategorySuggestion, key: "File_is_a_CommonJS_module_it_may_be_converted_to_an_ES_module_80001", text: "File is a CommonJS module; it may be converted to an ES module."} @@ -6372,6 +6374,7 @@ var allMessages = [...]**Message{ &The_content_mapper_returned_diagnostic_directives_with_overlapping_virtual_ranges, &The_invalid_diagnostic_directive_is_in_supplemental_output_0_returned_by_the_content_mapper, &Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex, + &Checking_workspace, &X_nodenext_if_module_is_nodenext_node16_if_module_is_node16_or_node18_otherwise_bundler, &File_is_a_CommonJS_module_it_may_be_converted_to_an_ES_module, &This_constructor_function_may_be_converted_to_a_class_declaration, diff --git a/tsc/internal/ls/lsutil/userpreferences.go b/tsc/internal/ls/lsutil/userpreferences.go index bf9273b7ad760..053cdf594490a 100644 --- a/tsc/internal/ls/lsutil/userpreferences.go +++ b/tsc/internal/ls/lsutil/userpreferences.go @@ -33,6 +33,7 @@ func NewDefaultUserPreferences() UserPreferences { ExcludeLibrarySymbolsInNavTo: core.TSTrue, WorkspaceSymbolsScope: WorkspaceSymbolsScopeAllOpenProjects, + WorkspaceDiagnosticsScope: WorkspaceDiagnosticsScopeOff, } } @@ -171,6 +172,19 @@ type UserPreferences struct { ExcludeLibrarySymbolsInNavTo core.Tristate `raw:"excludeLibrarySymbolsInNavTo" config:"workspaceSymbols.excludeLibrarySymbols"` WorkspaceSymbolsScope WorkspaceSymbolsScope `config:"workspaceSymbols.scope"` + // ------- Diagnostics ------- + + // How much of the workspace a `workspace/diagnostic` pull reports on. Off unless asked for; + // the server only offers the capability once it is set to something else. + WorkspaceDiagnosticsScope WorkspaceDiagnosticsScope `config:"experimental.workspaceDiagnostics.scope"` + // Whether the server keeps a document out of workspace reports while the client has it open. + // A client that pulls both kinds of diagnostics holds the results of each provider in its own + // collection and reconciles only within one, so a document reported by both appears twice; the + // server leaves open documents out to spare it that. A client that only pulls workspace + // diagnostics has nothing to collide with and would otherwise never hear about the documents it + // has open, so it turns this off. On unless set. + WorkspaceDiagnosticsServerDiagnosticsDeDuplication core.Tristate `config:"experimental.workspaceDiagnostics.serverDiagnosticsDeDuplication"` + // ------- Misc ------- EnableFormatting core.Tristate `raw:"formatEnabled" config:"format.enabled" fallbackConfig:"format.enable"` @@ -236,6 +250,32 @@ const ( WorkspaceSymbolsScopeCurrentProject WorkspaceSymbolsScope = "currentProject" ) +type WorkspaceDiagnosticsScope string + +const ( + // The default: nothing is reported and the capability is not offered. + WorkspaceDiagnosticsScopeOff WorkspaceDiagnosticsScope = "off" + // Projects that contain an open file. + WorkspaceDiagnosticsScopeOpenProjects WorkspaceDiagnosticsScope = "openProjects" + // Also the projects that reference them, so an edit surfaces breakage in consumers. + WorkspaceDiagnosticsScopeOpenProjectsAndDependents WorkspaceDiagnosticsScope = "openProjectsAndDependents" + // Every project in the workspace. + WorkspaceDiagnosticsScopeAllProjects WorkspaceDiagnosticsScope = "allProjects" +) + +// Enabled reports whether the scope asks for any workspace diagnostics. Unrecognized values are +// treated as off, so a typo cannot start a workspace-wide check. +func (s WorkspaceDiagnosticsScope) Enabled() bool { + switch s { + case WorkspaceDiagnosticsScopeOpenProjects, + WorkspaceDiagnosticsScopeOpenProjectsAndDependents, + WorkspaceDiagnosticsScopeAllProjects: + return true + default: + return false + } +} + const ( QuotePreferenceUnknown QuotePreference = "" QuotePreferenceAuto QuotePreference = "auto" diff --git a/tsc/internal/lsp/lsproto/lsp.go b/tsc/internal/lsp/lsproto/lsp.go index 6624fd554547a..89c195e5696c0 100644 --- a/tsc/internal/lsp/lsproto/lsp.go +++ b/tsc/internal/lsp/lsproto/lsp.go @@ -224,6 +224,15 @@ func (info NotificationInfo[Params]) NewNotificationMessage(params Params) *Requ } } +// WorkspaceDiagnosticPartialResultParams carries one chunk of a streamed `workspace/diagnostic` +// result. The generated [ProgressParams] narrows `value` to work done progress. +type WorkspaceDiagnosticPartialResultParams struct { + Token IntegerOrString `json:"token"` + Value WorkspaceDiagnosticReportPartialResult `json:"value"` +} + +var WorkspaceDiagnosticPartialResultInfo = NotificationInfo[*WorkspaceDiagnosticPartialResultParams]{Method: MethodProgress} + // UnmarshalParams decodes the params of an inbound request or notification // message into the requested type. Inbound messages store their params as a // raw [json.Value] (see [Message.UnmarshalJSON]); decoding is deferred to the diff --git a/tsc/testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline b/tsc/testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline index 38556f9e2e9df..fbaa0c13ab41d 100644 --- a/tsc/testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline +++ b/tsc/testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline @@ -591,6 +591,11 @@ Config:: "autoClosingTags": { "enabled": true }, + "experimental": { + "workspaceDiagnostics": { + "scope": "off" + } + }, "format": { "convertTabsToSpaces": true, "enabled": true, From b08511bb82ad1e21fda0f98a5899d0bcea52769a Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:10:50 -0700 Subject: [PATCH 02/10] Check a project across several diagnostics checkers A project's checkers were built for individual requests: one for diagnostics, a handful for queries, so a whole project was checked a file at a time on one checker. A project's diagnostics now run on as many checkers as a build of the program would use. A whole-program check shares its files between them from one queue, each checker taking the next file as it finishes the last, and a file is remembered against the checker that checked it, so a later pull on it goes back to where its types already are. Query checkers keep their own slots after them, since a query does not depend on which checker answers it and should not queue behind a check of the whole project. The program hands a whole-program check to a pool that runs one itself, and otherwise checks file by file as before. The compiler's own pool is unchanged. Each checker is taken for one file at a time rather than for the whole check. That costs an acquisition per file and buys the ability to let something else in between them, which the next changes need. The checkers are handed back when a caller is done with them. They hold the types of every file they reached, and keeping them buys nothing: a later pull that finds the project unchanged answers from what the client already holds without checking, and one that finds it changed needs new checkers anyway. Only a workspace pull checks a project this way, so the extra checkers are held only when that is switched on. Without it a project keeps the single diagnostics checker it has always had, and does not pay for the rest. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/compiler/program.go | 19 ++ tsc/internal/project/checkerpool.go | 273 +++++++++++++----- tsc/internal/project/checkerpool_test.go | 338 +++++++++++++++++++++-- tsc/internal/project/project.go | 4 +- tsc/internal/project/session.go | 7 + tsc/internal/project/snapshot.go | 11 + 6 files changed, 558 insertions(+), 94 deletions(-) diff --git a/tsc/internal/compiler/program.go b/tsc/internal/compiler/program.go index c81fb423908d0..cf033c2fa981e 100644 --- a/tsc/internal/compiler/program.go +++ b/tsc/internal/compiler/program.go @@ -716,6 +716,12 @@ func filterAndSortDiagnostics(diags []*ast.Diagnostic) []*ast.Diagnostic { })) } +// wholeProgramCheckerPool is a CheckerPool that runs a check of many files itself, so that the pool, +// rather than the program, decides which checker takes each file. +type wholeProgramCheckerPool interface { + ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) +} + // collectCheckerDiagnosticsFromFiles collects checker diagnostics for a list of files. func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, sourceFiles []*ast.SourceFile, collect func(context.Context, *checker.Checker, *ast.SourceFile) []*ast.Diagnostic) [][]*ast.Diagnostic { diagnostics := make([][]*ast.Diagnostic, len(sourceFiles)) @@ -723,6 +729,19 @@ func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, source p.compilerCheckerPool.forEachCheckerGroupDo(ctx, sourceFiles, p.SingleThreaded(), func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { diagnostics[fileIndex] = collect(ctx, c, file) }) + } else if pool, ok := p.checkerPool.(wholeProgramCheckerPool); ok { + files := make([]*ast.SourceFile, 0, len(sourceFiles)) + indices := make([]int, 0, len(sourceFiles)) + for i, file := range sourceFiles { + if p.SkipTypeChecking(file, false) { + continue + } + files = append(files, file) + indices = append(indices, i) + } + pool.ForEachCheckerGroupDo(ctx, files, p.SingleThreaded(), func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { + diagnostics[indices[fileIndex]] = collect(ctx, c, file) + }) } else { wg := core.NewWorkGroup(p.SingleThreaded()) for i, file := range sourceFiles { diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index bed324f6cc3a5..a93faea0a3e36 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" "sync" + "sync/atomic" "time" "github.com/microsoft/TypeScript/tsc/internal/ast" @@ -20,20 +21,24 @@ import ( const checkerHeldAnonymous = "" type CheckerPoolOptions struct { - // MaxCheckers controls the total number of checker slots per project - // (1 dedicated diagnostics checker + N-1 query checkers). Minimum 2. - // Zero uses the default (4). + // MaxCheckers bounds the query checkers a project keeps, at MaxCheckers-1 of them. Minimum 2. + // Zero uses the default (4). Diagnostics checkers are counted separately; see + // MatchBuildCheckerCount. MaxCheckers int // IdleTimeout controls how long an idle checker is kept // before being disposed. Zero uses the default (30s). IdleTimeout time.Duration + // MatchBuildCheckerCount makes diagnostics run on as many checkers as a build would use. Only a + // workspace pull checks a project that way; without it a project keeps one diagnostics checker + // and does not pay for the rest. + MatchBuildCheckerCount bool } // checkerPool manages three categories of type checkers for a project: // -// - Diagnostics (index 0): A single checker for LSP diagnostics, providing -// consistent walk order. Idle-cleaned. -// - Temporary (indices 1+): Ephemeral query checkers for LSP operations. +// - Diagnostics (indices 0 to diagnosticsCount-1): as many checkers as a build of this program +// would use. A file goes back to the one that last checked it. Idle-cleaned. +// - Temporary (indices diagnosticsCount+): Ephemeral query checkers for LSP operations. // Idle-cleaned after a configurable timeout. // - API: A single checker for API operations, providing stable // instance identity for reference equality on type/symbol handles. @@ -49,13 +54,19 @@ type checkerPool struct { // query checkers are not disposed until the pool is GC'd. discarded bool - // checkers[0] is the diagnostics checker. - // checkers[1:] are ephemeral query checkers. + // diagnosticsCount is how many checkers a build of this program would check it with. + diagnosticsCount int + // diagnosticsAffinity is the diagnostics checker that last checked each file, so the next check + // of it goes where its types already are. Guarded by mu. + diagnosticsAffinity map[*ast.SourceFile]int + + // checkers[:diagnosticsCount] are the diagnostics checkers. + // checkers[diagnosticsCount:] are ephemeral query checkers. // All are idle-cleaned. checkers []*checker.Checker heldBy []string // heldBy[i] is the requestID holding checker i, checkerHeldAnonymous, or "" if not held - fileAssociations map[*ast.SourceFile]int // file → query checker index (1+) - requestAssociations map[string]int // requestID → checker index + fileAssociations map[*ast.SourceFile]int // file → query checker index + requestAssociations map[string]int // requestID → query checker index // lastReleased tracks when each checker was last released. lastReleased []time.Time @@ -69,7 +80,9 @@ type checkerPool struct { persistentChecker *checker.Checker persistentHeld bool - diagSem chan struct{} + // Which diagnostics checker serves a request is decided by the file, so they get a slot each + // rather than sharing a counting semaphore the way the interchangeable query checkers do. + diagSems []chan struct{} querySem chan struct{} persistentSem chan struct{} @@ -85,25 +98,37 @@ func newCheckerPool(opts CheckerPoolOptions, program *compiler.Program, log func if opts.MaxCheckers <= 0 { opts.MaxCheckers = 4 } else if opts.MaxCheckers < 2 { - opts.MaxCheckers = 2 // at least 1 diagnostics + 1 query checker + opts.MaxCheckers = 2 // at least the diagnostics slot + 1 query checker } if opts.IdleTimeout <= 0 { opts.IdleTimeout = 30 * time.Second } querySlots := opts.MaxCheckers - 1 + // More than one only pays for itself when whole projects are checked. + diagnosticsCount := 1 + if opts.MatchBuildCheckerCount { + diagnosticsCount = diagnosticsCheckerCount(program) + } + diagSems := make([]chan struct{}, diagnosticsCount) + for i := range diagSems { + diagSems[i] = make(chan struct{}, 1) + } + slots := diagnosticsCount + querySlots pool := &checkerPool{ program: program, opts: opts, - checkers: make([]*checker.Checker, opts.MaxCheckers), - heldBy: make([]string, opts.MaxCheckers), + diagnosticsCount: diagnosticsCount, + checkers: make([]*checker.Checker, slots), + heldBy: make([]string, slots), fileAssociations: make(map[*ast.SourceFile]int), + diagnosticsAffinity: make(map[*ast.SourceFile]int), requestAssociations: make(map[string]int), - lastReleased: make([]time.Time, opts.MaxCheckers), - diagSem: make(chan struct{}, 1), + lastReleased: make([]time.Time, slots), + diagSems: diagSems, querySem: make(chan struct{}, querySlots), persistentSem: make(chan struct{}, 1), log: log, - globalDiagCheckerCount: make([]int, opts.MaxCheckers), + globalDiagCheckerCount: make([]int, slots), } if pool.log == nil { @@ -134,7 +159,7 @@ func (p *checkerPool) GetChecker(ctx context.Context, file *ast.SourceFile) (*ch switch lifetime { case core.CheckerLifetimeDiagnostics: - return p.getDiagnosticsChecker(ctx, requestID) + return p.getDiagnosticsChecker(requestID, file) case core.CheckerLifetimeAPI: return p.getPersistentChecker() default: @@ -143,18 +168,14 @@ func (p *checkerPool) GetChecker(ctx context.Context, file *ast.SourceFile) (*ch } // tryReacquireForRequest checks whether the given request already has an -// associated checker. If so, it either returns the checker directly (still held) -// or reacquires it by claiming a semaphore slot. The caller must provide the -// appropriate semaphore channel and indicate whether this is a diagnostics -// request (isDiag). If the associated checker is in the wrong category -// (e.g. a diagnostics index for a query request), the association is deleted -// and normal acquisition proceeds. +// associated query checker. If so, it either returns the checker directly (still +// held) or reacquires it by claiming a semaphore slot. // // Returns (checker, release, true) if the request was served (either still held // or reclaimed). Returns (nil, nil, false) if the caller must proceed with // normal acquisition — in this case, a semaphore slot has already been claimed. // Must NOT be called with p.mu held. -func (p *checkerPool) tryReacquireForRequest(requestID string, sem chan<- struct{}, isDiag bool) (*checker.Checker, func(), bool) { +func (p *checkerPool) tryReacquireForRequest(requestID string, sem chan<- struct{}) (*checker.Checker, func(), bool) { if requestID == "" { sem <- struct{}{} return nil, nil, false @@ -168,15 +189,6 @@ func (p *checkerPool) tryReacquireForRequest(requestID string, sem chan<- struct return nil, nil, false } - // Validate that the associated index matches the expected category. - // Index 0 is for diagnostics; indices 1+ are for queries. - if (isDiag && index != 0) || (!isDiag && index == 0) { - delete(p.requestAssociations, requestID) - p.mu.Unlock() - sem <- struct{}{} - return nil, nil, false - } - c := p.checkers[index] if c == nil { delete(p.requestAssociations, requestID) @@ -215,42 +227,145 @@ func (p *checkerPool) tryReacquireForRequest(requestID string, sem chan<- struct return nil, nil, false } -// getDiagnosticsChecker returns the dedicated diagnostics checker (index 0). -// Creates it on first use. Blocks on diagSem if it's currently in use. -func (p *checkerPool) getDiagnosticsChecker(ctx context.Context, requestID string) (*checker.Checker, func()) { - const diagIndex = 0 +// diagnosticsCheckerCount is how many checkers a build of this program would check it with: the +// program's checkers option, four by default, or one when single threaded. +func diagnosticsCheckerCount(program *compiler.Program) int { + count := 4 + if program.SingleThreaded() { + count = 1 + } else if c := program.Options().Checkers; c != nil { + count = *c + } + return max(min(count, len(program.SourceFiles()), 256), 1) +} - if c, release, ok := p.tryReacquireForRequest(requestID, p.diagSem, true); ok { - return c, release +// diagnosticsIndexFor returns the diagnostics checker that last checked a file, or the first one +// for a file none has. +func (p *checkerPool) diagnosticsIndexFor(file *ast.SourceFile) int { + if file == nil || p.diagnosticsCount == 1 { + return 0 } + p.mu.Lock() + defer p.mu.Unlock() + return p.diagnosticsAffinity[file] +} - // Token consumed — proceed with normal acquisition. +// noteDiagnosticsAffinity records which diagnostics checker checked a file. +func (p *checkerPool) noteDiagnosticsAffinity(file *ast.SourceFile, index int) { + if file == nil || p.diagnosticsCount == 1 { + return + } p.mu.Lock() defer p.mu.Unlock() + p.diagnosticsAffinity[file] = index +} - if p.checkers[diagIndex] == nil { - p.log("checkerpool: Creating diagnostics checker") - c, _ := checker.NewChecker(p.program, nil) - p.checkers[diagIndex] = c +// getDiagnosticsChecker returns the diagnostics checker that last checked the file. Unlike a query +// checker it is not reacquired by request: the file decides which one a caller gets, so a request +// is handed back the checker that already holds that file's types. A caller must not hold one +// diagnostics checker while asking for another, or two doing it in opposite orders would deadlock. +func (p *checkerPool) getDiagnosticsChecker(requestID string, file *ast.SourceFile) (*checker.Checker, func()) { + index := p.diagnosticsIndexFor(file) + c, release := p.acquireDiagnosticsChecker(index, requestID) + p.noteDiagnosticsAffinity(file, index) + p.log(fmt.Sprintf("checkerpool: Acquired diagnostics checker %d for request %s", index, holdTag(requestID))) + return c, release +} + +// acquireDiagnosticsChecker takes the numbered diagnostics checker, creating it on first use and +// blocking while another caller holds it. +func (p *checkerPool) acquireDiagnosticsChecker(index int, requestID string) (*checker.Checker, func()) { + p.diagSems[index] <- struct{}{} + + p.mu.Lock() + // Marking the slot held before letting go of the lock keeps idle cleanup and Discard off it + // while there is nothing in it to see. + p.heldBy[index] = holdTag(requestID) + c := p.checkers[index] + p.mu.Unlock() + + if c == nil { + // Built without the pool's lock: building a checker merges the globals of every file in + // the program, and a whole-program check builds one per checker at once. + p.log(fmt.Sprintf("checkerpool: Creating diagnostics checker %d", index)) + c, _ = checker.NewChecker(p.program, nil) + p.mu.Lock() + p.checkers[index] = c + p.mu.Unlock() } - c := p.checkers[diagIndex] - p.heldBy[diagIndex] = holdTag(requestID) - p.log("checkerpool: Acquired diagnostics checker for request " + holdTag(requestID)) - if requestID != "" { - if _, alreadyRegistered := p.requestAssociations[requestID]; !alreadyRegistered { - p.requestAssociations[requestID] = diagIndex - p.registerRequestCleanup(ctx, requestID) + return c, p.createRelease(requestID, index, c) +} + +// ForEachCheckerGroupDo runs a whole-program check on the diagnostics checkers. Each takes the next +// file from a shared queue as it finishes the last, so no checker sits idle while another has a +// backlog, and a file is remembered against the checker that checked it. Query checkers, handed +// out by request rather than by file, take no part in it. +func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { + var next atomic.Int64 + // Single threaded, one checker takes the whole queue anyway; make it the first, which is where a + // file no checker has seen goes too. + workers := p.diagnosticsCount + if singleThreaded { + workers = 1 + } + wg := core.NewWorkGroup(singleThreaded) + for index := range workers { + wg.Queue(func() { + requestID := core.GetRequestID(ctx) + for { + i := int(next.Add(1)) - 1 + if i >= len(files) { + // Nothing left, so don't build a checker to do it with. + return + } + // A cancelled caller discards what comes back anyway. Bailing leaves the rest of the + // files' diagnostics zero, so a caller must test for cancellation before reading them. + if ctx.Err() != nil { + return + } + // Taken a file at a time rather than for the whole check: a check of the whole + // project runs for as long as the project is big, and a pull on an open file + // would otherwise wait out all of it. + c, release := p.acquireDiagnosticsChecker(index, requestID) + cb(c, i, files[i]) + release() + p.noteDiagnosticsAffinity(files[i], index) + } + }) + } + wg.RunAndWait() +} + +// releaseDiagnosticsCheckers drops the checkers a whole-program check used. They hold the types of +// every file it reached, which is worth keeping only while something is likely to ask again. One +// another caller is holding is left to the idle timer. The global diagnostics they found are kept, +// since nothing else collects them. +func (p *checkerPool) releaseDiagnosticsCheckers() bool { + p.mu.Lock() + defer p.mu.Unlock() + released := false + for index := range p.diagnosticsCount { + c := p.checkers[index] + if c == nil || p.heldBy[index] != "" { + continue } + p.log(fmt.Sprintf("checkerpool: Releasing diagnostics checker %d on request", index)) + p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) + p.disposeCheckerLocked(index, c) + released = true } - return c, p.createRelease(requestID, diagIndex, c) + if released && !p.discarded { + p.scheduleCleanupLocked() + } + return released } -// getQueryChecker returns an ephemeral query checker from indices 1+. +// getQueryChecker returns an ephemeral query checker from the slots after the diagnostics ones. // Uses request affinity, then file affinity, then finds/creates. // Blocks on querySem if all query slots are in use. func (p *checkerPool) getQueryChecker(ctx context.Context, requestID string, file *ast.SourceFile) (*checker.Checker, func()) { - if c, release, ok := p.tryReacquireForRequest(requestID, p.querySem, false); ok { + if c, release, ok := p.tryReacquireForRequest(requestID, p.querySem); ok { return c, release } @@ -260,7 +375,7 @@ func (p *checkerPool) getQueryChecker(ctx context.Context, requestID string, fil // Try file affinity. if file != nil { - if index, ok := p.fileAssociations[file]; ok && index > 0 { + if index, ok := p.fileAssociations[file]; ok && index >= p.diagnosticsCount { if c := p.checkers[index]; c != nil && p.heldBy[index] == "" { p.heldBy[index] = holdTag(requestID) if requestID != "" { @@ -295,13 +410,13 @@ func (p *checkerPool) getQueryChecker(ctx context.Context, requestID string, fil // available. Must be called with p.mu held. func (p *checkerPool) findOrCreateQueryCheckerLocked() (*checker.Checker, int) { // Prefer an existing idle checker. - for i := 1; i < len(p.checkers); i++ { + for i := p.diagnosticsCount; i < len(p.checkers); i++ { if c := p.checkers[i]; c != nil && p.heldBy[i] == "" { return c, i } } // Create in the first empty slot. - for i := 1; i < len(p.checkers); i++ { + for i := p.diagnosticsCount; i < len(p.checkers); i++ { if p.checkers[i] == nil { p.log(fmt.Sprintf("checkerpool: Creating query checker %d", i)) c, _ := checker.NewChecker(p.program, nil) @@ -346,13 +461,19 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec return sync.OnceFunc(func() { p.mu.Lock() - if c.WasCanceled() { + switch { + case c.WasCanceled(): // Canceled checkers must be disposed. p.log(fmt.Sprintf("checkerpool: Checker %d for request %s was canceled, disposing", index, holdTag(requestID))) p.disposeCheckerLocked(index, c) - } else { + case p.discarded && index < p.diagnosticsCount: + // The program this checked has been replaced, so nothing will ask it for diagnostics + // again. Let go of it rather than holding a whole program's types until the pool is. + p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) + p.disposeCheckerLocked(index, c) + default: // Query checkers can produce incidental errors while serializing types. - if index == 0 { + if index < p.diagnosticsCount { p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) } p.heldBy[index] = "" @@ -360,7 +481,7 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec if !p.discarded { p.scheduleCleanupLocked() } - // If discarded, skip scheduling cleanup — checkers stay alive + // If discarded, skip scheduling cleanup — query checkers stay alive // until the pool is garbage collected so that API clients can // continue resolving type/symbol handles. } @@ -372,8 +493,8 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec p.mu.Unlock() // Release the semaphore slot. - if index == 0 { - <-p.diagSem + if index < p.diagnosticsCount { + <-p.diagSems[index] } else { <-p.querySem } @@ -487,6 +608,9 @@ func (p *checkerPool) mergeGlobalDiagnosticsFromCheckerLocked(index int, c *chec return } p.globalDiagCheckerCount[index] = len(globals) + if len(globals) == 0 { + return + } before := len(p.globalDiagAccumulated) p.globalDiagAccumulated = compiler.SortAndDeduplicateDiagnostics(append(p.globalDiagAccumulated, globals...)) if len(p.globalDiagAccumulated) != before { @@ -494,8 +618,8 @@ func (p *checkerPool) mergeGlobalDiagnosticsFromCheckerLocked(index int, c *chec } } -// GetGlobalDiagnostics returns the global diagnostics accumulated from the dedicated -// diagnostics checker across its instances during this pool's lifetime. +// GetGlobalDiagnostics returns the global diagnostics accumulated from the diagnostics +// checkers across their instances during this pool's lifetime. func (p *checkerPool) GetGlobalDiagnostics() []*ast.Diagnostic { p.mu.Lock() defer p.mu.Unlock() @@ -512,10 +636,10 @@ func (p *checkerPool) TakeNewGlobalDiagnostics() bool { return changed } -// Discard signals that this pool's program has been replaced. The pool -// remains functional but stops its idle-cleanup timer so that checkers -// are not disposed until the pool is GC'd. The API checker is unaffected -// since it is never idle-cleaned. +// Discard signals that this pool's program has been replaced. The pool remains functional; its +// diagnostics checkers are let go of, and its query checkers stay until it is GC'd so that an API +// client can go on resolving the handles they gave out. The API checker is unaffected, since it is +// never idle-cleaned. func (p *checkerPool) Discard() { p.mu.Lock() defer p.mu.Unlock() @@ -524,6 +648,15 @@ func (p *checkerPool) Discard() { } p.log("checkerpool: Discarding pool, stopping idle cleanup") p.discarded = true + // A discarded pool belongs to a program that has been replaced, so let go of the checkers a + // sweep left behind rather than holding a whole program's types for the life of the snapshot. + // Query checkers stay: an API client may still be resolving handles they handed out. + for index := range p.diagnosticsCount { + if c := p.checkers[index]; c != nil && p.heldBy[index] == "" { + p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) + p.disposeCheckerLocked(index, c) + } + } if p.cleanupTimer != nil { p.cleanupTimer.Stop() p.cleanupTimer = nil diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index 860cc3490c2e7..9940b655d4941 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -2,15 +2,19 @@ package project import ( "context" + "fmt" + "sync" "sync/atomic" "testing" "testing/synctest" "time" + "github.com/microsoft/TypeScript/tsc/internal/ast" "github.com/microsoft/TypeScript/tsc/internal/bundled" "github.com/microsoft/TypeScript/tsc/internal/checker" "github.com/microsoft/TypeScript/tsc/internal/compiler" "github.com/microsoft/TypeScript/tsc/internal/core" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" "github.com/microsoft/TypeScript/tsc/internal/project/logging" "github.com/microsoft/TypeScript/tsc/internal/vfs/vfstest" @@ -18,28 +22,43 @@ import ( ) func setupCheckerPoolSession(t *testing.T, opts CheckerPoolOptions) (*Session, *checkerPool) { + t.Helper() + return setupCheckerPoolSessionWithFiles(t, opts, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + }) +} + +func setupCheckerPoolSessionWithFiles(t *testing.T, opts CheckerPoolOptions, files map[string]any) (*Session, *checkerPool) { + t.Helper() + // A project only takes a build's worth of diagnostics checkers when the workspace pull is on, + // which is what these tests are about. See setupCheckerPoolSessionWithScope for the other way. + return setupCheckerPoolSessionWithScope(t, opts, files, lsutil.WorkspaceDiagnosticsScopeAllProjects) +} + +func setupCheckerPoolSessionWithScope(t *testing.T, opts CheckerPoolOptions, files map[string]any, scope lsutil.WorkspaceDiagnosticsScope) (*Session, *checkerPool) { t.Helper() if !bundled.Embedded { t.Skip("bundled files are not embedded") } - files := map[string]any{ - "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, - "/src/index.ts": "export const x: number = 1;", - } fs := bundled.WrapFS(vfstest.FromMap(files, false)) + options := &SessionOptions{ + CurrentDirectory: "/", + DefaultLibraryPath: bundled.LibPath(), + PositionEncoding: lsproto.PositionEncodingKindUTF8, + WatchEnabled: false, + LoggingEnabled: true, + CheckerPoolOptions: opts, + } + // Set directly rather than through Configure, which tells the client to refresh and there is + // no client here. The path from a preference to this is covered by the server's own tests. + options.workspaceDiagnosticsEnabled.Store(scope.Enabled()) session := NewSession(&SessionInit{ BackgroundCtx: context.Background(), - Options: &SessionOptions{ - CurrentDirectory: "/", - DefaultLibraryPath: bundled.LibPath(), - PositionEncoding: lsproto.PositionEncodingKindUTF8, - WatchEnabled: false, - LoggingEnabled: true, - CheckerPoolOptions: opts, - }, - FS: fs, - Logger: logging.NewTestLogger(), + Options: options, + FS: fs, + Logger: logging.NewTestLogger(), }) session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, "export const x: number = 1;", lsproto.LanguageKindTypeScript) @@ -53,9 +72,21 @@ func setupCheckerPoolSession(t *testing.T, opts CheckerPoolOptions) (*Session, * // newTestCheckerPool creates a checker pool inside the current goroutine context // (suitable for use inside synctest.Test) using the given program. func newTestCheckerPool(program *compiler.Program, opts CheckerPoolOptions) *checkerPool { + opts.MatchBuildCheckerCount = true return newCheckerPool(opts, program, func(string) {}) } +// manyCheckerPoolFiles is a program with more files than a build would give it checkers, so at +// least one checker owns several of them. +func manyCheckerPoolFiles() map[string]any { + files := map[string]any{"/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`} + files["/src/index.ts"] = "export const x: number = 1;\n" + for i := range 11 { + files[fmt.Sprintf("/src/f%d.ts", i)] = fmt.Sprintf("export const v%d = %d;\n", i, i) + } + return files +} + func TestCheckerPoolDiagnosticsRouting(t *testing.T) { t.Parallel() _, pool := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}) @@ -613,14 +644,13 @@ func TestCheckerPoolDiscardKeepsIdleCheckers(t *testing.T) { assert.Assert(t, pool.checkers[0] != nil, "diagnostics checker should exist") pool.mu.Unlock() - // Discard should keep idle checkers alive (they may be referenced by + // Discard should keep idle query checkers alive (they may be referenced by // API type handles) and just stop the cleanup timer. pool.Discard() pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == c1, "diagnostics checker should survive Discard") hasQuery := false - for i := 1; i < len(pool.checkers); i++ { + for i := pool.diagnosticsCount; i < len(pool.checkers); i++ { if pool.checkers[i] == c2 { hasQuery = true break @@ -634,7 +664,7 @@ func TestCheckerPoolDiscardKeepsIdleCheckers(t *testing.T) { synctest.Sleep(60 * time.Second) pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == c1, "diagnostics checker should persist indefinitely on discarded pool") + assert.Assert(t, pool.checkers[pool.diagnosticsCount] == c2, "query checker should persist indefinitely on discarded pool") pool.mu.Unlock() }) } @@ -764,7 +794,10 @@ func TestCheckerPoolDiagnosticsCheckerStableIdentity(t *testing.T) { }) } -func TestCheckerPoolDiagnosticsCheckerSurvivesDiscard(t *testing.T) { +// A discarded pool's program has been replaced, so its diagnostics checkers hold a whole program's +// worth of types that nothing will ask for again. They go, unlike the query checkers, which an API +// client may still be resolving handles against. +func TestCheckerPoolDiagnosticsCheckersDroppedOnDiscard(t *testing.T) { t.Parallel() session, _ := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}) ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") @@ -784,17 +817,20 @@ func TestCheckerPoolDiagnosticsCheckerSurvivesDiscard(t *testing.T) { pool.Discard() - // Diagnostics checker should survive Discard. pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == c, "diagnostics checker should survive Discard") + assert.Assert(t, pool.checkers[0] == nil, "diagnostics checker should be dropped on Discard") pool.mu.Unlock() - // Should still be acquirable and be the same instance. + // The pool stays usable; it just builds a new one. ctx2 := core.WithRequestID(context.Background(), "diag-discard-2") ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeDiagnostics) c2, release2 := pool.GetChecker(ctx2, nil) - assert.Assert(t, c2 == c, "diagnostics checker identity should be stable after Discard") + assert.Assert(t, c2 != nil) release2() + + pool.mu.Lock() + assert.Assert(t, pool.checkers[0] == nil, "a diagnostics checker released on a discarded pool goes too") + pool.mu.Unlock() }) } @@ -1281,3 +1317,259 @@ func TestCheckerPoolCleanupAfterDiscardIsNoop(t *testing.T) { pool.mu.Unlock() }) } + +// A file goes back to the diagnostics checker that last checked it, where its types already are. One +// no checker has seen goes to the first. +func TestCheckerPoolDiagnosticsReturnsAFileToItsLastChecker(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a = 1;", + "/src/b.ts": "export const b = 1;", + }) + files := pool.program.SourceFiles() + for _, file := range files { + assert.Equal(t, pool.diagnosticsIndexFor(file), 0, "a file no checker has seen goes to the first") + } + + // Nobody leaves their first file until every file has been taken, so each checker gets one: + // otherwise one could take them all, and returning each to its checker would prove nothing. + assert.Equal(t, pool.diagnosticsCount, len(files)) + checkedBy := make([]*checker.Checker, len(files)) + var mu sync.Mutex + var arrived sync.WaitGroup + arrived.Add(len(files)) + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + pool.ForEachCheckerGroupDo(ctx, files, false /*singleThreaded*/, func(c *checker.Checker, i int, _ *ast.SourceFile) { + mu.Lock() + checkedBy[i] = c + mu.Unlock() + arrived.Done() + arrived.Wait() + }) + distinct := map[*checker.Checker]bool{} + for _, c := range checkedBy { + distinct[c] = true + } + assert.Equal(t, len(distinct), len(files), "each file was checked on a checker of its own") + + for i, file := range files { + pullCtx := core.WithCheckerLifetime(core.WithRequestID(t.Context(), fmt.Sprintf("diag-%d", i)), core.CheckerLifetimeDiagnostics) + c, release := pool.GetChecker(pullCtx, file) + assert.Assert(t, c == checkedBy[i], "a pull on %s must get the checker that checked it", file.FileName()) + release() + } +} + +// Files on different checkers are checked at the same time; files on the same one wait for each +// other. +func TestCheckerPoolDiagnosticsCheckersAreHeldIndependently(t *testing.T) { + t.Parallel() + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a = 1;", + "/src/b.ts": "export const b = 1;", + "/src/c.ts": "export const c = 1;", + "/src/d.ts": "export const d = 1;", + "/src/e.ts": "export const e = 1;", + }) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + files := program.SourceFiles() + assert.Assert(t, pool.diagnosticsCount > 1 && len(files) >= 3) + + synctest.Test(t, func(t *testing.T) { + pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}) + + // Two files last checked on the same checker, and one on another. + shared := files[:2] + separate := files[2] + pool.noteDiagnosticsAffinity(shared[0], 0) + pool.noteDiagnosticsAffinity(shared[1], 0) + pool.noteDiagnosticsAffinity(separate, 1) + + diagCtx := func(id string) context.Context { + return core.WithCheckerLifetime(core.WithRequestID(context.Background(), id), core.CheckerLifetimeDiagnostics) + } + + held, releaseHeld := pool.GetChecker(diagCtx("diag-held"), shared[0]) + assert.Assert(t, held != nil) + + // The other file on the same checker has to wait for it. + var sameGot atomic.Bool + go func() { + c, release := pool.GetChecker(diagCtx("diag-same"), shared[1]) + sameGot.Store(c != nil) + release() + }() + synctest.Wait() + assert.Assert(t, !sameGot.Load(), "a file sharing a checker must wait for it") + + // A file on another checker does not. + other, releaseOther := pool.GetChecker(diagCtx("diag-other"), separate) + assert.Assert(t, other != nil && other != held, "a file on another checker gets another checker") + releaseOther() + + releaseHeld() + synctest.Wait() + assert.Assert(t, sameGot.Load(), "the waiting file gets the checker once it is released") + }) +} + +// A check of the whole project takes its checker a file at a time, so a pull on a file that checker +// last checked gets in between files rather than waiting out the whole project. +func TestCheckerPoolWholeProgramCheckYieldsBetweenFiles(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, manyCheckerPoolFiles()) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + // Single threaded, so the whole check runs on the first checker, which is also where a file no + // checker has seen goes. + files := program.SourceFiles() + const owner = 0 + owned := files + assert.Assert(t, len(owned) >= 3, "need enough files to interleave against") + + synctest.Test(t, func(t *testing.T) { + pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}) + + inFirstFile := make(chan struct{}) + finishFirstFile := make(chan struct{}) + var checked atomic.Int32 + go func() { + pool.ForEachCheckerGroupDo(context.Background(), files, true /*singleThreaded*/, func(c *checker.Checker, _ int, file *ast.SourceFile) { + if checked.Add(1) == 1 { + close(inFirstFile) + <-finishFirstFile + } + }) + }() + <-inFirstFile + + // A pull on another file the busy checker owns. + var checkedWhenPulled atomic.Int32 + checkedWhenPulled.Store(-1) + go func() { + ctx := core.WithRequestID(context.Background(), "diag-during-check") + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + _, release := pool.GetChecker(ctx, owned[1]) + checkedWhenPulled.Store(checked.Load()) + release() + }() + synctest.Wait() + assert.Equal(t, checkedWhenPulled.Load(), int32(-1), "the pull waits while the checker is on a file") + + close(finishFirstFile) + synctest.Wait() + assert.Assert(t, checkedWhenPulled.Load() > 0 && int(checkedWhenPulled.Load()) < len(owned), + "the pull must get the checker between files, not after the whole project") + }) +} + +// A whole-program check runs on the diagnostics checkers, of which there are as many as a build +// would use, and leaves each file remembered against the one that checked it. +func TestCheckerPoolChecksWholeProgramAcrossDiagnosticsCheckers(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + "/src/b.ts": "export const b = 1;", + }) + assert.Equal(t, pool.diagnosticsCount, diagnosticsCheckerCount(pool.program)) + assert.Assert(t, pool.diagnosticsCount > 1, "a three-file program gets more than one checker") + for index := range pool.checkers { + assert.Assert(t, pool.checkers[index] == nil, "nothing built before a check is asked for") + } + + diagnostics := pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, len(diagnostics) > 0, "expected the seeded error") + + for _, file := range pool.program.SourceFiles() { + index := pool.diagnosticsIndexFor(file) + assert.Assert(t, index < pool.diagnosticsCount && pool.checkers[index] != nil, + "%s must be remembered against the diagnostics checker that checked it", file.FileName()) + } + for index := pool.diagnosticsCount; index < len(pool.checkers); index++ { + assert.Assert(t, pool.checkers[index] == nil, "a whole-program check must not use a query checker") + } +} + +// Query checkers sit after the diagnostics ones, so a query never lands on a checker a whole-program +// check is partway through. +func TestCheckerPoolQueryCheckersSitAfterDiagnosticsCheckers(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + "/src/b.ts": "export const b = 1;", + }) + assert.Equal(t, len(pool.checkers), pool.diagnosticsCount+3, "MaxCheckers-1 query checkers on top of the diagnostics ones") + + ctx := core.WithCheckerLifetime(core.WithRequestID(t.Context(), "query-after-diag"), core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + defer release() + for index := range pool.diagnosticsCount { + assert.Assert(t, pool.checkers[index] != c, "a query must not use a diagnostics checker") + } +} + +// The checkers a sweep uses are handed back when it is done with them: they hold the types of every +// file in the program, and a later pull either finds the project unchanged, and answers from result +// ids without checking, or finds it changed and needs new ones anyway. +func TestCheckerPoolReleasesCheckersAfterAWholeProgramCheck(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + }) + + anyDiagnosticsChecker := func() bool { + for index := range pool.diagnosticsCount { + if pool.checkers[index] != nil { + return true + } + } + return false + } + + pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, anyDiagnosticsChecker()) + + assert.Assert(t, pool.releaseDiagnosticsCheckers(), "releasing reports that it dropped the checkers") + assert.Assert(t, !anyDiagnosticsChecker()) + assert.Assert(t, !pool.releaseDiagnosticsCheckers(), "nothing left to release") + + // Discarding is what the project system does once a program is replaced, and it has to happen + // before the next program's checkers are built or both generations are held at once. + pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, anyDiagnosticsChecker(), "a later check builds them again") + pool.Discard() + assert.Assert(t, !anyDiagnosticsChecker(), "a discarded pool must let go of its checkers") +} + +// A build's worth of diagnostics checkers is what a workspace pull needs. A session without the +// pull switched on keeps the single diagnostics checker it has always had, and does not pay for +// the memory of the rest. +func TestCheckerPoolKeepsOneDiagnosticsCheckerWithoutTheWorkspacePull(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithScope(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, + manyCheckerPoolFiles(), lsutil.WorkspaceDiagnosticsScopeOff) + + assert.Assert(t, diagnosticsCheckerCount(pool.program) > 1, "the program is big enough for a build to use several") + assert.Equal(t, pool.diagnosticsCount, 1, "without the pull, diagnostics stay on one checker") + assert.Equal(t, len(pool.checkers), 1+3, "one diagnostics checker plus MaxCheckers-1 query checkers") + + // Every file goes to that one checker. + for _, file := range pool.program.SourceFiles() { + assert.Equal(t, pool.diagnosticsIndexFor(file), 0) + } +} diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index 79954831210c9..09219738b7cd8 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -506,7 +506,9 @@ func (p *Project) CreateProgram() CreateProgramResult { // the same project never share a captured variable through a stale closure // stored in the old program's options. createCheckerPool := func(program *compiler.Program) compiler.CheckerPool { - return newCheckerPool(p.host.sessionOptions.CheckerPoolOptions, program, p.log) + opts := p.host.sessionOptions.CheckerPoolOptions + opts.MatchBuildCheckerCount = p.host.sessionOptions.workspaceDiagnosticsEnabled.Load() + return newCheckerPool(opts, program, p.log) } var cleanupModuleResolver func() createModuleResolver := func(options module.ResolverOptions) module.Resolver { diff --git a/tsc/internal/project/session.go b/tsc/internal/project/session.go index 08585aa678b49..39226c600346a 100644 --- a/tsc/internal/project/session.go +++ b/tsc/internal/project/session.go @@ -79,6 +79,12 @@ type SessionOptions struct { RunExternalCode bool DebounceDelay time.Duration CheckerPoolOptions CheckerPoolOptions + + // workspaceDiagnosticsEnabled tracks whether the workspace pull is switched on, so that a + // project only pays for a build's worth of checkers when something is going to check it that + // way. Written whenever the user's preferences change and read when a program's pool is built, + // so a session that turns the pull on picks it up as programs are rebuilt. + workspaceDiagnosticsEnabled atomic.Bool } type SessionInit struct { @@ -292,6 +298,7 @@ func (s *Session) Configure(config lsutil.UserPreferences) { oldConfig := s.workspaceUserPreferences s.workspaceUserPreferences = config s.userConfigRWMu.Unlock() + s.options.workspaceDiagnosticsEnabled.Store(config.WorkspaceDiagnosticsScope.Enabled()) if config.Locale != "" { oldLocale := s.client.GetLocale() diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 6040a639a2697..920766cb7b3bb 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -211,6 +211,17 @@ func (s *Snapshot) GetLanguageServiceProjectsContainingFile(uri lsproto.Document return s.ProjectCollection.GetLanguageServiceProjectsContainingFile(path) } +// ReleaseDiagnosticsCheckers drops the checkers a sweep used on a project. They hold the types of +// every file in it, which is the largest thing a pull creates, and keeping them buys nothing: a +// pull that finds the project unchanged answers from the result ids the client already holds +// without checking anything, and a pull that finds it changed needs new checkers regardless. +func (s *Snapshot) ReleaseDiagnosticsCheckers(project *Project) bool { + if project.checkerPool == nil { + return false + } + return project.checkerPool.releaseDiagnosticsCheckers() +} + func (s *Snapshot) GetFile(fileName string) FileHandle { return s.fs.GetFile(fileName) } From bc50faca967d5ded39e69db817f89b5892a10c2d Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:11:48 -0700 Subject: [PATCH 03/10] Stand a workspace pass down for work the user is waiting on A pass over the whole workspace keeps every checker of every project it is checking busy, and the machine with them. A pull on the document in front of the user arrives into that and waits: for the checker that owns the file, and then for a core to check it on. A snapshot update is worse, since it rebuilds the program under the session's snapshot lock, which every other request queues behind. The session counts the work a user is waiting on: an interactive document pull, and a snapshot update. A whole-program check waits for that count to reach zero before each file. It holds no checker while waiting, so what it stands aside for can take the checker it was about to use, and it gives up waiting if its own caller goes away. The trade is one-sided. A pass runs for as long as the workspace is big and nothing waits on it finishing sooner, so the time it gives up is time the user was going to spend waiting anyway. Only the pass waits; a pull never does, and counts from before it waits for a checker rather than from when it gets one, so the pass cannot take the checker out from under it. Requests are marked interactive by the handler rather than inferred from the checker lifetime, because a pass acquires diagnostics checkers itself, per file, and would otherwise stand aside for its own work and never finish. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/core/context.go | 12 + tsc/internal/project/checkerpool.go | 189 ++++++++--- tsc/internal/project/checkerpool_test.go | 323 ++++++++++++++++++- tsc/internal/project/idlecacheclean_test.go | 38 +++ tsc/internal/project/interactivework.go | 76 +++++ tsc/internal/project/interactivework_test.go | 101 ++++++ tsc/internal/project/project.go | 2 +- tsc/internal/project/session.go | 18 ++ tsc/internal/project/snapshot.go | 7 + tsc/internal/project/snapshothost.go | 5 + 10 files changed, 721 insertions(+), 50 deletions(-) create mode 100644 tsc/internal/project/idlecacheclean_test.go create mode 100644 tsc/internal/project/interactivework.go create mode 100644 tsc/internal/project/interactivework_test.go diff --git a/tsc/internal/core/context.go b/tsc/internal/core/context.go index 6747a03e2cc55..5a850744542e6 100644 --- a/tsc/internal/core/context.go +++ b/tsc/internal/core/context.go @@ -9,6 +9,7 @@ type key int const ( requestIDKey key = iota checkerLifetimeKey + interactiveRequestKey ) func WithRequestID(ctx context.Context, id string) context.Context { @@ -22,6 +23,17 @@ func GetRequestID(ctx context.Context) string { return "" } +// WithInteractiveRequest marks work a user is waiting on directly, as against work done ahead of +// being asked for it. Whole-workspace passes stand aside while any of it is outstanding. +func WithInteractiveRequest(ctx context.Context) context.Context { + return context.WithValue(ctx, interactiveRequestKey, true) +} + +func IsInteractiveRequest(ctx context.Context) bool { + interactive, _ := ctx.Value(interactiveRequestKey).(bool) + return interactive +} + type CheckerLifetime int const ( diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index a93faea0a3e36..51a915bbb2737 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -54,6 +54,16 @@ type checkerPool struct { // query checkers are not disposed until the pool is GC'd. discarded bool + // interactive is the session's count of work a user is waiting on. A whole-program check + // stands aside while any of it is outstanding, and an interactive check adds to it. + interactive *interactiveWork + + // sweeps counts the whole-program checks running on this pool, and diagInUse every hold of a + // diagnostics checker, whichever path took it. Both take their checker a file at a time, so a + // discarded pool must keep them until the work that is still asking has finished. + sweeps int + diagInUse int + // diagnosticsCount is how many checkers a build of this program would check it with. diagnosticsCount int // diagnosticsAffinity is the diagnostics checker that last checked each file, so the next check @@ -94,7 +104,7 @@ type checkerPool struct { var _ compiler.CheckerPool = (*checkerPool)(nil) -func newCheckerPool(opts CheckerPoolOptions, program *compiler.Program, log func(msg string)) *checkerPool { +func newCheckerPool(opts CheckerPoolOptions, program *compiler.Program, interactive *interactiveWork, log func(msg string)) *checkerPool { if opts.MaxCheckers <= 0 { opts.MaxCheckers = 4 } else if opts.MaxCheckers < 2 { @@ -117,6 +127,7 @@ func newCheckerPool(opts CheckerPoolOptions, program *compiler.Program, log func pool := &checkerPool{ program: program, opts: opts, + interactive: interactive, diagnosticsCount: diagnosticsCount, checkers: make([]*checker.Checker, slots), heldBy: make([]string, slots), @@ -159,7 +170,7 @@ func (p *checkerPool) GetChecker(ctx context.Context, file *ast.SourceFile) (*ch switch lifetime { case core.CheckerLifetimeDiagnostics: - return p.getDiagnosticsChecker(requestID, file) + return p.getDiagnosticsChecker(requestID, file, core.IsInteractiveRequest(ctx)) case core.CheckerLifetimeAPI: return p.getPersistentChecker() default: @@ -264,23 +275,61 @@ func (p *checkerPool) noteDiagnosticsAffinity(file *ast.SourceFile, index int) { // checker it is not reacquired by request: the file decides which one a caller gets, so a request // is handed back the checker that already holds that file's types. A caller must not hold one // diagnostics checker while asking for another, or two doing it in opposite orders would deadlock. -func (p *checkerPool) getDiagnosticsChecker(requestID string, file *ast.SourceFile) (*checker.Checker, func()) { +func (p *checkerPool) getDiagnosticsChecker(requestID string, file *ast.SourceFile, interactive bool) (*checker.Checker, func()) { + // Counted from before the wait for the checker rather than from when it is handed over, so a + // whole-program check stands aside for this rather than taking the checker out from under it. + var interactiveDone func() + if interactive { + interactiveDone = p.interactive.begin() + } + index := p.diagnosticsIndexFor(file) c, release := p.acquireDiagnosticsChecker(index, requestID) p.noteDiagnosticsAffinity(file, index) p.log(fmt.Sprintf("checkerpool: Acquired diagnostics checker %d for request %s", index, holdTag(requestID))) - return c, release + if interactiveDone == nil { + return c, release + } + return c, sync.OnceFunc(func() { + release() + interactiveDone() + }) } // acquireDiagnosticsChecker takes the numbered diagnostics checker, creating it on first use and // blocking while another caller holds it. func (p *checkerPool) acquireDiagnosticsChecker(index int, requestID string) (*checker.Checker, func()) { p.diagSems[index] <- struct{}{} + return p.fillDiagnosticsSlot(index, requestID) +} + +// tryAcquireDiagnosticsChecker is acquireDiagnosticsChecker for a caller that can give up. A +// cancelled whole-program check must not wait out a slot, still less build a checker to throw +// away: building one merges the globals of every file in the program, and the pull that cancelled +// it is waiting for that same slot. +func (p *checkerPool) tryAcquireDiagnosticsChecker(ctx context.Context, index int, requestID string) (*checker.Checker, func(), bool) { + select { + case p.diagSems[index] <- struct{}{}: + case <-ctx.Done(): + return nil, nil, false + } + // Cancelled while waiting for the slot: hand it straight back rather than build into it. + if ctx.Err() != nil { + <-p.diagSems[index] + return nil, nil, false + } + c, release := p.fillDiagnosticsSlot(index, requestID) + return c, release, true +} +// fillDiagnosticsSlot takes the numbered slot, whose semaphore the caller already holds, and +// creates the checker if this is its first use. +func (p *checkerPool) fillDiagnosticsSlot(index int, requestID string) (*checker.Checker, func()) { p.mu.Lock() // Marking the slot held before letting go of the lock keeps idle cleanup and Discard off it // while there is nothing in it to see. p.heldBy[index] = holdTag(requestID) + p.diagInUse++ c := p.checkers[index] p.mu.Unlock() @@ -302,6 +351,13 @@ func (p *checkerPool) acquireDiagnosticsChecker(index int, requestID string) (*c // backlog, and a file is remembered against the checker that checked it. Query checkers, handed // out by request rather than by file, take no part in it. func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { + // A caller the user is waiting on is what everything else stands aside for; waiting here would + // be waiting on itself, and the checkers would take turns rather than run together. + standAside := !core.IsInteractiveRequest(ctx) + // A snapshot update part way through discards this pool. The check is still going to ask for + // its checkers, so they have to outlast the discard; see endSweep. + p.beginSweep() + defer p.endSweep() var next atomic.Int64 // Single threaded, one checker takes the whole queue anyway; make it the first, which is where a // file no checker has seen goes too. @@ -319,15 +375,27 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So // Nothing left, so don't build a checker to do it with. return } - // A cancelled caller discards what comes back anyway. Bailing leaves the rest of the - // files' diagnostics zero, so a caller must test for cancellation before reading them. + // A cancelled caller discards what comes back anyway. Checked here rather than + // left to waitForIdle, which returns without looking at the context when nothing + // is outstanding. Bailing leaves the rest of the files' diagnostics zero, so a + // caller must test for cancellation before reading them. if ctx.Err() != nil { return } - // Taken a file at a time rather than for the whole check: a check of the whole - // project runs for as long as the project is big, and a pull on an open file - // would otherwise wait out all of it. - c, release := p.acquireDiagnosticsChecker(index, requestID) + // Stand aside between files for anything the user is waiting on. Nothing is held + // while waiting, so the work being waited for can take this checker if it needs it. + if standAside { + if err := p.interactive.waitForIdle(ctx); err != nil { + return + } + } + // A file at a time rather than for the whole check, so that standing aside is + // possible at all: a pull on an open file would otherwise wait out the entire + // project. + c, release, ok := p.tryAcquireDiagnosticsChecker(ctx, index, requestID) + if !ok { + return + } cb(c, i, files[i]) release() p.noteDiagnosticsAffinity(files[i], index) @@ -337,25 +405,49 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So wg.RunAndWait() } -// releaseDiagnosticsCheckers drops the checkers a whole-program check used. They hold the types of -// every file it reached, which is worth keeping only while something is likely to ask again. One -// another caller is holding is left to the idle timer. The global diagnostics they found are kept, -// since nothing else collects them. -func (p *checkerPool) releaseDiagnosticsCheckers() bool { +// beginSweep records a whole-program check as running on this pool. +func (p *checkerPool) beginSweep() { p.mu.Lock() defer p.mu.Unlock() + p.sweeps++ +} + +// endSweep records one as finished, and on a pool discarded while it ran does the letting go that +// its releases deferred. +func (p *checkerPool) endSweep() { + p.mu.Lock() + defer p.mu.Unlock() + p.sweeps-- + if p.discarded && p.sweeps == 0 { + p.disposeIdleDiagnosticsCheckersLocked() + } +} + +// disposeIdleDiagnosticsCheckersLocked lets go of every diagnostics checker no one is holding. +// Must be called with p.mu held. +func (p *checkerPool) disposeIdleDiagnosticsCheckersLocked() bool { released := false for index := range p.diagnosticsCount { c := p.checkers[index] if c == nil || p.heldBy[index] != "" { continue } - p.log(fmt.Sprintf("checkerpool: Releasing diagnostics checker %d on request", index)) p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) p.disposeCheckerLocked(index, c) released = true } - if released && !p.discarded { + return released +} + +// releaseDiagnosticsCheckers drops the checkers a whole-program check used. They hold the types of +// every file it reached, which is worth keeping only while something is likely to ask again. One +// another caller is holding is left to the idle timer. The global diagnostics they found are kept, +// since nothing else collects them. +func (p *checkerPool) releaseDiagnosticsCheckers() bool { + p.mu.Lock() + defer p.mu.Unlock() + released := p.disposeIdleDiagnosticsCheckersLocked() + if released { p.scheduleCleanupLocked() } return released @@ -466,11 +558,6 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec // Canceled checkers must be disposed. p.log(fmt.Sprintf("checkerpool: Checker %d for request %s was canceled, disposing", index, holdTag(requestID))) p.disposeCheckerLocked(index, c) - case p.discarded && index < p.diagnosticsCount: - // The program this checked has been replaced, so nothing will ask it for diagnostics - // again. Let go of it rather than holding a whole program's types until the pool is. - p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) - p.disposeCheckerLocked(index, c) default: // Query checkers can produce incidental errors while serializing types. if index < p.diagnosticsCount { @@ -478,12 +565,13 @@ func (p *checkerPool) createRelease(requestID string, index int, c *checker.Chec } p.heldBy[index] = "" p.lastReleased[index] = time.Now() - if !p.discarded { - p.scheduleCleanupLocked() - } - // If discarded, skip scheduling cleanup — query checkers stay alive - // until the pool is garbage collected so that API clients can - // continue resolving type/symbol handles. + // Scheduled on a discarded pool too: a checker is worth letting go of once nothing + // has come back for it, but not the moment it is handed back. + p.scheduleCleanupLocked() + } + + if index < p.diagnosticsCount { + p.diagInUse-- } // Unlock before releasing the semaphore slot. If we received from @@ -516,13 +604,18 @@ func (p *checkerPool) registerRequestCleanup(ctx context.Context, requestID stri // scheduleCleanupLocked resets (or starts) the cleanup timer so it fires at // the earliest pending checker-expiration deadline among all currently idle, // unheld checkers. -// Must be called with p.mu held. Must NOT be called on discarded pools. +// Must be called with p.mu held. func (p *checkerPool) scheduleCleanupLocked() { var earliestDeadline time.Time for i := range p.checkers { if p.checkers[i] == nil || p.heldBy[i] != "" || p.lastReleased[i].IsZero() { continue } + // Nothing collects a discarded pool's query checkers, so counting them here would keep + // resetting a timer that holds the pool, and the program it checked, alive for good. + if p.discarded && i >= p.diagnosticsCount { + continue + } deadline := p.lastReleased[i].Add(p.opts.IdleTimeout) if earliestDeadline.IsZero() || deadline.Before(earliestDeadline) { earliestDeadline = deadline @@ -552,10 +645,16 @@ func (p *checkerPool) scheduleCleanupLocked() { func (p *checkerPool) cleanupIdleCheckers() { p.mu.Lock() defer p.mu.Unlock() - // The timer callback may already have been in flight when Discard() called - // Stop() (which does not guarantee the callback won't run). Bail out without - // rescheduling so a discarded pool doesn't keep itself alive via a new timer. - if p.discarded { + // A check splits the files unevenly, so a checker given a small share goes untouched for + // longer than the timeout while the rest are still going. Collecting it would have the next + // file that lands on it rebuild one, part way through the check that is using it. + if p.sweeps > 0 || p.diagInUse > 0 { + // A whole timeout later, rather than at a deadline already passed, which would spin. + if p.cleanupTimer != nil { + p.cleanupTimer.Reset(p.opts.IdleTimeout) + } else { + p.cleanupTimer = time.AfterFunc(p.opts.IdleTimeout, p.cleanupIdleCheckers) + } return } now := time.Now() @@ -564,6 +663,11 @@ func (p *checkerPool) cleanupIdleCheckers() { if c == nil || p.heldBy[i] != "" { continue } + // A discarded pool collects only its diagnostics checkers. Query checkers stay until it + // is, so an API client can go on resolving the handles they gave out. + if p.discarded && i >= p.diagnosticsCount { + continue + } if p.lastReleased[i].IsZero() { continue } @@ -646,21 +750,14 @@ func (p *checkerPool) Discard() { if p.discarded { return // already discarded } - p.log("checkerpool: Discarding pool, stopping idle cleanup") + p.log("checkerpool: Discarding pool") p.discarded = true - // A discarded pool belongs to a program that has been replaced, so let go of the checkers a - // sweep left behind rather than holding a whole program's types for the life of the snapshot. - // Query checkers stay: an API client may still be resolving handles they handed out. - for index := range p.diagnosticsCount { - if c := p.checkers[index]; c != nil && p.heldBy[index] == "" { - p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) - p.disposeCheckerLocked(index, c) - } - } - if p.cleanupTimer != nil { - p.cleanupTimer.Stop() - p.cleanupTimer = nil + // Only once nothing is using it. Work in flight asks for its checker again a file at a time, + // and the idle timer collects whatever it leaves behind. + if p.sweeps == 0 && p.diagInUse == 0 { + p.disposeIdleDiagnosticsCheckersLocked() } + p.scheduleCleanupLocked() } func noop() {} diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index 9940b655d4941..072f8808f57a7 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -73,7 +73,7 @@ func setupCheckerPoolSessionWithScope(t *testing.T, opts CheckerPoolOptions, fil // (suitable for use inside synctest.Test) using the given program. func newTestCheckerPool(program *compiler.Program, opts CheckerPoolOptions) *checkerPool { opts.MatchBuildCheckerCount = true - return newCheckerPool(opts, program, func(string) {}) + return newCheckerPool(opts, program, nil /*interactive*/, func(string) {}) } // manyCheckerPoolFiles is a program with more files than a build would give it checkers, so at @@ -821,7 +821,9 @@ func TestCheckerPoolDiagnosticsCheckersDroppedOnDiscard(t *testing.T) { assert.Assert(t, pool.checkers[0] == nil, "diagnostics checker should be dropped on Discard") pool.mu.Unlock() - // The pool stays usable; it just builds a new one. + // The pool stays usable; it just builds a new one. Work that started before the discard + // goes on asking, a file at a time, so what it takes is not thrown away the moment it + // hands it back — rebuilding one merges the globals of every file in the program. ctx2 := core.WithRequestID(context.Background(), "diag-discard-2") ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeDiagnostics) c2, release2 := pool.GetChecker(ctx2, nil) @@ -829,7 +831,20 @@ func TestCheckerPoolDiagnosticsCheckersDroppedOnDiscard(t *testing.T) { release2() pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == nil, "a diagnostics checker released on a discarded pool goes too") + assert.Assert(t, pool.checkers[0] == c2, "a checker handed back on a discarded pool is kept for whoever asks next") + pool.mu.Unlock() + + // Asking again gets the same one rather than paying to build another. + c3, release3 := pool.GetChecker(ctx2, nil) + assert.Equal(t, c3, c2, "the next caller is given the checker that is already there") + release3() + + // Once nothing has come back for it, the idle timer collects it, so a discarded pool does + // not hold a whole program's types for the life of the snapshot. + time.Sleep(31 * time.Second) + synctest.Wait() + pool.mu.Lock() + assert.Assert(t, pool.checkers[0] == nil, "an idle diagnostics checker on a discarded pool is collected") pool.mu.Unlock() }) } @@ -1573,3 +1588,305 @@ func TestCheckerPoolKeepsOneDiagnosticsCheckerWithoutTheWorkspacePull(t *testing assert.Equal(t, pool.diagnosticsIndexFor(file), 0) } } + +// A whole-program check stands aside between files for anything the user is waiting on, so a pull +// on the file in front of them is not queued behind the rest of the workspace. +func TestCheckerPoolWholeProgramCheckStandsAsideForInteractiveWork(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, manyCheckerPoolFiles()) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + files := program.SourceFiles() + + synctest.Test(t, func(t *testing.T) { + interactive := newInteractiveWork() + pool := newCheckerPool(CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}, program, interactive, func(string) {}) + + // Something the user is waiting on is outstanding before the check starts. + interactiveDone := interactive.begin() + + var checked atomic.Int32 + var finished atomic.Bool + go func() { + pool.ForEachCheckerGroupDo(context.Background(), files, false /*singleThreaded*/, func(_ *checker.Checker, _ int, _ *ast.SourceFile) { + checked.Add(1) + }) + finished.Store(true) + }() + synctest.Wait() + assert.Equal(t, checked.Load(), int32(0), "no file may be checked while the user is waiting on something") + + interactiveDone() + synctest.Wait() + assert.Assert(t, finished.Load(), "the check resumes once the interactive work is done") + assert.Assert(t, checked.Load() > 0) + }) +} + +// An interactive pull counts as work the user is waiting on from before it waits for a checker, so +// a check of the whole program stands aside rather than taking the checker out from under it. +func TestCheckerPoolInteractivePullCountsAsInteractiveWork(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, manyCheckerPoolFiles()) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + synctest.Test(t, func(t *testing.T) { + interactive := newInteractiveWork() + pool := newCheckerPool(CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}, program, interactive, func(string) {}) + + ctx := core.WithRequestID(context.Background(), "doc-pull") + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + ctx = core.WithInteractiveRequest(ctx) + + c, release := pool.GetChecker(ctx, program.SourceFiles()[0]) + assert.Assert(t, c != nil) + + var idle atomic.Bool + go func() { + interactive.waitForIdle(context.Background()) + idle.Store(true) + }() + synctest.Wait() + assert.Assert(t, !idle.Load(), "a held interactive checker is work the user is waiting on") + + release() + synctest.Wait() + assert.Assert(t, idle.Load(), "releasing it clears the way for a workspace pass") + }) +} + +// A pull that is not interactive - the workspace pass's own per-file acquisitions - must not count, +// or the pass would stand aside for itself and never finish. +func TestCheckerPoolSweepAcquisitionIsNotInteractive(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, manyCheckerPoolFiles()) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + synctest.Test(t, func(t *testing.T) { + interactive := newInteractiveWork() + pool := newCheckerPool(CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}, program, interactive, func(string) {}) + + ctx := core.WithRequestID(context.Background(), "sweep") + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + + _, release := pool.GetChecker(ctx, program.SourceFiles()[0]) + defer release() + + var idle atomic.Bool + go func() { + interactive.waitForIdle(context.Background()) + idle.Store(true) + }() + synctest.Wait() + assert.Assert(t, idle.Load(), "a checker held by the pass itself is not work the user is waiting on") + }) +} + +// A whole-program check reached from a request the user is waiting on must not stand aside: it +// would be waiting on itself, and the checkers of one group would take turns rather than run +// together. Nothing does this today, but it is a cheap thing to leave armed. +func TestCheckerPoolInteractiveWholeProgramCheckDoesNotStandAside(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, manyCheckerPoolFiles()) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + files := program.SourceFiles() + + synctest.Test(t, func(t *testing.T) { + interactive := newInteractiveWork() + pool := newCheckerPool(CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}, program, interactive, func(string) {}) + + // Something else the user is waiting on never finishes. + defer interactive.begin()() + + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + ctx = core.WithInteractiveRequest(ctx) + + var finished atomic.Bool + go func() { + pool.ForEachCheckerGroupDo(ctx, files, false /*singleThreaded*/, func(*checker.Checker, int, *ast.SourceFile) {}) + finished.Store(true) + }() + synctest.Wait() + assert.Assert(t, finished.Load(), "an interactive check must not wait for the work it is part of") + }) +} + +// A snapshot update during a whole-program check discards the pool the check is running on. The +// check is still using it, one file at a time, so a pool that lets go of a diagnostics checker the +// moment it is handed back makes the next file build a fresh one — and building one merges the +// globals of every file in the program. On a large project that is minutes per file. +func TestCheckerPoolDiscardDoesNotRebuildPerFile(t *testing.T) { + t.Parallel() + + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, manyCheckerPoolFiles()) + t.Cleanup(session.Close) + + files := pool.program.SourceFiles() + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + + // Discard lands partway through, as a snapshot update would. + const discardAfter = 2 + seen := 0 + var checkers []*checker.Checker + pool.ForEachCheckerGroupDo(ctx, files, true /*singleThreaded*/, func(c *checker.Checker, _ int, _ *ast.SourceFile) { + checkers = append(checkers, c) + seen++ + if seen == discardAfter { + pool.Discard() + } + }) + + assert.Assert(t, len(checkers) == len(files), "every file is checked, got %d of %d", len(checkers), len(files)) + distinct := map[*checker.Checker]bool{} + for _, c := range checkers { + distinct[c] = true + } + assert.Assert(t, len(distinct) <= pool.diagnosticsCount, + "a discarded pool must not rebuild a checker per file: %d files were checked by %d distinct checkers, with only %d in the pool", + len(files), len(distinct), pool.diagnosticsCount) +} + +// Building a checker merges the globals of every file in the program, which on a large project is +// the slowest thing a pull does. A check that has been cancelled while waiting for a slot must +// give up rather than take the slot and build into it: the pull that cancelled it is waiting for +// that same slot. +func TestCheckerPoolCancelledSweepGivesUpTheSlot(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, manyCheckerPoolFiles()) + t.Cleanup(session.Close) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + synctest.Test(t, func(t *testing.T) { + pool := newTestCheckerPool(program, CheckerPoolOptions{}) + // Single threaded, so the check runs on the first checker only. + const owner = 0 + group := program.SourceFiles() + + // Something else holds the checker the group needs, so the check cannot start. + held, releaseHeld := pool.acquireDiagnosticsChecker(owner, "holder") + assert.Assert(t, held != nil) + + ctx, cancel := context.WithCancel(core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics)) + var checked atomic.Int64 + var returned atomic.Bool + go func() { + pool.ForEachCheckerGroupDo(ctx, group, true /*singleThreaded*/, func(*checker.Checker, int, *ast.SourceFile) { + checked.Add(1) + }) + returned.Store(true) + }() + + synctest.Wait() + assert.Assert(t, !returned.Load(), "the check is waiting for the checker the holder has") + + cancel() + synctest.Wait() + assert.Assert(t, returned.Load(), "a cancelled check must stop waiting for a slot it no longer needs") + assert.Equal(t, checked.Load(), int64(0), "it checked nothing") + + // The slot is the holder's to give back, not the abandoned check's. + releaseHeld() + synctest.Wait() + c, release := pool.acquireDiagnosticsChecker(owner, "after") + assert.Assert(t, c != nil, "the slot is free once the holder releases it") + release() + }) +} + +// The reference graph a pull needs resolves every file's imports through a type checker, taking +// one from the pool per file rather than through ForEachCheckerGroupDo. A snapshot update part +// way through discards the pool underneath it, and it goes on asking, so the discard must not +// make each of those calls build a checker of its own. +func TestCheckerPoolDiscardDoesNotRebuildPerAcquisition(t *testing.T) { + t.Parallel() + + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, manyCheckerPoolFiles()) + t.Cleanup(session.Close) + + files := pool.program.SourceFiles() + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + + // Discarding lets go of what nothing is holding, so the count that matters is how many + // checkers the calls after it are served by, not how many the whole run built. + const discardAfter = 2 + discarded := false + after := map[*checker.Checker]bool{} + for i, file := range files { + c, release := pool.GetChecker(ctx, file) + if discarded { + after[c] = true + } + release() + if i == discardAfter { + pool.Discard() + discarded = true + } + } + + served := len(files) - discardAfter - 1 + assert.Assert(t, len(after) <= pool.diagnosticsCount, + "a discarded pool must not rebuild a checker per acquisition: %d calls after the discard were served by %d distinct checkers, with only %d in the pool", + served, len(after), pool.diagnosticsCount) +} + +// A check of the whole program splits the files unevenly, so a checker given a small share goes +// untouched for longer than the idle timeout while the rest are still going. Collecting it would +// have the next file that lands on it rebuild a checker part way through the check using it. +func TestCheckerPoolIdleCleanupWaitsForARunningCheck(t *testing.T) { + t.Parallel() + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, manyCheckerPoolFiles()) + t.Cleanup(session.Close) + ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") + assert.NilError(t, err) + program := ls.GetProgram() + + synctest.Test(t, func(t *testing.T) { + pool := newTestCheckerPool(program, CheckerPoolOptions{IdleTimeout: 30 * time.Second}) + // Single threaded, so the check runs on the first checker only. + const owner = 0 + group := program.SourceFiles() + + // A checker the check is not on right now is built and handed back, as one given a small + // share of the files would be early on, while the rest of the check runs for minutes. + other := (owner + 1) % pool.diagnosticsCount + idle, releaseIdle := pool.acquireDiagnosticsChecker(other, "early") + assert.Assert(t, idle != nil) + releaseIdle() + + // Meanwhile the check is still running. + ctx, cancel := context.WithCancel(core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics)) + defer cancel() + started := make(chan struct{}) + blocked := make(chan struct{}) + go func() { + pool.ForEachCheckerGroupDo(ctx, group, true /*singleThreaded*/, func(*checker.Checker, int, *ast.SourceFile) { + select { + case <-started: + default: + close(started) + } + <-blocked + }) + }() + <-started + + time.Sleep(2 * pool.opts.IdleTimeout) + synctest.Wait() + pool.mu.Lock() + kept := pool.checkers[other] + pool.mu.Unlock() + assert.Assert(t, kept == idle, "an idle checker must not be collected while a check is still running") + + close(blocked) + synctest.Wait() + }) +} diff --git a/tsc/internal/project/idlecacheclean_test.go b/tsc/internal/project/idlecacheclean_test.go new file mode 100644 index 0000000000000..0187363c681ac --- /dev/null +++ b/tsc/internal/project/idlecacheclean_test.go @@ -0,0 +1,38 @@ +package project + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +// A snapshot update stands a pass over the workspace down, because the pass must not compete with +// work the user is waiting on. The idle clean updates the snapshot as well, and it is armed by a +// file event half a minute earlier, so on a project where a pass runs for minutes it lands in the +// middle of one. Nothing is waiting behind it, so the pass carries on. +func TestOnlyUpdatesSomeoneIsWaitingOnStandAPassDown(t *testing.T) { + t.Parallel() + + waiting := []UpdateReason{ + UpdateReasonUnknown, + UpdateReasonDidOpenFile, + UpdateReasonDidCloseFile, + UpdateReasonDidChangeCompilerOptionsForInferredProjects, + UpdateReasonRequestedLanguageServicePendingChanges, + UpdateReasonRequestedLanguageServiceProjectNotLoaded, + UpdateReasonRequestedLanguageServiceForFileNotOpen, + UpdateReasonRequestedLanguageServiceProjectDirty, + UpdateReasonRequestedLoadProjectTree, + UpdateReasonRequestedLanguageServiceWithAutoImports, + UpdateReasonDidChangeConfigFile, + UpdateReasonDidChangeContentMapperContributions, + } + for _, reason := range waiting { + assert.Assert(t, reason.someoneIsWaiting(), "reason %d is one a request waits behind", reason) + } + assert.Assert(t, !UpdateReasonIdleCleanDiskCache.someoneIsWaiting(), "the idle clean is housekeeping on a timer") + + // Every reason is accounted for above, so a new one cannot quietly be left unconsidered. + assert.Equal(t, len(waiting)+1, int(UpdateReasonDidChangeContentMapperContributions)+1, + "a new update reason needs deciding either way here") +} diff --git a/tsc/internal/project/interactivework.go b/tsc/internal/project/interactivework.go new file mode 100644 index 0000000000000..5da183cf64c10 --- /dev/null +++ b/tsc/internal/project/interactivework.go @@ -0,0 +1,76 @@ +package project + +import ( + "context" + "sync" +) + +// interactiveWork counts the work a user is waiting on: a pull on the document in front of them, or +// a snapshot update that every other request is queued behind. A pass over the whole workspace +// stands aside while any of it is outstanding. +// +// The trade is one-sided. A workspace pass runs for as long as the workspace is big and nothing +// waits on it finishing sooner, so the time it gives up is time the user was going to spend waiting +// anyway; whereas a pass that keeps every checker and every core busy makes the file in front of +// them answer minutes late. +// +// One of these is shared by every checker pool in a session, so a pull on one project stands the +// whole pass down rather than just the project that owns the file. +type interactiveWork struct { + mu sync.Mutex + count int + // idle is closed while nothing is outstanding, and replaced with an open channel when + // something starts. A waiter reads the channel rather than polling the count. + idle chan struct{} +} + +func newInteractiveWork() *interactiveWork { + idle := make(chan struct{}) + close(idle) + return &interactiveWork{idle: idle} +} + +// begin marks interactive work as started and returns the function that marks it done. Safe on a +// nil receiver, which is what a pool built outside a session gets. +func (w *interactiveWork) begin() func() { + if w == nil { + return noop + } + w.mu.Lock() + if w.count == 0 { + w.idle = make(chan struct{}) + } + w.count++ + w.mu.Unlock() + + return sync.OnceFunc(func() { + w.mu.Lock() + defer w.mu.Unlock() + w.count-- + if w.count == 0 { + close(w.idle) + } + }) +} + +// waitForIdle blocks until nothing interactive is outstanding, or ctx is done. A caller must not +// hold a checker while waiting, or the work it is standing aside for could be waiting on that. +func (w *interactiveWork) waitForIdle(ctx context.Context) error { + if w == nil { + return nil + } + for { + w.mu.Lock() + count, idle := w.count, w.idle + w.mu.Unlock() + if count == 0 { + return nil + } + select { + case <-idle: + // Something finished; look again, since more may have started. + case <-ctx.Done(): + return ctx.Err() + } + } +} diff --git a/tsc/internal/project/interactivework_test.go b/tsc/internal/project/interactivework_test.go new file mode 100644 index 0000000000000..2d1a22306c923 --- /dev/null +++ b/tsc/internal/project/interactivework_test.go @@ -0,0 +1,101 @@ +package project + +import ( + "context" + "sync/atomic" + "testing" + "testing/synctest" + + "gotest.tools/v3/assert" +) + +// Nothing outstanding means nothing to wait for. +func TestInteractiveWorkIdleByDefault(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + work := newInteractiveWork() + work.waitForIdle(context.Background()) + }) +} + +// A waiter is held until the last outstanding piece of work finishes, not the first. +func TestInteractiveWorkWaitsForAllOfIt(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + work := newInteractiveWork() + firstDone := work.begin() + secondDone := work.begin() + + var idle atomic.Bool + go func() { + work.waitForIdle(context.Background()) + idle.Store(true) + }() + synctest.Wait() + assert.Assert(t, !idle.Load(), "a waiter must not go while work is outstanding") + + firstDone() + synctest.Wait() + assert.Assert(t, !idle.Load(), "one of two finishing is not enough") + + secondDone() + synctest.Wait() + assert.Assert(t, idle.Load(), "the waiter goes once the last piece finishes") + }) +} + +// Work starting while a waiter is being woken must hold it back again, rather than letting it +// through on the strength of a moment of quiet. +func TestInteractiveWorkHoldsAcrossBackToBackWork(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + work := newInteractiveWork() + done := work.begin() + + var idle atomic.Bool + go func() { + work.waitForIdle(context.Background()) + idle.Store(true) + }() + synctest.Wait() + + next := work.begin() + done() + synctest.Wait() + assert.Assert(t, !idle.Load(), "the waiter must see the work that started behind the first") + + next() + synctest.Wait() + assert.Assert(t, idle.Load()) + }) +} + +// A cancelled caller stops waiting, so a superseded pass does not hang on a busy editor. +func TestInteractiveWorkStopsWaitingWhenCancelled(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + work := newInteractiveWork() + defer work.begin()() + + ctx, cancel := context.WithCancel(context.Background()) + var returned atomic.Bool + go func() { + work.waitForIdle(ctx) + returned.Store(true) + }() + synctest.Wait() + assert.Assert(t, !returned.Load()) + + cancel() + synctest.Wait() + assert.Assert(t, returned.Load(), "a cancelled waiter must not wait out the work") + }) +} + +// A pool built outside a session has no gate, and must not panic on one. +func TestInteractiveWorkNilIsInert(t *testing.T) { + t.Parallel() + var work *interactiveWork + work.begin()() + work.waitForIdle(context.Background()) +} diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index 09219738b7cd8..3e37444fc37de 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -508,7 +508,7 @@ func (p *Project) CreateProgram() CreateProgramResult { createCheckerPool := func(program *compiler.Program) compiler.CheckerPool { opts := p.host.sessionOptions.CheckerPoolOptions opts.MatchBuildCheckerCount = p.host.sessionOptions.workspaceDiagnosticsEnabled.Load() - return newCheckerPool(opts, program, p.log) + return newCheckerPool(opts, program, p.host.sessionOptions.interactiveWork, p.log) } var cleanupModuleResolver func() createModuleResolver := func(options module.ResolverOptions) module.Resolver { diff --git a/tsc/internal/project/session.go b/tsc/internal/project/session.go index 39226c600346a..85c5341d2b16e 100644 --- a/tsc/internal/project/session.go +++ b/tsc/internal/project/session.go @@ -54,6 +54,14 @@ const ( UpdateReasonDidChangeContentMapperContributions ) +// someoneIsWaiting reports whether a request is held up behind an update made for this reason. +// Every reason but the idle clean comes from something the user did or something a request asked +// for; the idle clean is housekeeping on a timer, and a pass over the workspace that stood down +// for it would be standing down for nobody. +func (r UpdateReason) someoneIsWaiting() bool { + return r != UpdateReasonIdleCleanDiskCache +} + type ContentMapperContributions struct { Mappers []*contentmapper.Mapper Extensions []string @@ -80,6 +88,10 @@ type SessionOptions struct { DebounceDelay time.Duration CheckerPoolOptions CheckerPoolOptions + // interactiveWork is shared by every checker pool in the session. Set by NewSnapshotHost; see + // interactiveWork for what it is for. + interactiveWork *interactiveWork + // workspaceDiagnosticsEnabled tracks whether the workspace pull is switched on, so that a // project only pays for a build's worth of checkers when something is going to check it that // way. Written whenever the user's preferences change and read when a program's pool is built, @@ -1361,6 +1373,12 @@ func (s *Session) updateSnapshotRef(ctx context.Context, overlays map[tspath.Pat } func (s *Session) updateSnapshot(ctx context.Context, overlays map[tspath.Path]*Overlay, change SnapshotChange, callerRef bool) *Snapshot { + // Rebuilding a program holds the snapshot write lock, so every request in the session waits on + // it. A workspace pass must not be competing for the machine while it runs. + if change.reason.someoneIsWaiting() { + defer s.options.interactiveWork.begin()() + } + s.snapshotMu.Lock() oldSnapshot := s.snapshot if !locale.HasLocale(ctx) { diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 920766cb7b3bb..d8d9a797297d0 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -211,6 +211,13 @@ func (s *Snapshot) GetLanguageServiceProjectsContainingFile(uri lsproto.Document return s.ProjectCollection.GetLanguageServiceProjectsContainingFile(path) } +// WaitForInteractiveIdle blocks until nothing a user is waiting on is outstanding, or ctx is done. +// A caller about to spend minutes on work nobody asked for uses this to let the work they did ask +// for go first. +func (s *Snapshot) WaitForInteractiveIdle(ctx context.Context) { + s.host.options.interactiveWork.waitForIdle(ctx) +} + // ReleaseDiagnosticsCheckers drops the checkers a sweep used on a project. They hold the types of // every file in it, which is the largest thing a pull creates, and keeping them buys nothing: a // pull that finds the project unchanged answers from the result ids the client already holds diff --git a/tsc/internal/project/snapshothost.go b/tsc/internal/project/snapshothost.go index 6659133490421..3bf636c03c7dc 100644 --- a/tsc/internal/project/snapshothost.go +++ b/tsc/internal/project/snapshothost.go @@ -76,6 +76,11 @@ func NewSnapshotHost(init *SessionInit) *SnapshotHost { if contentMappedParseCache == nil { contentMappedParseCache = NewContentMappedParseCache(RefCountCacheOptions{}) } + // Shared through the options because that is what a project's checker pool can reach. Set + // here rather than in NewSession so that every host has one, however it was built. + if init.Options.interactiveWork == nil { + init.Options.interactiveWork = newInteractiveWork() + } return &SnapshotHost{ options: init.Options, From 932d81975b2b0856254e41032236a29a13cb3515 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:12:14 -0700 Subject: [PATCH 04/10] Report how far a whole-program check has got A check of a large project runs for minutes inside a single call, so a caller that wants to say how far along it is cannot learn it from the call returning. The pool doing the work has to say. The pool takes a callback from the context and calls it as each file is finished. It is called once per file, from each of the checkers, so it has to be cheap and safe to call concurrently; what a caller does with it, and how often it acts on it, is the caller's business. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/project/checkerpool.go | 13 ++++++++- tsc/internal/project/checkerpool_test.go | 35 ++++++++++++++++++++++++ tsc/internal/project/checkprogress.go | 27 ++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tsc/internal/project/checkprogress.go diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index 51a915bbb2737..8e246e38b1377 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -354,11 +354,14 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So // A caller the user is waiting on is what everything else stands aside for; waiting here would // be waiting on itself, and the checkers would take turns rather than run together. standAside := !core.IsInteractiveRequest(ctx) + // Counted across the checkers rather than per checker, so a caller sees one number for the + // check rather than one per checker. + reportProgress := checkProgressFrom(ctx) // A snapshot update part way through discards this pool. The check is still going to ask for // its checkers, so they have to outlast the discard; see endSweep. p.beginSweep() defer p.endSweep() - var next atomic.Int64 + var next, checked atomic.Int64 // Single threaded, one checker takes the whole queue anyway; make it the first, which is where a // file no checker has seen goes too. workers := p.diagnosticsCount @@ -399,10 +402,18 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So cb(c, i, files[i]) release() p.noteDiagnosticsAffinity(files[i], index) + if done := checked.Add(1); reportProgress != nil && done%checkProgressBatchFiles == 0 { + reportProgress(int(done), len(files)) + } } }) } wg.RunAndWait() + // The last batch is rarely a whole one, so the final count is reported here rather than left + // short of where the check actually got to. + if reportProgress != nil { + reportProgress(int(checked.Load()), len(files)) + } } // beginSweep records a whole-program check as running on this pool. diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index 072f8808f57a7..6df2847a40e08 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -1890,3 +1890,38 @@ func TestCheckerPoolIdleCleanupWaitsForARunningCheck(t *testing.T) { synctest.Wait() }) } + +// A bar has a hundred positions however big the sweep, so a report per file is work no one can +// see. The final count is still reported, or the bar stops short of where the check got to. +func TestCheckerPoolBatchesProgressReports(t *testing.T) { + t.Parallel() + + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, manyCheckerPoolFiles()) + t.Cleanup(session.Close) + + files := pool.program.SourceFiles() + var mu sync.Mutex + var reports [][2]int + ctx := WithCheckProgress( + core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics), + func(checked, total int) { + mu.Lock() + defer mu.Unlock() + reports = append(reports, [2]int{checked, total}) + }) + + checked := 0 + pool.ForEachCheckerGroupDo(ctx, files, true /*singleThreaded*/, func(*checker.Checker, int, *ast.SourceFile) { + checked++ + }) + + assert.Equal(t, checked, len(files), "every file is checked") + assert.Equal(t, len(reports), checked/checkProgressBatchFiles+1, + "one report per %d files plus a final one, for %d files: %v", checkProgressBatchFiles, checked, reports) + last := reports[len(reports)-1] + assert.Equal(t, last[0], checked, "the last report is the final count") + assert.Equal(t, last[1], len(files), "the total is the files it was given") + for _, report := range reports[:len(reports)-1] { + assert.Equal(t, report[0]%checkProgressBatchFiles, 0, "batched reports land on a batch boundary: %v", reports) + } +} diff --git a/tsc/internal/project/checkprogress.go b/tsc/internal/project/checkprogress.go new file mode 100644 index 0000000000000..c8fa6a30ee132 --- /dev/null +++ b/tsc/internal/project/checkprogress.go @@ -0,0 +1,27 @@ +package project + +import "context" + +type checkProgressKey struct{} + +// checkProgressBatchFiles is how many files a check gets through between reports. A bar has a +// hundred positions however big the sweep, so reporting every file is work no one can see. +const checkProgressBatchFiles = 10 + +// CheckProgress is told how far a whole-program check has got: how many of the files it was given +// it has finished, and how many there were. It is called from each of the checkers doing the work, +// so it must be safe to call concurrently. It is called once per checkProgressBatchFiles files, +// and once more when the check finishes, so the last report is the final count. +type CheckProgress func(checked int, total int) + +// WithCheckProgress attaches a progress callback to a context. A check of a large project runs for +// minutes inside a single call, so a caller that wants to say how far along it is cannot learn it +// from the call returning; the pool doing the work has to say. +func WithCheckProgress(ctx context.Context, report CheckProgress) context.Context { + return context.WithValue(ctx, checkProgressKey{}, report) +} + +func checkProgressFrom(ctx context.Context) CheckProgress { + report, _ := ctx.Value(checkProgressKey{}).(CheckProgress) + return report +} From 86a818b275edf056cb737c2cc003e3963feb8dff Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:13:01 -0700 Subject: [PATCH 05/10] Re-check only the files an edit affects Checking a project checked every file of it, however little had changed. An edit to one file meant the next check re-did the workspace. A project now keeps the incremental view of its program: the file hashes, references and cached diagnostics one program leaves for the next to work out what a change reached. A caller asking through that gets a file the edit did not reach from what was cached rather than checking it again. The view is built on first use, since working it out walks every file and most programs are never asked, and what it worked out is carried to the next program without carrying the program itself - a caller that keeps the whole thing keeps every type reachable from it too. Building it resolves every file's imports, which needs a checker. It asks for a diagnostics checker rather than letting the project system read the bare context as a query: a query checker is never handed back when a check is done, so a program's worth of types would sit in the slots kept for hovers and completions until they idled out, and survive into the next program's generation. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/execute/incremental/program.go | 37 +++ .../execute/incremental/programtosnapshot.go | 114 +++++---- tsc/internal/project/checkerpool_test.go | 217 +++++++++++------- .../project/incrementalreferences_test.go | 116 ++++++++++ tsc/internal/project/incrementalstate.go | 107 +++++++++ tsc/internal/project/incrementalstate_test.go | 89 +++++++ tsc/internal/project/project.go | 4 + .../project/projectcollectionbuilder.go | 4 + tsc/internal/project/snapshot.go | 8 + 9 files changed, 567 insertions(+), 129 deletions(-) create mode 100644 tsc/internal/project/incrementalreferences_test.go create mode 100644 tsc/internal/project/incrementalstate.go create mode 100644 tsc/internal/project/incrementalstate_test.go diff --git a/tsc/internal/execute/incremental/program.go b/tsc/internal/execute/incremental/program.go index 4251a235457ee..78f0f10283700 100644 --- a/tsc/internal/execute/incremental/program.go +++ b/tsc/internal/execute/incremental/program.go @@ -72,6 +72,43 @@ type TestingData struct { UpdatedSignatureKinds map[tspath.Path]SignatureUpdateKind } +// PriorState is what one program leaves for the next to work out what a change reached: the file +// hashes, references and cached diagnostics it built, and none of the program they came from. A +// caller that keeps a whole Program for this keeps its program too, and every type reachable from +// it, for as long as it holds on. +type PriorState struct { + snapshot *snapshot +} + +// PriorState returns what this program has worked out, without the program itself. +func (p *Program) PriorState() *PriorState { + if p == nil { + return nil + } + return &PriorState{snapshot: p.snapshot} +} + +// NewProgramFromPriorState is NewProgram for a caller that kept only what the previous program +// worked out, rather than the program itself. +// +// reuseReferences says the program is a clone of the one prior came from. Working out what a file +// references means resolving each of its imports through a type checker, for every file in the +// program, which is the most expensive thing building this state does. A clone is only made when +// the replaced file's imports, module augmentations, ambient module names and reference directives +// are all unchanged, and no other file moves, so every file resolves to what it did before and the +// whole map can be carried over instead. +func NewProgramFromPriorState(program *compiler.Program, prior *PriorState, host Host, reuseReferences bool) *Program { + var oldSnapshot *snapshot + if prior != nil { + oldSnapshot = prior.snapshot + } + return &Program{ + snapshot: buildSnapshot(program, oldSnapshot, false /*hashWithText*/, reuseReferences && oldSnapshot != nil), + program: program, + host: host, + } +} + func (p *Program) GetTestingData() *TestingData { return p.testingData } diff --git a/tsc/internal/execute/incremental/programtosnapshot.go b/tsc/internal/execute/incremental/programtosnapshot.go index 27bfffb552b1c..746b4bcb8038d 100644 --- a/tsc/internal/execute/incremental/programtosnapshot.go +++ b/tsc/internal/execute/incremental/programtosnapshot.go @@ -17,15 +17,25 @@ func programToSnapshot(program *compiler.Program, oldProgram *Program, hashWithT if oldProgram != nil && oldProgram.program == program { return oldProgram.snapshot } + var oldSnapshot *snapshot + if oldProgram != nil { + oldSnapshot = oldProgram.snapshot + } + return buildSnapshot(program, oldSnapshot, hashWithText, false /*reuseReferences*/) +} + +// buildSnapshot works out what a program changed against what the one before it left behind. +func buildSnapshot(program *compiler.Program, oldSnapshot *snapshot, hashWithText bool, reuseReferences bool) *snapshot { snapshot := &snapshot{ options: program.Options(), hashWithText: hashWithText, checkPending: program.Options().NoCheck.IsTrue(), } to := &toProgramSnapshot{ - program: program, - oldProgram: oldProgram, - snapshot: snapshot, + program: program, + oldSnapshot: oldSnapshot, + snapshot: snapshot, + reuseReferences: reuseReferences, } if to.snapshot.canUseIncrementalState() { @@ -40,39 +50,42 @@ func programToSnapshot(program *compiler.Program, oldProgram *Program, hashWithT } type toProgramSnapshot struct { - program *compiler.Program - oldProgram *Program - snapshot *snapshot + program *compiler.Program + oldSnapshot *snapshot + snapshot *snapshot + // reuseReferences says every file references what it did in oldSnapshot; see + // NewProgramFromPriorState. + reuseReferences bool globalFileRemoved bool } func (t *toProgramSnapshot) reuseFromOldProgram() { - if t.oldProgram != nil { + if t.oldSnapshot != nil { if t.snapshot.options.Composite.IsTrue() { - t.snapshot.latestChangedDtsFile = t.oldProgram.snapshot.latestChangedDtsFile + t.snapshot.latestChangedDtsFile = t.oldSnapshot.latestChangedDtsFile } // Copy old snapshot's changed files set - t.oldProgram.snapshot.changedFilesSet.Range(func(key tspath.Path) bool { + t.oldSnapshot.changedFilesSet.Range(func(key tspath.Path) bool { t.snapshot.changedFilesSet.Add(key) return true }) - t.oldProgram.snapshot.affectedFilesPendingEmit.Range(func(key tspath.Path, emitKind FileEmitKind) bool { + t.oldSnapshot.affectedFilesPendingEmit.Range(func(key tspath.Path, emitKind FileEmitKind) bool { t.snapshot.affectedFilesPendingEmit.Store(key, emitKind) return true }) - t.snapshot.buildInfoEmitPending.Store(t.oldProgram.snapshot.buildInfoEmitPending.Load()) - t.snapshot.hasErrorsFromOldState = t.oldProgram.snapshot.hasErrors - t.snapshot.hasSemanticErrorsFromOldState = t.oldProgram.snapshot.hasSemanticErrors - t.snapshot.packageJsonsFromOldState = t.oldProgram.snapshot.packageJsons - t.snapshot.missingPackageJsonsFromOldState = t.oldProgram.snapshot.missingPackageJsons + t.snapshot.buildInfoEmitPending.Store(t.oldSnapshot.buildInfoEmitPending.Load()) + t.snapshot.hasErrorsFromOldState = t.oldSnapshot.hasErrors + t.snapshot.hasSemanticErrorsFromOldState = t.oldSnapshot.hasSemanticErrors + t.snapshot.packageJsonsFromOldState = t.oldSnapshot.packageJsons + t.snapshot.missingPackageJsonsFromOldState = t.oldSnapshot.missingPackageJsons } else { t.snapshot.buildInfoEmitPending.Store(t.snapshot.options.IsIncremental()) } } func (t *toProgramSnapshot) computeProgramFileChanges() { - canCopySemanticDiagnostics := t.oldProgram != nil && - !tsoptions.CompilerOptionsAffectSemanticDiagnostics(t.oldProgram.snapshot.options, t.program.Options()) + canCopySemanticDiagnostics := t.oldSnapshot != nil && + !tsoptions.CompilerOptionsAffectSemanticDiagnostics(t.oldSnapshot.options, t.program.Options()) // We can only reuse emit signatures (i.e. .d.ts signatures) if the .d.ts file is unchanged, // which will eg be depedent on change in options like declarationDir and outDir options are unchanged. // We need to look in oldState.compilerOptions, rather than oldCompilerOptions (i.e.we need to disregard useOldState) because @@ -80,12 +93,12 @@ func (t *toProgramSnapshot) computeProgramFileChanges() { // which would make useOldState as false since we can now use reference maps that are needed to track what to emit, what to check etc // but that option change does not affect d.ts file name so emitSignatures should still be reused. canCopyEmitSignatures := t.snapshot.options.Composite.IsTrue() && - t.oldProgram != nil && - !tsoptions.CompilerOptionsAffectDeclarationPath(t.oldProgram.snapshot.options, t.program.Options()) + t.oldSnapshot != nil && + !tsoptions.CompilerOptionsAffectDeclarationPath(t.oldSnapshot.options, t.program.Options()) copyDeclarationFileDiagnostics := canCopySemanticDiagnostics && - t.snapshot.options.SkipLibCheck.IsTrue() == t.oldProgram.snapshot.options.SkipLibCheck.IsTrue() + t.snapshot.options.SkipLibCheck.IsTrue() == t.oldSnapshot.options.SkipLibCheck.IsTrue() copyLibFileDiagnostics := copyDeclarationFileDiagnostics && - t.snapshot.options.SkipDefaultLibCheck.IsTrue() == t.oldProgram.snapshot.options.SkipDefaultLibCheck.IsTrue() + t.snapshot.options.SkipDefaultLibCheck.IsTrue() == t.oldSnapshot.options.SkipDefaultLibCheck.IsTrue() files := t.program.GetSourceFiles() wg := core.NewWorkGroup(t.program.SingleThreaded()) @@ -99,22 +112,22 @@ func (t *toProgramSnapshot) computeProgramFileChanges() { impliedNodeFormat := t.program.GetSourceFileMetaData(file.Path()).ImpliedNodeFormat affectsGlobalScope := fileAffectsGlobalScope(file) var signature string - newReferences := getReferencedFiles(t.program, file) + newReferences := t.referencesOf(file) if newReferences != nil { t.snapshot.referencedMap.storeReferences(file.Path(), newReferences) } - if t.oldProgram != nil { - if oldFileInfo, ok := t.oldProgram.snapshot.fileInfos.Load(file.Path()); ok { + if t.oldSnapshot != nil { + if oldFileInfo, ok := t.oldSnapshot.fileInfos.Load(file.Path()); ok { signature = oldFileInfo.signature if oldFileInfo.version != version || oldFileInfo.affectsGlobalScope != affectsGlobalScope || oldFileInfo.impliedNodeFormat != impliedNodeFormat { t.snapshot.addFileToChangeSet(file.Path()) - } else if oldReferences, _ := t.oldProgram.snapshot.referencedMap.getReferences(file.Path()); !newReferences.Equals(oldReferences) { + } else if oldReferences, _ := t.oldSnapshot.referencedMap.getReferences(file.Path()); !newReferences.Equals(oldReferences) { // Referenced files changed t.snapshot.addFileToChangeSet(file.Path()) } else if newReferences != nil { for refPath := range newReferences.Keys() { if t.program.GetSourceFileByPath(refPath) == nil { - if _, ok := t.oldProgram.snapshot.fileInfos.Load(refPath); ok { + if _, ok := t.oldSnapshot.fileInfos.Load(refPath); ok { // Referenced file was deleted in the new program t.snapshot.addFileToChangeSet(file.Path()) break @@ -126,22 +139,22 @@ func (t *toProgramSnapshot) computeProgramFileChanges() { t.snapshot.addFileToChangeSet(file.Path()) } if !t.snapshot.changedFilesSet.Has(file.Path()) { - if emitDiagnostics, ok := t.oldProgram.snapshot.emitDiagnosticsPerFile.Load(file.Path()); ok { + if emitDiagnostics, ok := t.oldSnapshot.emitDiagnosticsPerFile.Load(file.Path()); ok { t.snapshot.emitDiagnosticsPerFile.Store(file.Path(), repopulateDiagnosticsOfFile(emitDiagnostics, t.program, file)) } if canCopySemanticDiagnostics { if (!file.IsDeclarationFile || copyDeclarationFileDiagnostics) && (!t.program.IsSourceFileDefaultLibrary(file.Path()) || copyLibFileDiagnostics) { // Unchanged file copy diagnostics - if diagnostics, ok := t.oldProgram.snapshot.semanticDiagnosticsPerFile.Load(file.Path()); ok { + if diagnostics, ok := t.oldSnapshot.semanticDiagnosticsPerFile.Load(file.Path()); ok { t.snapshot.semanticDiagnosticsPerFile.Store(file.Path(), repopulateDiagnosticsOfFile(diagnostics, t.program, file)) } } } } if canCopyEmitSignatures { - if oldEmitSignature, ok := t.oldProgram.snapshot.emitSignatures.Load(file.Path()); ok { - t.snapshot.emitSignatures.Store(file.Path(), oldEmitSignature.getNewEmitSignature(t.oldProgram.snapshot.options, t.snapshot.options)) + if oldEmitSignature, ok := t.oldSnapshot.emitSignatures.Load(file.Path()); ok { + t.snapshot.emitSignatures.Store(file.Path(), oldEmitSignature.getNewEmitSignature(t.oldSnapshot.options, t.snapshot.options)) } } } else { @@ -160,9 +173,9 @@ func (t *toProgramSnapshot) computeProgramFileChanges() { } func (t *toProgramSnapshot) handleFileDelete() { - if t.oldProgram != nil { + if t.oldSnapshot != nil { // If the global file is removed, add all files as changed - t.oldProgram.snapshot.fileInfos.Range(func(filePath tspath.Path, oldInfo *FileInfo) bool { + t.oldSnapshot.fileInfos.Range(func(filePath tspath.Path, oldInfo *FileInfo) bool { if _, ok := t.snapshot.fileInfos.Load(filePath); !ok { if oldInfo.affectsGlobalScope { for _, file := range t.snapshot.getAllFilesExcludingDefaultLibraryFile(t.program, nil) { @@ -180,11 +193,11 @@ func (t *toProgramSnapshot) handleFileDelete() { } func (t *toProgramSnapshot) handleGlobalScopeChange() { - if t.oldProgram == nil || t.globalFileRemoved { + if t.oldSnapshot == nil || t.globalFileRemoved { return } globalScopeLost := false - t.oldProgram.snapshot.fileInfos.Range(func(filePath tspath.Path, oldInfo *FileInfo) bool { + t.oldSnapshot.fileInfos.Range(func(filePath tspath.Path, oldInfo *FileInfo) bool { if !oldInfo.affectsGlobalScope { return true } @@ -202,14 +215,14 @@ func (t *toProgramSnapshot) handleGlobalScopeChange() { } func (t *toProgramSnapshot) handlePendingEmit() { - if t.oldProgram != nil && !t.globalFileRemoved { + if t.oldSnapshot != nil && !t.globalFileRemoved { // If options affect emit, then we need to do complete emit per compiler options // otherwise only the js or dts that needs to emitted because its different from previously emitted options var pendingEmitKind FileEmitKind - if tsoptions.CompilerOptionsAffectEmit(t.oldProgram.snapshot.options, t.snapshot.options) { + if tsoptions.CompilerOptionsAffectEmit(t.oldSnapshot.options, t.snapshot.options) { pendingEmitKind = GetFileEmitKind(t.snapshot.options) } else { - pendingEmitKind = getPendingEmitKindWithOptions(t.snapshot.options, t.oldProgram.snapshot.options) + pendingEmitKind = getPendingEmitKindWithOptions(t.snapshot.options, t.oldSnapshot.options) } if pendingEmitKind != FileEmitKindNone { // Add all files to affectedFilesPendingEmit since emit changed @@ -225,9 +238,9 @@ func (t *toProgramSnapshot) handlePendingEmit() { } func (t *toProgramSnapshot) handlePendingCheck() { - if t.oldProgram != nil && + if t.oldSnapshot != nil && t.snapshot.semanticDiagnosticsPerFile.Size() != len(t.program.GetSourceFiles()) && - t.oldProgram.snapshot.checkPending != t.snapshot.checkPending { + t.oldSnapshot.checkPending != t.snapshot.checkPending { t.snapshot.buildInfoEmitPending.Store(true) } } @@ -287,6 +300,21 @@ func addReferencedFileFromFileName(program *compiler.Program, fileName string, r } } +// referencesOf returns what a file references, from the previous program's map where that map +// still describes it, and by resolving its imports through a type checker where it does not. +func (t *toProgramSnapshot) referencesOf(file *ast.SourceFile) *collections.Set[tspath.Path] { + if t.reuseReferences { + // Only for a file the previous program had. No entry in the map means it referenced + // nothing, which is an answer, so the file being known is what decides this rather than + // the lookup succeeding. + if _, known := t.oldSnapshot.fileInfos.Load(file.Path()); known { + references, _ := t.oldSnapshot.referencedMap.getReferences(file.Path()) + return references + } + } + return getReferencedFiles(t.program, file) +} + // Gets the referenced files for a file from the program with values for the keys as referenced file's path to be true func getReferencedFiles(program *compiler.Program, file *ast.SourceFile) *collections.Set[tspath.Path] { referencedFiles := collections.Set[tspath.Path]{} @@ -294,7 +322,13 @@ func getReferencedFiles(program *compiler.Program, file *ast.SourceFile) *collec // We need to use a set here since the code can contain the same import twice, // but that will only be one dependency. // To avoid invernal conversion, the key of the referencedFiles map must be of type Path - checker, done := program.GetTypeCheckerForFileExclusive(context.TODO(), file) + // The project system hands out checkers by what the caller wants them for, and this is the + // work of checking rather than a query: a query checker is never handed back by a sweep, so + // resolving every file's imports through one would leave a share of the program's types in + // each of the slots kept for hovers and completions until they idled out. The command line's + // pool does not distinguish, so this says nothing to it. + checkerCtx := core.WithCheckerLifetime(context.TODO(), core.CheckerLifetimeDiagnostics) + checker, done := program.GetTypeCheckerForFileExclusive(checkerCtx, file) defer done() for _, importName := range file.Imports() { addReferencedFilesFromImportLiteral(file, &referencedFiles, checker, importName) diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index 6df2847a40e08..2423d0a1d81fb 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -1487,105 +1487,42 @@ func TestCheckerPoolWholeProgramCheckYieldsBetweenFiles(t *testing.T) { }) } -// A whole-program check runs on the diagnostics checkers, of which there are as many as a build -// would use, and leaves each file remembered against the one that checked it. -func TestCheckerPoolChecksWholeProgramAcrossDiagnosticsCheckers(t *testing.T) { +// Building the incremental view of a program resolves every file's imports, which needs a checker. +// It has to be one of the checkers the caller is about to check with: a sweep only hands back the +// diagnostics checkers, so anything it leaves in the query slots holds a program's worth of types +// until it idles out, and survives into the next program's generation. +func TestCheckerPoolIncrementalViewUsesDiagnosticsCheckers(t *testing.T) { t.Parallel() - _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, - "/src/index.ts": "export const x: number = 1;", - "/src/a.ts": "export const a: string = 1;", - "/src/b.ts": "export const b = 1;", + "/src/index.ts": "import { a } from \"./a.js\";\nexport const x: number = a;\n", + "/src/a.ts": "import { b } from \"./b.js\";\nexport const a = b;\n", + "/src/b.ts": "export const b = 1;\n", }) - assert.Equal(t, pool.diagnosticsCount, diagnosticsCheckerCount(pool.program)) - assert.Assert(t, pool.diagnosticsCount > 1, "a three-file program gets more than one checker") - for index := range pool.checkers { - assert.Assert(t, pool.checkers[index] == nil, "nothing built before a check is asked for") - } - diagnostics := pool.program.GetSemanticDiagnostics(context.Background(), nil) - assert.Assert(t, len(diagnostics) > 0, "expected the seeded error") - - for _, file := range pool.program.SourceFiles() { - index := pool.diagnosticsIndexFor(file) - assert.Assert(t, index < pool.diagnosticsCount && pool.checkers[index] != nil, - "%s must be remembered against the diagnostics checker that checked it", file.FileName()) - } - for index := pool.diagnosticsCount; index < len(pool.checkers); index++ { - assert.Assert(t, pool.checkers[index] == nil, "a whole-program check must not use a query checker") - } -} + snapshot := session.Snapshot() + project := snapshot.ProjectCollection.ConfiguredProject("/src/tsconfig.json") + assert.Assert(t, project != nil) -// Query checkers sit after the diagnostics ones, so a query never lands on a checker a whole-program -// check is partway through. -func TestCheckerPoolQueryCheckersSitAfterDiagnosticsCheckers(t *testing.T) { - t.Parallel() - _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ - "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, - "/src/index.ts": "export const x: number = 1;", - "/src/a.ts": "export const a: string = 1;", - "/src/b.ts": "export const b = 1;", - }) - assert.Equal(t, len(pool.checkers), pool.diagnosticsCount+3, "MaxCheckers-1 query checkers on top of the diagnostics ones") + ctx := core.WithRequestID(t.Context(), "sweep") + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + assert.Assert(t, snapshot.IncrementalProgram(project) != nil) - ctx := core.WithCheckerLifetime(core.WithRequestID(t.Context(), "query-after-diag"), core.CheckerLifetimeTemporary) - c, release := pool.GetChecker(ctx, nil) - defer release() + diagnostics := 0 for index := range pool.diagnosticsCount { - assert.Assert(t, pool.checkers[index] != c, "a query must not use a diagnostics checker") - } -} - -// The checkers a sweep uses are handed back when it is done with them: they hold the types of every -// file in the program, and a later pull either finds the project unchanged, and answers from result -// ids without checking, or finds it changed and needs new ones anyway. -func TestCheckerPoolReleasesCheckersAfterAWholeProgramCheck(t *testing.T) { - t.Parallel() - _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ - "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, - "/src/index.ts": "export const x: number = 1;", - "/src/a.ts": "export const a: string = 1;", - }) - - anyDiagnosticsChecker := func() bool { - for index := range pool.diagnosticsCount { - if pool.checkers[index] != nil { - return true - } + if pool.checkers[index] != nil { + diagnostics++ } - return false + } + assert.Assert(t, diagnostics > 0, "resolving the imports must have used the diagnostics checkers") + for index := pool.diagnosticsCount; index < len(pool.checkers); index++ { + assert.Assert(t, pool.checkers[index] == nil, "query checker %d must not hold what a sweep will not hand back", index) } - pool.program.GetSemanticDiagnostics(context.Background(), nil) - assert.Assert(t, anyDiagnosticsChecker()) - - assert.Assert(t, pool.releaseDiagnosticsCheckers(), "releasing reports that it dropped the checkers") - assert.Assert(t, !anyDiagnosticsChecker()) - assert.Assert(t, !pool.releaseDiagnosticsCheckers(), "nothing left to release") - - // Discarding is what the project system does once a program is replaced, and it has to happen - // before the next program's checkers are built or both generations are held at once. - pool.program.GetSemanticDiagnostics(context.Background(), nil) - assert.Assert(t, anyDiagnosticsChecker(), "a later check builds them again") - pool.Discard() - assert.Assert(t, !anyDiagnosticsChecker(), "a discarded pool must let go of its checkers") -} - -// A build's worth of diagnostics checkers is what a workspace pull needs. A session without the -// pull switched on keeps the single diagnostics checker it has always had, and does not pay for -// the memory of the rest. -func TestCheckerPoolKeepsOneDiagnosticsCheckerWithoutTheWorkspacePull(t *testing.T) { - t.Parallel() - _, pool := setupCheckerPoolSessionWithScope(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, - manyCheckerPoolFiles(), lsutil.WorkspaceDiagnosticsScopeOff) - - assert.Assert(t, diagnosticsCheckerCount(pool.program) > 1, "the program is big enough for a build to use several") - assert.Equal(t, pool.diagnosticsCount, 1, "without the pull, diagnostics stay on one checker") - assert.Equal(t, len(pool.checkers), 1+3, "one diagnostics checker plus MaxCheckers-1 query checkers") - - // Every file goes to that one checker. - for _, file := range pool.program.SourceFiles() { - assert.Equal(t, pool.diagnosticsIndexFor(file), 0) + // And the sweep hands them all back. + assert.Assert(t, snapshot.ReleaseDiagnosticsCheckers(project)) + for index := range pool.diagnosticsCount { + assert.Assert(t, pool.checkers[index] == nil) } } @@ -1687,6 +1624,108 @@ func TestCheckerPoolSweepAcquisitionIsNotInteractive(t *testing.T) { }) } +// A whole-program check runs on the diagnostics checkers, of which there are as many as a build +// would use, and leaves each file remembered against the one that checked it. +func TestCheckerPoolChecksWholeProgramAcrossDiagnosticsCheckers(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + "/src/b.ts": "export const b = 1;", + }) + assert.Equal(t, pool.diagnosticsCount, diagnosticsCheckerCount(pool.program)) + assert.Assert(t, pool.diagnosticsCount > 1, "a three-file program gets more than one checker") + for index := range pool.checkers { + assert.Assert(t, pool.checkers[index] == nil, "nothing built before a check is asked for") + } + + diagnostics := pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, len(diagnostics) > 0, "expected the seeded error") + + for _, file := range pool.program.SourceFiles() { + index := pool.diagnosticsIndexFor(file) + assert.Assert(t, index < pool.diagnosticsCount && pool.checkers[index] != nil, + "%s must be remembered against the diagnostics checker that checked it", file.FileName()) + } + for index := pool.diagnosticsCount; index < len(pool.checkers); index++ { + assert.Assert(t, pool.checkers[index] == nil, "a whole-program check must not use a query checker") + } +} + +// Query checkers sit after the diagnostics ones, so a query never lands on a checker a whole-program +// check is partway through. +func TestCheckerPoolQueryCheckersSitAfterDiagnosticsCheckers(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + "/src/b.ts": "export const b = 1;", + }) + assert.Equal(t, len(pool.checkers), pool.diagnosticsCount+3, "MaxCheckers-1 query checkers on top of the diagnostics ones") + + ctx := core.WithCheckerLifetime(core.WithRequestID(t.Context(), "query-after-diag"), core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + defer release() + for index := range pool.diagnosticsCount { + assert.Assert(t, pool.checkers[index] != c, "a query must not use a diagnostics checker") + } +} + +// The checkers a sweep uses are handed back when it is done with them: they hold the types of every +// file in the program, and a later pull either finds the project unchanged, and answers from result +// ids without checking, or finds it changed and needs new ones anyway. +func TestCheckerPoolReleasesCheckersAfterAWholeProgramCheck(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, + "/src/index.ts": "export const x: number = 1;", + "/src/a.ts": "export const a: string = 1;", + }) + + anyDiagnosticsChecker := func() bool { + for index := range pool.diagnosticsCount { + if pool.checkers[index] != nil { + return true + } + } + return false + } + + pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, anyDiagnosticsChecker()) + + assert.Assert(t, pool.releaseDiagnosticsCheckers(), "releasing reports that it dropped the checkers") + assert.Assert(t, !anyDiagnosticsChecker()) + assert.Assert(t, !pool.releaseDiagnosticsCheckers(), "nothing left to release") + + // Discarding is what the project system does once a program is replaced, and it has to happen + // before the next program's checkers are built or both generations are held at once. + pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, anyDiagnosticsChecker(), "a later check builds them again") + pool.Discard() + assert.Assert(t, !anyDiagnosticsChecker(), "a discarded pool must let go of its checkers") +} + +// A build's worth of diagnostics checkers is what a workspace pull needs. A session without the +// pull switched on keeps the single diagnostics checker it has always had, and does not pay for +// the memory of the rest. +func TestCheckerPoolKeepsOneDiagnosticsCheckerWithoutTheWorkspacePull(t *testing.T) { + t.Parallel() + _, pool := setupCheckerPoolSessionWithScope(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}, + manyCheckerPoolFiles(), lsutil.WorkspaceDiagnosticsScopeOff) + + assert.Assert(t, diagnosticsCheckerCount(pool.program) > 1, "the program is big enough for a build to use several") + assert.Equal(t, pool.diagnosticsCount, 1, "without the pull, diagnostics stay on one checker") + assert.Equal(t, len(pool.checkers), 1+3, "one diagnostics checker plus MaxCheckers-1 query checkers") + + // Every file goes to that one checker. + for _, file := range pool.program.SourceFiles() { + assert.Equal(t, pool.diagnosticsIndexFor(file), 0) + } +} + // A whole-program check reached from a request the user is waiting on must not stand aside: it // would be waiting on itself, and the checkers of one group would take turns rather than run // together. Nothing does this today, but it is a cheap thing to leave armed. diff --git a/tsc/internal/project/incrementalreferences_test.go b/tsc/internal/project/incrementalreferences_test.go new file mode 100644 index 0000000000000..c7e4208785dbd --- /dev/null +++ b/tsc/internal/project/incrementalreferences_test.go @@ -0,0 +1,116 @@ +package project + +import ( + "context" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/bundled" + "github.com/microsoft/TypeScript/tsc/internal/core" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/project/logging" + "github.com/microsoft/TypeScript/tsc/internal/vfs/vfstest" + "gotest.tools/v3/assert" +) + +const referencesMain = "file:///src/main.ts" + +func referencesFiles() map[string]any { + return map[string]any{ + "/src/tsconfig.json": `{ "compilerOptions": { "strict": true } }`, + "/src/dep.ts": "export const dep: number = 1;\n", + "/src/main.ts": "import { dep } from './dep';\nexport const main = dep;\n", + } +} + +func referencesSession(t *testing.T) *Session { + t.Helper() + fs := bundled.WrapFS(vfstest.FromMap(referencesFiles(), false)) + options := &SessionOptions{ + CurrentDirectory: "/", + DefaultLibraryPath: bundled.LibPath(), + PositionEncoding: lsproto.PositionEncodingKindUTF8, + WatchEnabled: false, + LoggingEnabled: false, + } + options.workspaceDiagnosticsEnabled.Store(true) + session := NewSession(&SessionInit{ + BackgroundCtx: context.Background(), + Options: options, + FS: fs, + Logger: logging.NewTestLogger(), + }) + t.Cleanup(session.Close) + return session +} + +func referencesProject(t *testing.T, session *Session) (*Snapshot, *Project) { + t.Helper() + // Asking for the language service is what builds the program; the snapshot alone does not. + _, err := session.GetLanguageService(context.Background(), referencesMain) + assert.NilError(t, err) + snapshot := session.Snapshot() + project := snapshot.ProjectCollection.ConfiguredProject("/src/tsconfig.json") + assert.Assert(t, project != nil, "expected the configured project") + return snapshot, project +} + +func builtCheckers(pool *checkerPool) int { + pool.mu.Lock() + defer pool.mu.Unlock() + built := 0 + for _, c := range pool.checkers { + if c != nil { + built++ + } + } + return built +} + +// Working out what each file references means resolving every import in the program through a +// type checker, which on a large project takes minutes and blocks the pull that asked. A program +// cloned from another only replaces a file whose imports, augmentations, ambient module names and +// reference directives are all unchanged, so the answer cannot have moved and is carried over. +func TestIncrementalReferencesAreReusedAcrossAClonedProgram(t *testing.T) { + t.Parallel() + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + session := referencesSession(t) + session.DidOpenFile(context.Background(), referencesMain, 1, referencesFiles()["/src/main.ts"].(string), lsproto.LanguageKindTypeScript) + + // The first build has nothing to carry over, so it resolves everything itself. + snapshot, project := referencesProject(t, session) + assert.Assert(t, snapshot.IncrementalProgram(project) != nil) + + // The first edit after a load settles the project; the one after it is the ordinary case. + session.DidChangeFile(context.Background(), referencesMain, 2, []lsproto.TextDocumentContentChangePartialOrWholeDocument{ + {WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{ + Text: "import { dep } from './dep';\nexport const main = dep + 1;\n", + }}, + }) + snapshot, project = referencesProject(t, session) + assert.Assert(t, snapshot.IncrementalProgram(project) != nil) + + // An edit that leaves the imports alone clones the program. + session.DidChangeFile(context.Background(), referencesMain, 3, []lsproto.TextDocumentContentChangePartialOrWholeDocument{ + {WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{ + Text: "import { dep } from './dep';\nexport const main: string = dep;\n", + }}, + }) + snapshot, project = referencesProject(t, session) + assert.Equal(t, project.ProgramUpdateKind, ProgramUpdateKindCloned, "the edit should have cloned the program") + + before := builtCheckers(project.checkerPool) + program := snapshot.IncrementalProgram(project) + assert.Assert(t, program != nil) + assert.Equal(t, builtCheckers(project.checkerPool), before, + "carrying the reference map over must not need a type checker") + + // The edit is still seen: main.ts now assigns a number to a string. + file := project.Program.GetSourceFile("/src/main.ts") + assert.Assert(t, file != nil) + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + assert.Assert(t, len(program.GetSemanticDiagnostics(ctx, file)) > 0, + "the edited file is re-checked and its error reported") +} diff --git a/tsc/internal/project/incrementalstate.go b/tsc/internal/project/incrementalstate.go new file mode 100644 index 0000000000000..2af023bda972f --- /dev/null +++ b/tsc/internal/project/incrementalstate.go @@ -0,0 +1,107 @@ +package project + +import ( + "sync" + + "github.com/microsoft/TypeScript/tsc/internal/compiler" + "github.com/microsoft/TypeScript/tsc/internal/execute/incremental" +) + +// incrementalState carries what a project learned about which files a change reaches from one of +// its programs to the next, so a pull re-checks only the files an edit affected. It is held by +// pointer, so snapshots sharing a program share what it has built. +// +// It is built on the first pull that asks, not when the program is: building walks every file and +// resolves every import, and most programs are never pulled. It is built outside the lock, because +// next() runs while the session holds its snapshot write lock, with every request queued behind it. +type incrementalState struct { + mu sync.Mutex + // What the previous program left behind, holding no program of its own. + previous *incremental.PriorState + current *incremental.Program + // building is non-nil while a build is in flight and closed once it has published, so that a + // second caller waits on the build rather than on the lock. + building chan struct{} + // referencesValid says previous describes a program this one is a clone of, so what each file + // references has not moved. Working it out again means resolving every import in the program + // through a type checker, which on a large project is the slowest thing a pull does. + referencesValid bool +} + +// get returns the incremental view of the program, building it from the previous program's +// bookkeeping the first time it is asked for. +func (s *incrementalState) get(program *compiler.Program) *incremental.Program { + if s == nil { + // A project built before it had any state to carry; nothing to chain from. + return incremental.NewProgramFromPriorState(program, nil, nil, false /*reuseReferences*/) + } + for { + current, previous, built, mine := s.claimBuild() + switch { + case current != nil: + return current + case mine: + return s.publishBuild(incremental.NewProgramFromPriorState(program, previous, nil, s.referencesValid)) + default: + // Waiting on the build rather than the lock is what keeps next() off it. + <-built + } + } +} + +// claimBuild says what to do: use what is there, wait for the build in flight, or build it. A +// caller told to build must publish, or whoever waits on it waits forever. +func (s *incrementalState) claimBuild() (current *incremental.Program, previous *incremental.PriorState, built <-chan struct{}, mine bool) { + s.mu.Lock() + defer s.mu.Unlock() + switch { + case s.current != nil: + return s.current, nil, nil, false + case s.building != nil: + return nil, nil, s.building, false + default: + s.building = make(chan struct{}) + return nil, s.previous, nil, true + } +} + +// publishBuild hands the built view to whoever is waiting. A nil view means the build did not +// finish, and leaves the state as it was for the next caller to try again. +func (s *incrementalState) publishBuild(program *incremental.Program) *incremental.Program { + s.mu.Lock() + building := s.building + s.building = nil + if program != nil { + s.current = program + // The new view has taken what it needs; holding the old one keeps a program alive. + s.previous = nil + } + s.mu.Unlock() + // Closed after the state is published, so a waiter looking again finds it. + close(building) + return program +} + +// next returns the state a replacement program starts from. It keeps what this one worked out and +// drops the program it worked it out from, which is the largest thing a project holds. +// +// A build in flight is not waited for: what it produces describes the program being replaced, so +// the replacement chains from the baseline that build started from and sees the changes since as +// one. The build finishes into a state nothing reads. +// cloned says the replacement program was made by replacing one file in this one, which is only +// possible when nothing about what any file references changed. +func (s *incrementalState) next(cloned bool) *incrementalState { + if s == nil { + return &incrementalState{} + } + s.mu.Lock() + defer s.mu.Unlock() + if s.current != nil { + // previous describes the program being replaced, so the replacement being a clone of it is + // all that is needed. + return &incrementalState{previous: s.current.PriorState(), referencesValid: cloned} + } + // Nothing was built from previous, so it describes an earlier program still. It carries over + // only if every step from there to here was a clone. + return &incrementalState{previous: s.previous, referencesValid: cloned && s.referencesValid} +} diff --git a/tsc/internal/project/incrementalstate_test.go b/tsc/internal/project/incrementalstate_test.go new file mode 100644 index 0000000000000..573997bbac5dc --- /dev/null +++ b/tsc/internal/project/incrementalstate_test.go @@ -0,0 +1,89 @@ +package project + +import ( + "sync/atomic" + "testing" + "testing/synctest" + + "github.com/microsoft/TypeScript/tsc/internal/execute/incremental" + "gotest.tools/v3/assert" +) + +// next() is called while the session holds its snapshot write lock, with every request in the +// session queued behind it. Building the view walks every file of the program, so waiting for one +// in flight would freeze the session for as long as the build takes. +func TestIncrementalStateNextDoesNotWaitForABuildInFlight(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + state := &incrementalState{} + + // Claim the build without finishing it, standing in for a pull part way through one. + _, _, _, mine := state.claimBuild() + assert.Assert(t, mine, "the first caller builds") + + var returned atomic.Bool + go func() { + state.next(false /*cloned*/) + returned.Store(true) + }() + synctest.Wait() + assert.Assert(t, returned.Load(), "next must not wait for a build in flight") + }) +} + +// The replacement chains from the baseline the in-flight build started from, so the changes that +// build was going to describe are still seen - by the generation after it, all at once. +func TestIncrementalStateNextKeepsTheBaselineWhileBuilding(t *testing.T) { + t.Parallel() + baseline := &incremental.PriorState{} + state := &incrementalState{previous: baseline} + + _, previous, _, mine := state.claimBuild() + assert.Assert(t, mine) + assert.Equal(t, previous, baseline, "the build starts from the baseline") + + assert.Equal(t, state.next(false /*cloned*/).previous, baseline, "and so does the program that replaces it") +} + +// Only one caller builds; the rest wait on it rather than each building their own. +func TestIncrementalStateBuildsOnceForConcurrentCallers(t *testing.T) { + t.Parallel() + synctest.Test(t, func(t *testing.T) { + state := &incrementalState{} + _, _, _, mine := state.claimBuild() + assert.Assert(t, mine) + + var waiterDone atomic.Bool + go func() { + current, _, built, mine := state.claimBuild() + assert.Assert(t, current == nil && !mine, "a second caller waits rather than building") + <-built + waiterDone.Store(true) + }() + synctest.Wait() + assert.Assert(t, !waiterDone.Load(), "the waiter is held until the build publishes") + + state.publishBuild(&incremental.Program{}) + synctest.Wait() + assert.Assert(t, waiterDone.Load(), "publishing releases it") + assert.Assert(t, state.current != nil) + assert.Assert(t, state.previous == nil, "the view has taken what it needs from the baseline") + }) +} + +// A build that does not finish must leave the state as it was, so the next caller tries again +// rather than waiting on a channel nobody will ever close. +func TestIncrementalStateAbandonedBuildLetsTheNextCallerTry(t *testing.T) { + t.Parallel() + baseline := &incremental.PriorState{} + state := &incrementalState{previous: baseline} + + _, _, _, mine := state.claimBuild() + assert.Assert(t, mine) + state.publishBuild(nil) + + current, previous, _, mine := state.claimBuild() + assert.Assert(t, current == nil) + assert.Assert(t, mine, "the next caller builds it") + assert.Equal(t, previous, baseline, "still from the same baseline") +} diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index 3e37444fc37de..21984d61d2cd4 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -164,6 +164,8 @@ type Project struct { contentMapperWatchedFiles *collections.Set[tspath.Path] checkerPool *checkerPool + // incremental carries what a change reaches from one program to the next; see incrementalState. + incremental *incrementalState moduleResolverFactory ModuleResolverFactory moduleResolverID uint64 @@ -282,6 +284,7 @@ func NewProject( id: id, currentDirectory: currentDirectory, dirty: true, + incremental: &incrementalState{}, } project.programFilesWatch = NewWatchedFiles( @@ -414,6 +417,7 @@ func (p *Project) Clone() *Project { contentMapperWatchedFiles: p.contentMapperWatchedFiles, checkerPool: p.checkerPool, + incremental: p.incremental, moduleResolverFactory: p.moduleResolverFactory, moduleResolverID: p.moduleResolverID, diff --git a/tsc/internal/project/projectcollectionbuilder.go b/tsc/internal/project/projectcollectionbuilder.go index eb3752678485c..793b99d093e2c 100644 --- a/tsc/internal/project/projectcollectionbuilder.go +++ b/tsc/internal/project/projectcollectionbuilder.go @@ -1487,6 +1487,7 @@ func (b *ProjectCollectionBuilder) updateProgram(entry dirty.Value[*Project], lo oldHost := project.host oldProgram := project.Program oldCheckerPool := project.checkerPool + oldIncremental := project.incremental project.host = newCompilerHost(project.currentDirectory, project, b, logger.Fork("CompilerHost")) result := project.CreateProgram() var watchedFiles []string @@ -1527,6 +1528,9 @@ func (b *ProjectCollectionBuilder) updateProgram(entry dirty.Value[*Project], lo if oldCheckerPool != nil { oldCheckerPool.Discard() } + // Carries what the old program worked out about its files, without carrying the + // program. Built here rather than on first use so the old one can be let go of now. + project.incremental = oldIncremental.next(result.UpdateKind == ProgramUpdateKindCloned) }) }) } diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index d8d9a797297d0..a158a286b111d 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -13,6 +13,7 @@ import ( "github.com/microsoft/TypeScript/tsc/internal/collections" "github.com/microsoft/TypeScript/tsc/internal/contentmapper" "github.com/microsoft/TypeScript/tsc/internal/core" + "github.com/microsoft/TypeScript/tsc/internal/execute/incremental" "github.com/microsoft/TypeScript/tsc/internal/ls" "github.com/microsoft/TypeScript/tsc/internal/ls/autoimport" "github.com/microsoft/TypeScript/tsc/internal/ls/lsconv" @@ -229,6 +230,13 @@ func (s *Snapshot) ReleaseDiagnosticsCheckers(project *Project) bool { return project.checkerPool.releaseDiagnosticsCheckers() } +// IncrementalProgram returns a project's program together with the record of which files a change +// since the previous program reached, so a caller checking the project can skip the files it did +// not. Built on first use, and shared by every snapshot holding the same program. +func (s *Snapshot) IncrementalProgram(project *Project) *incremental.Program { + return project.incremental.get(project.Program) +} + func (s *Snapshot) GetFile(fileName string) FileHandle { return s.fs.GetFile(fileName) } From 9ee6a6da2d677030dfabf896d1444f03118fff26 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Tue, 22 Sep 2026 15:00:33 -0700 Subject: [PATCH 06/10] Keep what a cancelled check got through A check that was cancelled threw away every file it had finished, so the next one started from nothing. On a project where a check outlasts the gap between two edits, that meant it could never finish at all. It was thrown away because the results could not be told apart: an entry for a file the check never reached is nil, which reads the same as a file checked and found clean, and caching that would hide real errors. The collection now reports which files it got through, and only those are kept. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/compiler/program.go | 38 ++++++--- tsc/internal/execute/incremental/program.go | 8 +- tsc/internal/project/checkerpool.go | 6 +- .../project/incrementalpartial_test.go | 85 +++++++++++++++++++ 4 files changed, 118 insertions(+), 19 deletions(-) create mode 100644 tsc/internal/project/incrementalpartial_test.go diff --git a/tsc/internal/compiler/program.go b/tsc/internal/compiler/program.go index cf033c2fa981e..6b6db82e920cc 100644 --- a/tsc/internal/compiler/program.go +++ b/tsc/internal/compiler/program.go @@ -702,7 +702,8 @@ func (p *Program) collectCheckerDiagnostics(ctx context.Context, sourceFile *ast done() return filterAndSortDiagnostics(result) } - return filterAndSortDiagnostics(slices.Concat(p.collectCheckerDiagnosticsFromFiles(ctx, p.files, collect)...)) + diagnostics, _ := p.collectCheckerDiagnosticsFromFiles(ctx, p.files, collect) + return filterAndSortDiagnostics(slices.Concat(diagnostics...)) } func filterAndSortDiagnostics(diags []*ast.Diagnostic) []*ast.Diagnostic { @@ -722,41 +723,52 @@ type wholeProgramCheckerPool interface { ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) } -// collectCheckerDiagnosticsFromFiles collects checker diagnostics for a list of files. -func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, sourceFiles []*ast.SourceFile, collect func(context.Context, *checker.Checker, *ast.SourceFile) []*ast.Diagnostic) [][]*ast.Diagnostic { +// collectCheckerDiagnosticsFromFiles collects checker diagnostics for a list of files, and reports +// which of them it got through: a file a cancelled caller never reached is left nil, the same as +// one checked and found clean. +func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, sourceFiles []*ast.SourceFile, collect func(context.Context, *checker.Checker, *ast.SourceFile) []*ast.Diagnostic) ([][]*ast.Diagnostic, []bool) { diagnostics := make([][]*ast.Diagnostic, len(sourceFiles)) + checked := make([]bool, len(sourceFiles)) + check := func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { + result := collect(ctx, c, file) + // Cancellation can land part way through a file, so only an uninterrupted check counts. + if ctx.Err() == nil { + diagnostics[fileIndex] = result + checked[fileIndex] = true + } + } if p.compilerCheckerPool != nil { - p.compilerCheckerPool.forEachCheckerGroupDo(ctx, sourceFiles, p.SingleThreaded(), func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { - diagnostics[fileIndex] = collect(ctx, c, file) - }) + p.compilerCheckerPool.forEachCheckerGroupDo(ctx, sourceFiles, p.SingleThreaded(), check) } else if pool, ok := p.checkerPool.(wholeProgramCheckerPool); ok { files := make([]*ast.SourceFile, 0, len(sourceFiles)) indices := make([]int, 0, len(sourceFiles)) for i, file := range sourceFiles { if p.SkipTypeChecking(file, false) { + checked[i] = true continue } files = append(files, file) indices = append(indices, i) } pool.ForEachCheckerGroupDo(ctx, files, p.SingleThreaded(), func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { - diagnostics[indices[fileIndex]] = collect(ctx, c, file) + check(c, indices[fileIndex], file) }) } else { wg := core.NewWorkGroup(p.SingleThreaded()) for i, file := range sourceFiles { if p.SkipTypeChecking(file, false) { + checked[i] = true continue } wg.Queue(func() { c, done := p.checkerPool.GetChecker(ctx, file) - diagnostics[i] = collect(ctx, c, file) + check(c, i, file) done() }) } wg.RunAndWait() } - return diagnostics + return diagnostics, checked } func (p *Program) GetSyntacticDiagnostics(ctx context.Context, sourceFile *ast.SourceFile) []*ast.Diagnostic { @@ -821,12 +833,16 @@ func (p *Program) GetSemanticDiagnostics(ctx context.Context, sourceFile *ast.So // GetSemanticDiagnosticsForIncremental includes newly discovered globals in each // file's cached diagnostics and leaves noEmit filtering to the builder. func (p *Program) GetSemanticDiagnosticsForIncremental(ctx context.Context, sourceFiles []*ast.SourceFile) map[*ast.SourceFile][]*ast.Diagnostic { - allDiags := p.collectCheckerDiagnosticsFromFiles(ctx, sourceFiles, func(ctx context.Context, c *checker.Checker, file *ast.SourceFile) []*ast.Diagnostic { + allDiags, checked := p.collectCheckerDiagnosticsFromFiles(ctx, sourceFiles, func(ctx context.Context, c *checker.Checker, file *ast.SourceFile) []*ast.Diagnostic { return p.getBindAndCheckDiagnosticsWithChecker(ctx, c, file, true /*includeDeferredGlobals*/) }) result := make(map[*ast.SourceFile][]*ast.Diagnostic, len(sourceFiles)) for i, diags := range allDiags { - result[sourceFiles[i]] = filterAndSortDiagnostics(diags) + // Only the files this got through. A cancelled caller that kept the rest would be caching + // "no errors" for files nothing looked at. + if checked[i] { + result[sourceFiles[i]] = filterAndSortDiagnostics(diags) + } } return result } diff --git a/tsc/internal/execute/incremental/program.go b/tsc/internal/execute/incremental/program.go index 78f0f10283700..6e3ef8b51e868 100644 --- a/tsc/internal/execute/incremental/program.go +++ b/tsc/internal/execute/incremental/program.go @@ -340,11 +340,11 @@ func (p *Program) collectSemanticDiagnosticsOfAffectedFiles(ctx context.Context, } // Get their diagnostics and cache them + // Only the files it got through come back, so a cancelled check keeps what it finished + // rather than starting again from nothing the next time it is asked. On a project big enough + // that a check outlasts the gap between two edits, throwing the work away meant it could + // never finish at all. diagnosticsPerFile := p.program.GetSemanticDiagnosticsForIncremental(ctx, affectedFiles) - // commit changes if no err - if ctx.Err() != nil { - return - } // Commit changes to snapshot for file, diagnostics := range diagnosticsPerFile { diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index 8e246e38b1377..e03650c067848 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -378,10 +378,8 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So // Nothing left, so don't build a checker to do it with. return } - // A cancelled caller discards what comes back anyway. Checked here rather than - // left to waitForIdle, which returns without looking at the context when nothing - // is outstanding. Bailing leaves the rest of the files' diagnostics zero, so a - // caller must test for cancellation before reading them. + // Checked here rather than left to waitForIdle, which returns without looking at + // the context when nothing is outstanding. if ctx.Err() != nil { return } diff --git a/tsc/internal/project/incrementalpartial_test.go b/tsc/internal/project/incrementalpartial_test.go new file mode 100644 index 0000000000000..90af4aad54e72 --- /dev/null +++ b/tsc/internal/project/incrementalpartial_test.go @@ -0,0 +1,85 @@ +package project + +import ( + "context" + "fmt" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/bundled" + "github.com/microsoft/TypeScript/tsc/internal/core" + "gotest.tools/v3/assert" +) + +// erroringFiles is a project where every file has a semantic error, so a check that cached +// "clean" for a file it never looked at would be caught by the error going missing. +func erroringFiles(count int) map[string]any { + files := map[string]any{"/src/tsconfig.json": `{ "compilerOptions": { "strict": true } }`} + files["/src/index.ts"] = "export const x: string = 1;\n" + for i := range count { + files[fmt.Sprintf("/src/f%d.ts", i)] = fmt.Sprintf("export const v%d: string = %d;\n", i, i) + } + return files +} + +func allSemanticDiagnostics(t *testing.T, snapshot *Snapshot, project *Project, ctx context.Context) map[string]int { + t.Helper() + program := snapshot.IncrementalProgram(project) + counts := map[string]int{} + for _, file := range project.Program.SourceFiles() { + if diags := program.GetSemanticDiagnostics(ctx, file); len(diags) > 0 { + counts[file.FileName()] = len(diags) + } + } + return counts +} + +// A check of a large project outlasts the gap between two edits, so it is cancelled part way +// more often than it finishes. What it got through is kept, and keeping it must not cache +// "no errors" for the files it never reached. +func TestIncrementalCancelledCheckKeepsOnlyWhatItFinished(t *testing.T) { + t.Parallel() + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := erroringFiles(30) + + // What the answer is when nothing interrupts it. + full, fullProject := func() (map[string]int, *Project) { + session, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, files) + t.Cleanup(session.Close) + snapshot := session.Snapshot() + project := snapshot.ProjectCollection.ConfiguredProject("/src/tsconfig.json") + _ = pool + return allSemanticDiagnostics(t, snapshot, project, context.Background()), project + }() + assert.Assert(t, len(full) > 5, "the fixture should have errors in many files, got %d", len(full)) + + // Now the same, with the first pass cancelled part way through. + session, _ := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{}, files) + t.Cleanup(session.Close) + snapshot := session.Snapshot() + project := snapshot.ProjectCollection.ConfiguredProject("/src/tsconfig.json") + assert.Assert(t, project != nil) + assert.Equal(t, len(project.Program.SourceFiles()), len(fullProject.Program.SourceFiles())) + + program := snapshot.IncrementalProgram(project) + cancelCtx, cancel := context.WithCancel(core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics)) + cancelCtx = WithCheckProgress(cancelCtx, func(checked, total int) { + // Stop once it is under way but nowhere near done. + if checked >= checkProgressBatchFiles { + cancel() + } + }) + _ = program.GetSemanticDiagnostics(cancelCtx, nil) + cancel() + + // Asking again finishes the job, and the answer matches the run that was never interrupted. + after := map[string]int{} + for _, file := range project.Program.SourceFiles() { + if diags := program.GetSemanticDiagnostics(context.Background(), file); len(diags) > 0 { + after[file.FileName()] = len(diags) + } + } + assert.DeepEqual(t, after, full) +} From aedc48b6dbee71ec86029a4785cbee378d2eefe8 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:13:36 -0700 Subject: [PATCH 07/10] Expose the project state a workspace pull needs A pull has to know which projects to report on and which files are theirs, and none of that was reachable from outside the project system. OpenProjects is the projects an editor actually has a file open in, which is what the narrower scopes report on. ReferencedProjectPaths walks the reference graph, so a pull can find the projects that consume an open one. isOpen answers the question for one project, going through the memoized set for configured projects and scanning the open files for the inferred one, which is not in that set and of which there is only ever one. A collection also remembers which project trees it was last built for, so a request the loaded trees already cover is answered without building a snapshot to discover there was nothing to load. A pull asking for everything, every few seconds, would otherwise rebuild the collection each time. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/project/project.go | 13 ++++++ tsc/internal/project/projectcollection.go | 20 +++++++++ .../project/projectcollectionbuilder.go | 15 ++++++- tsc/internal/project/session.go | 14 +++--- tsc/internal/project/snapshot.go | 43 ++++++++++++++++--- 5 files changed, 91 insertions(+), 14 deletions(-) diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index 21984d61d2cd4..9633158396d8f 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -478,6 +478,19 @@ func (p *Project) setPotentialProjectReference(configFilePath tspath.Path) { p.potentialProjectReferences.Add(configFilePath) } +// ReferencedProjectPaths returns the config paths of the projects this project references. +func (p *Project) ReferencedProjectPaths() []tspath.Path { + if p.CommandLine == nil { + return nil + } + referenced := p.CommandLine.ResolvedProjectReferencePaths() + paths := make([]tspath.Path, 0, len(referenced)) + for _, path := range referenced { + paths = append(paths, p.toPath(path)) + } + return paths +} + func (p *Project) hasPotentialProjectReference(projectTreeRequest *ProjectTreeRequest) bool { if p.CommandLine != nil { for _, path := range p.CommandLine.ResolvedProjectReferencePaths() { diff --git a/tsc/internal/project/projectcollection.go b/tsc/internal/project/projectcollection.go index c43d9da8d633b..e88342dbee7ff 100644 --- a/tsc/internal/project/projectcollection.go +++ b/tsc/internal/project/projectcollection.go @@ -31,6 +31,9 @@ type ProjectCollection struct { // inferredProject is a fallback project that is used when no configured // project can be found for an open file. inferredProject *Project + // loadedProjectTrees is the project tree request this collection was last built for. A later + // request that it already covers needs no new snapshot to discover that nothing is missing. + loadedProjectTrees *ProjectTreeRequest // apiState tracks the projects and files that API clients have explicitly // opened so they are kept loaded across snapshots. apiState APIState @@ -199,6 +202,22 @@ func (c *ProjectCollection) GetOpenConfiguredProjects() *collections.Set[Configu return c.openConfiguredProjects } +// isOpen reports whether the project contains an open file. Configured projects come from the +// memoized set, which is indexed by default project; the inferred project is not in that set, but +// there is only ever one and open files are few. +func (c *ProjectCollection) isOpen(project *Project) bool { + if project == c.inferredProject { + for path := range c.openFiles.Keys() { + if project.containsFile(path) { + return true + } + } + return false + } + configuredID, configured := project.ID().Configured() + return configured && c.GetOpenConfiguredProjects().Has(configuredID) +} + func openFilePaths(overlays map[tspath.Path]*Overlay) collections.Set[tspath.Path] { openFiles := collections.Set[tspath.Path]{M: make(map[tspath.Path]struct{}, len(overlays))} for path := range overlays { @@ -335,6 +354,7 @@ func (c *ProjectCollection) clone() *ProjectCollection { openFiles: c.openFiles, inferredProject: c.inferredProject, fileDefaultProjects: c.fileDefaultProjects, + loadedProjectTrees: c.loadedProjectTrees, apiState: c.apiState, } } diff --git a/tsc/internal/project/projectcollectionbuilder.go b/tsc/internal/project/projectcollectionbuilder.go index 793b99d093e2c..189b65dd1e191 100644 --- a/tsc/internal/project/projectcollectionbuilder.go +++ b/tsc/internal/project/projectcollectionbuilder.go @@ -51,7 +51,9 @@ type ProjectCollectionBuilder struct { client Client // optional; used for project loading notifications - newSnapshotID uint64 + newSnapshotID uint64 + // loadedProjectTrees is what this build has loaded trees for, carried from the base collection. + loadedProjectTrees *ProjectTreeRequest programStructureChanged bool defaultProjectsInvalidated bool openFilesChanged bool @@ -102,6 +104,7 @@ func newProjectCollectionBuilder( configFileRegistryBuilder: newConfigFileRegistryBuilder(lsproto.GetClientCapabilities(ctx).Workspace.DidChangeWatchedFiles.RelativePatternSupport, fs, func(path tspath.Path) bool { _, ok := overlays[path]; return ok }, oldConfigFileRegistry, extendedConfigCache, newSnapshotID, sessionOptions, customConfigFileName, nil), newSnapshotID: newSnapshotID, openFilesChanged: !openFiles.Equals(&oldProjectCollection.openFiles), + loadedProjectTrees: oldProjectCollection.loadedProjectTrees, configuredProjects: dirty.NewSyncMap(oldProjectCollection.configuredProjects), syntheticProjects: dirty.NewSyncMap(oldProjectCollection.syntheticProjects), inferredProject: dirty.NewBox(oldProjectCollection.inferredProject), @@ -134,6 +137,11 @@ func (b *ProjectCollectionBuilder) Finalize(logger *logging.LogTree) (*ProjectCo newProjectCollection.syntheticProjects = syntheticProjects } + if newProjectCollection.loadedProjectTrees != b.loadedProjectTrees { + ensureCloned() + newProjectCollection.loadedProjectTrees = b.loadedProjectTrees + } + if b.openFilesChanged { ensureCloned() newProjectCollection.openFiles = openFilePaths(b.overlays) @@ -729,6 +737,11 @@ func (b *ProjectCollectionBuilder) DidRequestProject(projectID ID, logger *loggi func (b *ProjectCollectionBuilder) DidRequestProjectTrees(projectTreeRequest *ProjectTreeRequest, logger *logging.LogTree) { startTime := time.Now() + // Recorded so a later request this one covers can be answered without building a snapshot to + // discover there was nothing to load. + if !b.loadedProjectTrees.covers(projectTreeRequest) { + b.loadedProjectTrees = projectTreeRequest + } var currentProjects []ConfiguredProjectID b.configuredProjects.Range(func(sme *dirty.SyncMapEntry[ConfiguredProjectID, *Project]) bool { diff --git a/tsc/internal/project/session.go b/tsc/internal/project/session.go index 85c5341d2b16e..680b276cc926a 100644 --- a/tsc/internal/project/session.go +++ b/tsc/internal/project/session.go @@ -88,15 +88,15 @@ type SessionOptions struct { DebounceDelay time.Duration CheckerPoolOptions CheckerPoolOptions - // interactiveWork is shared by every checker pool in the session. Set by NewSnapshotHost; see - // interactiveWork for what it is for. - interactiveWork *interactiveWork - // workspaceDiagnosticsEnabled tracks whether the workspace pull is switched on, so that a // project only pays for a build's worth of checkers when something is going to check it that // way. Written whenever the user's preferences change and read when a program's pool is built, // so a session that turns the pull on picks it up as programs are rebuilt. workspaceDiagnosticsEnabled atomic.Bool + + // interactiveWork is shared by every checker pool in the session. Set by NewSnapshotHost; see + // interactiveWork for what it is for. + interactiveWork *interactiveWork } type SessionInit struct { @@ -1072,7 +1072,8 @@ func (s *Session) getSnapshot( var updateReason UpdateReason if len(request.Projects) > 0 { updateReason = UpdateReasonRequestedLanguageServiceProjectDirty - } else if request.ProjectTree != nil { + } else if request.ProjectTree != nil && !snapshot.ProjectCollection.loadedProjectTrees.covers(request.ProjectTree) { + // Only worth a new snapshot if there is something the loaded trees do not already cover. updateReason = UpdateReasonRequestedLoadProjectTree } else if request.AutoImports != "" { updateReason = UpdateReasonRequestedLanguageServiceWithAutoImports @@ -1872,7 +1873,8 @@ func (s *Session) refreshCodeLensIfNeeded(oldPrefs lsutil.UserPreferences, newPr func (s *Session) refreshDiagnosticsIfNeeded(oldPrefs lsutil.UserPreferences, newPrefs lsutil.UserPreferences) { if oldPrefs.CustomConfigFileName != newPrefs.CustomConfigFileName || oldPrefs.ReportStyleChecksAsWarnings != newPrefs.ReportStyleChecksAsWarnings || - oldPrefs.EnableValidation != newPrefs.EnableValidation { + oldPrefs.EnableValidation != newPrefs.EnableValidation || + oldPrefs.WorkspaceDiagnosticsScope != newPrefs.WorkspaceDiagnosticsScope { s.ScheduleDiagnosticsRefresh() } } diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index a158a286b111d..5b187a0de0706 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -212,13 +212,7 @@ func (s *Snapshot) GetLanguageServiceProjectsContainingFile(uri lsproto.Document return s.ProjectCollection.GetLanguageServiceProjectsContainingFile(path) } -// WaitForInteractiveIdle blocks until nothing a user is waiting on is outstanding, or ctx is done. -// A caller about to spend minutes on work nobody asked for uses this to let the work they did ask -// for go first. -func (s *Snapshot) WaitForInteractiveIdle(ctx context.Context) { - s.host.options.interactiveWork.waitForIdle(ctx) -} - +// OpenProjects returns the projects that contain at least one file open in the editor. // ReleaseDiagnosticsCheckers drops the checkers a sweep used on a project. They hold the types of // every file in it, which is the largest thing a pull creates, and keeping them buys nothing: a // pull that finds the project unchanged answers from the result ids the client already holds @@ -230,6 +224,13 @@ func (s *Snapshot) ReleaseDiagnosticsCheckers(project *Project) bool { return project.checkerPool.releaseDiagnosticsCheckers() } +// WaitForInteractiveIdle blocks until nothing a user is waiting on is outstanding, or ctx is done. +// A caller about to spend minutes on work nobody asked for uses this to let the work they did ask +// for go first. +func (s *Snapshot) WaitForInteractiveIdle(ctx context.Context) { + s.host.options.interactiveWork.waitForIdle(ctx) +} + // IncrementalProgram returns a project's program together with the record of which files a change // since the previous program reached, so a caller checking the project can skip the files it did // not. Built on first use, and shared by every snapshot holding the same program. @@ -237,6 +238,16 @@ func (s *Snapshot) IncrementalProgram(project *Project) *incremental.Program { return project.incremental.get(project.Program) } +func (s *Snapshot) OpenProjects() []*Project { + var open []*Project + for _, project := range s.ProjectCollection.Projects() { + if s.ProjectCollection.isOpen(project) { + open = append(open, project) + } + } + return open +} + func (s *Snapshot) GetFile(fileName string) FileHandle { return s.fs.GetFile(fileName) } @@ -393,6 +404,24 @@ func (p *ProjectTreeRequest) IsProjectReferenced(projectID tspath.Path) bool { return p.referencedProjects.Has(projectID) } +// covers reports whether having loaded p also loaded everything other asks for. +func (p *ProjectTreeRequest) covers(other *ProjectTreeRequest) bool { + switch { + case p == nil: + return false + case p.IsAllProjects(): + return true + case other.IsAllProjects(): + return false + } + for project := range other.referencedProjects.Keys() { + if !p.referencedProjects.Has(project) { + return false + } + } + return true +} + func (p *ProjectTreeRequest) Projects() []tspath.Path { if p.referencedProjects == nil { return nil From 24e0915a44fece46963b0b786e8405b9312b6086 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:14:01 -0700 Subject: [PATCH 08/10] Collect a project's diagnostics for a workspace pull What a pull reports for a project, in terms the language service already speaks. WorkspaceDiagnosticFiles is the files of a project worth reporting on. It leaves out what is not the user's code to fix: default libraries, anything reached as an external library, and a referenced project's sources or emitted declarations, which the project that owns them reports itself. A content-mapped file's projection goes too, since its canonical file reports under the same URI. A project narrows the rest the way it narrows a build, with its tsconfig. WorkspaceDiagnosticsForProject checks the project in one call and returns what each file should report, keyed by file. Checking everything at once lets the pool share the work across its checkers rather than having it driven a file at a time from outside. The program is passed in rather than taken from the language service, because a pull hands over the incremental view of it. Suggestions are left out: nothing caches them, so asking would re-check every file and undo the point of the incremental view. Co-Authored-By: Claude Opus 5.5 (1M context) --- tsc/internal/ls/diagnostics.go | 93 +++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 14 deletions(-) diff --git a/tsc/internal/ls/diagnostics.go b/tsc/internal/ls/diagnostics.go index 3638efdac813a..c4a166c95ced4 100644 --- a/tsc/internal/ls/diagnostics.go +++ b/tsc/internal/ls/diagnostics.go @@ -30,26 +30,49 @@ func getAllDiagnostics(ctx context.Context, program *compiler.Program, file *ast } func (l *LanguageService) ProvideDiagnostics(ctx context.Context, uri lsproto.DocumentUri) (lsproto.DocumentDiagnosticResponse, error) { - program, file := l.getProgramAndFile(uri) - - if l.UserPreferences().EnableValidation.IsFalse() { - diagnostics := []*lsproto.Diagnostic{} - return lsproto.RelatedFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport{ - FullDocumentDiagnosticReport: &lsproto.RelatedFullDocumentDiagnosticReport{ - Items: diagnostics, - }, - }, nil - } - - diagnostics := getAllDiagnostics(ctx, program, file) - + _, file := l.getProgramAndFile(uri) return lsproto.RelatedFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport{ FullDocumentDiagnosticReport: &lsproto.RelatedFullDocumentDiagnosticReport{ - Items: l.toLSPDiagnostics(ctx, diagnostics), + Items: l.ProvideDiagnosticsForFile(ctx, file), }, }, nil } +// ProvideDiagnosticsForFile computes diagnostics for a file of this project's program, for callers +// that already hold it and need not re-resolve it by URI. +func (l *LanguageService) ProvideDiagnosticsForFile(ctx context.Context, file *ast.SourceFile) []*lsproto.Diagnostic { + if l.UserPreferences().EnableValidation.IsFalse() { + return []*lsproto.Diagnostic{} + } + return l.toLSPDiagnostics(ctx, getAllDiagnostics(ctx, l.program, file)) +} + +// WorkspaceDiagnosticFiles returns the files a workspace pull should report, in program order. +func (l *LanguageService) WorkspaceDiagnosticFiles() []*ast.SourceFile { + program := l.program + files := make([]*ast.SourceFile, 0, len(program.SourceFiles())) + for _, file := range program.SourceFiles() { + // Dependencies are not the user's code to fix. + if program.IsSourceFileDefaultLibrary(file.Path()) || program.IsSourceFileFromExternalLibrary(file) { + continue + } + // A referenced project's source, reached through the redirect; it reports its own. + if program.IsSourceFromProjectReference(file.Path()) { + continue + } + // A referenced project's emitted declarations, consumed when the redirect is disabled. + if program.GetProjectReferenceFromOutputDts(file.Path()) != nil { + continue + } + // A projection of a content-mapped file; its canonical file reports it under the same URI. + if file.CanonicalSourceFile() != nil { + continue + } + files = append(files, file) + } + return files +} + func (l *LanguageService) toLSPDiagnostics(ctx context.Context, diagnostics ...[]*ast.Diagnostic) []*lsproto.Diagnostic { reportStyleChecksAsWarnings := l.UserPreferences().ReportStyleChecksAsWarnings.IsTrue() size := 0 @@ -118,3 +141,45 @@ func worstCategory(diags []*ast.Diagnostic) diagnostics.Category { } return worst } + +// WorkspaceDiagnosticsForProject checks a project in one call and returns what each of its files +// should report, keyed by file. Checking everything in one call lets the pool share the work across +// its checkers and keeps its own coordination rather than repeating it per file. +// +// The program is passed in rather than taken from the language service because a sweep hands over +// the incremental view of it, which re-checks only the files a change reached and serves the rest +// from what it cached last time. Suggestions are left out: nothing caches them, so asking would +// re-check every file and undo that. +func (l *LanguageService) WorkspaceDiagnosticsForProject(ctx context.Context, program compiler.ProgramLike, files []*ast.SourceFile) map[*ast.SourceFile][]*lsproto.Diagnostic { + reports := make(map[*ast.SourceFile][]*lsproto.Diagnostic, len(files)) + if l.UserPreferences().EnableValidation.IsFalse() { + for _, file := range files { + reports[file] = []*lsproto.Diagnostic{} + } + return reports + } + + byFile := make(map[*ast.SourceFile][]*ast.Diagnostic, len(files)) + collect := func(diagnostics []*ast.Diagnostic) { + for _, diagnostic := range diagnostics { + if file := diagnostic.File(); file != nil { + byFile[file] = append(byFile[file], diagnostic) + } + } + } + collect(program.GetSyntacticDiagnostics(ctx, nil)) + collect(program.GetSemanticDiagnostics(ctx, nil)) + if program.Options().GetEmitDeclarations() { + collect(program.GetDeclarationDiagnostics(ctx, nil)) + } + + for _, file := range files { + // A file's supplemental sources report under the file itself, as they do for a pull on it. + diagnostics := byFile[file] + for _, supplemental := range file.SupplementalSourceFiles() { + diagnostics = append(diagnostics, byFile[supplemental]...) + } + reports[file] = l.toLSPDiagnostics(ctx, diagnostics) + } + return reports +} From b67269d91969b95db88c3257f7a6a0ce78d8255c Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:14:51 -0700 Subject: [PATCH 09/10] Implement the workspace/diagnostic handler Reports diagnostics for the whole workspace, not just the files the editor has open. A pull runs off the dispatch loop, because checking a large workspace takes minutes and the server has to keep answering everything else meanwhile. Files within a project are checked in one call so the pool shares the work; the projects themselves run a few at a time, bounded so the two together do not take the machine. Reports stream through the partial result token as projects finish, and always in the same order. Three things keep the cost down between pulls, because the client pulls every couple of seconds for as long as it is open: A result id per file, hashed from its diagnostics and remembered against the program version that produced it. A project is rebuilt as a unit, so an unchanged generation answers every file in it without checking anything. A fingerprint of what the last answer was computed from - the snapshot and the settings. A pull that matches it, and whose client still holds every result id handed out, reports nothing at all rather than walking every file to say so. Superseding: a pull cancels the one before it, with a cause saying why. The reason decides what the client is told. A pull the user cancelled answers RequestCancelled; one a newer pull replaced answers ServerCancelled carrying DiagnosticServerCancellationData, because a client that cannot tell a server standing down from a failure counts it as one, and a handful is enough for it to stop pulling the workspace for the session. Pulls are superseded on the dispatch loop, so they replace one another in the order the client sent them rather than the order their goroutines start in. Progress is reported against the client's work done token, or one the server creates when the client did not send one - and clients that pull the workspace do not. It advances as files are checked rather than as projects report, since a workspace can be a single project and would otherwise sit at nothing for the whole run. A report is only sent when the percentage moves, which bounds it to a hundred notifications however large the sweep. Open documents are left to the client's own per-document pull, which it reconciles poorly with workspace results; a client that only pulls the workspace can ask for them to be included. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/lsp/server.go | 50 +- .../lsp/server_workspacediagnostics_test.go | 1221 +++++++++++++++++ tsc/internal/lsp/workspacediagnostics.go | 541 ++++++++ .../lsp/workspacediagnostics_internal_test.go | 102 ++ tsc/internal/lsp/workspacediagnosticscache.go | 165 +++ tsc/internal/lsp/workspacediagnosticsscope.go | 135 ++ 6 files changed, 2211 insertions(+), 3 deletions(-) create mode 100644 tsc/internal/lsp/server_workspacediagnostics_test.go create mode 100644 tsc/internal/lsp/workspacediagnostics.go create mode 100644 tsc/internal/lsp/workspacediagnostics_internal_test.go create mode 100644 tsc/internal/lsp/workspacediagnosticscache.go create mode 100644 tsc/internal/lsp/workspacediagnosticsscope.go diff --git a/tsc/internal/lsp/server.go b/tsc/internal/lsp/server.go index 1d32d1e97b6be..61fb590444540 100644 --- a/tsc/internal/lsp/server.go +++ b/tsc/internal/lsp/server.go @@ -80,6 +80,7 @@ func NewServer(opts *ServerOptions) *Server { startWatchdog: opts.SetParentProcessID, initComplete: make(chan struct{}), progressDelay: opts.ProgressDelay, + workspaceDiagnostics: newWorkspaceDiagnosticsCache(), } s.logger = newLogger(s) @@ -250,6 +251,15 @@ type Server struct { startWatchdog func(parentPID int) flakeLogging lsproto.DiagnosticFlakeLogLevel + + // workspaceDiagnostics remembers, across `workspace/diagnostic` pulls, which program version + // produced the result id a client holds for each file. + workspaceDiagnostics *workspaceDiagnosticsCache + + // workspaceDiagnosticsPull is the `workspace/diagnostic` request currently running, if any. A + // newer pull supersedes it; see supersedeWorkspaceDiagnostics. + workspaceDiagnosticsMu sync.Mutex + workspaceDiagnosticsPull *workspaceDiagnosticsPull } func (s *Server) Session() *project.Session { return s.session } @@ -1100,6 +1110,16 @@ func (s *Server) sendResult(id *jsonrpc.ID, result any) error { }) } +// errorWithData is an error response whose `data` the client reads to decide what to do next, as +// the diagnostic requests do: without it a server-cancelled pull looks like a failure. +type errorWithData struct { + code lsproto.ErrorCode + data any +} + +func (e errorWithData) Error() string { return e.code.Error() } +func (e errorWithData) Unwrap() error { return e.code } + type userFacingRequestFailedError string func (e userFacingRequestFailedError) Error() string { return string(e) } @@ -1116,12 +1136,16 @@ func (s *Server) sendError(id *jsonrpc.ID, err error) error { if errCode, ok := errors.AsType[lsproto.ErrorCode](err); ok { code = errCode } - // TODO(jakebailey): error data + var data any + if withData, ok := errors.AsType[errorWithData](err); ok { + data = withData.data + } return s.sendResponse(&lsproto.ResponseMessage{ ID: id, Error: &jsonrpc.ResponseError{ Code: int32(code), Message: err.Error(), + Data: data, }, }) } @@ -1146,11 +1170,11 @@ func (s *Server) handleRequestOrNotification(ctx context.Context, req *lsproto.R if handler := handlers()[req.Method]; handler != nil { start := time.Now() - doAsyncWork, err := handler(s, ctx, req) idStr := "" if req.ID != nil { idStr = " (" + req.ID.String() + ")" } + doAsyncWork, err := handler(s, ctx, req) if err != nil { if resp, ok := contentMapperFallbackResponse(req.Method, err); ok { if !s.logger.IsTracing() { @@ -1278,6 +1302,7 @@ var handlers = sync.OnceValue(func() handlerMap { handlers.registerRequestHandler(lsproto.CallHierarchyIncomingCallsInfo, (*Server).handleCallHierarchyIncomingCalls) handlers.registerRequestHandler(lsproto.CallHierarchyOutgoingCallsInfo, (*Server).handleCallHierarchyOutgoingCalls) + handlers.registerWorkspaceDiagnosticHandler() handlers.registerRequestHandler(lsproto.WorkspaceSymbolInfo, (*Server).handleWorkspaceSymbol) handlers.registerRequestHandler(lsproto.CompletionItemResolveInfo, (*Server).handleCompletionItemResolve) handlers.registerRequestHandler(lsproto.CodeLensResolveInfo, (*Server).handleCodeLensResolve) @@ -1801,7 +1826,8 @@ func (s *Server) handleDidChangeWorkspaceConfiguration(ctx context.Context, para if params.Settings == nil { return nil } else if settings, ok := params.Settings.(map[string]any); ok { - s.session.Configure(lsutil.ParseUserPreferences(settings)) + preferences := lsutil.ParseUserPreferences(settings) + s.session.Configure(preferences) } return nil } @@ -1848,6 +1874,9 @@ func (s *Server) handleSetLogVerbosity(_ context.Context, params *lsproto.SetLog func (s *Server) handleDocumentDiagnostic(ctx context.Context, languageService *ls.LanguageService, params *lsproto.DocumentDiagnosticParams) (lsproto.DocumentDiagnosticResponse, error) { ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + // The client asked for this file, and is showing the result where the user is looking. A pass + // over the whole workspace stands aside for it. + ctx = core.WithInteractiveRequest(ctx) if s.flakeLogging == lsproto.DiagnosticFlakeLogLevelOff { return languageService.ProvideDiagnostics(ctx, params.TextDocument.Uri) } @@ -1904,6 +1933,21 @@ func generateDiagnosticDiffString(missingFromPre []*lsproto.Diagnostic, missingF return b.String() } +// handleWorkspaceDiagnostic answers a `workspace/diagnostic` pull. Unlike the other handlers it is +// not registered with registerRequestHandler: a pull of a large workspace runs for minutes, so it +// has to answer off the dispatch loop. registerWorkspaceDiagnosticHandler does that part. +func (s *Server) handleWorkspaceDiagnostic(ctx context.Context, params *lsproto.WorkspaceDiagnosticParams) (lsproto.WorkspaceDiagnosticResponse, error) { + resp, err := s.pullWorkspaceDiagnostics(ctx, params) + // Why the pull stopped decides what the client is told, and the context carries the reason: a + // pull the user cancelled answers RequestCancelled, which the dispatch loop derives from the + // context error. Checked after the pull rather than where it gives up, since it can be + // superseded at any moment, including once it has an answer. + if errors.Is(context.Cause(ctx), errWorkspaceDiagnosticsSuperseded) { + return nil, supersededWorkspaceDiagnosticsError + } + return resp, err +} + func (s *Server) handleHover(ctx context.Context, ls *ls.LanguageService, params *lsproto.HoverParams) (lsproto.HoverResponse, error) { return ls.ProvideHover(ctx, params) } diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go new file mode 100644 index 0000000000000..58bdc42c8acad --- /dev/null +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -0,0 +1,1221 @@ +package lsp_test + +import ( + "context" + "fmt" + "io" + "slices" + "strings" + "sync" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/bundled" + "github.com/microsoft/TypeScript/tsc/internal/json" + "github.com/microsoft/TypeScript/tsc/internal/lsp" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/testutil/lsptestutil" + "github.com/microsoft/TypeScript/tsc/internal/vfs/vfstest" + "gotest.tools/v3/assert" +) + +type workspaceDiagnosticReport = lsproto.WorkspaceFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport + +// progressRecorder collects the $/progress notifications the server sends for a request, so tests +// can assert on streamed partial results and work done progress. +type progressRecorder struct { + mu sync.Mutex + partial []workspaceDiagnosticReport + kinds []string + percentages []uint32 + registered []string + unregistered []string + workspaceRegos int +} + +func (p *progressRecorder) recordRegistration(req *lsproto.RequestMessage) { + raw, ok := req.Params.(json.Value) + if !ok { + return + } + p.mu.Lock() + defer p.mu.Unlock() + switch req.Method { + case lsproto.MethodClientRegisterCapability: + var params lsproto.RegistrationParams + if json.Unmarshal(raw, ¶ms) != nil { + return + } + for _, registration := range params.Registrations { + p.registered = append(p.registered, registration.Id) + if opts := registration.RegisterOptions; opts != nil && opts.TextDocumentDiagnostic != nil && opts.TextDocumentDiagnostic.WorkspaceDiagnostics { + p.workspaceRegos++ + } + } + case lsproto.MethodClientUnregisterCapability: + var params lsproto.UnregistrationParams + if json.Unmarshal(raw, ¶ms) != nil { + return + } + for _, unregistration := range params.Unregisterations { + p.unregistered = append(p.unregistered, unregistration.Id) + } + } +} + +func (p *progressRecorder) registrationIDs() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.registered...) +} + +func (p *progressRecorder) unregistrationIDs() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.unregistered...) +} + +func (p *progressRecorder) record(req *lsproto.RequestMessage) { + if req.Method != lsproto.MethodProgress { + return + } + raw, ok := req.Params.(json.Value) + if !ok { + return + } + var partial lsproto.WorkspaceDiagnosticPartialResultParams + if err := json.Unmarshal(raw, &partial); err == nil && len(partial.Value.Items) > 0 { + p.mu.Lock() + p.partial = append(p.partial, partial.Value.Items...) + p.mu.Unlock() + return + } + var workDone lsproto.ProgressParams + if err := json.Unmarshal(raw, &workDone); err != nil { + return + } + p.mu.Lock() + defer p.mu.Unlock() + switch { + case workDone.Value.Begin != nil: + p.kinds = append(p.kinds, "begin") + case workDone.Value.Report != nil: + p.kinds = append(p.kinds, "report") + if pct := workDone.Value.Report.Percentage; pct != nil { + p.percentages = append(p.percentages, *pct) + } + case workDone.Value.End != nil: + p.kinds = append(p.kinds, "end") + } +} + +func (p *progressRecorder) partialItems() []workspaceDiagnosticReport { + p.mu.Lock() + defer p.mu.Unlock() + return append([]workspaceDiagnosticReport(nil), p.partial...) +} + +func (p *progressRecorder) workDonePercentages() []uint32 { + p.mu.Lock() + defer p.mu.Unlock() + return append([]uint32(nil), p.percentages...) +} + +func (p *progressRecorder) workDoneKinds() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.kinds...) +} + +// initWorkspaceDiagnosticsClient brings up a client and turns workspace diagnostics on at the given +// scope. The capability is never advertised at initialize, so every test has to opt in the same way +// a user would. +func initWorkspaceDiagnosticsClient(t *testing.T, files map[string]string) (*lsptestutil.LSPClient, *progressRecorder) { + t.Helper() + return initWorkspaceDiagnosticsClientWithScope(t, files, "allProjects") +} + +func initWorkspaceDiagnosticsClientWithScope(t *testing.T, files map[string]string, scope string) (*lsptestutil.LSPClient, *progressRecorder) { + t.Helper() + client, progress := startWorkspaceDiagnosticsClient(t, files) + if scope != "" { + setWorkspaceDiagnosticsScope(t, client, scope) + } + return client, progress +} + +func startWorkspaceDiagnosticsClient(t *testing.T, files map[string]string) (*lsptestutil.LSPClient, *progressRecorder) { + t.Helper() + + fs := bundled.WrapFS(vfstest.FromMap(files, false)) + progress := &progressRecorder{} + + onServerRequest := func(_ context.Context, req *lsproto.RequestMessage) *lsproto.ResponseMessage { + switch req.Method { + case lsproto.MethodClientRegisterCapability, lsproto.MethodClientUnregisterCapability: + progress.recordRegistration(req) + return &lsproto.ResponseMessage{ID: req.ID, JSONRPC: req.JSONRPC, Result: lsproto.Null{}} + case lsproto.MethodWindowWorkDoneProgressCreate: + return &lsproto.ResponseMessage{ID: req.ID, JSONRPC: req.JSONRPC, Result: lsproto.Null{}} + default: + return nil + } + } + + client, closeClient := lsptestutil.NewLSPClient(t, lsp.ServerOptions{ + Err: io.Discard, + Cwd: "/home/projects", + FS: fs, + DefaultLibraryPath: bundled.LibPath(), + }, onServerRequest) + t.Cleanup(func() { _ = closeClient() }) + + client.OnServerNotification = func(_ context.Context, req *lsproto.RequestMessage) { + progress.record(req) + } + + initMsg, initResult, ok := client.SendRequest(t, lsproto.InitializeInfo, &lsproto.InitializeParams{ + Capabilities: &lsproto.ClientCapabilities{ + TextDocument: &lsproto.TextDocumentClientCapabilities{ + Diagnostic: &lsproto.DiagnosticClientCapabilities{DynamicRegistration: new(true)}, + }, + }, + }) + assert.Assert(t, ok && initMsg.AsResponse().Error == nil, "Initialize failed") + assert.Assert(t, !initResult.Capabilities.DiagnosticProvider.Options.WorkspaceDiagnostics, + "workspace diagnostics must not be advertised at initialize") + client.SendNotification(t, lsproto.InitializedInfo, &lsproto.InitializedParams{}) + <-client.Server.InitComplete() + + return client, progress +} + +func setWorkspaceDiagnosticsScope(t *testing.T, client *lsptestutil.LSPClient, scope string) { + t.Helper() + client.SendNotification(t, lsproto.WorkspaceDidChangeConfigurationInfo, &lsproto.DidChangeConfigurationParams{ + Settings: map[string]any{ + "typescript": map[string]any{"experimental": map[string]any{"workspaceDiagnostics": map[string]any{"scope": scope}}}, + }, + }) +} + +// workspaceDiagnosticsFiles is a project with an error in one file, a clean file, and a dependency +// that must not be reported. +var workspaceDiagnosticsFiles = map[string]string{ + "/home/projects/tsconfig.json": `{}`, + // Opened by tests. An open document is left out of workspace reports, so tests open this one + // and assert on the others. + "/home/projects/open.ts": "export const shared = 1;", + "/home/projects/index.ts": "import { shared } from \"./open.js\";\nexport const x: string = shared;\n", + "/home/projects/other.ts": "export const y = 1;", + "/home/projects/node_modules/dep/package.json": `{"name": "dep", "types": "index.d.ts"}`, + "/home/projects/node_modules/dep/index.d.ts": "export declare const z: string = 1;", + "/home/projects/node_modules/@types/x/package.json": `{"name": "@types/x", "types": "index.d.ts"}`, +} + +func openWorkspaceDiagnosticsProject(t *testing.T, client *lsptestutil.LSPClient) { + t.Helper() + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/open.ts", + LanguageId: "typescript", + Version: 1, + Text: workspaceDiagnosticsFiles["/home/projects/open.ts"], + }, + }) +} + +func pullWorkspaceDiagnostics(t *testing.T, client *lsptestutil.LSPClient, params *lsproto.WorkspaceDiagnosticParams) *lsproto.WorkspaceDiagnosticReport { + t.Helper() + msg, resp, ok := client.SendRequest(t, lsproto.WorkspaceDiagnosticInfo, params) + assert.Assert(t, ok, "expected a response") + assert.Assert(t, msg.AsResponse().Error == nil, "expected no error") + return resp +} + +func reportURIs(reports []workspaceDiagnosticReport) []string { + uris := make([]string, 0, len(reports)) + for _, report := range reports { + if report.FullDocumentDiagnosticReport != nil { + uris = append(uris, string(report.FullDocumentDiagnosticReport.Uri)) + } else { + uris = append(uris, string(report.UnchangedDocumentDiagnosticReport.Uri)) + } + } + return uris +} + +func findFullReport(t *testing.T, reports []workspaceDiagnosticReport, uri lsproto.DocumentUri) *lsproto.WorkspaceFullDocumentDiagnosticReport { + t.Helper() + for _, report := range reports { + if report.FullDocumentDiagnosticReport != nil && report.FullDocumentDiagnosticReport.Uri == uri { + return report.FullDocumentDiagnosticReport + } + } + t.Fatalf("no full report for %s in %v", uri, reportURIs(reports)) + return nil +} + +func findUnchangedReport(t *testing.T, reports []workspaceDiagnosticReport, uri lsproto.DocumentUri) *lsproto.WorkspaceUnchangedDocumentDiagnosticReport { + t.Helper() + for _, report := range reports { + if report.UnchangedDocumentDiagnosticReport != nil && report.UnchangedDocumentDiagnosticReport.Uri == uri { + return report.UnchangedDocumentDiagnosticReport + } + } + t.Fatalf("no unchanged report for %s in %v", uri, reportURIs(reports)) + return nil +} + +func previousResultIDs(reports []workspaceDiagnosticReport) []lsproto.PreviousResultId { + ids := make([]lsproto.PreviousResultId, 0, len(reports)) + for _, report := range reports { + if full := report.FullDocumentDiagnosticReport; full != nil && full.ResultId != nil { + ids = append(ids, lsproto.PreviousResultId{Uri: full.Uri, Value: *full.ResultId}) + } else if unchanged := report.UnchangedDocumentDiagnosticReport; unchanged != nil { + ids = append(ids, lsproto.PreviousResultId{Uri: unchanged.Uri, Value: unchanged.ResultId}) + } + } + return ids +} + +func TestWorkspaceDiagnosticsReportsEveryProjectFile(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, _ := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + + assert.DeepEqual(t, reportURIs(resp.Items), []string{ + "file:///home/projects/index.ts", + "file:///home/projects/other.ts", + }) + + // The opened document is deliberately absent: the client pulls it directly. + withError := findFullReport(t, resp.Items, "file:///home/projects/index.ts") + assert.Assert(t, withError.Version.Integer == nil) + assert.Equal(t, len(withError.Items), 1) + assert.Assert(t, strings.Contains(withError.Items[0].Message.AsString(), "not assignable")) + + clean := findFullReport(t, resp.Items, "file:///home/projects/other.ts") + assert.Assert(t, clean.Version.Integer == nil) + assert.Equal(t, len(clean.Items), 0) +} + +func TestWorkspaceDiagnosticsReportsUnchangedForKnownResultIDs(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, _ := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + ids := previousResultIDs(first.Items) + assert.Equal(t, len(ids), 2) + + // Nothing has moved and the client holds all of it, so there is nothing to say. The client + // pulls the workspace every couple of seconds, and repeating the answer would make it + // reconcile a report per file each time for no change. + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: ids, + }) + assert.Equal(t, len(second.Items), 0, "a pull with nothing to add reports nothing, got %v", reportURIs(second.Items)) + + // Editing the open document changes what a closed file reports, so that file comes back in + // full. The other file, which the edit did not reach, is acknowledged rather than re-reported. + client.SendNotification(t, lsproto.TextDocumentDidChangeInfo, &lsproto.DidChangeTextDocumentParams{ + TextDocument: lsproto.VersionedTextDocumentIdentifier{Uri: "file:///home/projects/open.ts", Version: 2}, + ContentChanges: []lsproto.TextDocumentContentChangePartialOrWholeDocument{ + {WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{Text: "export const shared = \"ok\";"}}, + }, + }) + + third := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: ids, + }) + assert.DeepEqual(t, reportURIs(third.Items), reportURIs(first.Items)) + fixed := findFullReport(t, third.Items, "file:///home/projects/index.ts") + assert.Equal(t, len(fixed.Items), 0) + unchanged := findUnchangedReport(t, third.Items, "file:///home/projects/other.ts") + assert.Equal(t, unchanged.ResultId, ids[1].Value) +} + +func TestWorkspaceDiagnosticsClearsDocumentsNoLongerReported(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, _ := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{ + {Uri: "file:///home/projects/deleted.ts", Value: "stale"}, + }, + }) + + cleared := findFullReport(t, resp.Items, "file:///home/projects/deleted.ts") + assert.Equal(t, len(cleared.Items), 0) + assert.Assert(t, cleared.ResultId == nil) +} + +func TestWorkspaceDiagnosticsStreamsPartialResults(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, progress := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + PartialResultToken: &lsproto.IntegerOrString{String: new("workspace-diagnostics")}, + WorkDoneToken: &lsproto.IntegerOrString{String: new("workspace-diagnostics-progress")}, + }) + + // Everything was streamed, so the response itself carries no reports. + assert.Equal(t, len(resp.Items), 0) + assert.DeepEqual(t, reportURIs(progress.partialItems()), []string{ + "file:///home/projects/index.ts", + "file:///home/projects/other.ts", + }) + + kinds := progress.workDoneKinds() + assert.Assert(t, len(kinds) >= 2, "expected work done progress, got %v", kinds) + assert.Equal(t, kinds[0], "begin") + assert.Equal(t, kinds[len(kinds)-1], "end") +} + +func TestWorkspaceDiagnosticsDisabledByScope(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, _ := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + setWorkspaceDiagnosticsScope(t, client, "off") + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{ + {Uri: "file:///home/projects/index.ts", Value: "stale"}, + }, + }) + + // Nothing is checked, but whatever the client still holds is cleared. + assert.DeepEqual(t, reportURIs(resp.Items), []string{"file:///home/projects/index.ts"}) + assert.Equal(t, len(findFullReport(t, resp.Items, "file:///home/projects/index.ts").Items), 0) +} + +// compositeSolutionFiles is a solution-style build of two composite projects, where b references a +// and a has an error. +var compositeSolutionFiles = map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./a"}, {"path": "./b"}]}`, + "/home/projects/a/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib"}}`, + "/home/projects/a/index.ts": "export const a: string = 1;", + "/home/projects/b/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib"}, "references": [{"path": "../a"}]}`, + "/home/projects/b/index.ts": "import { a } from \"../a/index.js\";\nexport const b = a;\n", + "/home/projects/b/open.ts": "export const opened = 1;\n", +} + +func openAndPull(t *testing.T, files map[string]string, open lsproto.DocumentUri) *lsproto.WorkspaceDiagnosticReport { + t.Helper() + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: open, + LanguageId: "typescript", + Version: 1, + Text: files[open.FileName()], + }, + }) + return pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) +} + +// A file reached through the source-of-project-reference redirect belongs to the project that owns +// it, so it is reported once even though it appears in both programs. +func TestWorkspaceDiagnosticsAttributesReferencedSourcesToOwningProject(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + resp := openAndPull(t, compositeSolutionFiles, "file:///home/projects/b/open.ts") + + assert.DeepEqual(t, reportURIs(resp.Items), []string{ + "file:///home/projects/a/index.ts", + "file:///home/projects/b/index.ts", + }) + assert.Equal(t, len(findFullReport(t, resp.Items, "file:///home/projects/a/index.ts").Items), 1) + assert.Equal(t, len(findFullReport(t, resp.Items, "file:///home/projects/b/index.ts").Items), 0) +} + +// With the redirect disabled, b consumes a's emitted declarations. Those are build output, not +// something the user edits, so they must not be reported. +func TestWorkspaceDiagnosticsSkipsReferencedProjectOutputs(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./a"}, {"path": "./b"}]}`, + "/home/projects/a/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib"}}`, + "/home/projects/a/index.ts": "export const a: string = \"ok\";", + "/home/projects/a/lib/index.d.ts": "export declare const a: string = 1;\n", + "/home/projects/a/lib/index.js": "export const a = \"ok\";\n", + "/home/projects/b/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib", "disableSourceOfProjectReferenceRedirect": true}, "references": [{"path": "../a"}]}`, + "/home/projects/b/index.ts": "import { a } from \"../a/index.js\";\nexport const b = a;\n", + "/home/projects/b/open.ts": "export const opened = 1;\n", + } + + resp := openAndPull(t, files, "file:///home/projects/b/open.ts") + + assert.DeepEqual(t, reportURIs(resp.Items), []string{ + "file:///home/projects/a/index.ts", + "file:///home/projects/b/index.ts", + }) +} + +// disableReferencedProjectLoad keeps the referenced project out of the editor entirely, so its +// files are not reported even though the referencing project's program contains them. +func TestWorkspaceDiagnosticsHonorsDisableReferencedProjectLoad(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/a/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib"}}`, + "/home/projects/a/index.ts": "export const a: string = 1;", + "/home/projects/b/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "lib", "disableReferencedProjectLoad": true}, "references": [{"path": "../a"}]}`, + "/home/projects/b/index.ts": "import { a } from \"../a/index.js\";\nexport const b: number = a;\n", + "/home/projects/b/open.ts": "export const opened = 1;\n", + } + + resp := openAndPull(t, files, "file:///home/projects/b/open.ts") + + assert.DeepEqual(t, reportURIs(resp.Items), []string{"file:///home/projects/b/index.ts"}) + assert.Equal(t, len(findFullReport(t, resp.Items, "file:///home/projects/b/index.ts").Items), 1) +} + +// manyProjectFiles builds a solution of independent sibling projects, each with a few files and an +// error in one of them, to exercise checking more than one project at a time. +func manyProjectFiles(projects, filesPerProject int) map[string]string { + files := map[string]string{} + var refs strings.Builder + for p := range projects { + name := fmt.Sprintf("p%d", p) + if p > 0 { + refs.WriteString(", ") + } + fmt.Fprintf(&refs, `{"path": "./%s"}`, name) + files[fmt.Sprintf("/home/projects/%s/tsconfig.json", name)] = `{"compilerOptions": {"composite": true, "outDir": "lib"}}` + for f := range filesPerProject { + body := fmt.Sprintf("export const v%d = %d;", f, f) + if f == 0 { + body = "export const bad: string = 1;" + } + files[fmt.Sprintf("/home/projects/%s/f%d.ts", name, f)] = body + "\n" + } + } + files["/home/projects/p0/open.ts"] = "export const opened = 1;\n" + files["/home/projects/tsconfig.json"] = fmt.Sprintf(`{"files": [], "references": [%s]}`, refs.String()) + return files +} + +// Projects are checked concurrently, but a pull must still report the same files in the same order +// every time. +func TestWorkspaceDiagnosticsOrdersReportsDeterministically(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + const projects, filesPerProject = 6, 4 + files := manyProjectFiles(projects, filesPerProject) + + var want []string + for p := range projects { + for f := range filesPerProject { + want = append(want, fmt.Sprintf("file:///home/projects/p%d/f%d.ts", p, f)) + } + } + + for range 3 { + resp := openAndPull(t, files, "file:///home/projects/p0/open.ts") + assert.DeepEqual(t, reportURIs(resp.Items), want) + for p := range projects { + uri := lsproto.DocumentUri(fmt.Sprintf("file:///home/projects/p%d/f0.ts", p)) + assert.Equal(t, len(findFullReport(t, resp.Items, uri).Items), 1, "expected the error in %s", uri) + } + } +} + +// Editing one project rebuilds only that project's program, so the untouched projects are +// acknowledged from the cache instead of being checked again. +func TestWorkspaceDiagnosticsRechecksOnlyTheEditedProject(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./p0"}, {"path": "./p1"}, {"path": "./p2"}]}`, + "/home/projects/p0/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}}`, + "/home/projects/p0/open.ts": "export const shared = 1;\n", + "/home/projects/p0/consumer.ts": "import { shared } from \"./open.js\";\nexport const use: string = shared;\n", + "/home/projects/p1/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}}`, + "/home/projects/p1/index.ts": "export const p1 = 1;\n", + "/home/projects/p2/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}}`, + "/home/projects/p2/index.ts": "export const p2 = 1;\n", + } + opened := lsproto.DocumentUri("file:///home/projects/p0/open.ts") + + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: opened, LanguageId: "typescript", Version: 1, + Text: files["/home/projects/p0/open.ts"], + }, + }) + + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + consumer := lsproto.DocumentUri("file:///home/projects/p0/consumer.ts") + assert.Equal(t, len(findFullReport(t, first.Items, consumer).Items), 1) + ids := previousResultIDs(first.Items) + + // Fix the error by editing p0's open file. Only p0's program is rebuilt. + client.SendNotification(t, lsproto.TextDocumentDidChangeInfo, &lsproto.DidChangeTextDocumentParams{ + TextDocument: lsproto.VersionedTextDocumentIdentifier{Uri: opened, Version: 2}, + ContentChanges: []lsproto.TextDocumentContentChangePartialOrWholeDocument{ + {WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{Text: "export const shared = \"ok\";\n"}}, + }, + }) + + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: ids}) + + // The consumer's diagnostics changed, so it comes back in full and now reports nothing. + assert.Equal(t, len(findFullReport(t, second.Items, consumer).Items), 0) + + // Every file of every other project is acknowledged as unchanged. + for _, report := range second.Items { + if full := report.FullDocumentDiagnosticReport; full != nil { + assert.Equal(t, full.Uri, consumer, "only the affected file should be reported in full") + continue + } + assert.Assert(t, report.UnchangedDocumentDiagnosticReport != nil) + } +} + +// A setting that changes what a diagnostic says is invisible to a program generation, so the cache +// must not answer "unchanged" across such a change. +func TestWorkspaceDiagnosticsInvalidatesCacheOnSeverityPreferenceChange(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + uri := lsproto.DocumentUri("file:///home/projects/index.ts") + files := map[string]string{ + "/home/projects/tsconfig.json": `{"compilerOptions": {"noUnusedLocals": true}}`, + "/home/projects/index.ts": "export function f() { const unused = 1; }\n", + "/home/projects/open.ts": "export const opened = 1;\n", + } + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/open.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/open.ts"], + }, + }) + + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + before := findFullReport(t, first.Items, uri) + assert.Equal(t, len(before.Items), 1) + assert.Equal(t, *before.Items[0].Severity, lsproto.DiagnosticSeverityWarning) + + // Style checks become errors. The program is untouched, so only the fingerprint catches this. + // Configuration always arrives as a full snapshot, so the scope has to be repeated or it would + // fall back to its default and turn the feature off. + client.SendNotification(t, lsproto.WorkspaceDidChangeConfigurationInfo, &lsproto.DidChangeConfigurationParams{ + Settings: map[string]any{"typescript": map[string]any{ + "reportStyleChecksAsWarnings": false, + "experimental": map[string]any{"workspaceDiagnostics": map[string]any{"scope": "allProjects"}}, + }}, + }) + + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: previousResultIDs(first.Items), + }) + after := findFullReport(t, second.Items, uri) + assert.Equal(t, len(after.Items), 1) + assert.Equal(t, *after.Items[0].Severity, lsproto.DiagnosticSeverityError) +} + +// scopedSolutionFiles is a three-project solution: lib is referenced by app, and standalone is +// unrelated to both. Each has one error. +var scopedSolutionFiles = map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./lib"}, {"path": "./app"}, {"path": "./standalone"}]}`, + "/home/projects/lib/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}}`, + "/home/projects/lib/index.ts": "export const libBad: string = 1;\n", + "/home/projects/lib/open.ts": "export const opened = 1;\n", + "/home/projects/app/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}, "references": [{"path": "../lib"}]}`, + "/home/projects/app/index.ts": "import { libBad } from \"../lib/index.js\";\nexport const appBad: number = libBad;\n", + "/home/projects/standalone/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out"}}`, + "/home/projects/standalone/index.ts": "export const aloneBad: string = 1;\n", +} + +func pullWithScope(t *testing.T, scope string, open lsproto.DocumentUri) []string { + t.Helper() + client, _ := initWorkspaceDiagnosticsClientWithScope(t, scopedSolutionFiles, scope) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: open, LanguageId: "typescript", Version: 1, + Text: scopedSolutionFiles[open.FileName()], + }, + }) + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + return reportURIs(resp.Items) +} + +// Opening a file in lib, each scope reports a different slice of the solution: lib alone, lib plus +// the app that consumes it, or everything including the unrelated project. +func TestWorkspaceDiagnosticsScopes(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + opened := lsproto.DocumentUri("file:///home/projects/lib/open.ts") + + t.Run("openProjects", func(t *testing.T) { + t.Parallel() + assert.DeepEqual(t, pullWithScope(t, "openProjects", opened), []string{ + "file:///home/projects/lib/index.ts", + }) + }) + + t.Run("openProjectsAndDependents", func(t *testing.T) { + t.Parallel() + assert.DeepEqual(t, pullWithScope(t, "openProjectsAndDependents", opened), []string{ + "file:///home/projects/app/index.ts", + "file:///home/projects/lib/index.ts", + }) + }) + + t.Run("allProjects", func(t *testing.T) { + t.Parallel() + assert.DeepEqual(t, pullWithScope(t, "allProjects", opened), []string{ + "file:///home/projects/app/index.ts", + "file:///home/projects/lib/index.ts", + "file:///home/projects/standalone/index.ts", + }) + }) + + t.Run("off", func(t *testing.T) { + t.Parallel() + assert.Equal(t, len(pullWithScope(t, "off", opened)), 0) + }) +} + +// A document the client has open is pulled directly through textDocument/diagnostic, and the client +// only reconciles document and workspace results within a single diagnostic provider. Workspace +// diagnostics ride on their own provider, so reporting an open file here would show every problem +// in it twice. Opening a file that was previously reported must also clear it. +func TestWorkspaceDiagnosticsExcludesOpenDocuments(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{}`, + "/home/projects/broken.ts": "export const bad: string = 1;\n", + "/home/projects/clean.ts": "export const fine = 1;\n", + } + broken := lsproto.DocumentUri("file:///home/projects/broken.ts") + + client, _ := initWorkspaceDiagnosticsClient(t, files) + + // Open only the clean file. The error in the unopened file is reported by the workspace pull. + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/clean.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/clean.ts"], + }, + }) + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.DeepEqual(t, reportURIs(first.Items), []string{string(broken)}) + assert.Equal(t, len(findFullReport(t, first.Items, broken).Items), 1) + + // Now open the file with the error. It must be reported empty rather than left in place, so the + // workspace collection drops it and only the document pull shows the problem. + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: broken, LanguageId: "typescript", Version: 1, + Text: files["/home/projects/broken.ts"], + }, + }) + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: previousResultIDs(first.Items), + }) + cleared := findFullReport(t, second.Items, broken) + assert.Equal(t, len(cleared.Items), 0, "an opened file must be cleared from the workspace report") + + // Closing it hands ownership back to the workspace pull. + client.SendNotification(t, lsproto.TextDocumentDidCloseInfo, &lsproto.DidCloseTextDocumentParams{ + TextDocument: lsproto.TextDocumentIdentifier{Uri: broken}, + }) + third := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: previousResultIDs(second.Items), + }) + assert.Equal(t, len(findFullReport(t, third.Items, broken).Items), 1) +} + +// What a pull reports is what the program has, so a project narrows it the way it narrows a build: +// with the tsconfig's exclude. +func TestWorkspaceDiagnosticsHonorsTsconfigExclude(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{"exclude": ["vendor"]}`, + "/home/projects/open.ts": "export const opened = 1;\n", + "/home/projects/src/index.ts": "export const bad: string = 1;\n", + "/home/projects/vendor/lib.ts": "export const vendored: string = 1;\n", + } + + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/open.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/open.ts"], + }, + }) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + + assert.DeepEqual(t, reportURIs(resp.Items), []string{"file:///home/projects/src/index.ts"}) + assert.Equal(t, len(findFullReport(t, resp.Items, "file:///home/projects/src/index.ts").Items), 1) +} + +// Dependencies stay out without any exclude to say so: an imported package and an automatically +// included @types package are both in the program, and both have errors, and neither is the user's +// code to fix. +func TestWorkspaceDiagnosticsLeavesOutDependencies(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{}`, + "/home/projects/open.ts": "export const opened = 1;\n", + "/home/projects/index.ts": "import { z } from \"dep\";\nexport const x: string = 1;\nexport { z };\n", + "/home/projects/node_modules/dep/package.json": `{"name": "dep", "types": "index.d.ts"}`, + "/home/projects/node_modules/dep/index.d.ts": "export declare const z: string;\nexport declare const bad: string = 1;\n", + "/home/projects/node_modules/@types/x/package.json": `{"name": "@types/x", "types": "index.d.ts"}`, + "/home/projects/node_modules/@types/x/index.d.ts": "declare const globalBad: string = 1;\n", + } + + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/open.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/open.ts"], + }, + }) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.DeepEqual(t, reportURIs(resp.Items), []string{"file:///home/projects/index.ts"}) +} + +// A file with no tsconfig lands in the inferred project, which the memoized open-configured-projects +// set does not cover, so the open-project scopes have to account for it separately. +func TestWorkspaceDiagnosticsCoversInferredProject(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/index.ts": "import { helper } from \"./helper.js\";\nexport const x = helper;\n", + "/home/projects/helper.ts": "export const helper: string = 1;\n", + } + + client, _ := initWorkspaceDiagnosticsClientWithScope(t, files, "openProjects") + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/index.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/index.ts"], + }, + }) + + resp := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + + helper := lsproto.DocumentUri("file:///home/projects/helper.ts") + assert.DeepEqual(t, reportURIs(resp.Items), []string{string(helper)}) + assert.Equal(t, len(findFullReport(t, resp.Items, helper).Items), 1) +} + +// Single threaded mode takes a different path that spawns no goroutines. It has to produce the same +// reports, and it must not deadlock: core.NewWorkGroup's single threaded form defers work to +// RunAndWait, which cannot drive a drain that runs as projects finish. +func TestWorkspaceDiagnosticsSingleThreaded(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./p0"}, {"path": "./p1"}]}`, + "/home/projects/p0/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out", "singleThreaded": true}}`, + "/home/projects/p0/open.ts": "export const opened = 1;\n", + "/home/projects/p0/bad.ts": "export const a: string = 1;\n", + "/home/projects/p1/tsconfig.json": `{"compilerOptions": {"composite": true, "outDir": "out", "singleThreaded": true}}`, + "/home/projects/p1/bad.ts": "export const b: string = 1;\n", + } + + resp := openAndPull(t, files, "file:///home/projects/p0/open.ts") + + assert.DeepEqual(t, reportURIs(resp.Items), []string{ + "file:///home/projects/p0/bad.ts", + "file:///home/projects/p1/bad.ts", + }) + for _, uri := range reportURIs(resp.Items) { + assert.Equal(t, len(findFullReport(t, resp.Items, lsproto.DocumentUri(uri)).Items), 1) + } +} + +// "projects that reference them" is transitive: app references mid references base, so opening a +// file in base must reach app as well, while an unrelated project stays out. +func TestWorkspaceDiagnosticsDependentsAreTransitive(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + composite := `{"compilerOptions": {"composite": true, "outDir": "out"}%s}` + files := map[string]string{ + "/home/projects/tsconfig.json": `{"files": [], "references": [{"path": "./base"}, {"path": "./mid"}, {"path": "./app"}, {"path": "./unrelated"}]}`, + "/home/projects/base/tsconfig.json": fmt.Sprintf(composite, ""), + "/home/projects/base/open.ts": "export const opened = 1;\n", + "/home/projects/base/index.ts": "export const base: string = 1;\n", + "/home/projects/mid/tsconfig.json": fmt.Sprintf(composite, `, "references": [{"path": "../base"}]`), + "/home/projects/mid/index.ts": "export const mid: string = 1;\n", + "/home/projects/app/tsconfig.json": fmt.Sprintf(composite, `, "references": [{"path": "../mid"}]`), + "/home/projects/app/index.ts": "export const app: string = 1;\n", + "/home/projects/unrelated/tsconfig.json": fmt.Sprintf(composite, ""), + "/home/projects/unrelated/index.ts": "export const alone: string = 1;\n", + } + + pull := func(scope string) []string { + client, _ := initWorkspaceDiagnosticsClientWithScope(t, files, scope) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/base/open.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/base/open.ts"], + }, + }) + got := reportURIs(pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }).Items) + slices.Sort(got) + return got + } + + assert.DeepEqual(t, pull("openProjectsAndDependents"), []string{ + "file:///home/projects/app/index.ts", + "file:///home/projects/base/index.ts", + "file:///home/projects/mid/index.ts", + }) + + // The unrelated project is reachable and gets loaded, so its absence above is the scope + // filtering it out rather than the loader never finding it. + assert.DeepEqual(t, pull("allProjects"), []string{ + "file:///home/projects/app/index.ts", + "file:///home/projects/base/index.ts", + "file:///home/projects/mid/index.ts", + "file:///home/projects/unrelated/index.ts", + }) +} + +// An edit should cost what it affects, not what the project contains. These files form a chain +// where each consumes the previous file's interface, so widening one breaks its direct importer and +// nothing beyond it. +func TestWorkspaceDiagnosticsRechecksOnlyWhatAnEditAffects(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + const n = 12 + body := func(f int, extra string) string { + prev := "" + if f > 0 { + prev = fmt.Sprintf("import type { I%d } from \"./f%d.js\";\nexport const uses%d: I%d = { a: \"x\", b: %d };\n", f-1, f-1, f, f-1, f) + } + return fmt.Sprintf("%sexport interface I%d { a: string; b: number%s }\n", prev, f, extra) + } + files := map[string]string{"/home/projects/tsconfig.json": `{"compilerOptions":{"strict":true}}`} + for f := range n { + files[fmt.Sprintf("/home/projects/f%d.ts", f)] = body(f, "") + } + + client, _ := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/f0.ts", LanguageId: "typescript", Version: 1, Text: body(0, ""), + }, + }) + + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + var ids []lsproto.PreviousResultId + for _, item := range first.Items { + if full := item.FullDocumentDiagnosticReport; full != nil && full.ResultId != nil { + ids = append(ids, lsproto.PreviousResultId{Uri: full.Uri, Value: *full.ResultId}) + } + } + + // Requiring a new member of I0 breaks f1, which builds one, and leaves f2 onwards alone. + client.SendNotification(t, lsproto.TextDocumentDidChangeInfo, &lsproto.DidChangeTextDocumentParams{ + TextDocument: lsproto.VersionedTextDocumentIdentifier{Uri: "file:///home/projects/f0.ts", Version: 2}, + ContentChanges: []lsproto.TextDocumentContentChangePartialOrWholeDocument{ + {WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{Text: body(0, "; c: string")}}, + }, + }) + + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: ids}) + + var reportedInFull []string + for _, item := range second.Items { + if full := item.FullDocumentDiagnosticReport; full != nil { + reportedInFull = append(reportedInFull, string(full.Uri)) + } + } + assert.DeepEqual(t, reportedInFull, []string{"file:///home/projects/f1.ts"}) + + broken := findFullReport(t, second.Items, "file:///home/projects/f1.ts") + assert.Equal(t, len(broken.Items), 1) + assert.Assert(t, strings.Contains(broken.Items[0].Message.AsString(), "c")) +} + +// A client that never pulls per document has nothing for a workspace report to collide with, and +// would otherwise never hear about the files it has open. +func TestWorkspaceDiagnosticsReportsOpenDocumentsWithoutServerDeDuplication(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, _ := initWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + openWorkspaceDiagnosticsProject(t, client) + + // On by default, so the open document is left to the pull the client makes for it. + deDuplicated := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.Assert(t, !slices.Contains(reportURIs(deDuplicated.Items), "file:///home/projects/open.ts")) + + client.SendNotification(t, lsproto.WorkspaceDidChangeConfigurationInfo, &lsproto.DidChangeConfigurationParams{ + Settings: map[string]any{ + "typescript": map[string]any{"experimental": map[string]any{"workspaceDiagnostics": map[string]any{ + "scope": "allProjects", + "serverDiagnosticsDeDuplication": false, + }}}, + }, + }) + + reported := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.Assert(t, slices.Contains(reportURIs(reported.Items), "file:///home/projects/open.ts"), + "expected the open document, got %v", reportURIs(reported.Items)) + + // It is reported with the version the client has, so the client can tell which text it is for. + open := findFullReport(t, reported.Items, "file:///home/projects/open.ts") + assert.Assert(t, open.Version.Integer != nil, "an open document reports the version it was checked at") +} + +// The client pulls the workspace every couple of seconds for as long as it is open. A pull with +// nothing to add reports nothing, which is also how it avoids the work: the reports are what +// enumerating and checking the projects produces. +func TestWorkspaceDiagnosticsIdlePullReportsNothing(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := manyProjectFiles(6, 10) + client, _ := initWorkspaceDiagnosticsClient(t, files) + const open = lsproto.DocumentUri("file:///home/projects/p0/open.ts") + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: open, LanguageId: "typescript", Version: 1, Text: files[open.FileName()], + }, + }) + + first := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.Assert(t, len(first.Items) > 0, "the first pull reports the workspace") + ids := previousResultIDs(first.Items) + + // Whatever the client asks, an idle pull says nothing, and says it without touching a project. + for range 3 { + idle := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: ids}) + assert.Equal(t, len(idle.Items), 0, "an idle pull reports nothing, got %v", reportURIs(idle.Items)) + } + + // A client that has lost what it held is told again, rather than being left with nothing. + forgotten := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.DeepEqual(t, reportURIs(forgotten.Items), reportURIs(first.Items)) +} + +// Pulls that overlap must not pile up behind each other: the last one answers in full, and the ones +// it superseded end instead of checking a workspace the client is about to be told about anyway. +func TestWorkspaceDiagnosticsSupersedesOverlappingPulls(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := manyProjectFiles(6, 20) + client, _ := initWorkspaceDiagnosticsClient(t, files) + const open = lsproto.DocumentUri("file:///home/projects/p0/open.ts") + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: open, + LanguageId: "typescript", + Version: 1, + Text: files[open.FileName()], + }, + }) + + const pulls = 3 + waiters := make([]func() (*lsproto.Message, *lsproto.WorkspaceDiagnosticReport, bool), pulls) + for i := range waiters { + waiters[i] = client.SendRequestAsync(t, lsproto.WorkspaceDiagnosticInfo, &lsproto.WorkspaceDiagnosticParams{}) + } + + // The last pull is superseded by nothing, so it must report. + msg, report, ok := waiters[pulls-1]() + assert.Assert(t, ok, "expected a response to the last pull") + assert.Assert(t, msg.AsResponse().Error == nil, "the last pull must not be cancelled") + assert.Assert(t, len(report.Items) > 0, "the last pull must report the workspace") + + // The earlier ones were superseded by their successor, unless one got in ahead of it. Either way + // they answer, rather than running on behind the pull that replaced them. + superseded := 0 + for _, wait := range waiters[:pulls-1] { + msg, _, _ := wait() + if err := msg.AsResponse().Error; err != nil { + // ServerCancelled carrying cancellation data: a client that reads this as a failure + // gives up on workspace diagnostics after a handful of them. + assert.Equal(t, err.Code, int32(lsproto.ErrorCodeServerCancelled), "a superseded pull is server-cancelled, not failed") + data, ok := err.Data.(map[string]any) + assert.Assert(t, ok, "a superseded pull must carry cancellation data, got %#v", err.Data) + assert.Equal(t, data["retriggerRequest"], false, "the pull that replaced this one already reports what it would have") + superseded++ + } + } + assert.Assert(t, superseded > 0, "a pull sent while another is checking this many files must supersede it") +} + +// A project reports only once it is wholly checked, and a workspace can be a single project, so +// progress paced by reports would sit at nothing for the whole run and then jump to the end. It +// has to be paced by the files the checkers get through. +func TestWorkspaceDiagnosticsProgressAdvancesWhileCheckingOneProject(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + // One project, enough files that a file is worth well under a percent of the whole. + files := map[string]string{"/home/projects/tsconfig.json": `{}`} + files["/home/projects/open.ts"] = "export const opened = 1;\n" + for i := range 60 { + files[fmt.Sprintf("/home/projects/f%d.ts", i)] = fmt.Sprintf("export const v%d: number = %d;\n", i, i) + } + + client, progress := initWorkspaceDiagnosticsClient(t, files) + client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{ + TextDocument: &lsproto.TextDocumentItem{ + Uri: "file:///home/projects/open.ts", LanguageId: "typescript", Version: 1, + Text: files["/home/projects/open.ts"], + }, + }) + report := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + WorkDoneToken: &lsproto.IntegerOrString{String: new("wsdiag-progress")}, + PreviousResultIds: []lsproto.PreviousResultId{}, + }) + assert.Assert(t, len(report.Items) > 0, "the pull reports the workspace") + + percentages := progress.workDonePercentages() + assert.Assert(t, len(percentages) > 0, "expected progress reports, got none") + for i, pct := range percentages { + assert.Assert(t, pct <= 100, "percentage out of range: %d", pct) + if i > 0 { + assert.Assert(t, pct > percentages[i-1], "progress must move forwards, got %v", percentages) + } + } + assert.Assert(t, percentages[0] < 100, + "progress must advance while the single project is checked, not jump to the end: %v", percentages) +} diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go new file mode 100644 index 0000000000000..5d0a74bb14ae0 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -0,0 +1,541 @@ +package lsp + +import ( + "context" + "errors" + "fmt" + "sync/atomic" + "time" + + "github.com/microsoft/TypeScript/tsc/internal/ast" + "github.com/microsoft/TypeScript/tsc/internal/collections" + "github.com/microsoft/TypeScript/tsc/internal/core" + "github.com/microsoft/TypeScript/tsc/internal/diagnostics" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsconv" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/project" + "github.com/microsoft/TypeScript/tsc/internal/tspath" +) + +const ( + // How much of a streamed report is buffered before being flushed. + workspaceDiagnosticsChunkFiles = 100 + workspaceDiagnosticsChunkInterval = 500 * time.Millisecond +) + +type workspaceDiagnosticReport = lsproto.WorkspaceFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport + +func (handlers handlerMap) registerWorkspaceDiagnosticHandler() { + handlers[lsproto.WorkspaceDiagnosticInfo.Method] = func(s *Server, ctx context.Context, req *lsproto.RequestMessage) (func() error, error) { + if s.session == nil { + return nil, lsproto.ErrorCodeServerNotInitialized + } + params, err := req.UnmarshalParams[*lsproto.WorkspaceDiagnosticParams]() + if err != nil { + return nil, err + } + // Superseded here, on the dispatch loop, so pulls replace one another in the order the + // client sent them rather than the order their goroutines happen to start in. + ctx, cancel := context.WithCancelCause(ctx) + pull := &workspaceDiagnosticsPull{cancel: cancel} + s.supersedeWorkspaceDiagnostics(pull) + + // A pull can run for minutes, so it stays off the dispatch loop. + return func() error { + defer s.recover(req) + defer cancel(nil) + defer s.finishWorkspaceDiagnostics(pull) + + resp, lsErr := s.handleWorkspaceDiagnostic(ctx, params) + if lsErr != nil { + return lsErr + } + if ctx.Err() != nil { + return ctx.Err() + } + return s.sendResult(req.ID, resp) + }, nil + } +} + +// supersededWorkspaceDiagnosticsError is what a pull that a newer one replaced answers with. The +// code has to be ServerCancelled carrying DiagnosticServerCancellationData: a client that cannot +// tell a server-cancelled pull from a failed one counts it against the handful of failures it +// allows before it stops pulling the workspace altogether. retriggerRequest is false because the +// pull that replaced this one is already reporting what it would have. +var supersededWorkspaceDiagnosticsError = errorWithData{ + code: lsproto.ErrorCodeServerCancelled, + data: &lsproto.DiagnosticServerCancellationData{RetriggerRequest: false}, +} + +// errWorkspaceDiagnosticsSuperseded is the cancellation cause of a pull that a newer one replaced. +// The context carries the reason so that it does not have to be tracked alongside it, and so that +// it cannot disagree with whether the context is actually done. +var errWorkspaceDiagnosticsSuperseded = errors.New("workspace diagnostics superseded by a newer pull") + +// workspaceDiagnosticsPull is one running `workspace/diagnostic` request, identified by pointer so +// a pull can tell whether it is still the current one. +type workspaceDiagnosticsPull struct { + cancel context.CancelCauseFunc +} + +// supersedeWorkspaceDiagnostics makes pull the current one and cancels whatever was running before +// it. A pull reports the whole workspace as of the snapshot it starts from, so an older one can +// only answer with a program version the newer one is about to cover; finishing it would spend +// minutes of checking to tell the client something it is already being told. +func (s *Server) supersedeWorkspaceDiagnostics(pull *workspaceDiagnosticsPull) { + s.workspaceDiagnosticsMu.Lock() + superseded := s.workspaceDiagnosticsPull + s.workspaceDiagnosticsPull = pull + s.workspaceDiagnosticsMu.Unlock() + if superseded != nil { + superseded.cancel(errWorkspaceDiagnosticsSuperseded) + } +} + +// finishWorkspaceDiagnostics clears the current pull, unless a newer one has already replaced it. +func (s *Server) finishWorkspaceDiagnostics(pull *workspaceDiagnosticsPull) { + s.workspaceDiagnosticsMu.Lock() + defer s.workspaceDiagnosticsMu.Unlock() + if s.workspaceDiagnosticsPull == pull { + s.workspaceDiagnosticsPull = nil + } +} + +// pullWorkspaceDiagnostics reports the workspace as of the snapshot it reads. +func (s *Server) pullWorkspaceDiagnostics(ctx context.Context, params *lsproto.WorkspaceDiagnosticParams) (lsproto.WorkspaceDiagnosticResponse, error) { + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + run := newWorkspaceDiagnosticsRun(ctx, s, params) + + scope := s.session.Config().WorkspaceDiagnosticsScope + // An empty (non-nil) set loads no trees beyond what is already loaded. + var trees *collections.Set[tspath.Path] + if scope != lsutil.WorkspaceDiagnosticsScopeAllProjects { + trees = &collections.Set[tspath.Path]{} + if scope == lsutil.WorkspaceDiagnosticsScopeOpenProjectsAndDependents { + for _, open := range s.session.Snapshot().OpenProjects() { + trees.Add(tspath.Path(open.Id())) + } + } + } + + repeat := false + s.session.WithSnapshotLoadingProjectTree(ctx, trees, func(snapshot *project.Snapshot) { + preferences := snapshot.UserPreferences() + settings := workspaceDiagnosticsSettings{preferences: preferences, locale: s.GetLocale().String()} + // A program generation cannot see a settings change, so the cache is keyed on them too. + s.workspaceDiagnostics.useSettings(settings) + if !scope.Enabled() || preferences.EnableValidation.IsFalse() { + // Nothing is reported, and the cleanup pass below clears whatever the client holds. + return + } + run.fingerprint = newWorkspaceDiagnosticsFingerprint(snapshot, settings) + if s.workspaceDiagnostics.repeatsLastAnswer(run.fingerprint, run.previous) { + repeat = true + return + } + run.collect(snapshot, projectsInScope(snapshot, scope)) + }) + + if repeat { + // The client pulls every couple of seconds for as long as it is open, so most pulls have + // nothing to tell it. Saying so in full would walk every file of every project and make + // the client reconcile a report each, for no change. Reporting nothing leaves it alone. + return &lsproto.WorkspaceDiagnosticReport{Items: []workspaceDiagnosticReport{}}, nil + } + + // A cancelled run covered only part of the workspace; the cleanup below would mistake the files + // it never reached for files that no longer have diagnostics. + if err := ctx.Err(); err != nil { + run.endProgress() + return nil, err + } + + // Report empty for anything the client holds that no project reported, so it clears. + for _, previous := range params.PreviousResultIds { + if !run.reported.Has(previous.Uri) { + run.add(workspaceDiagnosticReport{ + FullDocumentDiagnosticReport: &lsproto.WorkspaceFullDocumentDiagnosticReport{ + Uri: previous.Uri, + Items: []*lsproto.Diagnostic{}, + }, + }) + } + } + + if run.collected { + s.workspaceDiagnostics.retain(&run.reported, run.fingerprint) + if s.logger.IsVerbose() { + stats := s.workspaceDiagnostics.stats() + s.logger.Logf("workspace diagnostics: reported %d files, cached %d files across %d projects", + run.filesDone, stats.Files, stats.Projects) + } + } + + // Checking can surface global diagnostics the owning tsconfig has not published yet. + s.session.EnqueuePublishGlobalDiagnostics() + + return run.finish(), nil +} + +// workspaceDiagnosticsRun accumulates the reports of one `workspace/diagnostic` request. +type workspaceDiagnosticsRun struct { + server *Server + ctx context.Context + + partialResultToken *lsproto.IntegerOrString + workDoneToken *lsproto.IntegerOrString + // The token progress is reported against: the client's if it sent one, otherwise one the + // server created. Nil until a sweep with something to report begins. + progressToken *lsproto.IntegerOrString + // Result ids the client already holds. + previous map[lsproto.DocumentUri]string + // Documents already covered, so a file in several projects is reported once. + reported collections.Set[lsproto.DocumentUri] + + // Reports not yet flushed; without a partial result token this holds all of them. + pending []workspaceDiagnosticReport + // Paces flushes and progress so neither is sent per file. + sinceTick int + lastTick time.Time + + filesDone int + filesTotal int + begun bool + // The projects being checked, so progress can be totalled from wherever it is reported. + work []*workspaceDiagnosticsProject + // The last percentage sent, so progress never goes backwards and unchanged ticks say nothing. + lastPercentage atomic.Int64 + // Whether a sweep actually ran, so a disabled pull does not prune the cache. + collected bool + // What this answer was computed from, recorded with it so the next pull can tell whether it + // has anything to add. + fingerprint workspaceDiagnosticsFingerprint + + cache *workspaceDiagnosticsCache +} + +func newWorkspaceDiagnosticsRun(ctx context.Context, server *Server, params *lsproto.WorkspaceDiagnosticParams) *workspaceDiagnosticsRun { + previous := make(map[lsproto.DocumentUri]string, len(params.PreviousResultIds)) + for _, id := range params.PreviousResultIds { + previous[id.Uri] = id.Value + } + return &workspaceDiagnosticsRun{ + server: server, + ctx: ctx, + partialResultToken: params.PartialResultToken, + workDoneToken: params.WorkDoneToken, + previous: previous, + lastTick: time.Now(), + cache: server.workspaceDiagnostics, + } +} + +// collect reports every file owned by every project in scope. A project's files are shared across +// its diagnostics checkers, so checking one project already uses several checkers; projects run +// concurrently on top of that, bounded so the two together do not take the machine. +func (r *workspaceDiagnosticsRun) collect(snapshot *project.Snapshot, projects []*project.Project) { + r.collected = true + // Enumerating the files walks every one of them, which is worth standing aside for too. + snapshot.WaitForInteractiveIdle(r.ctx) + work := r.assignFilesToProjects(snapshot, projects) + r.work = work + // Only files that still need checking count towards progress. + r.filesTotal = 0 + for _, pf := range work { + r.filesTotal += pf.toCheck + } + r.beginProgress() + + if concurrency := workspaceDiagnosticsConcurrency(work); concurrency > 1 { + r.checkConcurrently(snapshot, work, concurrency) + } else { + r.checkSequentially(snapshot, work) + } +} + +// checkSequentially is the single threaded path: no goroutines are spawned at all, so a run can be +// stepped through. core.NewWorkGroup's single threaded form cannot serve here because it defers +// every task to RunAndWait, and this drains projects as they finish. +func (r *workspaceDiagnosticsRun) checkSequentially(snapshot *project.Snapshot, work []*workspaceDiagnosticsProject) { + for _, pf := range work { + if pf.toCheck == 0 { + r.emitProject(pf) + continue + } + completed := r.checkProject(snapshot, pf) + snapshot.ReleaseDiagnosticsCheckers(pf.project) + if !completed { + return + } + r.emitProject(pf) + } +} + +// checkConcurrently gives each project its own slot and drains them in project order as they fill, +// so reports stream as they finish but always come out in the same order. +func (r *workspaceDiagnosticsRun) checkConcurrently(snapshot *project.Snapshot, work []*workspaceDiagnosticsProject, concurrency int) { + completed := make([]bool, len(work)) + done := make([]chan struct{}, len(work)) + for i := range done { + done[i] = make(chan struct{}) + } + + slots := make(chan struct{}, concurrency) + wg := core.NewWorkGroup(false /*singleThreaded*/) + for i, pf := range work { + if pf.toCheck == 0 { + // Answered entirely from the cache: no checker, no slot. + completed[i] = true + close(done[i]) + continue + } + wg.Queue(func() { + defer close(done[i]) + select { + case slots <- struct{}{}: + defer func() { <-slots }() + case <-r.ctx.Done(): + return + } + // Hand back the checkers before the next project builds its own, so a sweep holds + // only as many programs' worth of types as it is checking at once. + defer snapshot.ReleaseDiagnosticsCheckers(pf.project) + completed[i] = r.checkProject(snapshot, pf) + }) + } + + for i, pf := range work { + <-done[i] + if !completed[i] { + break + } + r.emitProject(pf) + } + wg.RunAndWait() +} + +// checkProject fills in the reports for the files of one project, reporting whether it got through +// them all. A cancelled project must not be emitted: its remaining reports are still zero values. +// +// The program checks them all in one call, so the work is shared across the project's checkers +// rather than being driven a file at a time from here; the trade is that a project reports once it +// is done rather than streaming as each of its files finishes. +func (r *workspaceDiagnosticsRun) checkProject(snapshot *project.Snapshot, pf *workspaceDiagnosticsProject) bool { + files := make([]*ast.SourceFile, 0, len(pf.files)) + for _, file := range pf.files { + if file != nil { + files = append(files, file) + } + } + // The check runs for as long as the project is big, so it says how far along it is as it + // goes. It counts the files of the program, which is more than the files this reports on - + // the libraries among them are checked but never reported - so the count is scaled onto what + // this project contributes to the bar rather than used directly. + ctx := project.WithCheckProgress(r.ctx, func(checked, total int) { + if total <= 0 { + return + } + pf.progress.Store(min(int64(pf.toCheck), int64(checked)*int64(pf.toCheck)/int64(total))) + // Called from the checkers, a batch of files at a time. reportProgress says nothing + // unless the percentage moved, which bounds the notifications to a hundred on top of that. + r.reportProgress() + }) + + // Ask through the incremental view, so a change is re-checked where it landed rather than + // across the whole project. + program := snapshot.IncrementalProgram(pf.project) + reports := pf.languageService.WorkspaceDiagnosticsForProject(ctx, program, files) + if r.ctx.Err() != nil { + return false + } + for j, file := range pf.files { + if file == nil { + continue + } + pf.reports[j] = r.reportForFile(snapshot, file, reports[file]) + } + return true +} + +// emitProject hands a finished project's reports to the client and remembers which program version +// produced each result id, so the next pull can skip the file. +func (r *workspaceDiagnosticsRun) emitProject(pf *workspaceDiagnosticsProject) { + // Whatever the check reported along the way, the project is done now. + pf.progress.Store(int64(pf.toCheck)) + for j, report := range pf.reports { + if pf.files[j] != nil { + r.filesDone++ + if full := report.FullDocumentDiagnosticReport; full != nil && full.ResultId != nil { + r.cache.store(lsconv.FileNameToDocumentURI(pf.files[j].FileName()), workspaceDiagnosticsCacheEntry{ + project: tspath.Path(pf.project.Id()), + generation: pf.generation, + resultID: *full.ResultId, + }) + } + } + r.add(report) + } +} + +func (r *workspaceDiagnosticsRun) reportForFile(snapshot *project.Snapshot, file *ast.SourceFile, items []*lsproto.Diagnostic) workspaceDiagnosticReport { + uri := lsconv.FileNameToDocumentURI(file.FileName()) + resultID := workspaceDiagnosticsResultID(items) + version := openDocumentVersion(snapshot, file.FileName()) + + if previous, ok := r.previous[uri]; ok && resultID != "" && previous == resultID { + return workspaceDiagnosticReport{ + UnchangedDocumentDiagnosticReport: &lsproto.WorkspaceUnchangedDocumentDiagnosticReport{ + Uri: uri, + Version: version, + ResultId: resultID, + }, + } + } + full := &lsproto.WorkspaceFullDocumentDiagnosticReport{ + Uri: uri, + Version: version, + Items: items, + } + if resultID != "" { + full.ResultId = &resultID + } + return workspaceDiagnosticReport{FullDocumentDiagnosticReport: full} +} + +func (r *workspaceDiagnosticsRun) add(report workspaceDiagnosticReport) { + r.pending = append(r.pending, report) + r.sinceTick++ + if r.sinceTick < workspaceDiagnosticsChunkFiles && time.Since(r.lastTick) < workspaceDiagnosticsChunkInterval { + return + } + r.sinceTick = 0 + r.lastTick = time.Now() + r.flush() + r.reportProgress() +} + +// flush streams buffered reports to the partial result token, if the client gave one. +func (r *workspaceDiagnosticsRun) flush() { + if r.partialResultToken == nil || len(r.pending) == 0 { + return + } + _ = r.server.sendNotification(lsproto.WorkspaceDiagnosticPartialResultInfo, &lsproto.WorkspaceDiagnosticPartialResultParams{ + Token: *r.partialResultToken, + Value: lsproto.WorkspaceDiagnosticReportPartialResult{Items: r.pending}, + }) + r.pending = nil +} + +func (r *workspaceDiagnosticsRun) finish() lsproto.WorkspaceDiagnosticResponse { + // With a partial result token everything was streamed already; without one, pending holds it all. + r.flush() + r.endProgress() + items := r.pending + if items == nil { + items = []workspaceDiagnosticReport{} + } + r.pending = nil + return &lsproto.WorkspaceDiagnosticReport{Items: items} +} + +// beginProgress starts reporting how far through the workspace the sweep is. A sweep of a large +// workspace runs for minutes, so the client needs something to show for it. +// +// The token is the client's when it sent one. Clients that pull the workspace generally do not: +// they send a partial result token to stream the reports and nothing else, which leaves the server +// to create a progress token of its own. Both are the same notification once a token exists; only +// who creates it differs. +func (r *workspaceDiagnosticsRun) beginProgress() { + if r.filesTotal == 0 { + // Everything was answered from the cache, so there is nothing to watch. + return + } + switch { + case r.workDoneToken != nil: + r.progressToken = r.workDoneToken + case r.server.clientCapabilities.Window.WorkDoneProgress: + // Named after the request so two pulls cannot report against one token. + token := lsproto.IntegerOrString{String: new("tsgo-workspace-diagnostics-" + core.GetRequestID(r.ctx))} + r.progressToken = &token + // Not waited on: the client answers with null, and a slow one would hold up the sweep. The + // notifications below leave on the same queue behind it, so they cannot overtake it. + if err := r.server.sendClientRequestFireAndForget(lsproto.WindowWorkDoneProgressCreateInfo, &lsproto.WorkDoneProgressCreateParams{ + Token: token, + }); err != nil { + r.progressToken = nil + return + } + default: + return + } + + r.begun = true + r.sendProgress(lsproto.WorkDoneProgressBeginOrReportOrEnd{ + Begin: &lsproto.WorkDoneProgressBegin{ + Title: diagnostics.Checking_workspace.Localize(r.server.GetLocale()), + Message: new(fmt.Sprintf("0/%d", r.filesTotal)), + Percentage: new(uint32(0)), + }, + }) +} + +// filesChecked totals what the projects have got through. Reported files rather than checked ones, +// so it can be read against filesTotal. +func (r *workspaceDiagnosticsRun) filesChecked() int { + done := 0 + for _, pf := range r.work { + done += int(pf.progress.Load()) + } + return done +} + +// reportProgress sends where the sweep has got to, unless it has not moved. Called from each of +// the checkers as they finish a file, and as reports are added, so it only ever moves forwards and +// says nothing when it has not. +func (r *workspaceDiagnosticsRun) reportProgress() { + if !r.begun || r.filesTotal == 0 { + return + } + done := min(r.filesChecked(), r.filesTotal) + percentage := int64(done * 100 / r.filesTotal) + for { + last := r.lastPercentage.Load() + if percentage <= last { + return + } + if r.lastPercentage.CompareAndSwap(last, percentage) { + break + } + } + r.sendProgress(lsproto.WorkDoneProgressBeginOrReportOrEnd{ + Report: &lsproto.WorkDoneProgressReport{ + Message: new(fmt.Sprintf("%d/%d", done, r.filesTotal)), + Percentage: new(uint32(percentage)), + }, + }) +} + +func (r *workspaceDiagnosticsRun) endProgress() { + if !r.begun { + return + } + r.begun = false + r.sendProgress(lsproto.WorkDoneProgressBeginOrReportOrEnd{End: &lsproto.WorkDoneProgressEnd{}}) +} + +func (r *workspaceDiagnosticsRun) sendProgress(value lsproto.WorkDoneProgressBeginOrReportOrEnd) { + _ = r.server.sendNotification(lsproto.ProgressInfo, &lsproto.ProgressParams{ + Token: *r.progressToken, + Value: value, + }) +} + +// openDocumentVersion returns the LSP version of an open file, and null otherwise. +func openDocumentVersion(snapshot *project.Snapshot, fileName string) lsproto.IntegerOrNull { + if handle := snapshot.GetFile(fileName); handle != nil && handle.IsOverlay() { + return lsproto.IntegerOrNull{Integer: new(handle.Version())} + } + return lsproto.IntegerOrNull{} +} diff --git a/tsc/internal/lsp/workspacediagnostics_internal_test.go b/tsc/internal/lsp/workspacediagnostics_internal_test.go new file mode 100644 index 0000000000000..95e42a2cbf600 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnostics_internal_test.go @@ -0,0 +1,102 @@ +package lsp + +import ( + "errors" + "reflect" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" + "gotest.tools/v3/assert" +) + +// The cache compares whole preference structs rather than listing the ones that matter, so a new +// preference cannot be forgotten and leave errors in the problem list that no longer exist. That +// relies on reflect.DeepEqual holding for equal values, which is not true of funcs or channels: +// two non-nil ones never compare equal, so a single such field would make every pull see settings +// as changed and discard the cache. Comparing values cannot catch that, since the zero of both is +// nil and nil does compare equal, so check the type instead. +func TestUserPreferencesStayComparableByValue(t *testing.T) { + t.Parallel() + + var offenders []string + var walk func(t reflect.Type, path string, seen map[reflect.Type]bool) + walk = func(t reflect.Type, path string, seen map[reflect.Type]bool) { + if seen[t] { + return + } + seen[t] = true + switch t.Kind() { + case reflect.Func, reflect.Chan, reflect.UnsafePointer: + offenders = append(offenders, path+" is a "+t.Kind().String()) + case reflect.Struct: + for field := range t.Fields() { + walk(field.Type, path+"."+field.Name, seen) + } + case reflect.Pointer, reflect.Slice, reflect.Array: + walk(t.Elem(), path+"[]", seen) + case reflect.Map: + walk(t.Key(), path+"[key]", seen) + walk(t.Elem(), path+"[value]", seen) + } + } + walk(reflect.TypeFor[lsutil.UserPreferences](), "UserPreferences", map[reflect.Type]bool{}) + + assert.Equal(t, len(offenders), 0, + "reflect.DeepEqual cannot compare these, so workspaceDiagnosticsSettings.Equal would discard the cache on every pull: %v", offenders) +} + +// Equal must react to a preference the handler reads. +func TestWorkspaceDiagnosticsSettingsEqual(t *testing.T) { + t.Parallel() + + settings := func(scope lsutil.WorkspaceDiagnosticsScope, locale string) workspaceDiagnosticsSettings { + return workspaceDiagnosticsSettings{ + preferences: lsutil.UserPreferences{ + WorkspaceDiagnosticsScope: scope, + AutoImportFileExcludePatterns: []string{"**/vendor/**"}, + }, + locale: locale, + } + } + base := settings(lsutil.WorkspaceDiagnosticsScopeOpenProjects, "en") + + assert.Assert(t, base.Equal(settings(lsutil.WorkspaceDiagnosticsScopeOpenProjects, "en")), + "separately built but equal settings must compare equal") + assert.Assert(t, !base.Equal(settings(lsutil.WorkspaceDiagnosticsScopeAllProjects, "en")), + "a changed preference must invalidate the cache") + assert.Assert(t, !base.Equal(settings(lsutil.WorkspaceDiagnosticsScopeOpenProjects, "de")), + "a changed locale must invalidate the cache, since it changes what a diagnostic says") +} + +// Only the newest pull is worth finishing: an older one reports the workspace as of a snapshot the +// newer one has already moved past, so it is cancelled rather than left to check for minutes. +func TestWorkspaceDiagnosticsPullSupersedesTheOneBeforeIt(t *testing.T) { + t.Parallel() + + s := &Server{} + // The cause each pull was cancelled with, which is how a pull tells why it stopped. + causes := make([]error, 3) + pulls := make([]*workspaceDiagnosticsPull, len(causes)) + for i := range pulls { + pulls[i] = &workspaceDiagnosticsPull{cancel: func(cause error) { causes[i] = cause }} + } + + s.supersedeWorkspaceDiagnostics(pulls[0]) + assert.Assert(t, causes[0] == nil, "the only pull running is not superseded") + + s.supersedeWorkspaceDiagnostics(pulls[1]) + assert.Assert(t, errors.Is(causes[0], errWorkspaceDiagnosticsSuperseded), + "a newer pull cancels the one it replaces, saying why: %v", causes[0]) + + s.supersedeWorkspaceDiagnostics(pulls[2]) + assert.Assert(t, errors.Is(causes[1], errWorkspaceDiagnosticsSuperseded), "every pull but the newest is cancelled") + assert.Assert(t, causes[2] == nil, "the newest pull runs to completion") + + // A superseded pull unwinding afterwards must not clear the pull that replaced it. + s.finishWorkspaceDiagnostics(pulls[0]) + s.finishWorkspaceDiagnostics(pulls[1]) + assert.Assert(t, s.workspaceDiagnosticsPull == pulls[2]) + + s.finishWorkspaceDiagnostics(pulls[2]) + assert.Assert(t, s.workspaceDiagnosticsPull == nil, "nothing is running once the newest pull finishes") +} diff --git a/tsc/internal/lsp/workspacediagnosticscache.go b/tsc/internal/lsp/workspacediagnosticscache.go new file mode 100644 index 0000000000000..ea6b0226069cb --- /dev/null +++ b/tsc/internal/lsp/workspacediagnosticscache.go @@ -0,0 +1,165 @@ +package lsp + +import ( + "reflect" + "strconv" + "sync" + + "github.com/microsoft/TypeScript/tsc/internal/collections" + "github.com/microsoft/TypeScript/tsc/internal/json" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/project" + "github.com/microsoft/TypeScript/tsc/internal/tspath" + "github.com/zeebo/xxh3" +) + +// workspaceDiagnosticsCache remembers which program version produced the result id a client holds +// for a file. A program is rebuilt as a unit, so an unchanged generation means every file in the +// project can be answered "unchanged" without checking it. +type workspaceDiagnosticsCache struct { + mu sync.Mutex + settings workspaceDiagnosticsSettings + entries map[lsproto.DocumentUri]workspaceDiagnosticsCacheEntry + // answered is what the last pull that reported was computed from, so a later pull can tell + // that it would only repeat itself. Nil until a pull has reported. + answered *workspaceDiagnosticsFingerprint +} + +// workspaceDiagnosticsFingerprint is everything a pull's answer depends on: the snapshot it reads +// and the settings it is computed under. A pull's answer is a function of the two, so two pulls +// that share a fingerprint have the same answer. +// +// The snapshot rather than the program generations, which is what decides whether a file needs +// checking: what a pull reports also turns on which documents are open, which projects are loaded, +// and what is on disk. Those all move the snapshot, and the per-file cache already answers an +// unnecessary sweep without checking anything. +type workspaceDiagnosticsFingerprint struct { + snapshot uint64 + settings workspaceDiagnosticsSettings +} + +func newWorkspaceDiagnosticsFingerprint(snapshot *project.Snapshot, settings workspaceDiagnosticsSettings) workspaceDiagnosticsFingerprint { + return workspaceDiagnosticsFingerprint{snapshot: snapshot.ID(), settings: settings} +} + +func (f workspaceDiagnosticsFingerprint) Equal(other workspaceDiagnosticsFingerprint) bool { + return f.snapshot == other.snapshot && f.settings.Equal(other.settings) +} + +// workspaceDiagnosticsSettings is the settings an entry was computed under. It compares every +// preference rather than the ones known to matter: one that changes what a diagnostic says but was +// left off such a list would strand errors in the client's problem list, which is worse than the +// occasional extra sweep. +type workspaceDiagnosticsSettings struct { + preferences lsutil.UserPreferences + locale string +} + +func (s workspaceDiagnosticsSettings) Equal(other workspaceDiagnosticsSettings) bool { + return s.locale == other.locale && reflect.DeepEqual(s.preferences, other.preferences) +} + +type workspaceDiagnosticsCacheEntry struct { + project tspath.Path + generation uint64 + resultID string +} + +func newWorkspaceDiagnosticsCache() *workspaceDiagnosticsCache { + return &workspaceDiagnosticsCache{entries: map[lsproto.DocumentUri]workspaceDiagnosticsCacheEntry{}} +} + +// useSettings discards the cache if the settings behind it changed. It compares the snapshot's +// copy rather than reacting to a configuration notification, so a pull already in flight cannot +// repopulate under settings that have moved on. +func (c *workspaceDiagnosticsCache) useSettings(settings workspaceDiagnosticsSettings) { + c.mu.Lock() + defer c.mu.Unlock() + if !c.settings.Equal(settings) { + c.settings = settings + c.entries = map[lsproto.DocumentUri]workspaceDiagnosticsCacheEntry{} + c.answered = nil + } +} + +// repeatsLastAnswer reports whether a pull would tell the client only what it already has. The +// programs being unchanged is not enough on its own: a client that has forgotten a result id we +// recorded, or one that never had it, has to be told again. +func (c *workspaceDiagnosticsCache) repeatsLastAnswer(fingerprint workspaceDiagnosticsFingerprint, clientHolds map[lsproto.DocumentUri]string) bool { + c.mu.Lock() + defer c.mu.Unlock() + if c.answered == nil || !c.answered.Equal(fingerprint) { + return false + } + for uri, entry := range c.entries { + if clientHolds[uri] != entry.resultID { + return false + } + } + return true +} + +// unchangedResultID returns the result id to acknowledge, if the client still holds what we last +// computed for this generation. +func (c *workspaceDiagnosticsCache) unchangedResultID(uri lsproto.DocumentUri, project tspath.Path, generation uint64, clientHolds string) (string, bool) { + if clientHolds == "" { + return "", false + } + c.mu.Lock() + defer c.mu.Unlock() + entry, ok := c.entries[uri] + if !ok || entry.project != project || entry.generation != generation || entry.resultID != clientHolds { + return "", false + } + return entry.resultID, true +} + +func (c *workspaceDiagnosticsCache) store(uri lsproto.DocumentUri, entry workspaceDiagnosticsCacheEntry) { + c.mu.Lock() + defer c.mu.Unlock() + c.entries[uri] = entry +} + +// retain drops everything the sweep did not report, and records what the answer was computed from +// so the next pull can tell whether it has anything to add. +func (c *workspaceDiagnosticsCache) retain(reported *collections.Set[lsproto.DocumentUri], fingerprint workspaceDiagnosticsFingerprint) { + c.mu.Lock() + defer c.mu.Unlock() + for uri := range c.entries { + if !reported.Has(uri) { + delete(c.entries, uri) + } + } + c.answered = &fingerprint +} + +// workspaceDiagnosticsCacheStats describes what the cache holds, so a pull that takes longer than +// expected can be read against what it was able to skip. +type workspaceDiagnosticsCacheStats struct { + Files int + Projects int +} + +func (c *workspaceDiagnosticsCache) stats() workspaceDiagnosticsCacheStats { + c.mu.Lock() + defer c.mu.Unlock() + projects := collections.Set[tspath.Path]{} + for _, entry := range c.entries { + projects.Add(entry.project) + } + return workspaceDiagnosticsCacheStats{Files: len(c.entries), Projects: projects.Len()} +} + +// workspaceDiagnosticsResultID hashes a file's diagnostics, so the next pull can tell whether they +// moved. Returns "" if they cannot be hashed, which forces a full report. +func workspaceDiagnosticsResultID(items []*lsproto.Diagnostic) string { + if len(items) == 0 { + return "empty" + } + encoded, err := json.Marshal(items) + if err != nil { + return "" + } + return strconv.FormatUint(xxh3.Hash(encoded), 36) +} diff --git a/tsc/internal/lsp/workspacediagnosticsscope.go b/tsc/internal/lsp/workspacediagnosticsscope.go new file mode 100644 index 0000000000000..3931be02d44c1 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnosticsscope.go @@ -0,0 +1,135 @@ +package lsp + +import ( + "runtime" + "slices" + "sync/atomic" + + "github.com/microsoft/TypeScript/tsc/internal/ast" + "github.com/microsoft/TypeScript/tsc/internal/collections" + "github.com/microsoft/TypeScript/tsc/internal/ls" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsconv" + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/project" + "github.com/microsoft/TypeScript/tsc/internal/tspath" +) + +// Each concurrent project holds a build's worth of diagnostics checkers, so this bounds peak memory. +const workspaceDiagnosticsMaxProjects = 4 + +// workspaceDiagnosticsConcurrency returns how many projects to check at once, mirroring the default +// the build orchestrator uses for --builders: four, or one under single threaded mode. Unlike a +// build, a pull runs while the user is typing, so it also leaves half the processors for the +// requests they are waiting on. +func workspaceDiagnosticsConcurrency(work []*workspaceDiagnosticsProject) int { + for _, pf := range work { + if pf.languageService.GetProgram().SingleThreaded() { + return 1 + } + } + return min(len(work), workspaceDiagnosticsMaxProjects, max(1, runtime.GOMAXPROCS(0)/2)) +} + +// projectsInScope narrows the loaded projects to the ones the scope reports on, in snapshot order. +func projectsInScope(snapshot *project.Snapshot, scope lsutil.WorkspaceDiagnosticsScope) []*project.Project { + all := snapshot.ProjectCollection.Projects() + if scope == lsutil.WorkspaceDiagnosticsScopeAllProjects { + return all + } + + wanted := collections.Set[tspath.Path]{} + for _, open := range snapshot.OpenProjects() { + wanted.Add(tspath.Path(open.Id())) + } + if scope == lsutil.WorkspaceDiagnosticsScopeOpenProjectsAndDependents { + // Walk reference edges backwards to a fixed point to find consumers of the open projects. + // The graph is tiny, so repeated passes beat building a reverse index. + for changed := true; changed; { + changed = false + for _, p := range all { + if wanted.Has(tspath.Path(p.Id())) { + continue + } + if slices.ContainsFunc(p.ReferencedProjectPaths(), wanted.Has) { + wanted.Add(tspath.Path(p.Id())) + changed = true + } + } + } + } + + inScope := make([]*project.Project, 0, wanted.Len()) + for _, p := range all { + if wanted.Has(tspath.Path(p.Id())) { + inScope = append(inScope, p) + } + } + return inScope +} + +type workspaceDiagnosticsProject struct { + languageService *ls.LanguageService + project *project.Project + generation uint64 + // Index aligned. A file answered from the cache has its report filled in and its entry nil. + files []*ast.SourceFile + reports []workspaceDiagnosticReport + toCheck int + // How much of toCheck this project has got through, for the progress report. Written by the + // checkers doing the work and read by whoever is reporting, so it is an atomic. + progress atomic.Int64 +} + +// assignFilesToProjects decides which project reports which file and answers from the cache where +// it can. Enumerating files needs the program but not a checker, so this runs before any checking. +func (r *workspaceDiagnosticsRun) assignFilesToProjects(snapshot *project.Snapshot, projects []*project.Project) []*workspaceDiagnosticsProject { + var work []*workspaceDiagnosticsProject + // A client that pulls per document reconciles the two providers' results poorly, so open + // documents are left to that pull. One that only pulls the workspace needs them included. + deDuplicate := !snapshot.UserPreferences().WorkspaceDiagnosticsServerDiagnosticsDeDuplication.IsFalse() + for _, p := range projects { + program := p.GetProgram() + if program == nil { + continue + } + // Id rather than ConfigFilePath: the inferred project has no config file and would panic. + projectPath := tspath.Path(p.Id()) + generation := p.ProgramLastUpdate + languageService := ls.NewLanguageService(p.ID(), program, snapshot, "") + + pf := &workspaceDiagnosticsProject{languageService: languageService, project: p, generation: generation} + for _, file := range languageService.WorkspaceDiagnosticFiles() { + if deDuplicate { + if handle := snapshot.GetFile(file.FileName()); handle != nil && handle.IsOverlay() { + // The client pulls open documents directly. Reporting them here too would + // duplicate every problem, since the client only reconciles the two within one + // provider. Leaving the file out of `reported` clears anything it still holds. + continue + } + } + uri := lsconv.FileNameToDocumentURI(file.FileName()) + if !r.reported.AddIfAbsent(uri) { + continue + } + if resultID, ok := r.cache.unchangedResultID(uri, projectPath, generation, r.previous[uri]); ok { + pf.files = append(pf.files, nil) + pf.reports = append(pf.reports, workspaceDiagnosticReport{ + UnchangedDocumentDiagnosticReport: &lsproto.WorkspaceUnchangedDocumentDiagnosticReport{ + Uri: uri, + Version: openDocumentVersion(snapshot, file.FileName()), + ResultId: resultID, + }, + }) + continue + } + pf.files = append(pf.files, file) + pf.reports = append(pf.reports, workspaceDiagnosticReport{}) + pf.toCheck++ + } + if len(pf.files) > 0 { + work = append(work, pf) + } + } + return work +} From 5dfcc2d557759ba5f2bed2ebb0b06cefe4b930be Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 18 Sep 2026 13:15:07 -0700 Subject: [PATCH 10/10] Offer workspace diagnostics through dynamic registration The capability cannot be advertised at initialize. The setting that turns workspace diagnostics on arrives after it, and a server that claimed the capability while the setting was off would have clients pulling a workspace nobody asked it to check. It is registered and unregistered dynamically instead, as the setting changes, and only ever by the one provider. The client runs a workspace pull per provider that asks for it, into that provider's own collection, so a second provider carrying the capability would report every problem twice; the content mapper's registration deliberately leaves it off, and the provider that does carry it covers content-mapped files too. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/lsp/server.go | 9 ++ ...r_workspacediagnosticsregistration_test.go | 85 +++++++++++++++++++ .../lsp/workspacediagnosticsregistration.go | 65 ++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 tsc/internal/lsp/server_workspacediagnosticsregistration_test.go create mode 100644 tsc/internal/lsp/workspacediagnosticsregistration.go diff --git a/tsc/internal/lsp/server.go b/tsc/internal/lsp/server.go index 61fb590444540..b4cdf55930e69 100644 --- a/tsc/internal/lsp/server.go +++ b/tsc/internal/lsp/server.go @@ -256,6 +256,9 @@ type Server struct { // produced the result id a client holds for each file. workspaceDiagnostics *workspaceDiagnosticsCache + workspaceDiagnosticsRegistrationMu sync.Mutex + workspaceDiagnosticsRegistered bool + // workspaceDiagnosticsPull is the `workspace/diagnostic` request currently running, if any. A // newer pull supersedes it; see supersedeWorkspaceDiagnostics. workspaceDiagnosticsMu sync.Mutex @@ -525,6 +528,10 @@ func (s *Server) RegisterContentMapperExtensions(ctx context.Context, extensions { Id: contentMapperDiagnosticRegistrationID, RegisterOptions: &lsproto.RegisterOptions{ + // Must not set WorkspaceDiagnostics: the client runs one workspace pull per provider + // that asks for it, into that provider's own collection, so a second one would report + // every problem twice. workspaceDiagnosticsRegistrationID is the only provider that + // carries it, and it covers content-mapped files too. TextDocumentDiagnostic: &lsproto.DiagnosticRegistrationOptions{ DocumentSelector: selector, Identifier: new("typescript"), @@ -1778,6 +1785,7 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali return err } s.session.InitializeWithUserConfig(userPreferences) + s.syncWorkspaceDiagnosticsRegistration(ctx, userPreferences) _, err = s.sendClientRequest(ctx, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{ Registrations: []*lsproto.Registration{ @@ -1828,6 +1836,7 @@ func (s *Server) handleDidChangeWorkspaceConfiguration(ctx context.Context, para } else if settings, ok := params.Settings.(map[string]any); ok { preferences := lsutil.ParseUserPreferences(settings) s.session.Configure(preferences) + s.syncWorkspaceDiagnosticsRegistration(ctx, preferences) } return nil } diff --git a/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go b/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go new file mode 100644 index 0000000000000..63df4837ba557 --- /dev/null +++ b/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go @@ -0,0 +1,85 @@ +package lsp_test + +import ( + "slices" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/bundled" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "gotest.tools/v3/assert" +) + +// The capability is withheld at initialize and only offered once the setting asks for it, then +// withdrawn when it is turned back off. +func TestWorkspaceDiagnosticsCapabilityFollowsScope(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + + // Nothing is offered while the setting sits at its default. + assert.Assert(t, !slices.Contains(progress.registrationIDs(), "workspace-diagnostics"), + "workspace diagnostics should not be registered by default, got %v", progress.registrationIDs()) + + setWorkspaceDiagnosticsScope(t, client, "allProjects") + openWorkspaceDiagnosticsProject(t, client) + pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}}) + assert.Assert(t, slices.Contains(progress.registrationIDs(), "workspace-diagnostics"), + "expected a workspace diagnostics registration, got %v", progress.registrationIDs()) + + setWorkspaceDiagnosticsScope(t, client, "off") + pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}}) + assert.Assert(t, slices.Contains(progress.unregistrationIDs(), "workspace-diagnostics"), + "expected the registration to be withdrawn, got %v", progress.unregistrationIDs()) +} + +// Moving between two enabled scopes must not churn the registration. +func TestWorkspaceDiagnosticsCapabilityRegisteredOnce(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + setWorkspaceDiagnosticsScope(t, client, "openProjects") + setWorkspaceDiagnosticsScope(t, client, "allProjects") + setWorkspaceDiagnosticsScope(t, client, "openProjectsAndDependents") + openWorkspaceDiagnosticsProject(t, client) + pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}}) + + progress.mu.Lock() + defer progress.mu.Unlock() + assert.Equal(t, progress.workspaceRegos, 1, "expected exactly one workspace diagnostics registration") + assert.Assert(t, !slices.Contains(progress.unregistered, "workspace-diagnostics")) +} + +// Turning validation off silences diagnostics entirely, so the capability is withdrawn rather than +// left in place for a client that would keep pulling the workspace every couple of seconds. +func TestWorkspaceDiagnosticsCapabilityWithdrawnWhenValidationDisabled(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles) + setWorkspaceDiagnosticsScope(t, client, "allProjects") + openWorkspaceDiagnosticsProject(t, client) + pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}}) + assert.Assert(t, slices.Contains(progress.registrationIDs(), "workspace-diagnostics")) + + // The scope still asks for every project, but validation is off. + client.SendNotification(t, lsproto.WorkspaceDidChangeConfigurationInfo, &lsproto.DidChangeConfigurationParams{ + Settings: map[string]any{"typescript": map[string]any{ + "validate": map[string]any{"enabled": false}, + "experimental": map[string]any{"workspaceDiagnostics": map[string]any{"scope": "allProjects"}}, + }}, + }) + pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}}) + assert.Assert(t, slices.Contains(progress.unregistrationIDs(), "workspace-diagnostics"), + "expected the registration to be withdrawn, got %v", progress.unregistrationIDs()) +} diff --git a/tsc/internal/lsp/workspacediagnosticsregistration.go b/tsc/internal/lsp/workspacediagnosticsregistration.go new file mode 100644 index 0000000000000..46d4c2b0744e6 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnosticsregistration.go @@ -0,0 +1,65 @@ +package lsp + +import ( + "context" + + "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" +) + +const workspaceDiagnosticsRegistrationID = "workspace-diagnostics" + +// syncWorkspaceDiagnosticsRegistration offers workspace diagnostics to the client, or withdraws the +// offer, to match the current settings. Workspace support is a property of a diagnostic provider, +// so it is offered by registering one. A client that holds the capability re-pulls on a timer, so +// withdrawing it matters as much as offering it. +func (s *Server) syncWorkspaceDiagnosticsRegistration(ctx context.Context, preferences lsutil.UserPreferences) { + if !s.clientCapabilities.TextDocument.Diagnostic.DynamicRegistration { + return + } + + s.workspaceDiagnosticsRegistrationMu.Lock() + defer s.workspaceDiagnosticsRegistrationMu.Unlock() + + // Validation off silences diagnostics whatever the scope says. + wanted := preferences.WorkspaceDiagnosticsScope.Enabled() && !preferences.EnableValidation.IsFalse() + if wanted == s.workspaceDiagnosticsRegistered { + return + } + + if !wanted { + if _, err := s.sendClientRequest(ctx, lsproto.ClientUnregisterCapabilityInfo, &lsproto.UnregistrationParams{ + Unregisterations: []*lsproto.Unregistration{ + {Id: workspaceDiagnosticsRegistrationID, Method: string(lsproto.MethodTextDocumentDiagnostic)}, + }, + }); err != nil { + s.logger.Error("failed to unregister workspace diagnostics: ", err) + return + } + s.workspaceDiagnosticsRegistered = false + return + } + + // The empty document selector is deliberate: document diagnostics are served by the provider + // advertised at initialize, and matching no document keeps this one from pulling them twice. + if _, err := s.sendClientRequest(ctx, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{ + Registrations: []*lsproto.Registration{ + { + Id: workspaceDiagnosticsRegistrationID, + RegisterOptions: &lsproto.RegisterOptions{ + TextDocumentDiagnostic: &lsproto.DiagnosticRegistrationOptions{ + DocumentSelector: lsproto.DocumentSelectorOrNull{DocumentSelector: &[]lsproto.TextDocumentFilterLanguageOrSchemeOrPattern{}}, + Identifier: new("typescript-workspace"), + InterFileDependencies: true, + WorkspaceDiagnostics: true, + Id: new(workspaceDiagnosticsRegistrationID), + }, + }, + }, + }, + }); err != nil { + s.logger.Error("failed to register workspace diagnostics: ", err) + return + } + s.workspaceDiagnosticsRegistered = true +}