From 8a05f18af5c95504001a8a5297c79ce689ae303b Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 13:11:24 -0700 Subject: [PATCH 01/13] Add the workspace diagnostics settings Nothing reads them yet. - experimental.workspaceDiagnostics.scope, which decides whether the editor reports on projects the user has not opened - experimentalWorkspaceDiagnosticsExclude, the paths a project does not want reported. Dependencies reached by module resolution are excluded by the compiler's own predicates, and this defaults to node_modules, which cannot be inferred that way because locally installed typings are program roots. tsconfig's own exclude cannot serve here: it filters root discovery, so imported files enter the program regardless. How much of the machine a pull uses is not settable: it checks projects at the concurrency a build uses for --builders, and each project with the checkers a build of it would use. Co-Authored-By: Claude Opus 5 (1M context) --- packages/vscode-typescript/package.json | 30 ++++++++++++++ packages/vscode-typescript/package.nls.json | 6 +++ tsc/internal/core/compileroptions.go | 1 + .../diagnostics/diagnostics_generated.go | 4 ++ .../diagnostics/extraDiagnosticMessages.json | 4 ++ tsc/internal/ls/lsutil/userpreferences.go | 40 +++++++++++++++++++ tsc/internal/project/session.go | 3 +- tsc/internal/tsoptions/commandlineoption.go | 4 ++ tsc/internal/tsoptions/declscompiler.go | 9 +++++ tsc/internal/tsoptions/parsinghelpers.go | 2 + .../state/codeLensAcrossProjects.baseline | 5 +++ .../reference/tsc/commandLine/help-all.js | 5 +++ 12 files changed, 112 insertions(+), 1 deletion(-) diff --git a/packages/vscode-typescript/package.json b/packages/vscode-typescript/package.json index 3be3c59bd7b72..5ebd2c679a356 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/core/compileroptions.go b/tsc/internal/core/compileroptions.go index 55401a499f3a8..03a100fc66111 100644 --- a/tsc/internal/core/compileroptions.go +++ b/tsc/internal/core/compileroptions.go @@ -45,6 +45,7 @@ type CompilerOptions struct { ForceConsistentCasingInFileNames Tristate `json:"forceConsistentCasingInFileNames,omitzero"` IsolatedModules Tristate `json:"isolatedModules,omitzero"` IsolatedDeclarations Tristate `json:"isolatedDeclarations,omitzero"` + ExperimentalWorkspaceDiagnosticsExclude []string `json:"experimentalWorkspaceDiagnosticsExclude,omitzero"` IgnoreConfig Tristate `json:"ignoreConfig,omitzero"` IgnoreDeprecations string `json:"ignoreDeprecations,omitzero"` ImportHelpers Tristate `json:"importHelpers,omitzero"` diff --git a/tsc/internal/diagnostics/diagnostics_generated.go b/tsc/internal/diagnostics/diagnostics_generated.go index e323564b5d377..40df1bbb461f1 100644 --- a/tsc/internal/diagnostics/diagnostics_generated.go +++ b/tsc/internal/diagnostics/diagnostics_generated.go @@ -4424,6 +4424,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: 100068, category: CategoryMessage, key: "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_100068", text: "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'."} +var Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on = &Message{code: 100070, category: CategoryMessage, key: "Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on_100070", text: "Paths that workspace-wide diagnostics in the editor should not report on."} + func keyToMessage(key Key) *Message { switch key { case "Unterminated_string_literal_1002": @@ -8848,6 +8850,8 @@ func keyToMessage(key Key) *Message { return The_invalid_diagnostic_directive_is_in_supplemental_output_0_returned_by_the_content_mapper case "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_100068": return Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex + case "Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on_100070": + return Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on default: return nil } diff --git a/tsc/internal/diagnostics/extraDiagnosticMessages.json b/tsc/internal/diagnostics/extraDiagnosticMessages.json index 41c7b8d8abc35..4c534ba8eae34 100644 --- a/tsc/internal/diagnostics/extraDiagnosticMessages.json +++ b/tsc/internal/diagnostics/extraDiagnosticMessages.json @@ -338,5 +338,9 @@ "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'.": { "category": "Message", "code": 100068 + }, + "Paths that workspace-wide diagnostics in the editor should not report on.": { + "category": "Message", + "code": 100070 } } 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/project/session.go b/tsc/internal/project/session.go index f682a0b5bfbce..658db60bd7c67 100644 --- a/tsc/internal/project/session.go +++ b/tsc/internal/project/session.go @@ -1918,7 +1918,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/tsoptions/commandlineoption.go b/tsc/internal/tsoptions/commandlineoption.go index 1346e5c0bbd5e..5e57446380389 100644 --- a/tsc/internal/tsoptions/commandlineoption.go +++ b/tsc/internal/tsoptions/commandlineoption.go @@ -108,6 +108,10 @@ var commandLineOptionElements = map[string]*CommandLineOption{ Kind: CommandLineOptionTypeEnum, // libMap, DefaultValueDescription: core.TSUnknown, }, + "experimentalWorkspaceDiagnosticsExclude": { + Name: "experimentalWorkspaceDiagnosticsExclude", + Kind: CommandLineOptionTypeString, + }, "rootDirs": { Name: "rootDirs", Kind: CommandLineOptionTypeString, diff --git a/tsc/internal/tsoptions/declscompiler.go b/tsc/internal/tsoptions/declscompiler.go index 46fb42faee7c5..6f4a97453f6ab 100644 --- a/tsc/internal/tsoptions/declscompiler.go +++ b/tsc/internal/tsoptions/declscompiler.go @@ -1082,6 +1082,15 @@ var optionsForCompiler = []*CommandLineOption{ Description: diagnostics.Opt_a_project_out_of_multi_project_reference_checking_when_editing, DefaultValueDescription: false, }, + { + Name: "experimentalWorkspaceDiagnosticsExclude", + Kind: CommandLineOptionTypeList, + IsTSConfigOnly: true, + allowConfigDirTemplateSubstitution: true, + Category: diagnostics.Projects, + Description: diagnostics.Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on, + DefaultValueDescription: "**/node_modules/**", + }, { Name: "disableReferencedProjectLoad", Kind: CommandLineOptionTypeBoolean, diff --git a/tsc/internal/tsoptions/parsinghelpers.go b/tsc/internal/tsoptions/parsinghelpers.go index fa7491b1e0c06..4b0b6e808f1d2 100644 --- a/tsc/internal/tsoptions/parsinghelpers.go +++ b/tsc/internal/tsoptions/parsinghelpers.go @@ -324,6 +324,8 @@ func parseCompilerOptions(key string, value any, allOptions *core.CompilerOption allOptions.DisableSourceOfProjectReferenceRedirect = ParseTristate(value) case "disableSolutionSearching": allOptions.DisableSolutionSearching = ParseTristate(value) + case "experimentalWorkspaceDiagnosticsExclude": + allOptions.ExperimentalWorkspaceDiagnosticsExclude = ParseStringArray(value) case "disableReferencedProjectLoad": allOptions.DisableReferencedProjectLoad = ParseTristate(value) case "declarationMap": 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, diff --git a/tsc/testdata/baselines/reference/tsc/commandLine/help-all.js b/tsc/testdata/baselines/reference/tsc/commandLine/help-all.js index db9dd22705973..39343a4a0abd1 100644 --- a/tsc/testdata/baselines/reference/tsc/commandLine/help-all.js +++ b/tsc/testdata/baselines/reference/tsc/commandLine/help-all.js @@ -348,6 +348,11 @@ Disable preferring source files instead of declaration files when referencing co type: boolean default: false +--experimentalWorkspaceDiagnosticsExclude +Paths that workspace-wide diagnostics in the editor should not report on. +one or more: string +default: **/node_modules/** + --incremental, -i Save .tsbuildinfo files to allow for incremental compilation of projects. type: boolean From 1173d931586b4c567174afe835e9534ec03c1d0c Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 13:12:22 -0700 Subject: [PATCH 02/13] Add workspace diagnostics scaffolding Protocol and language service plumbing, with no handler wired up yet: - WorkspaceDiagnosticPartialResultParams, since the generated ProgressParams narrows $/progress values to work done progress - ProvideDiagnosticsForFile, extracted from ProvideDiagnostics so a caller holding a source file need not re-resolve it by URI - WorkspaceDiagnosticFiles, the set of files a project owns for reporting, which is where experimentalWorkspaceDiagnosticsExclude is applied - A 'Checking workspace' progress message Co-Authored-By: Claude Opus 5 (1M context) --- .../diagnostics/diagnostics_generated.go | 4 ++ .../diagnostics/extraDiagnosticMessages.json | 4 ++ tsc/internal/ls/diagnostics.go | 69 +++++++++++++++---- tsc/internal/lsp/lsproto/lsp.go | 9 +++ 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/tsc/internal/diagnostics/diagnostics_generated.go b/tsc/internal/diagnostics/diagnostics_generated.go index 40df1bbb461f1..4e67dd03a2556 100644 --- a/tsc/internal/diagnostics/diagnostics_generated.go +++ b/tsc/internal/diagnostics/diagnostics_generated.go @@ -4424,6 +4424,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: 100068, category: CategoryMessage, key: "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_100068", text: "Diagnostic directive {0} returned by the content mapper has an invalid 'unusedExpectDirectiveIndex'."} +var Checking_workspace = &Message{code: 100069, category: CategoryMessage, key: "Checking_workspace_100069", text: "Checking workspace"} + var Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on = &Message{code: 100070, category: CategoryMessage, key: "Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on_100070", text: "Paths that workspace-wide diagnostics in the editor should not report on."} func keyToMessage(key Key) *Message { @@ -8850,6 +8852,8 @@ func keyToMessage(key Key) *Message { return The_invalid_diagnostic_directive_is_in_supplemental_output_0_returned_by_the_content_mapper case "Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex_100068": return Diagnostic_directive_0_returned_by_the_content_mapper_has_an_invalid_unusedExpectDirectiveIndex + case "Checking_workspace_100069": + return Checking_workspace case "Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on_100070": return Paths_that_workspace_wide_diagnostics_in_the_editor_should_not_report_on default: diff --git a/tsc/internal/diagnostics/extraDiagnosticMessages.json b/tsc/internal/diagnostics/extraDiagnosticMessages.json index 4c534ba8eae34..e593d82f16faf 100644 --- a/tsc/internal/diagnostics/extraDiagnosticMessages.json +++ b/tsc/internal/diagnostics/extraDiagnosticMessages.json @@ -339,6 +339,10 @@ "category": "Message", "code": 100068 }, + "Checking workspace": { + "category": "Message", + "code": 100069 + }, "Paths that workspace-wide diagnostics in the editor should not report on.": { "category": "Message", "code": 100070 diff --git a/tsc/internal/ls/diagnostics.go b/tsc/internal/ls/diagnostics.go index 3638efdac813a..c322b75feea6f 100644 --- a/tsc/internal/ls/diagnostics.go +++ b/tsc/internal/ls/diagnostics.go @@ -11,6 +11,7 @@ import ( "github.com/microsoft/TypeScript/tsc/internal/ls/lsconv" "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" "github.com/microsoft/TypeScript/tsc/internal/spanmap" + "github.com/microsoft/TypeScript/tsc/internal/vfs/vfsmatch" ) // getAllDiagnostics collects all diagnostics for a file: syntactic, semantic, @@ -30,24 +31,66 @@ 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) + _, file := l.getProgramAndFile(uri) + return lsproto.RelatedFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport{ + FullDocumentDiagnosticReport: &lsproto.RelatedFullDocumentDiagnosticReport{ + 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() { - diagnostics := []*lsproto.Diagnostic{} - return lsproto.RelatedFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport{ - FullDocumentDiagnosticReport: &lsproto.RelatedFullDocumentDiagnosticReport{ - Items: diagnostics, - }, - }, nil + return []*lsproto.Diagnostic{} } + return l.toLSPDiagnostics(ctx, getAllDiagnostics(ctx, l.program, file)) +} - diagnostics := getAllDiagnostics(ctx, program, file) +// defaultWorkspaceDiagnosticsExclude applies when a project does not set +// experimentalWorkspaceDiagnosticsExclude. Dependencies reached by resolution are filtered out separately; this +// catches locally installed typings, which are program roots and so not external library imports. +var defaultWorkspaceDiagnosticsExclude = []string{"**/node_modules/**"} - return lsproto.RelatedFullDocumentDiagnosticReportOrUnchangedDocumentDiagnosticReport{ - FullDocumentDiagnosticReport: &lsproto.RelatedFullDocumentDiagnosticReport{ - Items: l.toLSPDiagnostics(ctx, diagnostics), - }, - }, nil +// workspaceDiagnosticsExcludeMatcher compiles the project's exclusion patterns relative to its +// tsconfig, falling back to the default when the option is unset. +func (l *LanguageService) workspaceDiagnosticsExcludeMatcher() *vfsmatch.SpecMatcher { + specs := l.program.Options().ExperimentalWorkspaceDiagnosticsExclude + if specs == nil { + specs = defaultWorkspaceDiagnosticsExclude + } + return vfsmatch.NewSpecMatcher(specs, l.program.CommandLine().GetCurrentDirectory(), vfsmatch.UsageExclude, l.UseCaseSensitiveFileNames()) +} + +// WorkspaceDiagnosticFiles returns the files a workspace pull should report, in program order. +func (l *LanguageService) WorkspaceDiagnosticFiles() []*ast.SourceFile { + program := l.program + excluded := l.workspaceDiagnosticsExcludeMatcher() + 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 + } + if excluded != nil && excluded.MatchString(file.FileName()) { + 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 { diff --git a/tsc/internal/lsp/lsproto/lsp.go b/tsc/internal/lsp/lsproto/lsp.go index 4941077b9ca0c..62e8976399e76 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 From 9e9d6281285c2fcddb73e47ff88dee37c5c60d2d Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Mon, 31 Aug 2026 14:14:56 -0700 Subject: [PATCH 03/13] Expose the project state workspace diagnostics needs - OpenProjects and ReferencedProjectPaths, to resolve the scopes that report on the open projects and the projects referencing them - isOpen, to tell a project the user is working in from one only a sweep has reason to look at Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/project/project.go | 13 +++++++++++++ tsc/internal/project/projectcollection.go | 15 +++++++++++++++ tsc/internal/project/snapshot.go | 11 +++++++++++ 3 files changed, 39 insertions(+) diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index 80cacf0b7b834..a317e5eefd46e 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -369,6 +369,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 1bd2f8148a466..969c663d97ede 100644 --- a/tsc/internal/project/projectcollection.go +++ b/tsc/internal/project/projectcollection.go @@ -169,6 +169,21 @@ func (c *ProjectCollection) GetOpenConfiguredProjects() *collections.Set[tspath. 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 + } + return c.GetOpenConfiguredProjects().Has(project.configFilePath) +} + 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 { diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 97b2961d3f5c8..56f80beea4524 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -303,6 +303,17 @@ func (s *Snapshot) GetProjectsContainingFile(uri lsproto.DocumentUri) []ls.Proje return s.ProjectCollection.GetProjectsContainingFile(path) } +// OpenProjects returns the projects that contain at least one file open in the editor. +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) } From 8f90d5c0f190d8826bb8cfb1d8be1ef12c75c6ff Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 13:13:27 -0700 Subject: [PATCH 04/13] Implement the workspace/diagnostic handler Three files, since the request has three separable concerns: - workspacediagnosticscache.go decides what need not be checked. It remembers which program version produced the result id a client holds, so an unchanged generation answers a whole project without checking it. Settings are compared as whole preference structs rather than a list of the ones known to matter: the auto-import registry can list its own because a stale entry there costs a missing completion, whereas here it would leave errors in the problem list that no longer exist. - workspacediagnosticsscope.go decides what to check: which projects the scope admits, which project reports each file, and how many run at once. - workspacediagnostics.go runs it, streaming reports through a partial result token and reporting progress. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/lsp/server.go | 8 +- .../lsp/server_workspacediagnostics_test.go | 885 ++++++++++++++++++ tsc/internal/lsp/workspacediagnostics.go | 375 ++++++++ .../lsp/workspacediagnostics_internal_test.go | 68 ++ tsc/internal/lsp/workspacediagnosticscache.go | 122 +++ tsc/internal/lsp/workspacediagnosticsscope.go | 126 +++ 6 files changed, 1583 insertions(+), 1 deletion(-) 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 c850f4a2ad474..d229f7cf41476 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,10 @@ 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 } func (s *Server) Session() *project.Session { return s.session } @@ -1146,11 +1151,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 +1283,7 @@ var handlers = sync.OnceValue(func() handlerMap { registerRequestHandler(handlers, lsproto.CallHierarchyIncomingCallsInfo, (*Server).handleCallHierarchyIncomingCalls) registerRequestHandler(handlers, lsproto.CallHierarchyOutgoingCallsInfo, (*Server).handleCallHierarchyOutgoingCalls) + registerWorkspaceDiagnosticHandler(handlers) registerRequestHandler(handlers, lsproto.WorkspaceSymbolInfo, (*Server).handleWorkspaceSymbol) registerRequestHandler(handlers, lsproto.CompletionItemResolveInfo, (*Server).handleCompletionItemResolve) registerRequestHandler(handlers, lsproto.CodeLensResolveInfo, (*Server).handleCodeLensResolve) diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go new file mode 100644 index 0000000000000..00789eae9f004 --- /dev/null +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -0,0 +1,885 @@ +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 +} + +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") + 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) 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, + 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 := lsptestutil.SendRequest(t, client, 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") + lsptestutil.SendNotification(t, client, lsproto.InitializedInfo, &lsproto.InitializedParams{}) + <-client.Server.InitComplete() + + return client, progress +} + +func setWorkspaceDiagnosticsScope(t *testing.T, client *lsptestutil.LSPClient, scope string) { + t.Helper() + lsptestutil.SendNotification(t, client, 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() + lsptestutil.SendNotification(t, client, 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 := lsptestutil.SendRequest(t, client, 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 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) + + second := pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{ + PreviousResultIds: ids, + }) + assert.DeepEqual(t, reportURIs(second.Items), reportURIs(first.Items)) + for _, report := range second.Items { + assert.Assert(t, report.UnchangedDocumentDiagnosticReport != nil, "expected an unchanged report, got %v", report) + } + + // Editing the open document changes what a closed file reports, so that file comes back in full. + lsptestutil.SendNotification(t, client, 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, + }) + fixed := findFullReport(t, third.Items, "file:///home/projects/index.ts") + assert.Equal(t, len(fixed.Items), 0) +} + +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) + lsptestutil.SendNotification(t, client, 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) + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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) + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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) + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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) +} + +// The node_modules exclusion is a default, not a rule: a project can say what workspace diagnostics +// should skip. +func TestWorkspaceDiagnosticsHonorsExcludeOption(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + files := map[string]string{ + "/home/projects/tsconfig.json": `{"compilerOptions": {"experimentalWorkspaceDiagnosticsExclude": ["**/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) + lsptestutil.SendNotification(t, client, 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{}, + }) + + // vendor/ is excluded by the option; src/ is reported even though the default would not have + // excluded vendor/ and this project's setting replaces that default. + 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) +} + +// 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") + lsptestutil.SendNotification(t, client, 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) + lsptestutil.SendNotification(t, client, 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", + }) +} diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go new file mode 100644 index 0000000000000..1a49381a925f5 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -0,0 +1,375 @@ +package lsp + +import ( + "context" + "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" + "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 registerWorkspaceDiagnosticHandler(handlers handlerMap) { + 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 := lsproto.UnmarshalParams[*lsproto.WorkspaceDiagnosticParams](req) + if err != nil { + return nil, err + } + // A pull can run for minutes, so it stays off the dispatch loop. + return func() error { + defer s.recover(req) + resp, lsErr := s.computeWorkspaceDiagnostics(ctx, params) + if lsErr != nil { + return lsErr + } + if ctx.Err() != nil { + return ctx.Err() + } + return s.sendResult(req.ID, resp) + }, nil + } +} + +func (s *Server) computeWorkspaceDiagnostics(ctx context.Context, params *lsproto.WorkspaceDiagnosticParams) (lsproto.WorkspaceDiagnosticResponse, error) { + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) + run := newWorkspaceDiagnosticsRun(s, ctx, 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(open.Id()) + } + } + } + + s.session.WithSnapshotLoadingProjectTree(ctx, trees, func(snapshot *project.Snapshot) { + preferences := snapshot.UserPreferences() + // A program generation cannot see a settings change, so the cache is keyed on them too. + s.workspaceDiagnostics.useSettings(workspaceDiagnosticsSettings{ + preferences: preferences, + locale: s.GetLocale().String(), + }) + if !scope.Enabled() || preferences.EnableValidation.IsFalse() { + // Nothing is reported, and the cleanup pass below clears whatever the client holds. + return + } + run.collect(snapshot, scope) + }) + + // 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) + 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 + // 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 + // Whether a sweep actually ran, so a disabled pull does not prune the cache. + collected bool + + cache *workspaceDiagnosticsCache +} + +func newWorkspaceDiagnosticsRun(server *Server, ctx context.Context, 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. Files within a project are checked +// one at a time because they share its single diagnostics checker, which exists to keep the walk +// order consistent; checker pools are per project, so whole projects run concurrently. +func (r *workspaceDiagnosticsRun) collect(snapshot *project.Snapshot, scope lsutil.WorkspaceDiagnosticsScope) { + r.collected = true + work := r.assignFilesToProjects(snapshot, projectsInScope(snapshot, scope)) + // 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) + 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 + } + 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. +func (r *workspaceDiagnosticsRun) checkProject(snapshot *project.Snapshot, pf workspaceDiagnosticsProject) bool { + for j, file := range pf.files { + if file == nil { + continue + } + if r.ctx.Err() != nil { + return false + } + pf.reports[j] = r.reportForFile(snapshot, pf.languageService, 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) { + 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: pf.project.Id(), + generation: pf.generation, + resultID: *full.ResultId, + }) + } + } + r.add(report) + } +} + +func (r *workspaceDiagnosticsRun) reportForFile(snapshot *project.Snapshot, languageService *ls.LanguageService, file *ast.SourceFile) workspaceDiagnosticReport { + uri := lsconv.FileNameToDocumentURI(file.FileName()) + items := languageService.ProvideDiagnosticsForFile(r.ctx, file) + 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 + } + _ = sendNotification(r.server, 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} +} + +func (r *workspaceDiagnosticsRun) beginProgress() { + if r.workDoneToken == nil || r.filesTotal == 0 { + return + } + r.begun = true + r.sendProgress(lsproto.WorkDoneProgressBeginOrReportOrEnd{ + Begin: &lsproto.WorkDoneProgressBegin{ + Title: diagnostics.Checking_workspace.Localize(r.server.GetLocale()), + Percentage: new(uint32(0)), + }, + }) +} + +func (r *workspaceDiagnosticsRun) reportProgress() { + if !r.begun { + return + } + r.sendProgress(lsproto.WorkDoneProgressBeginOrReportOrEnd{ + Report: &lsproto.WorkDoneProgressReport{ + Percentage: new(uint32(r.filesDone * 100 / r.filesTotal)), + }, + }) +} + +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) { + _ = sendNotification(r.server, lsproto.ProgressInfo, &lsproto.ProgressParams{ + Token: *r.workDoneToken, + 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..5c354c482f319 --- /dev/null +++ b/tsc/internal/lsp/workspacediagnostics_internal_test.go @@ -0,0 +1,68 @@ +package lsp + +import ( + "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") +} diff --git a/tsc/internal/lsp/workspacediagnosticscache.go b/tsc/internal/lsp/workspacediagnosticscache.go new file mode 100644 index 0000000000000..21d4ef4fc809b --- /dev/null +++ b/tsc/internal/lsp/workspacediagnosticscache.go @@ -0,0 +1,122 @@ +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/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 +} + +// workspaceDiagnosticsSettings is the settings an entry was computed under. Unlike the equivalent +// in the auto-import registry, which lists the preferences it depends on, this compares all of +// them: a preference that changes what a diagnostic says but is missing from such a list would +// leave stale 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. Comparing the whole preference +// set rather than the fields known to matter means a new preference cannot silently leave stale +// entries in place, and comparing the snapshot's copy rather than reacting to a configuration +// notification means 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{} + } +} + +// 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. +func (c *workspaceDiagnosticsCache) retain(reported *collections.Set[lsproto.DocumentUri]) { + c.mu.Lock() + defer c.mu.Unlock() + for uri := range c.entries { + if !reported.Has(uri) { + delete(c.entries, uri) + } + } +} + +// workspaceDiagnosticsCacheStats describes what the cache holds. The auto-import registry reports +// its buckets the same way: a cache that decides whether a file is re-checked is worth being able +// to see when a pull takes longer than expected. +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..23c14139fd8ac --- /dev/null +++ b/tsc/internal/lsp/workspacediagnosticsscope.go @@ -0,0 +1,126 @@ +package lsp + +import ( + "runtime" + "slices" + + "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 its own diagnostics checker, 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(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(p.Id()) { + continue + } + if slices.ContainsFunc(p.ReferencedProjectPaths(), wanted.Has) { + wanted.Add(p.Id()) + changed = true + } + } + } + } + + inScope := make([]*project.Project, 0, wanted.Len()) + for _, p := range all { + if wanted.Has(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 +} + +// 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 + 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 := p.Id() + generation := p.ProgramLastUpdate + languageService := ls.NewLanguageService(projectPath, program, snapshot, "") + + pf := workspaceDiagnosticsProject{languageService: languageService, project: p, generation: generation} + for _, file := range languageService.WorkspaceDiagnosticFiles() { + 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 e57c3c7c233db1db22f5e75066301874299d7073 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Mon, 31 Aug 2026 16:49:57 -0700 Subject: [PATCH 05/13] Offer workspace diagnostics through dynamic registration The capability is absent from the initialize response and registered only once the scope setting asks for it, so a client does not start pulling the workspace until someone opts in. Workspace support is a property of a diagnostic provider rather than a method of its own, so it is offered by registering one; the document selector is empty because document diagnostics are already served by the provider advertised at initialize. Withdrawing matters as much as offering: a client holding the capability re-pulls on a timer, so the registration is dropped when the scope goes back to off or validation is disabled. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/lsp/server.go | 12 ++- .../lsp/server_workspacediagnostics_test.go | 57 ++++++++++-- ...r_workspacediagnosticsregistration_test.go | 86 +++++++++++++++++++ .../lsp/workspacediagnosticsregistration.go | 65 ++++++++++++++ 4 files changed, 214 insertions(+), 6 deletions(-) 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 d229f7cf41476..781df8e88fb1d 100644 --- a/tsc/internal/lsp/server.go +++ b/tsc/internal/lsp/server.go @@ -255,6 +255,9 @@ type Server struct { // workspaceDiagnostics remembers, across `workspace/diagnostic` pulls, which program version // produced the result id a client holds for each file. workspaceDiagnostics *workspaceDiagnosticsCache + + workspaceDiagnosticsRegistrationMu sync.Mutex + workspaceDiagnosticsRegistered bool } func (s *Server) Session() *project.Session { return s.session } @@ -520,6 +523,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"), @@ -1761,6 +1768,7 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali return err } s.session.InitializeWithUserConfig(userPreferences) + s.syncWorkspaceDiagnosticsRegistration(ctx, userPreferences) _, err = sendClientRequest(ctx, s, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{ Registrations: []*lsproto.Registration{ @@ -1809,7 +1817,9 @@ 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) + s.syncWorkspaceDiagnosticsRegistration(ctx, preferences) } return nil } diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go index 00789eae9f004..bfcfc40c3c77e 100644 --- a/tsc/internal/lsp/server_workspacediagnostics_test.go +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -23,9 +23,54 @@ type workspaceDiagnosticReport = lsproto.WorkspaceFullDocumentDiagnosticReportOr // 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 + mu sync.Mutex + partial []workspaceDiagnosticReport + kinds []string + 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) { @@ -96,8 +141,10 @@ func startWorkspaceDiagnosticsClient(t *testing.T, files map[string]string) (*ls onServerRequest := func(_ context.Context, req *lsproto.RequestMessage) *lsproto.ResponseMessage { switch req.Method { - case lsproto.MethodClientRegisterCapability, lsproto.MethodClientUnregisterCapability, - lsproto.MethodWindowWorkDoneProgressCreate: + 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 diff --git a/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go b/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go new file mode 100644 index 0000000000000..5fea85a8306d5 --- /dev/null +++ b/tsc/internal/lsp/server_workspacediagnosticsregistration_test.go @@ -0,0 +1,86 @@ +package lsp_test + +import ( + "slices" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/bundled" + "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" + "github.com/microsoft/TypeScript/tsc/internal/testutil/lsptestutil" + "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. + lsptestutil.SendNotification(t, client, 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..7243ff4d1d440 --- /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 := sendClientRequest(ctx, s, 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 := sendClientRequest(ctx, s, 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 +} From 6345374ba15457b458c14d49c79d0a057046307f Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Thu, 3 Sep 2026 15:05:35 -0700 Subject: [PATCH 06/13] Check a project's diagnostics the way a build does A pull checked a whole project through the one checker individual requests share, however many checkers a build of the same project would use. Which checker sees a file is part of how a program is checked, so the two could disagree, and one checker had to do all the work. CheckerPool grows the two things checking a whole program needs, which were buried in the compiler's own implementation of it: iterating checkers as groups, so each is taken once for the files assigned to it, and reporting the global diagnostics they found. The project's pool answers both by building the compiler's pool on first use, so a whole-program check gets that count and that partition, and the compiler's grouped iteration drives it rather than anything here. Requests for a single file are untouched: they still share one checker, keep their request affinity, and never contend with a sweep. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/compiler/checkerpool.go | 18 ++++++++++-- tsc/internal/compiler/program.go | 23 ++++------------ tsc/internal/project/checkerpool.go | 31 +++++++++++++++++++++ tsc/internal/project/checkerpool_test.go | 35 +++++++++++++++++++++--- 4 files changed, 83 insertions(+), 24 deletions(-) diff --git a/tsc/internal/compiler/checkerpool.go b/tsc/internal/compiler/checkerpool.go index c7961118a4ce5..cc7c7067752d5 100644 --- a/tsc/internal/compiler/checkerpool.go +++ b/tsc/internal/compiler/checkerpool.go @@ -19,8 +19,22 @@ import ( // The returned checker must not be accessed concurrently; each acquisition is exclusive. // If file is non-nil, the pool may use it as an affinity hint to return the same // checker for the same file across calls. +// CheckerPool owns the checkers a program is checked with: one per the program's `checkers` option, +// with the program's files partitioned across them. Which checker sees a file is part of how a +// program is checked, so anything wanting to check one the way the command line does has to check +// it through this. type CheckerPool interface { GetChecker(ctx context.Context, file *ast.SourceFile) (*checker.Checker, func()) + // ForEachCheckerGroupDo runs one task per checker rather than one per file, so each checker is + // taken once for the whole group of files assigned to it. + ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) + GetGlobalDiagnostics() []*ast.Diagnostic +} + +// NewCheckerPool returns the pool the compiler would check this program with. A Program builds its +// own, so this is for callers that supply a pool of their own and need the compiler's for checking. +func NewCheckerPool(program *Program) CheckerPool { + return newCheckerPool(program) } type checkerPool struct { @@ -466,10 +480,10 @@ func (p *checkerPool) GetGlobalDiagnostics() []*ast.Diagnostic { return SortAndDeduplicateDiagnostics(slices.Concat(globalDiagnostics...)) } -// forEachCheckerGroupDo runs one task per checker in parallel. Each task iterates +// ForEachCheckerGroupDo runs one task per checker in parallel. Each task iterates // the provided files, processing only those assigned to its checker. Within each // checker's set, files are visited in their original order. -func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { +func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { p.createCheckers() checkerCount := len(p.checkers) diff --git a/tsc/internal/compiler/program.go b/tsc/internal/compiler/program.go index 34f713918a6ad..ea0c82f23c675 100644 --- a/tsc/internal/compiler/program.go +++ b/tsc/internal/compiler/program.go @@ -698,24 +698,11 @@ func filterAndSortDiagnostics(diags []*ast.Diagnostic) []*ast.Diagnostic { // 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)) - 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) - }) - } else { - wg := core.NewWorkGroup(p.SingleThreaded()) - for i, file := range sourceFiles { - if p.SkipTypeChecking(file, false) { - continue - } - wg.Queue(func() { - c, done := p.checkerPool.GetChecker(ctx, file) - diagnostics[i] = collect(ctx, c, file) - done() - }) - } - wg.RunAndWait() - } + // A file is checked by the checker its pool assigned it, and each checker is taken once for its + // whole group rather than once per file. + p.checkerPool.ForEachCheckerGroupDo(ctx, sourceFiles, p.SingleThreaded(), func(c *checker.Checker, fileIndex int, file *ast.SourceFile) { + diagnostics[fileIndex] = collect(ctx, c, file) + }) return diagnostics } diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index 726199fac4fe2..b7e1b730246fb 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -49,6 +49,12 @@ type checkerPool struct { // query checkers are not disposed until the pool is GC'd. discarded bool + // checkingPool is the pool the compiler would check this program with. A whole-program check + // goes through it so each file is checked by the same checker, out of the same number of + // checkers, that a build would use. Individual requests keep using the checkers below, which + // this pool knows nothing about. Built on first use and dropped once a sweep is done with it. + checkingPool compiler.CheckerPool + // checkers[0] is the diagnostics checker. // checkers[1:] are ephemeral query checkers. // All are idle-cleaned. @@ -246,6 +252,19 @@ func (p *checkerPool) getDiagnosticsChecker(ctx context.Context, requestID strin return c, p.createRelease(requestID, diagIndex, c) } +// ForEachCheckerGroupDo implements compiler.CheckerPool, so a whole-program check runs on the +// compiler's own checkers rather than the single diagnostics checker individual requests share. +func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { + p.mu.Lock() + if p.checkingPool == nil { + p.log("checkerpool: Creating checking pool") + p.checkingPool = compiler.NewCheckerPool(p.program) + } + pool := p.checkingPool + p.mu.Unlock() + pool.ForEachCheckerGroupDo(ctx, files, singleThreaded, cb) +} + // getQueryChecker returns an ephemeral query checker from indices 1+. // Uses request affinity, then file affinity, then finds/creates. // Blocks on querySem if all query slots are in use. @@ -484,6 +503,15 @@ func (p *checkerPool) mergeGlobalDiagnosticsFromCheckerLocked(index int, c *chec return } p.globalDiagCheckerCount[index] = len(globals) + p.mergeGlobalDiagnosticsLocked(globals) +} + +// mergeGlobalDiagnosticsLocked merges global diagnostics into the accumulated set. +// Must be called with p.mu held. +func (p *checkerPool) mergeGlobalDiagnosticsLocked(globals []*ast.Diagnostic) { + if len(globals) == 0 { + return + } before := len(p.globalDiagAccumulated) p.globalDiagAccumulated = compiler.SortAndDeduplicateDiagnostics(append(p.globalDiagAccumulated, globals...)) if len(p.globalDiagAccumulated) != before { @@ -521,6 +549,9 @@ func (p *checkerPool) Discard() { } p.log("checkerpool: Discarding pool, stopping idle cleanup") p.discarded = true + // A discarded pool belongs to a project nothing is using, so let go of the checkers a sweep + // left behind rather than holding their types for the life of the snapshot. + p.checkingPool = nil 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 76fdcf12a7e10..eea32bc6345f9 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -18,15 +18,19 @@ 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() 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)) session := NewSession(&SessionInit{ BackgroundCtx: context.Background(), @@ -1293,3 +1297,26 @@ func TestCheckerPoolCleanupAfterDiscardIsNoop(t *testing.T) { pool.mu.Unlock() }) } + +// A whole-program check must run on the pool the compiler would have built, so a pull in the editor +// checks each file with the same checker, out of the same number of checkers, that a build uses. +// The checkers individual requests share are a different, smaller pool. +func TestCheckerPoolChecksWholeProgramWithCompilerPool(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.Assert(t, pool.checkingPool == 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") + assert.Assert(t, pool.checkingPool != nil, "a whole-program check must go through the compiler's pool") + + // 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.Discard() + assert.Assert(t, pool.checkingPool == nil, "a discarded pool must let go of its checkers") +} From 014e366be24a26e31215c21b59f83176f89a47db Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 15:10:18 -0700 Subject: [PATCH 07/13] Sweep a project in one call instead of a file at a time The sweep asked for each file's diagnostics separately, which meant it, rather than the program, decided how checking was spread over checkers. WorkspaceDiagnosticsForProject checks the program once and keys the result by file, so the program splits the work as a build would and the sweep only formats what comes back. A project now reports when it is done rather than streaming as each of its files finishes; projects still stream as they finish. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/ls/diagnostics.go | 44 ++++++++++++++++++++++++ tsc/internal/lsp/workspacediagnostics.go | 23 +++++++++---- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/tsc/internal/ls/diagnostics.go b/tsc/internal/ls/diagnostics.go index c322b75feea6f..94278236c61d3 100644 --- a/tsc/internal/ls/diagnostics.go +++ b/tsc/internal/ls/diagnostics.go @@ -161,3 +161,47 @@ func worstCategory(diags []*ast.Diagnostic) diagnostics.Category { } return worst } + +// WorkspaceDiagnosticsForProject checks the whole program at once and returns what each file it +// owns should report. Checking every file in one call lets the program split the work across the +// checkers a build would use, and keeps the checker pool's own coordination rather than repeating +// it per file; the diagnostics come back keyed by the file they belong to. +func (l *LanguageService) WorkspaceDiagnosticsForProject(ctx context.Context, 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)) + for _, diagnostics := range [][]*ast.Diagnostic{ + l.program.GetSyntacticDiagnostics(ctx, nil), + l.program.GetSemanticDiagnostics(ctx, nil), + l.program.GetSuggestionDiagnostics(ctx, nil), + } { + for _, diagnostic := range diagnostics { + if file := diagnostic.File(); file != nil { + byFile[file] = append(byFile[file], diagnostic) + } + } + } + if l.program.Options().GetEmitDeclarations() { + for _, diagnostic := range l.program.GetDeclarationDiagnostics(ctx, nil) { + if file := diagnostic.File(); file != nil { + byFile[file] = append(byFile[file], diagnostic) + } + } + } + + 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 +} diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go index 1a49381a925f5..1144bacc3a6ae 100644 --- a/tsc/internal/lsp/workspacediagnostics.go +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -8,7 +8,6 @@ import ( "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" "github.com/microsoft/TypeScript/tsc/internal/ls/lsconv" "github.com/microsoft/TypeScript/tsc/internal/ls/lsutil" "github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto" @@ -234,15 +233,26 @@ func (r *workspaceDiagnosticsRun) checkConcurrently(snapshot *project.Snapshot, // 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. +// checkProject checks a project's files and builds their reports. The program checks them all in +// one call, so the work is split across the checkers a build would use 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) + } + } + reports := pf.languageService.WorkspaceDiagnosticsForProject(r.ctx, files) + if r.ctx.Err() != nil { + return false + } for j, file := range pf.files { if file == nil { continue } - if r.ctx.Err() != nil { - return false - } - pf.reports[j] = r.reportForFile(snapshot, pf.languageService, file) + pf.reports[j] = r.reportForFile(snapshot, file, reports[file]) } return true } @@ -265,9 +275,8 @@ func (r *workspaceDiagnosticsRun) emitProject(pf workspaceDiagnosticsProject) { } } -func (r *workspaceDiagnosticsRun) reportForFile(snapshot *project.Snapshot, languageService *ls.LanguageService, file *ast.SourceFile) workspaceDiagnosticReport { +func (r *workspaceDiagnosticsRun) reportForFile(snapshot *project.Snapshot, file *ast.SourceFile, items []*lsproto.Diagnostic) workspaceDiagnosticReport { uri := lsconv.FileNameToDocumentURI(file.FileName()) - items := languageService.ProvideDiagnosticsForFile(r.ctx, file) resultID := workspaceDiagnosticsResultID(items) version := openDocumentVersion(snapshot, file.FileName()) From e1ac311b8c7e2861b837050a91bfc2fe7da21de8 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 15:40:52 -0700 Subject: [PATCH 08/13] Hand back a swept project's checkers when the sweep is done The checkers a whole-program check uses hold the types of every file in the program, which is the largest thing a pull creates, and they were kept until the program was replaced. On a workspace of any size that is most of the memory a pull costs, held indefinitely. Nothing was buying it: a later pull that finds the project unchanged answers from the result ids the client already holds without checking anything, and one that finds it changed needs new checkers regardless. Measured on a 300 file project, releasing takes retained heap from 60 MiB to 26 MiB with the sweep and the repeat pull unchanged at 130ms and 1.1ms. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/lsp/workspacediagnostics.go | 4 ++++ tsc/internal/project/checkerpool.go | 15 +++++++++++++++ tsc/internal/project/checkerpool_test.go | 21 +++++++++++++++++++++ tsc/internal/project/snapshot.go | 11 +++++++++++ 4 files changed, 51 insertions(+) diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go index 1144bacc3a6ae..2686c6b4724ef 100644 --- a/tsc/internal/lsp/workspacediagnostics.go +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -184,6 +184,7 @@ func (r *workspaceDiagnosticsRun) checkSequentially(snapshot *project.Snapshot, continue } completed := r.checkProject(snapshot, pf) + snapshot.ReleaseCheckingPool(pf.project) if !completed { return } @@ -217,6 +218,9 @@ func (r *workspaceDiagnosticsRun) checkConcurrently(snapshot *project.Snapshot, 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.ReleaseCheckingPool(pf.project) completed[i] = r.checkProject(snapshot, pf) }) } diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index b7e1b730246fb..f69dc6e2f1cd1 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -265,6 +265,21 @@ func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.So pool.ForEachCheckerGroupDo(ctx, files, singleThreaded, cb) } +// releaseCheckingPool drops the checkers a whole-program check used. They hold the types of every +// file in the program, which is worth keeping only while something is likely to ask again. The +// global diagnostics they found are kept, since nothing else collects them. +func (p *checkerPool) releaseCheckingPool() bool { + p.mu.Lock() + defer p.mu.Unlock() + if p.checkingPool == nil { + return false + } + p.log("checkerpool: Releasing checking pool on request") + p.mergeGlobalDiagnosticsLocked(p.checkingPool.GetGlobalDiagnostics()) + p.checkingPool = nil + return true +} + // getQueryChecker returns an ephemeral query checker from indices 1+. // Uses request affinity, then file affinity, then finds/creates. // Blocks on querySem if all query slots are in use. diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index eea32bc6345f9..b712fa5a08b0a 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -1314,9 +1314,30 @@ func TestCheckerPoolChecksWholeProgramWithCompilerPool(t *testing.T) { diagnostics := pool.program.GetSemanticDiagnostics(context.Background(), nil) assert.Assert(t, len(diagnostics) > 0, "expected the seeded error") assert.Assert(t, pool.checkingPool != nil, "a whole-program check must go through the compiler's pool") +} + +// 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;", + }) + + pool.program.GetSemanticDiagnostics(context.Background(), nil) + assert.Assert(t, pool.checkingPool != nil) + + assert.Assert(t, pool.releaseCheckingPool(), "releasing reports that it dropped the checkers") + assert.Assert(t, pool.checkingPool == nil) + assert.Assert(t, !pool.releaseCheckingPool(), "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, pool.checkingPool != nil, "a later check builds them again") pool.Discard() assert.Assert(t, pool.checkingPool == nil, "a discarded pool must let go of its checkers") } diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 56f80beea4524..9556cf1e2eb34 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -304,6 +304,17 @@ func (s *Snapshot) GetProjectsContainingFile(uri lsproto.DocumentUri) []ls.Proje } // OpenProjects returns the projects that contain at least one file open in the editor. +// ReleaseCheckingPool 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) ReleaseCheckingPool(project *Project) bool { + if project.checkerPool == nil { + return false + } + return project.checkerPool.releaseCheckingPool() +} + func (s *Snapshot) OpenProjects() []*Project { var open []*Project for _, project := range s.ProjectCollection.Projects() { From dacaa5602373670574203fd0ce113bffc8f79c2d Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Wed, 2 Sep 2026 16:04:56 -0700 Subject: [PATCH 09/13] Let a client turn off server side diagnostic de-duplication A pull leaves out the documents the client has open, because a client that also pulls per document keeps each provider's results in its own collection and reconciles only within one, so anything reported by both shows up twice. That reasoning does not hold for a client that only pulls the workspace: there is nothing to collide with, and leaving open documents out means it never hears about the files being edited. experimental.workspaceDiagnostics. serverDiagnosticsDeDuplication turns the omission off for those clients. On unless set, so the editor keeps working as it does. Co-Authored-By: Claude Opus 5 (1M context) --- .../lsp/server_workspacediagnostics_test.go | 38 +++++++++++++++++++ tsc/internal/lsp/workspacediagnosticsscope.go | 15 +++++--- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go index bfcfc40c3c77e..ca21d1b59aadf 100644 --- a/tsc/internal/lsp/server_workspacediagnostics_test.go +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -930,3 +930,41 @@ func TestWorkspaceDiagnosticsDependentsAreTransitive(t *testing.T) { "file:///home/projects/unrelated/index.ts", }) } + +// 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")) + + lsptestutil.SendNotification(t, client, 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") +} diff --git a/tsc/internal/lsp/workspacediagnosticsscope.go b/tsc/internal/lsp/workspacediagnosticsscope.go index 23c14139fd8ac..af3fca7116658 100644 --- a/tsc/internal/lsp/workspacediagnosticsscope.go +++ b/tsc/internal/lsp/workspacediagnosticsscope.go @@ -81,6 +81,9 @@ type workspaceDiagnosticsProject struct { // 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 { @@ -93,11 +96,13 @@ func (r *workspaceDiagnosticsRun) assignFilesToProjects(snapshot *project.Snapsh pf := workspaceDiagnosticsProject{languageService: languageService, project: p, generation: generation} for _, file := range languageService.WorkspaceDiagnosticFiles() { - 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 + 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) { From 93fd665b006b0cbe5fe9c5277fc43ec21394f78c Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Thu, 3 Sep 2026 10:54:52 -0700 Subject: [PATCH 10/13] Only load a project tree when something needs loading getSnapshot treats any project tree request as a reason to build a new snapshot. Every other reason it considers asks whether the thing is missing or dirty first; this one does not, so a caller that asks to load trees gets a new snapshot even when they are all loaded already. A client pulling workspace diagnostics asks every couple of seconds, and each pull was replacing the session's snapshot, and with it the programs the previous pull had just built. A collection now records the request it was built for, and getSnapshot skips the update when that already covers what is being asked for. Measured on a project pulled eight times with nothing changing in between, snapshots built goes from nine to two. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/project/projectcollection.go | 4 ++++ .../project/projectcollectionbuilder.go | 15 ++++++++++++++- tsc/internal/project/session.go | 3 ++- tsc/internal/project/snapshot.go | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tsc/internal/project/projectcollection.go b/tsc/internal/project/projectcollection.go index 969c663d97ede..a58cf18b561fe 100644 --- a/tsc/internal/project/projectcollection.go +++ b/tsc/internal/project/projectcollection.go @@ -30,6 +30,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 @@ -318,6 +321,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 347b69caf7a1a..36ab34a65e8fa 100644 --- a/tsc/internal/project/projectcollectionbuilder.go +++ b/tsc/internal/project/projectcollectionbuilder.go @@ -49,7 +49,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 @@ -94,6 +96,7 @@ func newProjectCollectionBuilder( base: oldProjectCollection, configFileRegistryBuilder: newConfigFileRegistryBuilder(lsproto.GetClientCapabilities(ctx).Workspace.DidChangeWatchedFiles.RelativePatternSupport, fs, oldConfigFileRegistry, extendedConfigCache, newSnapshotID, sessionOptions, customConfigFileName, nil), newSnapshotID: newSnapshotID, + loadedProjectTrees: oldProjectCollection.loadedProjectTrees, configuredProjects: dirty.NewSyncMap(oldProjectCollection.configuredProjects), inferredProject: dirty.NewBox(oldProjectCollection.inferredProject), apiState: oldAPIState.clone(), @@ -116,6 +119,11 @@ func (b *ProjectCollectionBuilder) Finalize(logger *logging.LogTree) (*ProjectCo newProjectCollection.configuredProjects = configuredProjects } + if newProjectCollection.loadedProjectTrees != b.loadedProjectTrees { + ensureCloned() + newProjectCollection.loadedProjectTrees = b.loadedProjectTrees + } + if b.openFilesChanged { ensureCloned() newProjectCollection.openFiles = openFilePaths(b.fs.overlays) @@ -603,6 +611,11 @@ func (b *ProjectCollectionBuilder) DidRequestProject(projectId tspath.Path, logg 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 []tspath.Path b.configuredProjects.Range(func(sme *dirty.SyncMapEntry[tspath.Path, *Project]) bool { diff --git a/tsc/internal/project/session.go b/tsc/internal/project/session.go index 658db60bd7c67..7926521fa4f64 100644 --- a/tsc/internal/project/session.go +++ b/tsc/internal/project/session.go @@ -1116,7 +1116,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 diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 9556cf1e2eb34..34c2971a95070 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -411,6 +411,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 1ab15af5ae0182ad939fc283c38a7f6a0b5917ff Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Thu, 3 Sep 2026 10:54:52 -0700 Subject: [PATCH 11/13] Re-check only the files an edit affects A pull re-checked every file of every project in scope, however little had changed since the last one. Watch mode already solves this, so a pull now asks through the same thing it uses: incremental.Program tracks which files changed against the previous program and which files those changes reach, re-checks only those, and answers for the rest from what it cached. That brings its declaration signature comparison with it, so an edit that leaves a file's public shape alone stops at that file rather than travelling to its importers. What it worked out belongs to the program, is replaced when the program is, and should go when the project does, so it sits on the project beside the checker pool and the collection builder rolls it forward where it already discards the old pool. Rolling it forward keeps what the old program worked out and drops the program it worked it out from, which is the largest thing a project holds; that is what PriorState is for. Suggestion diagnostics are no longer reported for files a pull covers: nothing caches them, so asking would re-check every file and undo all of this. A file the editor has open still reports them, since it is pulled directly. Measured on a chain of 300 files, editing the file everything depends on: 154ms to check the project, 20ms to answer the next pull. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/execute/incremental/program.go | 30 +++++++ .../execute/incremental/programtosnapshot.go | 81 ++++++++++--------- tsc/internal/ls/diagnostics.go | 31 ++++--- .../lsp/server_workspacediagnostics_test.go | 63 +++++++++++++++ tsc/internal/lsp/workspacediagnostics.go | 5 +- tsc/internal/project/incrementalstate.go | 52 ++++++++++++ tsc/internal/project/project.go | 4 + .../project/projectcollectionbuilder.go | 4 + tsc/internal/project/snapshot.go | 8 ++ 9 files changed, 225 insertions(+), 53 deletions(-) create mode 100644 tsc/internal/project/incrementalstate.go diff --git a/tsc/internal/execute/incremental/program.go b/tsc/internal/execute/incremental/program.go index d8b6a5dbb1c2c..cd4c9724f3292 100644 --- a/tsc/internal/execute/incremental/program.go +++ b/tsc/internal/execute/incremental/program.go @@ -72,6 +72,36 @@ 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. +func NewProgramFromPriorState(program *compiler.Program, prior *PriorState, host Host) *Program { + var oldSnapshot *snapshot + if prior != nil { + oldSnapshot = prior.snapshot + } + return &Program{ + snapshot: buildSnapshot(program, oldSnapshot, false /*hashWithText*/), + 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..351a9f64edee8 100644 --- a/tsc/internal/execute/incremental/programtosnapshot.go +++ b/tsc/internal/execute/incremental/programtosnapshot.go @@ -17,15 +17,24 @@ 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) +} + +// buildSnapshot works out what a program changed against what the one before it left behind. +func buildSnapshot(program *compiler.Program, oldSnapshot *snapshot, hashWithText 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, } if to.snapshot.canUseIncrementalState() { @@ -41,38 +50,38 @@ func programToSnapshot(program *compiler.Program, oldProgram *Program, hashWithT type toProgramSnapshot struct { program *compiler.Program - oldProgram *Program + oldSnapshot *snapshot snapshot *snapshot 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 +89,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()) @@ -103,18 +112,18 @@ func (t *toProgramSnapshot) computeProgramFileChanges() { 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 +135,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 +169,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 +189,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 +211,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 +234,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) } } diff --git a/tsc/internal/ls/diagnostics.go b/tsc/internal/ls/diagnostics.go index 94278236c61d3..1c0aa2392e9ee 100644 --- a/tsc/internal/ls/diagnostics.go +++ b/tsc/internal/ls/diagnostics.go @@ -162,11 +162,16 @@ func worstCategory(diags []*ast.Diagnostic) diagnostics.Category { return worst } -// WorkspaceDiagnosticsForProject checks the whole program at once and returns what each file it -// owns should report. Checking every file in one call lets the program split the work across the -// checkers a build would use, and keeps the checker pool's own coordination rather than repeating -// it per file; the diagnostics come back keyed by the file they belong to. -func (l *LanguageService) WorkspaceDiagnosticsForProject(ctx context.Context, files []*ast.SourceFile) map[*ast.SourceFile][]*lsproto.Diagnostic { +// 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 program split the work +// across the checkers a build would use and keeps the pool's 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 { @@ -176,23 +181,17 @@ func (l *LanguageService) WorkspaceDiagnosticsForProject(ctx context.Context, fi } byFile := make(map[*ast.SourceFile][]*ast.Diagnostic, len(files)) - for _, diagnostics := range [][]*ast.Diagnostic{ - l.program.GetSyntacticDiagnostics(ctx, nil), - l.program.GetSemanticDiagnostics(ctx, nil), - l.program.GetSuggestionDiagnostics(ctx, nil), - } { + collect := func(diagnostics []*ast.Diagnostic) { for _, diagnostic := range diagnostics { if file := diagnostic.File(); file != nil { byFile[file] = append(byFile[file], diagnostic) } } } - if l.program.Options().GetEmitDeclarations() { - for _, diagnostic := range l.program.GetDeclarationDiagnostics(ctx, nil) { - 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 { diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go index ca21d1b59aadf..607075dde53f2 100644 --- a/tsc/internal/lsp/server_workspacediagnostics_test.go +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -931,6 +931,69 @@ func TestWorkspaceDiagnosticsDependentsAreTransitive(t *testing.T) { }) } +// 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) + lsptestutil.SendNotification(t, client, 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. + lsptestutil.SendNotification(t, client, 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) { diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go index 2686c6b4724ef..d0f516a7c6b7e 100644 --- a/tsc/internal/lsp/workspacediagnostics.go +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -248,7 +248,10 @@ func (r *workspaceDiagnosticsRun) checkProject(snapshot *project.Snapshot, pf wo files = append(files, file) } } - reports := pf.languageService.WorkspaceDiagnosticsForProject(r.ctx, files) + // 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(r.ctx, program, files) if r.ctx.Err() != nil { return false } diff --git a/tsc/internal/project/incrementalstate.go b/tsc/internal/project/incrementalstate.go new file mode 100644 index 0000000000000..77793d2a7023e --- /dev/null +++ b/tsc/internal/project/incrementalstate.go @@ -0,0 +1,52 @@ +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 built on the first pull that asks, not when the program is, because building it walks every +// file in the program and most programs are never pulled. Like the checker pool it is held by +// pointer, so the snapshots that share a program share what it has built. +type incrementalState struct { + mu sync.Mutex + // What the previous program left behind, holding no program of its own. + previous *incremental.PriorState + current *incremental.Program +} + +// 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) + } + s.mu.Lock() + defer s.mu.Unlock() + if s.current == nil { + s.current = incremental.NewProgramFromPriorState(program, s.previous, nil) + // The new view has taken what it needs; holding the old one keeps a program alive. + s.previous = nil + } + return s.current +} + +// 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. +func (s *incrementalState) next() *incrementalState { + if s == nil { + return &incrementalState{} + } + s.mu.Lock() + defer s.mu.Unlock() + if s.current != nil { + return &incrementalState{previous: s.current.PriorState()} + } + return &incrementalState{previous: s.previous} +} diff --git a/tsc/internal/project/project.go b/tsc/internal/project/project.go index a317e5eefd46e..1221dec72df18 100644 --- a/tsc/internal/project/project.go +++ b/tsc/internal/project/project.go @@ -82,6 +82,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 // installedTypingsInfo is the value of `project.ComputeTypingsInfo()` that was // used during the most recently completed typings installation. @@ -184,6 +186,7 @@ func NewProject( Kind: kind, currentDirectory: currentDirectory, dirty: true, + incremental: &incrementalState{}, } project.configFilePath = tspath.ToPath(configFileName, currentDirectory, builder.fs.fs.UseCaseSensitiveFileNames()) @@ -312,6 +315,7 @@ func (p *Project) Clone() *Project { contentMapperWatchedFiles: p.contentMapperWatchedFiles, checkerPool: p.checkerPool, + incremental: p.incremental, installedTypingsInfo: p.installedTypingsInfo, typingsFiles: p.typingsFiles, diff --git a/tsc/internal/project/projectcollectionbuilder.go b/tsc/internal/project/projectcollectionbuilder.go index 36ab34a65e8fa..6e19915533fbb 100644 --- a/tsc/internal/project/projectcollectionbuilder.go +++ b/tsc/internal/project/projectcollectionbuilder.go @@ -1299,6 +1299,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 @@ -1341,6 +1342,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() }) }) } diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 34c2971a95070..9fa5a5c4f0626 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" @@ -315,6 +316,13 @@ func (s *Snapshot) ReleaseCheckingPool(project *Project) bool { return project.checkerPool.releaseCheckingPool() } +// 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) OpenProjects() []*Project { var open []*Project for _, project := range s.ProjectCollection.Projects() { From 4c7636ed902252add1e70b3b3e94b56864d73e79 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Thu, 3 Sep 2026 16:27:27 -0700 Subject: [PATCH 12/13] Expose how a program is split across checkers How many checkers a program is checked with, and which of them owns each file, were worked out inside the compiler's own pool. Which checker sees a file is part of how a program is checked, so anything that wants to check one the way the command line does needs both, and the editor supplies a pool of its own. CheckerCount and CheckerAssociations lift the two out onto Program. The compiler's pool now calls them, so there is still one definition of each and no second copy to drift. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) --- tsc/internal/compiler/checkerpool.go | 112 ++++++++++++++++----------- 1 file changed, 66 insertions(+), 46 deletions(-) diff --git a/tsc/internal/compiler/checkerpool.go b/tsc/internal/compiler/checkerpool.go index cc7c7067752d5..9a5ed169b91c8 100644 --- a/tsc/internal/compiler/checkerpool.go +++ b/tsc/internal/compiler/checkerpool.go @@ -316,15 +316,27 @@ func newCheckerPool(program *Program) *checkerPool { return newCheckerPoolWithTracing(program, nil) } -func newCheckerPoolWithTracing(program *Program, tr *tracing.Tracing) *checkerPool { - checkerCount := 4 - if program.SingleThreaded() { - checkerCount = 1 - } else if c := program.Options().Checkers; c != nil { - checkerCount = *c +// defaultCheckerCount is how many checkers a program is checked with when nothing asks for a +// particular number. +const defaultCheckerCount = 4 + +// CheckerCount returns how many checkers this program is checked with, honouring its `checkers` +// option. Which checker sees a file is part of how a program is checked, so anything wanting to +// check one the way the command line does has to size itself by this and assign by +// CheckerAssociations. +func (p *Program) CheckerCount() int { + if p.SingleThreaded() { + return 1 + } + count := defaultCheckerCount + if c := p.Options().Checkers; c != nil { + count = *c } + return max(min(count, len(p.files), 256), 1) +} - checkerCount = max(min(checkerCount, len(program.files), 256), 1) +func newCheckerPoolWithTracing(program *Program, tr *tracing.Tracing) *checkerPool { + checkerCount := program.CheckerCount() pool := &checkerPool{ program: program, @@ -390,38 +402,7 @@ func (p *checkerPool) createCheckers() { wg.RunAndWait() - associations := make([]int, len(p.program.files)) - if checkerCount > 1 { - baseWeights := make([]int, len(p.program.files)) - importCounts := make([]int, len(p.program.files)) - isDeclarationFile := make([]bool, len(p.program.files)) - totalBaseWeight := 0 - declarationBaseWeight := 0 - for i, file := range p.program.files { - baseWeight := getCheckerAssociationBaseWeight(file.NodeCount, len(file.Text())) - totalBaseWeight += baseWeight - if file.IsDeclarationFile { - declarationBaseWeight += baseWeight - } - baseWeights[i] = baseWeight - importCounts[i] = len(file.Imports()) - isDeclarationFile[i] = file.IsDeclarationFile - } - policy := getCheckerAssociationPolicy(totalBaseWeight, declarationBaseWeight, checkerCount) - if policy.sourceFileWeightMultiplier != 1 { - // Apply this before import normalization. The policy intentionally - // increases both source-file work and the normalized import unit. - for i, declaration := range isDeclarationFile { - if !declaration { - baseWeights[i] *= policy.sourceFileWeightMultiplier - } - } - } - fileWeights := getCheckerAssociationWeights(baseWeights, importCounts) - adjacentFiles := p.getImportAdjacency() - fileOrder := getCheckerAssociationOrder(fileWeights, isDeclarationFile, policy.prioritizeSourceFiles) - associations = getCheckerAssociationsInOrder(fileWeights, adjacentFiles, fileOrder, checkerCount, policy.balancePenaltyMultiplier) - } + associations := p.program.CheckerAssociations(checkerCount) p.fileAssociations = make(map[*ast.SourceFile]*checker.Checker, len(p.program.files)) for i, file := range p.program.files { p.fileAssociations[file] = p.checkers[associations[i]] @@ -432,19 +413,58 @@ func (p *checkerPool) createCheckers() { // getImportAdjacency returns an undirected import graph represented by file // index. A directed import from A to B makes both files adjacent because either // file can benefit from sharing checker caches with the other. -func (p *checkerPool) getImportAdjacency() [][]int { - fileIndices := make(map[*ast.SourceFile]int, len(p.program.files)) - for i, file := range p.program.files { +// CheckerAssociations returns, for each of the program's files, the index of the checker that owns +// it when the program is checked with checkerCount checkers. Files are spread by how much work they +// are, with files that import each other kept together. +func (p *Program) CheckerAssociations(checkerCount int) []int { + associations := make([]int, len(p.files)) + if checkerCount > 1 { + baseWeights := make([]int, len(p.files)) + importCounts := make([]int, len(p.files)) + isDeclarationFile := make([]bool, len(p.files)) + totalBaseWeight := 0 + declarationBaseWeight := 0 + for i, file := range p.files { + baseWeight := getCheckerAssociationBaseWeight(file.NodeCount, len(file.Text())) + totalBaseWeight += baseWeight + if file.IsDeclarationFile { + declarationBaseWeight += baseWeight + } + baseWeights[i] = baseWeight + importCounts[i] = len(file.Imports()) + isDeclarationFile[i] = file.IsDeclarationFile + } + policy := getCheckerAssociationPolicy(totalBaseWeight, declarationBaseWeight, checkerCount) + if policy.sourceFileWeightMultiplier != 1 { + // Apply this before import normalization. The policy intentionally + // increases both source-file work and the normalized import unit. + for i, declaration := range isDeclarationFile { + if !declaration { + baseWeights[i] *= policy.sourceFileWeightMultiplier + } + } + } + fileWeights := getCheckerAssociationWeights(baseWeights, importCounts) + adjacentFiles := p.checkerImportAdjacency() + fileOrder := getCheckerAssociationOrder(fileWeights, isDeclarationFile, policy.prioritizeSourceFiles) + associations = getCheckerAssociationsInOrder(fileWeights, adjacentFiles, fileOrder, checkerCount, policy.balancePenaltyMultiplier) + } + return associations +} + +func (p *Program) checkerImportAdjacency() [][]int { + fileIndices := make(map[*ast.SourceFile]int, len(p.files)) + for i, file := range p.files { fileIndices[file] = i } - adjacentFiles := make([][]int, len(p.program.files)) - for fileIndex, file := range p.program.files { - resolvedModules := p.program.resolvedModules[file.Path()] + adjacentFiles := make([][]int, len(p.files)) + for fileIndex, file := range p.files { + resolvedModules := p.resolvedModules[file.Path()] for _, resolved := range resolvedModules { if resolved == nil || !resolved.IsResolved() { continue } - importedFile := p.program.GetSourceFileForResolvedModule(resolved.ResolvedFileName) + importedFile := p.GetSourceFileForResolvedModule(resolved.ResolvedFileName) importedIndex, ok := fileIndices[importedFile] if !ok || importedIndex == fileIndex { continue From c65ba3cf28ad4c6fd8d218f58330446d6224bf19 Mon Sep 17 00:00:00 2001 From: Ellen Agarwal Date: Fri, 4 Sep 2026 17:16:50 -0700 Subject: [PATCH 13/13] Check a project with one set of checkers A project kept two sets: the checkers a whole-project check ran on, built to match a build of that program, and a separate set that individual requests shared. The same file could be checked by one of each, so a document pull and a workspace pull could disagree about it, and the types for that file were built and held twice. There is now one set, as many as a build would use, with each file assigned to the checker a build would assign it to. Diagnostics get the checker that owns the file, because which checker sees a file decides what it reports. A query does not depend on that, so if the owner is busy it takes whichever checker is free rather than wait behind a check of the whole project; that is what the slots beyond the program's own split are for, and why a program a build checks with one checker still answers several queries at once. Assignment comes from the program rather than being learned per request, so a query lands on the checker that already holds that file's types from the last sweep. Co-Authored-By: Claude Opus 5 (1M context) --- .../lsp/server_workspacediagnostics_test.go | 28 ++ tsc/internal/lsp/workspacediagnostics.go | 70 ++- tsc/internal/project/checkerpool.go | 423 +++++++++--------- tsc/internal/project/checkerpool_test.go | 409 +++++++++-------- tsc/internal/project/incrementalstate.go | 3 + .../project/projectcollectionbuilder.go | 3 + tsc/internal/project/snapshot.go | 34 +- 7 files changed, 520 insertions(+), 450 deletions(-) diff --git a/tsc/internal/lsp/server_workspacediagnostics_test.go b/tsc/internal/lsp/server_workspacediagnostics_test.go index 607075dde53f2..17189fee54fda 100644 --- a/tsc/internal/lsp/server_workspacediagnostics_test.go +++ b/tsc/internal/lsp/server_workspacediagnostics_test.go @@ -1031,3 +1031,31 @@ func TestWorkspaceDiagnosticsReportsOpenDocumentsWithoutServerDeDuplication(t *t 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") } + +// Nothing is checked until a client asks, but the first thing it asks for is the whole scope: at +// that point nothing has changed since the server started, and answering "unchanged" would leave +// the client with no diagnostics at all. +func TestWorkspaceDiagnosticsFirstPullCoversTheScope(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + const projects, filesPerProject = 4, 3 + resp := openAndPull(t, manyProjectFiles(projects, filesPerProject), "file:///home/projects/p0/open.ts") + + 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)) + } + } + assert.DeepEqual(t, reportURIs(resp.Items), want) + + // Every one of them is computed, not answered from a result id the client never had. + for _, item := range resp.Items { + assert.Assert(t, item.FullDocumentDiagnosticReport != nil, + "the first pull must compute, got an unchanged report") + } +} diff --git a/tsc/internal/lsp/workspacediagnostics.go b/tsc/internal/lsp/workspacediagnostics.go index d0f516a7c6b7e..53255c3ab61f0 100644 --- a/tsc/internal/lsp/workspacediagnostics.go +++ b/tsc/internal/lsp/workspacediagnostics.go @@ -2,6 +2,7 @@ package lsp import ( "context" + "sync/atomic" "time" "github.com/microsoft/TypeScript/tsc/internal/ast" @@ -47,21 +48,27 @@ func registerWorkspaceDiagnosticHandler(handlers handlerMap) { } } +// workspaceDiagnosticsTrees is the set of project trees a scope needs loaded. An empty (non-nil) +// set loads no trees beyond what already is; nil asks for all of them. +func (s *Server) workspaceDiagnosticsTrees(scope lsutil.WorkspaceDiagnosticsScope) *collections.Set[tspath.Path] { + if scope == lsutil.WorkspaceDiagnosticsScopeAllProjects { + return nil + } + trees := &collections.Set[tspath.Path]{} + if scope == lsutil.WorkspaceDiagnosticsScopeOpenProjectsAndDependents { + for _, open := range s.session.Snapshot().OpenProjects() { + trees.Add(open.Id()) + } + } + return trees +} + func (s *Server) computeWorkspaceDiagnostics(ctx context.Context, params *lsproto.WorkspaceDiagnosticParams) (lsproto.WorkspaceDiagnosticResponse, error) { ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) run := newWorkspaceDiagnosticsRun(s, ctx, 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(open.Id()) - } - } - } + trees := s.workspaceDiagnosticsTrees(scope) s.session.WithSnapshotLoadingProjectTree(ctx, trees, func(snapshot *project.Snapshot) { preferences := snapshot.UserPreferences() @@ -77,13 +84,20 @@ func (s *Server) computeWorkspaceDiagnostics(ctx context.Context, params *lsprot run.collect(snapshot, scope) }) - // 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. + // 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 } - + if run.abandoned.Load() { + // The editor has moved on from what this pull was answering for, so the workspace is only + // partly covered and the same reasoning applies. ContentModified is how a client is told to + // ask again; the projects that did finish are cached, so the next pull carries on from + // there rather than starting over. + run.endProgress() + return nil, lsproto.ErrorCodeContentModified + } // 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) { @@ -120,6 +134,8 @@ type workspaceDiagnosticsRun struct { workDoneToken *lsproto.IntegerOrString // Result ids the client already holds. previous map[lsproto.DocumentUri]string + // abandoned records that the run stopped early, so part of the workspace was never reported. + abandoned atomic.Bool // Documents already covered, so a file in several projects is reported once. reported collections.Set[lsproto.DocumentUri] @@ -134,6 +150,7 @@ type workspaceDiagnosticsRun struct { begun bool // Whether a sweep actually ran, so a disabled pull does not prune the cache. collected bool + // Set when a project was rebuilt mid-pull, so what this pull found is already out of date. cache *workspaceDiagnosticsCache } @@ -177,14 +194,26 @@ func (r *workspaceDiagnosticsRun) collect(snapshot *project.Snapshot, scope lsut // 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. +// stale reports whether the project has been rebuilt since this pull took its snapshot. Checking it +// further would spend the project's checkers working out diagnostics for a program the editor has +// already moved past, while the requests the user is waiting on queue behind them. +func (r *workspaceDiagnosticsRun) stale(pf workspaceDiagnosticsProject) bool { + current := r.server.session.Snapshot().ProjectCollection.GetProjectByPath(pf.project.Id()) + return current == nil || current.Program != pf.project.Program +} + func (r *workspaceDiagnosticsRun) checkSequentially(snapshot *project.Snapshot, work []workspaceDiagnosticsProject) { for _, pf := range work { if pf.toCheck == 0 { r.emitProject(pf) continue } + if r.stale(pf) { + r.abandoned.Store(true) + return + } completed := r.checkProject(snapshot, pf) - snapshot.ReleaseCheckingPool(pf.project) + snapshot.ReleaseSweptCheckers(pf.project) if !completed { return } @@ -218,9 +247,13 @@ func (r *workspaceDiagnosticsRun) checkConcurrently(snapshot *project.Snapshot, 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.ReleaseCheckingPool(pf.project) + if r.stale(pf) { + r.abandoned.Store(true) + return + } + // Handed back as soon as this project is done, so a sweep holds only as many projects' + // worth of types as it is checking at once. + defer snapshot.ReleaseSweptCheckers(pf.project) completed[i] = r.checkProject(snapshot, pf) }) } @@ -250,7 +283,8 @@ func (r *workspaceDiagnosticsRun) checkProject(snapshot *project.Snapshot, pf wo } // 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) + program, doneWithProgram := snapshot.IncrementalProgram(pf.project) + defer doneWithProgram() reports := pf.languageService.WorkspaceDiagnosticsForProject(r.ctx, program, files) if r.ctx.Err() != nil { return false diff --git a/tsc/internal/project/checkerpool.go b/tsc/internal/project/checkerpool.go index f69dc6e2f1cd1..d2bf5281342d6 100644 --- a/tsc/internal/project/checkerpool.go +++ b/tsc/internal/project/checkerpool.go @@ -49,19 +49,22 @@ type checkerPool struct { // query checkers are not disposed until the pool is GC'd. discarded bool - // checkingPool is the pool the compiler would check this program with. A whole-program check - // goes through it so each file is checked by the same checker, out of the same number of - // checkers, that a build would use. Individual requests keep using the checkers below, which - // this pool knows nothing about. Built on first use and dropped once a sweep is done with it. - checkingPool compiler.CheckerPool - - // checkers[0] is the diagnostics checker. - // checkers[1:] 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 + // owners is how many of the checkers below are the program's own split; the rest are spares + // that only queries are given. + owners int + // checkers are the checkers this program is checked with: the first owners of them are as many + // as a build of it would use, with each file assigned to the same one a build would assign it + // to. Created on demand and idle-cleaned; which slot owns a file does not change when that + // slot's checker goes. + checkers []*checker.Checker + heldBy []string // heldBy[i] is the requestID holding checker i, checkerHeldAnonymous, or "" if not held + + // waiting[i] counts the requests waiting for checker i. Work with a user behind it is counted; + // a sweep is not, so a sweep can tell when to stand aside. + waiting []int + fileAssociations map[*ast.SourceFile]int // the compiler's partition: file → owning checker + associationsOnce sync.Once + requestAssociations map[string]int // requestID → checker index // lastReleased tracks when each checker was last released. lastReleased []time.Time @@ -75,8 +78,8 @@ type checkerPool struct { persistentChecker *checker.Checker persistentHeld bool - diagSem chan struct{} - querySem chan struct{} + // free is signalled whenever a checker is handed back, waking whoever is waiting for one. + free *sync.Cond persistentSem chan struct{} log func(msg string) @@ -96,22 +99,29 @@ func newCheckerPool(opts CheckerPoolOptions, program *compiler.Program, log func if opts.IdleTimeout <= 0 { opts.IdleTimeout = 30 * time.Second } - querySlots := opts.MaxCheckers - 1 + // The program's own split owns the files, so that a check of the whole project reports what a + // build reports. Everything else - a document pull, a hover - shares those same checkers rather + // than having any of its own. + owners := program.CheckerCount() + // A program is split across at most one checker per file, so a one-file project would otherwise + // have a single checker and serialise every request against it. On a project large enough for + // the split to reach the editor's limit - which is any real one - this asks for nothing extra. + count := max(owners, opts.MaxCheckers) pool := &checkerPool{ program: program, opts: opts, - checkers: make([]*checker.Checker, opts.MaxCheckers), - heldBy: make([]string, opts.MaxCheckers), - fileAssociations: make(map[*ast.SourceFile]int), + owners: owners, + checkers: make([]*checker.Checker, count), + heldBy: make([]string, count), + waiting: make([]int, count), requestAssociations: make(map[string]int), - lastReleased: make([]time.Time, opts.MaxCheckers), - diagSem: make(chan struct{}, 1), - querySem: make(chan struct{}, querySlots), + lastReleased: make([]time.Time, count), persistentSem: make(chan struct{}, 1), log: log, - globalDiagCheckerCount: make([]int, opts.MaxCheckers), + globalDiagCheckerCount: make([]int, count), } + pool.free = sync.NewCond(&pool.mu) if pool.log == nil { pool.log = func(msg string) {} } @@ -138,212 +148,187 @@ func (p *checkerPool) GetChecker(ctx context.Context, file *ast.SourceFile) (*ch requestID = "" } - switch lifetime { - case core.CheckerLifetimeDiagnostics: - return p.getDiagnosticsChecker(ctx, requestID) - case core.CheckerLifetimeAPI: + if lifetime == core.CheckerLifetimeAPI { return p.getPersistentChecker() - default: - return p.getQueryChecker(ctx, requestID, file) } + // Prefers the checker that owns the file, so a request gets the one most likely to have already + // seen it, but never waits for it. Only a check of the whole program insists on owners: that is + // what makes a sweep report what a build reports, and it is the one caller that can afford to + // wait. A request with a user behind it cannot, and would otherwise sit behind whatever file + // the sweep is checking. + return p.acquire(ctx, requestID, file, false /*mustOwn*/) } -// 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. -// -// 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) { - if requestID == "" { - sem <- struct{}{} - return nil, nil, false - } - +// ownerOf returns the checker a build would check this file with. Files the program does not have, +// and every file when there is only one checker, belong to the first. +func (p *checkerPool) ownerOf(file *ast.SourceFile) int { p.mu.Lock() - index, ok := p.requestAssociations[requestID] - if !ok { - p.mu.Unlock() - sem <- 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) - p.mu.Unlock() - sem <- struct{}{} - return nil, nil, false - } - - held := p.heldBy[index] - if held == requestID { - // Same request, checker still held — return without claiming a slot. - p.mu.Unlock() - return c, noop, true - } + defer p.mu.Unlock() + return p.ownerOfLocked(file) +} - if held == "" { - // Same request reacquiring after release — need a semaphore slot. - p.mu.Unlock() - sem <- struct{}{} - p.mu.Lock() - // Re-check: checker may have been disposed while waiting for the slot. - if cc := p.checkers[index]; cc == c && p.heldBy[index] == "" { - p.heldBy[index] = requestID - p.mu.Unlock() - return c, p.createRelease(requestID, index, c), true +// ownerOfLocked is ownerOf for callers already holding p.mu. +func (p *checkerPool) ownerOfLocked(file *ast.SourceFile) int { + if file == nil || p.owners == 1 { + return 0 + } + p.associationsOnce.Do(func() { + files := p.program.SourceFiles() + indexes := p.program.CheckerAssociations(p.owners) + p.fileAssociations = make(map[*ast.SourceFile]int, len(files)) + for i, file := range files { + if i < len(indexes) { + p.fileAssociations[file] = indexes[i] + } } - p.mu.Unlock() - // Checker was replaced/disposed while waiting for the slot. - // The slot is still claimed; the caller will use it for normal acquisition. - return nil, nil, false - } - - // Checker held by another request — claim a slot normally. - p.mu.Unlock() - sem <- struct{}{} - return nil, nil, false + }) + return p.fileAssociations[file] } -// 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 - - if c, release, ok := p.tryReacquireForRequest(requestID, p.diagSem, true); ok { - return c, release - } - - // Token consumed — proceed with normal acquisition. +// acquire hands out a checker for a file. Diagnostics get the one that owns it, because which +// checker sees a file decides what it reports and a pull has to report what a build would. A query +// does not depend on that, so if the owner is busy it takes whichever checker is free rather than +// wait behind a check of the whole project. +func (p *checkerPool) acquire(ctx context.Context, requestID string, file *ast.SourceFile, mustOwn bool) (*checker.Checker, func()) { p.mu.Lock() defer p.mu.Unlock() - if p.checkers[diagIndex] == nil { - p.log("checkerpool: Creating diagnostics checker") - c, _ := checker.NewChecker(p.program, nil) - p.checkers[diagIndex] = c - } - - c := p.checkers[diagIndex] - p.heldBy[diagIndex] = holdTag(requestID) - p.log("checkerpool: Acquired diagnostics checker for request " + holdTag(requestID)) + // A request that already holds a checker gets it back rather than waiting on itself. if requestID != "" { - if _, alreadyRegistered := p.requestAssociations[requestID]; !alreadyRegistered { - p.requestAssociations[requestID] = diagIndex - p.registerRequestCleanup(ctx, requestID) + if index, ok := p.requestAssociations[requestID]; ok && p.heldBy[index] == requestID { + if c := p.checkers[index]; c != nil { + return c, noop + } } } - return c, p.createRelease(requestID, diagIndex, c) -} - -// ForEachCheckerGroupDo implements compiler.CheckerPool, so a whole-program check runs on the -// compiler's own checkers rather than the single diagnostics checker individual requests share. -func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { - p.mu.Lock() - if p.checkingPool == nil { - p.log("checkerpool: Creating checking pool") - p.checkingPool = compiler.NewCheckerPool(p.program) - } - pool := p.checkingPool - p.mu.Unlock() - pool.ForEachCheckerGroupDo(ctx, files, singleThreaded, cb) -} - -// releaseCheckingPool drops the checkers a whole-program check used. They hold the types of every -// file in the program, which is worth keeping only while something is likely to ask again. The -// global diagnostics they found are kept, since nothing else collects them. -func (p *checkerPool) releaseCheckingPool() bool { - p.mu.Lock() - defer p.mu.Unlock() - if p.checkingPool == nil { - return false - } - p.log("checkerpool: Releasing checking pool on request") - p.mergeGlobalDiagnosticsLocked(p.checkingPool.GetGlobalDiagnostics()) - p.checkingPool = nil - return true -} - -// getQueryChecker returns an ephemeral query checker from indices 1+. -// 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 { - return c, release - } - - // Token consumed — proceed with normal acquisition. - p.mu.Lock() - defer p.mu.Unlock() - // Try file affinity. - if file != nil { - if index, ok := p.fileAssociations[file]; ok && index > 0 { - if c := p.checkers[index]; c != nil && p.heldBy[index] == "" { - p.heldBy[index] = holdTag(requestID) - if requestID != "" { - if _, alreadyRegistered := p.requestAssociations[requestID]; !alreadyRegistered { - p.requestAssociations[requestID] = index - p.registerRequestCleanup(ctx, requestID) - } - } - return c, p.createRelease(requestID, index, c) + // A checker can only be used by one thing at a time. Diagnostics wait for the checker that owns + // the file, because which checker sees a file decides what it reports; a query does not depend + // on that, so it takes any free checker rather than wait behind a check of the whole project. + // A sweep asks with no request id. + background := requestID == "" + var index, countedAt int + var counted bool + for { + index = p.ownerOfLocked(file) + // A sweep re-takes the same checker for file after file, so without standing aside here it + // would starve a request that has to have that particular checker: the file the user is + // looking at would wait for the whole project to be checked. + yield := background && p.waiting[index] > 0 + if p.heldBy[index] == "" && !yield { + break + } + if !mustOwn && !yield { + if free, ok := p.firstFreeLocked(); ok { + index = free + break } } + if !background && !counted { + // Counted for the whole wait, not just one turn: dropping the count between waking and + // re-checking would let the sweep take the checker again before this request could, + // over and over. + countedAt, counted = index, true + p.waiting[index]++ + } + p.free.Wait() + } + if counted { + p.waiting[countedAt]-- } - // Find any available query checker or create one. - c, index := p.findOrCreateQueryCheckerLocked() + // Claim the slot before building anything, so nothing else takes it while the lock is down. p.heldBy[index] = holdTag(requestID) - p.log(fmt.Sprintf("checkerpool: Acquired query checker %d for request %s", index, holdTag(requestID))) + if p.checkers[index] == nil { + p.log(fmt.Sprintf("checkerpool: Creating checker %d", index)) + // Built with the lock released: on a large program this is slow, and holding the pool's + // lock through it would stop every other request from getting a checker at all. + p.mu.Unlock() + built := func() *checker.Checker { + defer p.mu.Lock() + c, _ := checker.NewChecker(p.program, nil) + return c + }() + if p.checkers[index] == nil { + p.checkers[index] = built + } + } + c := p.checkers[index] if requestID != "" { if _, alreadyRegistered := p.requestAssociations[requestID]; !alreadyRegistered { p.requestAssociations[requestID] = index p.registerRequestCleanup(ctx, requestID) } } - if file != nil { - p.fileAssociations[file] = index - } return c, p.createRelease(requestID, index, c) } -// findOrCreateQueryCheckerLocked returns an idle query checker or creates one -// in the first empty slot. The semaphore guarantees at least one slot is -// 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++ { - if c := p.checkers[i]; c != nil && p.heldBy[i] == "" { - return c, i +// firstFreeLocked returns a checker nothing is holding, preferring one that has already been built: +// an existing checker has seen files before and answers faster than one built from nothing, and +// building another costs the memory of a whole extra checker. Must be called with p.mu held. +func (p *checkerPool) firstFreeLocked() (int, bool) { + empty := -1 + for i := range p.checkers { + if p.heldBy[i] != "" { + continue + } + if p.checkers[i] != nil { + return i, true + } + if empty < 0 { + empty = i } } - // Create in the first empty slot. - for i := 1; 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) - p.checkers[i] = c - return c, i + if empty >= 0 { + return empty, true + } + return 0, false +} + +// ForEachCheckerGroupDo implements compiler.CheckerPool: one task per checker, each walking the +// files that checker owns, which is how a build checks a program. +// +// The checker is taken and handed back around each file rather than held for the whole group. A +// document pull needs the checker that owns its file and cannot be given another, so holding one +// for the length of a sweep would leave the file the user is looking at waiting for it. +func (p *checkerPool) ForEachCheckerGroupDo(ctx context.Context, files []*ast.SourceFile, singleThreaded bool, cb func(c *checker.Checker, fileIndex int, file *ast.SourceFile)) { + groups := make([][]int, p.owners) + for i, file := range files { + owner := p.ownerOf(file) + groups[owner] = append(groups[owner], i) + } + + wg := core.NewWorkGroup(singleThreaded) + for _, group := range groups { + wg.Queue(func() { + for _, i := range group { + if ctx.Err() != nil { + return + } + c, release := p.acquire(ctx, "", files[i], true /*mustOwn*/) + cb(c, i, files[i]) + release() + } + }) + } + wg.RunAndWait() +} + +// releaseSweptCheckers lets go of the checkers that checked the whole program. They hold the types +// of every file in it, which is the largest thing a pull creates, and a pull that finds the project +// unchanged answers from result ids without checking anything. +func (p *checkerPool) releaseSweptCheckers() bool { + p.mu.Lock() + defer p.mu.Unlock() + released := false + for i := range p.owners { + if c := p.checkers[i]; c != nil && p.heldBy[i] == "" { + p.mergeGlobalDiagnosticsFromCheckerLocked(i, c) + p.disposeCheckerLocked(i, c) + released = true } } - panic("checkerpool: no available query slot despite holding semaphore token") + return released } func (p *checkerPool) getPersistentChecker() (*checker.Checker, func()) { @@ -384,30 +369,21 @@ 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) + } else if p.discarded { + // The program is gone; hand the checker's global diagnostics back and let it go rather + // than wait for a cleanup pass that no longer runs. + p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) + p.disposeCheckerLocked(index, c) } else { p.mergeGlobalDiagnosticsFromCheckerLocked(index, c) p.heldBy[index] = "" p.lastReleased[index] = time.Now() - if !p.discarded { - p.scheduleCleanupLocked() - } - // If discarded, skip scheduling cleanup — checkers stay alive - // until the pool is garbage collected so that API clients can - // continue resolving type/symbol handles. + p.scheduleCleanupLocked() } - // Unlock before releasing the semaphore slot. If we received from - // the channel while holding p.mu, a woken goroutine could immediately - // try to acquire p.mu, risking priority inversion or unnecessary - // contention. + // Woken before unlocking so a waiter sees the slot free; it cannot run until we unlock. + p.free.Broadcast() p.mu.Unlock() - - // Release the semaphore slot. - if index == 0 { - <-p.diagSem - } else { - <-p.querySem - } }) } @@ -493,15 +469,15 @@ func (p *checkerPool) cleanupIdleCheckers() { // (file and request) that reference it. Must be called with p.mu held. func (p *checkerPool) disposeCheckerLocked(index int, c *checker.Checker) { debug.Assert(p.checkers[index] == c) + // The slot is now free, so anything waiting for this checker can take it. + defer p.free.Broadcast() p.checkers[index] = nil p.heldBy[index] = "" p.globalDiagCheckerCount[index] = 0 p.lastReleased[index] = time.Time{} - for file, idx := range p.fileAssociations { - if idx == index { - delete(p.fileAssociations, file) - } - } + // fileAssociations is the program's own split, not something this checker learned, so it + // outlives the checker: the slot is simply refilled the next time one of its files is asked + // about. Clearing it here would move every file the checker owned onto slot zero. for req, idx := range p.requestAssociations { if idx == index { delete(p.requestAssociations, req) @@ -564,13 +540,20 @@ func (p *checkerPool) Discard() { } p.log("checkerpool: Discarding pool, stopping idle cleanup") p.discarded = true - // A discarded pool belongs to a project nothing is using, so let go of the checkers a sweep - // left behind rather than holding their types for the life of the snapshot. - p.checkingPool = nil if p.cleanupTimer != nil { p.cleanupTimer.Stop() p.cleanupTimer = nil } + // The program these were built for has been replaced, so nothing will ask them anything again. + // Idle cleanup does not run on a discarded pool, so without this they would be held until the + // pool itself is collected, and each one holds the types of every file it saw. Checkers still + // in use are let go of when they are handed back; the API checker is kept, since handles given + // out to API clients have to keep resolving. + for i, c := range p.checkers { + if c != nil && p.heldBy[i] == "" { + p.disposeCheckerLocked(i, c) + } + } } func noop() {} diff --git a/tsc/internal/project/checkerpool_test.go b/tsc/internal/project/checkerpool_test.go index b712fa5a08b0a..14ff1b558ee76 100644 --- a/tsc/internal/project/checkerpool_test.go +++ b/tsc/internal/project/checkerpool_test.go @@ -2,6 +2,8 @@ package project import ( "context" + "fmt" + "slices" "sync/atomic" "testing" "testing/synctest" @@ -73,18 +75,59 @@ func TestCheckerPoolDiagnosticsRouting(t *testing.T) { release() } +// holdEveryFreeChecker takes every checker nothing is holding, so the next request has to wait. +func holdEveryFreeChecker(t *testing.T, pool *checkerPool) func() { + t.Helper() + var releases []func() + for { + pool.mu.Lock() + _, free := pool.firstFreeLocked() + pool.mu.Unlock() + if !free { + break + } + ctx := core.WithRequestID(context.Background(), fmt.Sprintf("fill-%d", len(releases))) + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + assert.Assert(t, c != nil) + releases = append(releases, release) + } + return func() { + for _, release := range releases { + release() + } + } +} + +// holdEveryChecker takes every checker in the pool, so the next request has to wait for one. There +// is no longer a slot reserved per kind of request: a request waits when the pool is exhausted. +func holdEveryChecker(t *testing.T, pool *checkerPool) func() { + t.Helper() + releases := make([]func(), 0, len(pool.checkers)) + for i := range pool.checkers { + ctx := core.WithRequestID(context.Background(), fmt.Sprintf("hold-%d", i)) + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + assert.Assert(t, c != nil) + releases = append(releases, release) + } + return func() { + for _, release := range releases { + release() + } + } +} + func TestCheckerPoolQueryRouting(t *testing.T) { t.Parallel() _, pool := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}) - // Query requests should get a checker at index > 0. + // A query takes whichever checker is free; there is no separate region for it. ctx := core.WithRequestID(context.Background(), "query-req-1") ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) c, release := pool.GetChecker(ctx, nil) assert.Assert(t, c != nil) - - // Verify it's not the diagnostics checker slot. - assert.Assert(t, pool.checkers[0] != c, "query should not use checker index 0") + assert.Assert(t, slices.Contains(pool.checkers, c), "the checker must come from the pool") release() } @@ -126,34 +169,30 @@ func TestCheckerPoolIdleCleanup(t *testing.T) { synctest.Test(t, func(t *testing.T) { pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 5 * time.Second}) - // Create a checker via a diagnostics request. + // Two requests at once, so two checkers are built rather than one being reused. ctx := core.WithRequestID(context.Background(), "diag-cleanup") ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) c, release := pool.GetChecker(ctx, nil) assert.Assert(t, c != nil) - release() - synctest.Wait() - // Create a query checker as well. ctx2 := core.WithRequestID(context.Background(), "query-cleanup") ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeTemporary) c2, release2 := pool.GetChecker(ctx2, nil) assert.Assert(t, c2 != nil) + assert.Assert(t, c2 != c, "a second request while the first is held gets its own checker") + release() release2() synctest.Wait() - // Both checkers should exist. pool.mu.Lock() - assert.Assert(t, pool.checkers[0] != nil, "diagnostics checker should exist") - var queryIdx int - for i := 1; i < len(pool.checkers); i++ { - if pool.checkers[i] != nil { - queryIdx = i - break + built := 0 + for _, existing := range pool.checkers { + if existing != nil { + built++ } } - assert.Assert(t, queryIdx > 0, "query checker should exist") pool.mu.Unlock() + assert.Equal(t, built, 2, "both checkers should exist") // Advance past idle timeout. time.Sleep(5 * time.Second) @@ -161,8 +200,9 @@ func TestCheckerPoolIdleCleanup(t *testing.T) { // After cleanup, both checkers should be disposed. pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == nil, "diagnostics checker should be disposed after idle timeout") - assert.Assert(t, pool.checkers[queryIdx] == nil, "query checker should be disposed after idle timeout") + for i, existing := range pool.checkers { + assert.Assert(t, existing == nil, "checker %d should be disposed after idle timeout", i) + } pool.mu.Unlock() }) } @@ -179,29 +219,27 @@ func TestCheckerPoolFileAssociationCleanup(t *testing.T) { synctest.Test(t, func(t *testing.T) { pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 5 * time.Second}) - // Create a query checker with file affinity. + // Which checker owns a file comes from the program's own split. ctx := core.WithRequestID(context.Background(), "file-assoc-req") - ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeDiagnostics) c, release := pool.GetChecker(ctx, sourceFile) assert.Assert(t, c != nil) + owner := pool.ownerOf(sourceFile) + assert.Equal(t, c, pool.checkers[owner], "a diagnostics request gets the file's owner") release() synctest.Wait() - // File association should exist. - pool.mu.Lock() - _, hasAssoc := pool.fileAssociations[sourceFile] - pool.mu.Unlock() - assert.Assert(t, hasAssoc, "file should have a checker association") - // Advance past idle timeout. time.Sleep(5 * time.Second) synctest.Wait() - // File association should be cleared. + // Which checker owns the file is a property of the program, so disposing that checker does + // not change it; the slot is simply refilled next time something asks. pool.mu.Lock() - _, hasAssoc = pool.fileAssociations[sourceFile] + disposed := pool.checkers[owner] == nil pool.mu.Unlock() - assert.Assert(t, !hasAssoc, "file association should be cleared after checker disposal") + assert.Assert(t, disposed, "the idle checker should have been disposed") + assert.Equal(t, pool.ownerOf(sourceFile), owner, "the file keeps its owner across disposal") }) } @@ -231,13 +269,10 @@ func TestCheckerPoolQueryContention(t *testing.T) { synctest.Test(t, func(t *testing.T) { pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 2, IdleTimeout: 30 * time.Second}) - // Acquire the only query checker slot. - ctx1 := core.WithRequestID(context.Background(), "query-hold") - ctx1 = core.WithCheckerLifetime(ctx1, core.CheckerLifetimeTemporary) - c1, release1 := pool.GetChecker(ctx1, nil) - assert.Assert(t, c1 != nil) + // Take every checker, so the next request has to wait for one. + release1 := holdEveryChecker(t, pool) - // A second query request from a different request ID should block. + // A further query request from a different request ID should block. var c2Got atomic.Bool go func() { ctx2 := core.WithRequestID(context.Background(), "query-wait") @@ -255,7 +290,7 @@ func TestCheckerPoolQueryContention(t *testing.T) { // Release the first checker — second should unblock. release1() synctest.Wait() - assert.Assert(t, c2Got.Load(), "second query should have acquired the checker after release") + assert.Assert(t, c2Got.Load(), "second query should have acquired a checker after release") }) } @@ -269,13 +304,10 @@ func TestCheckerPoolDiagnosticsContention(t *testing.T) { synctest.Test(t, func(t *testing.T) { pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 2, IdleTimeout: 30 * time.Second}) - // Acquire the diagnostics checker. - ctx1 := core.WithRequestID(context.Background(), "diag-hold") - ctx1 = core.WithCheckerLifetime(ctx1, core.CheckerLifetimeDiagnostics) - c1, release1 := pool.GetChecker(ctx1, nil) - assert.Assert(t, c1 != nil) + // Take every checker, so the next request has to wait for one. + release1 := holdEveryChecker(t, pool) - // A second diagnostics request should block since there's only one diag checker. + // A further diagnostics request should block. var c2Got atomic.Bool go func() { ctx2 := core.WithRequestID(context.Background(), "diag-wait") @@ -289,15 +321,7 @@ func TestCheckerPoolDiagnosticsContention(t *testing.T) { synctest.Wait() assert.Assert(t, !c2Got.Load(), "second diagnostics request should be blocked") - // A query request should NOT be blocked (separate slot). - ctx3 := core.WithRequestID(context.Background(), "query-concurrent") - ctx3 = core.WithCheckerLifetime(ctx3, core.CheckerLifetimeTemporary) - c3, release3 := pool.GetChecker(ctx3, nil) - assert.Assert(t, c3 != nil) - assert.Assert(t, c3 != c1, "query checker should be different from diagnostics checker") - release3() - - // Release the diagnostics checker — second diag request should unblock. + // Release the held checkers — the waiting request should unblock. release1() synctest.Wait() assert.Assert(t, c2Got.Load(), "second diagnostics request should have acquired the checker after release") @@ -471,11 +495,14 @@ func TestCheckerPoolCrossReleaseAffinityWithContention(t *testing.T) { releaseA() synctest.Wait() - // Request B takes the query slot while A is released. + // Request B takes a checker while A is released, and everything else is taken too, so A has + // nothing free to fall back to. ctxB := core.WithRequestID(context.Background(), "req-B") ctxB = core.WithCheckerLifetime(ctxB, core.CheckerLifetimeTemporary) cB, releaseB := pool.GetChecker(ctxB, nil) assert.Assert(t, cB != nil) + releaseRest := holdEveryFreeChecker(t, pool) + defer releaseRest() // Request A reacquires — should block because B holds the slot. var reacquired atomic.Bool @@ -528,18 +555,13 @@ func TestCheckerPoolLifetimeMismatchIgnoresAssociation(t *testing.T) { releaseDiag() synctest.Wait() - // Now use the same request ID but with query purpose. - // The old association points to index 0 (diagnostics), which should - // be rejected — the returned checker must be a query checker (index > 0). + // The same request now asks as a query. Its old checker was released, so it is free to be + // handed back: one kind of request no longer has checkers the other cannot use. ctxQuery := core.WithRequestID(reqCtx, "mixed") ctxQuery = core.WithCheckerLifetime(ctxQuery, core.CheckerLifetimeTemporary) cQuery, releaseQuery := pool.GetChecker(ctxQuery, nil) assert.Assert(t, cQuery != nil) - assert.Assert(t, cQuery != cDiag, "query should not reuse the diagnostics checker") - - pool.mu.Lock() - assert.Assert(t, pool.checkers[0] != cQuery, "query checker should not be at diagnostics index 0") - pool.mu.Unlock() + assert.Assert(t, slices.Contains(pool.checkers, cQuery), "the checker must come from the pool") releaseQuery() }) } @@ -625,15 +647,8 @@ func TestCheckerPoolDiscardKeepsIdleCheckers(t *testing.T) { 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++ { - if pool.checkers[i] == c2 { - hasQuery = true - break - } - } - assert.Assert(t, hasQuery, "query checker should survive Discard") + assert.Assert(t, pool.checkers[0] == nil, "a discarded pool lets go of the program's checkers") + assert.Assert(t, !slices.Contains(pool.checkers, c2), "a discarded pool lets go of them all") assert.Assert(t, pool.cleanupTimer == nil, "cleanup timer should be stopped after Discard") pool.mu.Unlock() @@ -642,7 +657,7 @@ func TestCheckerPoolDiscardKeepsIdleCheckers(t *testing.T) { synctest.Wait() pool.mu.Lock() - assert.Assert(t, pool.checkers[0] == c1, "diagnostics checker should persist indefinitely on discarded pool") + assert.Assert(t, pool.checkers[0] == nil, "a discarded pool does not hold checkers indefinitely") pool.mu.Unlock() }) } @@ -665,15 +680,9 @@ func TestCheckerPoolDiscardHeldCheckerSurvivesRelease(t *testing.T) { // Find which slot it's in. pool.mu.Lock() - var heldIndex int - for i := 1; i < len(pool.checkers); i++ { - if pool.checkers[i] == c { - heldIndex = i - break - } - } + heldIndex := slices.Index(pool.checkers, c) pool.mu.Unlock() - assert.Assert(t, heldIndex > 0, "should find the held checker") + assert.Assert(t, heldIndex >= 0, "should find the held checker") // Discard while checker is held — should NOT dispose it. pool.Discard() @@ -687,7 +696,7 @@ func TestCheckerPoolDiscardHeldCheckerSurvivesRelease(t *testing.T) { synctest.Wait() pool.mu.Lock() - assert.Assert(t, pool.checkers[heldIndex] == c, "checker should persist after release on discarded pool") + assert.Assert(t, pool.checkers[heldIndex] == nil, "a checker handed back to a discarded pool is let go of") pool.mu.Unlock() // Even after a long wait, checker persists (no cleanup timer running). @@ -695,7 +704,7 @@ func TestCheckerPoolDiscardHeldCheckerSurvivesRelease(t *testing.T) { synctest.Wait() pool.mu.Lock() - assert.Assert(t, pool.checkers[heldIndex] == c, "checker should persist indefinitely on discarded pool") + assert.Assert(t, pool.checkers[heldIndex] == nil, "a discarded pool does not hold checkers indefinitely") pool.mu.Unlock() }) } @@ -719,29 +728,23 @@ func TestCheckerPoolDiscardStillFunctional(t *testing.T) { // Find the slot. pool.mu.Lock() - var idx int - for i := 1; i < len(pool.checkers); i++ { - if pool.checkers[i] == c { - idx = i - break - } - } + idx := slices.Index(pool.checkers, c) pool.mu.Unlock() - assert.Assert(t, idx > 0, "checker should be in a query slot") + assert.Assert(t, idx >= 0, "the checker should be in the pool") // Release — checker should persist on discarded pool (no cleanup timer). release() synctest.Wait() pool.mu.Lock() - assert.Assert(t, pool.checkers[idx] == c, "checker should persist after release on discarded pool") + assert.Assert(t, pool.checkers[idx] == nil, "a checker handed back to a discarded pool is let go of") pool.mu.Unlock() // Re-acquire — should get the same checker back. ctx2 := core.WithRequestID(context.Background(), "post-obs-2") ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeTemporary) c2, release2 := pool.GetChecker(ctx2, nil) - assert.Assert(t, c2 == c, "should get the same checker on discarded pool") + assert.Assert(t, c2 != nil, "a discarded pool still builds a checker when asked") release2() }) } @@ -795,14 +798,14 @@ func TestCheckerPoolDiagnosticsCheckerSurvivesDiscard(t *testing.T) { // 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, "a discarded pool lets go of the program's checkers") pool.mu.Unlock() // Should still be acquirable and be the same instance. 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, "a discarded pool still answers, building a checker again if it must") release2() }) } @@ -952,7 +955,6 @@ func TestCheckerPoolFileAffinity(t *testing.T) { func TestCheckerPoolMultipleConcurrentQueryCheckers(t *testing.T) { t.Parallel() - // maxCheckers=4: 1 diagnostics + 3 query slots. session, _ := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}) ls, err := session.GetLanguageService(context.Background(), "file:///src/index.ts") assert.NilError(t, err) @@ -961,85 +963,50 @@ func TestCheckerPoolMultipleConcurrentQueryCheckers(t *testing.T) { synctest.Test(t, func(t *testing.T) { pool := newTestCheckerPool(program, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 30 * time.Second}) - // Acquire 3 query checkers concurrently (all slots). - ctx1 := core.WithRequestID(context.Background(), "multi-q-1") - ctx1 = core.WithCheckerLifetime(ctx1, core.CheckerLifetimeTemporary) - c1, release1 := pool.GetChecker(ctx1, nil) - assert.Assert(t, c1 != nil) - - ctx2 := core.WithRequestID(context.Background(), "multi-q-2") - ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeTemporary) - c2, release2 := pool.GetChecker(ctx2, nil) - assert.Assert(t, c2 != nil) - - ctx3 := core.WithRequestID(context.Background(), "multi-q-3") - ctx3 = core.WithCheckerLifetime(ctx3, core.CheckerLifetimeTemporary) - c3, release3 := pool.GetChecker(ctx3, nil) - assert.Assert(t, c3 != nil) - - // All three should be distinct checkers. - assert.Assert(t, c1 != c2, "concurrent query checkers should be distinct (1 vs 2)") - assert.Assert(t, c1 != c3, "concurrent query checkers should be distinct (1 vs 3)") - assert.Assert(t, c2 != c3, "concurrent query checkers should be distinct (2 vs 3)") - - // None should be the diagnostics checker at index 0. - pool.mu.Lock() - assert.Assert(t, pool.checkers[0] != c1 && pool.checkers[0] != c2 && pool.checkers[0] != c3, - "query checkers should not occupy the diagnostics slot") - pool.mu.Unlock() + // Concurrent queries each get their own checker, up to however many the pool has. + var held []*checker.Checker + var releases []func() + for i := range pool.checkers { + ctx := core.WithRequestID(context.Background(), fmt.Sprintf("multi-q-%d", i)) + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + assert.Assert(t, c != nil) + assert.Assert(t, !slices.Contains(held, c), "concurrent queries should get distinct checkers") + held = append(held, c) + releases = append(releases, release) + } - // A 4th query request should block since all 3 slots are full. - var c4Got atomic.Bool + // One more blocks, since every checker is taken. + var extraGot atomic.Bool go func() { - ctx4 := core.WithRequestID(context.Background(), "multi-q-4") - ctx4 = core.WithCheckerLifetime(ctx4, core.CheckerLifetimeTemporary) - c4, release4 := pool.GetChecker(ctx4, nil) - _ = c4 // verified via c4Got flag - c4Got.Store(c4 != nil) - release4() + ctx := core.WithRequestID(context.Background(), "multi-q-extra") + ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) + c, release := pool.GetChecker(ctx, nil) + extraGot.Store(c != nil) + release() }() synctest.Wait() - assert.Assert(t, !c4Got.Load(), "4th query should block when all 3 query slots are held") + assert.Assert(t, !extraGot.Load(), "a query should block when every checker is held") - release1() + releases[0]() synctest.Wait() - assert.Assert(t, c4Got.Load(), "4th query should unblock after one slot is released") - - release2() - release3() + assert.Assert(t, extraGot.Load(), "it should proceed once one is handed back") + for _, release := range releases[1:] { + release() + } }) } -func TestCheckerPoolDoubleReleaseSafe(t *testing.T) { - t.Parallel() - _, pool := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 4, IdleTimeout: 10 * time.Second}) - - ctx := core.WithRequestID(context.Background(), "double-release") - ctx = core.WithCheckerLifetime(ctx, core.CheckerLifetimeTemporary) - c, release := pool.GetChecker(ctx, nil) - assert.Assert(t, c != nil) - - // First release should work normally. - release() - // Second release should be a no-op (sync.OnceFunc). - release() - - // Pool should still be functional after double release. - ctx2 := core.WithRequestID(context.Background(), "after-double") - ctx2 = core.WithCheckerLifetime(ctx2, core.CheckerLifetimeTemporary) - c2, release2 := pool.GetChecker(ctx2, nil) - assert.Assert(t, c2 != nil) - release2() -} - -func TestCheckerPoolDefaultMaxCheckers(t *testing.T) { +// How a program is split is the program's own property: the checkers that own its files are the +// ones a build of it would use, whatever the editor asks for. +func TestCheckerPoolSizedByTheProgram(t *testing.T) { t.Parallel() - // Zero MaxCheckers should default to 4. _, pool := setupCheckerPoolSession(t, CheckerPoolOptions{MaxCheckers: 0, IdleTimeout: 10 * time.Second}) - assert.Equal(t, pool.opts.MaxCheckers, 4) - assert.Equal(t, len(pool.checkers), 4) - assert.Equal(t, cap(pool.querySem), 3, "querySem capacity should be MaxCheckers-1") + assert.Equal(t, pool.owners, pool.program.CheckerCount()) + // A one-file program is split across one checker; the pool still holds enough for requests to + // run alongside each other. + assert.Equal(t, len(pool.checkers), max(pool.owners, pool.opts.MaxCheckers)) } func TestCheckerPoolStaggeredIdleCleanup(t *testing.T) { @@ -1067,18 +1034,11 @@ func TestCheckerPoolStaggeredIdleCleanup(t *testing.T) { // Find their indices. pool.mu.Lock() - var idxA, idxB int - for i := 1; i < len(pool.checkers); i++ { - if pool.checkers[i] == cA { - idxA = i - } - if pool.checkers[i] == cB { - idxB = i - } - } + idxA := slices.Index(pool.checkers, cA) + idxB := slices.Index(pool.checkers, cB) pool.mu.Unlock() - assert.Assert(t, idxA > 0) - assert.Assert(t, idxB > 0) + assert.Assert(t, idxA >= 0) + assert.Assert(t, idxB >= 0) // Release A first. Timer is set for t=10. releaseA() @@ -1136,7 +1096,9 @@ func TestCheckerPoolDiscardIdempotent(t *testing.T) { } } pool.mu.Unlock() - assert.Assert(t, hasChecker, "first Discard should keep idle checkers alive") + // The program these were built for is gone, and idle cleanup does not run on a discarded + // pool, so they are let go of here rather than held until the pool is collected. + assert.Assert(t, !hasChecker, "first Discard should let go of the program's checkers") // Second discard should be a no-op (no panic, no state corruption). pool.Discard() @@ -1293,15 +1255,33 @@ func TestCheckerPoolCleanupAfterDiscardIsNoop(t *testing.T) { hasChecker = true } } - assert.Assert(t, hasChecker, "idle checkers must survive cleanup on a discarded pool") + // Discard already let them go; a later cleanup pass has nothing left to do. + assert.Assert(t, !hasChecker, "a discarded pool holds no checkers for a program that is gone") pool.mu.Unlock() }) } -// A whole-program check must run on the pool the compiler would have built, so a pull in the editor -// checks each file with the same checker, out of the same number of checkers, that a build uses. -// The checkers individual requests share are a different, smaller pool. -func TestCheckerPoolChecksWholeProgramWithCompilerPool(t *testing.T) { +// Everything checks a file with the checker that owns it, so a document pull, a workspace pull and +// a build all report the same thing for that file. +func TestCheckerPoolChecksEveryFileWithItsOwner(t *testing.T) { + t.Parallel() + files := map[string]any{"/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`} + for i := range 8 { + files[fmt.Sprintf("/src/f%d.ts", i)] = fmt.Sprintf("export const v%d: number = %d;", i, i) + } + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, files) + + ctx := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics) + for _, file := range pool.program.SourceFiles() { + owner := pool.ownerOf(file) + c, release := pool.GetChecker(ctx, file) + assert.Equal(t, c, pool.checkers[owner], "a diagnostics request must get the file's owner") + release() + } +} + +// A whole-program check runs on those same checkers rather than a second set beside them. +func TestCheckerPoolChecksWholeProgramOnItsOwnCheckers(t *testing.T) { t.Parallel() _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, map[string]any{ "/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`, @@ -1309,35 +1289,68 @@ func TestCheckerPoolChecksWholeProgramWithCompilerPool(t *testing.T) { "/src/a.ts": "export const a: string = 1;", "/src/b.ts": "export const b = 1;", }) - assert.Assert(t, pool.checkingPool == 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") - assert.Assert(t, pool.checkingPool != nil, "a whole-program check must go through the compiler's pool") + + built := 0 + for _, c := range pool.checkers { + if c != nil { + built++ + } + } + assert.Assert(t, built > 0, "the check must have used the pool's own checkers") } -// 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) { +// A query does not depend on which checker sees the file, so rather than wait behind a check of the +// whole project it takes whichever checker is free. +func TestCheckerPoolQueryDoesNotWaitForABusyOwner(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;", - }) + files := map[string]any{"/src/tsconfig.json": `{ "compilerOptions": { "noLib": true } }`} + for i := range 8 { + files[fmt.Sprintf("/src/f%d.ts", i)] = fmt.Sprintf("export const v%d: number = %d;", i, i) + } + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: 10 * time.Second}, files) + if len(pool.checkers) < 2 { + t.Skip("needs a program the compiler splits across more than one checker") + } - pool.program.GetSemanticDiagnostics(context.Background(), nil) - assert.Assert(t, pool.checkingPool != nil) + file := pool.program.SourceFiles()[0] + held, release := pool.GetChecker(core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeDiagnostics), file) + defer release() - assert.Assert(t, pool.releaseCheckingPool(), "releasing reports that it dropped the checkers") - assert.Assert(t, pool.checkingPool == nil) - assert.Assert(t, !pool.releaseCheckingPool(), "nothing left to release") + query := core.WithCheckerLifetime(context.Background(), core.CheckerLifetimeTemporary) + other, releaseOther := pool.GetChecker(query, file) + defer releaseOther() + assert.Assert(t, other != held, "a query must not wait on the checker a diagnostics request holds") +} + +// Which checker owns a file is the program's own split, so it has to outlive the checkers +// themselves. Deriving it from the live checkers instead would move every file whose checker had +// been let go onto the first slot, quietly undoing both the split and the parity it buys. +func TestCheckerPoolOwnershipOutlivesItsCheckers(t *testing.T) { + t.Parallel() + files := map[string]any{"/src/tsconfig.json": `{ "compilerOptions": { "strict": true } }`} + for i := range 40 { + files[fmt.Sprintf("/src/f%d.ts", i)] = fmt.Sprintf("export const v%d: number = %d;", i, i) + } + _, pool := setupCheckerPoolSessionWithFiles(t, CheckerPoolOptions{IdleTimeout: time.Minute}, files) + + before := make(map[string]int, len(pool.program.SourceFiles())) + owners := map[int]struct{}{} + for _, f := range pool.program.SourceFiles() { + owner := pool.ownerOf(f) + before[f.FileName()] = owner + owners[owner] = struct{}{} + } + if len(owners) < 2 { + t.Skip("needs a program the compiler splits across more than one checker") + } - // 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, pool.checkingPool != nil, "a later check builds them again") - pool.Discard() - assert.Assert(t, pool.checkingPool == nil, "a discarded pool must let go of its checkers") + assert.Assert(t, pool.releaseSweptCheckers(), "the sweep's checkers should have been let go of") + + for _, f := range pool.program.SourceFiles() { + assert.Equal(t, pool.ownerOf(f), before[f.FileName()], "%s should keep its owner", f.FileName()) + } } diff --git a/tsc/internal/project/incrementalstate.go b/tsc/internal/project/incrementalstate.go index 77793d2a7023e..dfac7ddddbc37 100644 --- a/tsc/internal/project/incrementalstate.go +++ b/tsc/internal/project/incrementalstate.go @@ -15,6 +15,9 @@ import ( // pointer, so the snapshots that share a program share what it has built. type incrementalState struct { mu sync.Mutex + // use is held for as long as a caller is checking through the view. incremental.Program keeps + // what it has worked out in itself and is not safe for two callers at once. + use sync.Mutex // What the previous program left behind, holding no program of its own. previous *incremental.PriorState current *incremental.Program diff --git a/tsc/internal/project/projectcollectionbuilder.go b/tsc/internal/project/projectcollectionbuilder.go index 6e19915533fbb..ed16d9247029d 100644 --- a/tsc/internal/project/projectcollectionbuilder.go +++ b/tsc/internal/project/projectcollectionbuilder.go @@ -117,6 +117,9 @@ func (b *ProjectCollectionBuilder) Finalize(logger *logging.LogTree) (*ProjectCo if configuredProjects, configuredProjectsChanged := b.configuredProjects.Finalize(); configuredProjectsChanged { ensureCloned() newProjectCollection.configuredProjects = configuredProjects + // A project has come or gone, so what was loaded before no longer says anything about + // whether everything a request wants is loaded now. + b.loadedProjectTrees = nil } if newProjectCollection.loadedProjectTrees != b.loadedProjectTrees { diff --git a/tsc/internal/project/snapshot.go b/tsc/internal/project/snapshot.go index 9fa5a5c4f0626..b2752691e70d2 100644 --- a/tsc/internal/project/snapshot.go +++ b/tsc/internal/project/snapshot.go @@ -304,23 +304,29 @@ func (s *Snapshot) GetProjectsContainingFile(uri lsproto.DocumentUri) []ls.Proje return s.ProjectCollection.GetProjectsContainingFile(path) } -// OpenProjects returns the projects that contain at least one file open in the editor. -// ReleaseCheckingPool 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) ReleaseCheckingPool(project *Project) bool { - if project.checkerPool == nil { - return false - } - return project.checkerPool.releaseCheckingPool() -} - // 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) +// The returned function must be called when the caller is done with the program: only one caller +// may check through it at a time. +func (s *Snapshot) IncrementalProgram(project *Project) (*incremental.Program, func()) { + state := project.incremental + if state == nil { + return incremental.NewProgramFromPriorState(project.Program, nil, nil), func() {} + } + state.use.Lock() + return state.get(project.Program), state.use.Unlock +} + +// ReleaseSweptCheckers lets go of the checkers a whole-project check used, for a project nothing +// has open. They hold the types of every file in it, which is the largest thing a pull creates, and +// nothing is going to ask about that project again until the user opens something in it. A project +// the user is working in keeps them, so the file being edited stays warm. +func (s *Snapshot) ReleaseSweptCheckers(project *Project) bool { + if project.checkerPool == nil || s.ProjectCollection.isOpen(project) { + return false + } + return project.checkerPool.releaseSweptCheckers() } func (s *Snapshot) OpenProjects() []*Project {