From 574ee8564391cf2562217a8dd02a98c8462192f3 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 12:56:44 -0800 Subject: [PATCH 01/13] wip prog diags in lsp --- internal/ls/diagnostics.go | 56 +------------ internal/ls/lsconv/converters.go | 84 +++++++++++++++++++ internal/lsp/server.go | 7 ++ internal/project/client.go | 1 + internal/project/session.go | 51 +++++++++++ .../projecttestutil/clientmock_generated.go | 51 +++++++++++ 6 files changed, 195 insertions(+), 55 deletions(-) diff --git a/internal/ls/diagnostics.go b/internal/ls/diagnostics.go index f223bf5dbe..844fd8ece8 100644 --- a/internal/ls/diagnostics.go +++ b/internal/ls/diagnostics.go @@ -2,11 +2,9 @@ package ls import ( "context" - "slices" "strings" "github.com/microsoft/typescript-go/internal/ast" - "github.com/microsoft/typescript-go/internal/diagnostics" "github.com/microsoft/typescript-go/internal/diagnosticwriter" "github.com/microsoft/typescript-go/internal/ls/lsconv" "github.com/microsoft/typescript-go/internal/lsp/lsproto" @@ -41,64 +39,12 @@ func (l *LanguageService) toLSPDiagnostics(ctx context.Context, diagnostics ...[ lspDiagnostics := make([]*lsproto.Diagnostic, 0, size) for _, diagSlice := range diagnostics { for _, diag := range diagSlice { - lspDiagnostics = append(lspDiagnostics, l.toLSPDiagnostic(ctx, diag)) + lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPull(ctx, l.converters, diag)) } } return lspDiagnostics } -func (l *LanguageService) toLSPDiagnostic(ctx context.Context, diagnostic *ast.Diagnostic) *lsproto.Diagnostic { - clientOptions := lsproto.GetClientCapabilities(ctx).TextDocument.Diagnostic - var severity lsproto.DiagnosticSeverity - switch diagnostic.Category() { - case diagnostics.CategorySuggestion: - severity = lsproto.DiagnosticSeverityHint - case diagnostics.CategoryMessage: - severity = lsproto.DiagnosticSeverityInformation - case diagnostics.CategoryWarning: - severity = lsproto.DiagnosticSeverityWarning - default: - severity = lsproto.DiagnosticSeverityError - } - - var relatedInformation []*lsproto.DiagnosticRelatedInformation - if clientOptions.RelatedInformation { - relatedInformation = make([]*lsproto.DiagnosticRelatedInformation, 0, len(diagnostic.RelatedInformation())) - for _, related := range diagnostic.RelatedInformation() { - relatedInformation = append(relatedInformation, &lsproto.DiagnosticRelatedInformation{ - Location: lsproto.Location{ - Uri: lsconv.FileNameToDocumentURI(related.File().FileName()), - Range: l.converters.ToLSPRange(related.File(), related.Loc()), - }, - Message: related.Message(), - }) - } - } - - var tags []lsproto.DiagnosticTag - if len(clientOptions.TagSupport.ValueSet) > 0 && (diagnostic.ReportsUnnecessary() || diagnostic.ReportsDeprecated()) { - tags = make([]lsproto.DiagnosticTag, 0, 2) - if diagnostic.ReportsUnnecessary() && slices.Contains(clientOptions.TagSupport.ValueSet, lsproto.DiagnosticTagUnnecessary) { - tags = append(tags, lsproto.DiagnosticTagUnnecessary) - } - if diagnostic.ReportsDeprecated() && slices.Contains(clientOptions.TagSupport.ValueSet, lsproto.DiagnosticTagDeprecated) { - tags = append(tags, lsproto.DiagnosticTagDeprecated) - } - } - - return &lsproto.Diagnostic{ - Range: l.converters.ToLSPRange(diagnostic.File(), diagnostic.Loc()), - Code: &lsproto.IntegerOrString{ - Integer: ptrTo(diagnostic.Code()), - }, - Severity: &severity, - Message: messageChainToString(diagnostic), - Source: ptrTo("ts"), - RelatedInformation: ptrToSliceIfNonEmpty(relatedInformation), - Tags: ptrToSliceIfNonEmpty(tags), - } -} - func messageChainToString(diagnostic *ast.Diagnostic) string { if len(diagnostic.MessageChain()) == 0 { return diagnostic.Message() diff --git a/internal/ls/lsconv/converters.go b/internal/ls/lsconv/converters.go index 3b290b446c..a5beb0bb5c 100644 --- a/internal/ls/lsconv/converters.go +++ b/internal/ls/lsconv/converters.go @@ -1,6 +1,7 @@ package lsconv import ( + "context" "fmt" "net/url" "slices" @@ -8,7 +9,9 @@ import ( "unicode/utf16" "unicode/utf8" + "github.com/microsoft/typescript-go/internal/ast" "github.com/microsoft/typescript-go/internal/core" + "github.com/microsoft/typescript-go/internal/diagnostics" "github.com/microsoft/typescript-go/internal/lsp/lsproto" "github.com/microsoft/typescript-go/internal/tspath" ) @@ -199,3 +202,84 @@ func (c *Converters) PositionToLineAndCharacter(script Script, position core.Tex func ptrTo[T any](v T) *T { return &v } + +type diagnosticCapabilities struct { + relatedInformation bool + tagValueSet []lsproto.DiagnosticTag +} + +// DiagnosticToLSPPull converts a diagnostic for pull diagnostics (textDocument/diagnostic) +func DiagnosticToLSPPull(ctx context.Context, converters *Converters, diagnostic *ast.Diagnostic) *lsproto.Diagnostic { + clientCaps := lsproto.GetClientCapabilities(ctx).TextDocument.Diagnostic + return diagnosticToLSP(converters, diagnostic, diagnosticCapabilities{ + relatedInformation: clientCaps.RelatedInformation, + tagValueSet: clientCaps.TagSupport.ValueSet, + }) +} + +// DiagnosticToLSPPush converts a diagnostic for push diagnostics (textDocument/publishDiagnostics) +func DiagnosticToLSPPush(ctx context.Context, converters *Converters, diagnostic *ast.Diagnostic) *lsproto.Diagnostic { + clientCaps := lsproto.GetClientCapabilities(ctx).TextDocument.PublishDiagnostics + return diagnosticToLSP(converters, diagnostic, diagnosticCapabilities{ + relatedInformation: clientCaps.RelatedInformation, + tagValueSet: clientCaps.TagSupport.ValueSet, + }) +} + +func diagnosticToLSP(converters *Converters, diagnostic *ast.Diagnostic, caps diagnosticCapabilities) *lsproto.Diagnostic { + var severity lsproto.DiagnosticSeverity + switch diagnostic.Category() { + case diagnostics.CategorySuggestion: + severity = lsproto.DiagnosticSeverityHint + case diagnostics.CategoryMessage: + severity = lsproto.DiagnosticSeverityInformation + case diagnostics.CategoryWarning: + severity = lsproto.DiagnosticSeverityWarning + default: + severity = lsproto.DiagnosticSeverityError + } + + var relatedInformation []*lsproto.DiagnosticRelatedInformation + if caps.relatedInformation { + relatedInformation = make([]*lsproto.DiagnosticRelatedInformation, 0, len(diagnostic.RelatedInformation())) + for _, related := range diagnostic.RelatedInformation() { + relatedInformation = append(relatedInformation, &lsproto.DiagnosticRelatedInformation{ + Location: lsproto.Location{ + Uri: FileNameToDocumentURI(related.File().FileName()), + Range: converters.ToLSPRange(related.File(), related.Loc()), + }, + Message: related.Message(), + }) + } + } + + var tags []lsproto.DiagnosticTag + if len(caps.tagValueSet) > 0 && (diagnostic.ReportsUnnecessary() || diagnostic.ReportsDeprecated()) { + tags = make([]lsproto.DiagnosticTag, 0, 2) + if diagnostic.ReportsUnnecessary() && slices.Contains(caps.tagValueSet, lsproto.DiagnosticTagUnnecessary) { + tags = append(tags, lsproto.DiagnosticTagUnnecessary) + } + if diagnostic.ReportsDeprecated() && slices.Contains(caps.tagValueSet, lsproto.DiagnosticTagDeprecated) { + tags = append(tags, lsproto.DiagnosticTagDeprecated) + } + } + + return &lsproto.Diagnostic{ + Range: converters.ToLSPRange(diagnostic.File(), diagnostic.Loc()), + Code: &lsproto.IntegerOrString{ + Integer: ptrTo(diagnostic.Code()), + }, + Severity: &severity, + Message: diagnostic.Message(), + Source: ptrTo("ts"), + RelatedInformation: ptrToSliceIfNonEmpty(relatedInformation), + Tags: ptrToSliceIfNonEmpty(tags), + } +} + +func ptrToSliceIfNonEmpty[T any](s []T) *[]T { + if len(s) == 0 { + return nil + } + return &s +} diff --git a/internal/lsp/server.go b/internal/lsp/server.go index de880048c1..ea5d67bef5 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -219,6 +219,13 @@ func (s *Server) RefreshDiagnostics(ctx context.Context) error { return nil } +// PublishDiagnostics implements project.Client. +func (s *Server) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { + notification := lsproto.TextDocumentPublishDiagnosticsInfo.NewNotificationMessage(params) + s.outgoingQueue <- notification.Message() + return nil +} + func (s *Server) RequestConfiguration(ctx context.Context) (*lsutil.UserPreferences, error) { caps := lsproto.GetClientCapabilities(ctx) if !caps.Workspace.Configuration { diff --git a/internal/project/client.go b/internal/project/client.go index e223389eb6..46b5393a05 100644 --- a/internal/project/client.go +++ b/internal/project/client.go @@ -10,4 +10,5 @@ type Client interface { WatchFiles(ctx context.Context, id WatcherID, watchers []*lsproto.FileSystemWatcher) error UnwatchFiles(ctx context.Context, id WatcherID) error RefreshDiagnostics(ctx context.Context) error + PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error } diff --git a/internal/project/session.go b/internal/project/session.go index 0e2b2de6b1..08b45a214f 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -14,6 +14,7 @@ import ( "github.com/microsoft/typescript-go/internal/compiler" "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/ls" + "github.com/microsoft/typescript-go/internal/ls/lsconv" "github.com/microsoft/typescript-go/internal/ls/lsutil" "github.com/microsoft/typescript-go/internal/lsp/lsproto" "github.com/microsoft/typescript-go/internal/project/ata" @@ -439,6 +440,7 @@ func (s *Session) UpdateSnapshot(ctx context.Context, overlays map[tspath.Path]* s.logger.Log(err) } } + s.publishProgramDiagnostics(oldSnapshot, newSnapshot) }) return newSnapshot @@ -689,6 +691,55 @@ func (s *Session) NpmInstall(cwd string, npmInstallArgs []string) ([]byte, error return s.npmExecutor.NpmInstall(cwd, npmInstallArgs) } +func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot *Snapshot) { + collections.DiffOrderedMaps( + oldSnapshot.ProjectCollection.ProjectsByPath(), + newSnapshot.ProjectCollection.ProjectsByPath(), + func(_ tspath.Path, addedProject *Project) { + }, + func(projectPath tspath.Path, removedProject *Project) { + if removedProject.Kind != KindConfigured { + return + } + if err := s.client.PublishDiagnostics(context.Background(), &lsproto.PublishDiagnosticsParams{ + Uri: lsproto.DocumentUri(removedProject.ConfigFileName()), + Diagnostics: []*lsproto.Diagnostic{}, + }); err != nil && s.options.LoggingEnabled { + s.logger.Logf("Error clearing diagnostics for removed project: %v", err) + } + }, + func(projectPath tspath.Path, oldProject, newProject *Project) { + }, + ) + + for _, project := range newSnapshot.ProjectCollection.Projects() { + if project.Program == nil { + continue + } + + if project.ProgramLastUpdate != newSnapshot.ID() { + continue + } + + if project.Kind != KindConfigured { + continue + } + + programDiagnostics := project.Program.GetProgramDiagnostics() + lspDiagnostics := make([]*lsproto.Diagnostic, 0, len(programDiagnostics)) + for _, diag := range programDiagnostics { + lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPush(context.TODO(), newSnapshot.converters, diag)) + } + + if err := s.client.PublishDiagnostics(context.TODO(), &lsproto.PublishDiagnosticsParams{ + Uri: lsproto.DocumentUri(project.ConfigFileName()), + Diagnostics: lspDiagnostics, + }); err != nil && s.options.LoggingEnabled { + s.logger.Logf("Error publishing diagnostics: %v", err) + } + } +} + func (s *Session) triggerATAForUpdatedProjects(newSnapshot *Snapshot) { for _, project := range newSnapshot.ProjectCollection.Projects() { if project.ShouldTriggerATA(newSnapshot.ID()) { diff --git a/internal/testutil/projecttestutil/clientmock_generated.go b/internal/testutil/projecttestutil/clientmock_generated.go index f2dff8fad5..2c310ceb13 100644 --- a/internal/testutil/projecttestutil/clientmock_generated.go +++ b/internal/testutil/projecttestutil/clientmock_generated.go @@ -21,6 +21,9 @@ var _ project.Client = &ClientMock{} // // // make and configure a mocked project.Client // mockedClient := &ClientMock{ +// PublishDiagnosticsFunc: func(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { +// panic("mock out the PublishDiagnostics method") +// }, // RefreshDiagnosticsFunc: func(ctx context.Context) error { // panic("mock out the RefreshDiagnostics method") // }, @@ -37,6 +40,9 @@ var _ project.Client = &ClientMock{} // // } type ClientMock struct { + // PublishDiagnosticsFunc mocks the PublishDiagnostics method. + PublishDiagnosticsFunc func(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error + // RefreshDiagnosticsFunc mocks the RefreshDiagnostics method. RefreshDiagnosticsFunc func(ctx context.Context) error @@ -48,6 +54,13 @@ type ClientMock struct { // calls tracks calls to the methods. calls struct { + // PublishDiagnostics holds details about calls to the PublishDiagnostics method. + PublishDiagnostics []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Params is the params argument value. + Params *lsproto.PublishDiagnosticsParams + } // RefreshDiagnostics holds details about calls to the RefreshDiagnostics method. RefreshDiagnostics []struct { // Ctx is the ctx argument value. @@ -70,11 +83,49 @@ type ClientMock struct { Watchers []*lsproto.FileSystemWatcher } } + lockPublishDiagnostics sync.RWMutex lockRefreshDiagnostics sync.RWMutex lockUnwatchFiles sync.RWMutex lockWatchFiles sync.RWMutex } +// PublishDiagnostics calls PublishDiagnosticsFunc. +func (mock *ClientMock) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { + callInfo := struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams + }{ + Ctx: ctx, + Params: params, + } + mock.lockPublishDiagnostics.Lock() + mock.calls.PublishDiagnostics = append(mock.calls.PublishDiagnostics, callInfo) + mock.lockPublishDiagnostics.Unlock() + if mock.PublishDiagnosticsFunc == nil { + var errOut error + return errOut + } + return mock.PublishDiagnosticsFunc(ctx, params) +} + +// PublishDiagnosticsCalls gets all the calls that were made to PublishDiagnostics. +// Check the length with: +// +// len(mockedClient.PublishDiagnosticsCalls()) +func (mock *ClientMock) PublishDiagnosticsCalls() []struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams +} { + var calls []struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams + } + mock.lockPublishDiagnostics.RLock() + calls = mock.calls.PublishDiagnostics + mock.lockPublishDiagnostics.RUnlock() + return calls +} + // RefreshDiagnostics calls RefreshDiagnosticsFunc. func (mock *ClientMock) RefreshDiagnostics(ctx context.Context) error { callInfo := struct { From f8e919e8ec7d6d33120c2c0cd3d2509bdd491cc5 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 19:12:29 -0800 Subject: [PATCH 02/13] Simplify --- internal/project/session.go | 61 ++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/internal/project/session.go b/internal/project/session.go index 08b45a214f..0cd7f8972d 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -692,51 +692,48 @@ func (s *Session) NpmInstall(cwd string, npmInstallArgs []string) ([]byte, error } func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot *Snapshot) { + ctx := context.Background() collections.DiffOrderedMaps( oldSnapshot.ProjectCollection.ProjectsByPath(), newSnapshot.ProjectCollection.ProjectsByPath(), func(_ tspath.Path, addedProject *Project) { + if addedProject.Kind != KindConfigured || addedProject.Program == nil { + return + } + if addedProject.ProgramLastUpdate != newSnapshot.ID() { + return + } + s.publishProjectDiagnostics(ctx, addedProject, addedProject.Program.GetProgramDiagnostics(), newSnapshot.converters) }, - func(projectPath tspath.Path, removedProject *Project) { + func(_ tspath.Path, removedProject *Project) { if removedProject.Kind != KindConfigured { return } - if err := s.client.PublishDiagnostics(context.Background(), &lsproto.PublishDiagnosticsParams{ - Uri: lsproto.DocumentUri(removedProject.ConfigFileName()), - Diagnostics: []*lsproto.Diagnostic{}, - }); err != nil && s.options.LoggingEnabled { - s.logger.Logf("Error clearing diagnostics for removed project: %v", err) - } + s.publishProjectDiagnostics(ctx, removedProject, nil, newSnapshot.converters) }, - func(projectPath tspath.Path, oldProject, newProject *Project) { + func(_ tspath.Path, oldProject, newProject *Project) { + if newProject.Kind != KindConfigured || newProject.Program == nil { + return + } + if newProject.ProgramLastUpdate != newSnapshot.ID() { + return + } + s.publishProjectDiagnostics(ctx, newProject, newProject.Program.GetProgramDiagnostics(), newSnapshot.converters) }, ) +} - for _, project := range newSnapshot.ProjectCollection.Projects() { - if project.Program == nil { - continue - } - - if project.ProgramLastUpdate != newSnapshot.ID() { - continue - } - - if project.Kind != KindConfigured { - continue - } - - programDiagnostics := project.Program.GetProgramDiagnostics() - lspDiagnostics := make([]*lsproto.Diagnostic, 0, len(programDiagnostics)) - for _, diag := range programDiagnostics { - lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPush(context.TODO(), newSnapshot.converters, diag)) - } +func (s *Session) publishProjectDiagnostics(ctx context.Context, project *Project, diagnostics []*ast.Diagnostic, converters *lsconv.Converters) { + lspDiagnostics := make([]*lsproto.Diagnostic, 0, len(diagnostics)) + for _, diag := range diagnostics { + lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPush(ctx, converters, diag)) + } - if err := s.client.PublishDiagnostics(context.TODO(), &lsproto.PublishDiagnosticsParams{ - Uri: lsproto.DocumentUri(project.ConfigFileName()), - Diagnostics: lspDiagnostics, - }); err != nil && s.options.LoggingEnabled { - s.logger.Logf("Error publishing diagnostics: %v", err) - } + if err := s.client.PublishDiagnostics(ctx, &lsproto.PublishDiagnosticsParams{ + Uri: lsproto.DocumentUri(project.ConfigFileName()), + Diagnostics: lspDiagnostics, + }); err != nil && s.options.LoggingEnabled { + s.logger.Logf("Error publishing diagnostics: %v", err) } } From 9621a3e688e569f3dc319273d2797e884e6fe6f6 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 19:16:13 -0800 Subject: [PATCH 03/13] Simplify --- internal/project/session.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/project/session.go b/internal/project/session.go index 0cd7f8972d..cad96199b8 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -697,10 +697,7 @@ func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot * oldSnapshot.ProjectCollection.ProjectsByPath(), newSnapshot.ProjectCollection.ProjectsByPath(), func(_ tspath.Path, addedProject *Project) { - if addedProject.Kind != KindConfigured || addedProject.Program == nil { - return - } - if addedProject.ProgramLastUpdate != newSnapshot.ID() { + if addedProject.Kind != KindConfigured || addedProject.Program == nil || addedProject.ProgramLastUpdate != newSnapshot.ID() { return } s.publishProjectDiagnostics(ctx, addedProject, addedProject.Program.GetProgramDiagnostics(), newSnapshot.converters) @@ -712,10 +709,7 @@ func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot * s.publishProjectDiagnostics(ctx, removedProject, nil, newSnapshot.converters) }, func(_ tspath.Path, oldProject, newProject *Project) { - if newProject.Kind != KindConfigured || newProject.Program == nil { - return - } - if newProject.ProgramLastUpdate != newSnapshot.ID() { + if newProject.Kind != KindConfigured || newProject.Program == nil || newProject.ProgramLastUpdate != newSnapshot.ID() { return } s.publishProjectDiagnostics(ctx, newProject, newProject.Program.GetProgramDiagnostics(), newSnapshot.converters) From 953928aad036342557ec13a34723ee6b91fd87b7 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 19:32:03 -0800 Subject: [PATCH 04/13] fix path --- internal/project/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/project/session.go b/internal/project/session.go index cad96199b8..153c15c1ea 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -724,7 +724,7 @@ func (s *Session) publishProjectDiagnostics(ctx context.Context, project *Projec } if err := s.client.PublishDiagnostics(ctx, &lsproto.PublishDiagnosticsParams{ - Uri: lsproto.DocumentUri(project.ConfigFileName()), + Uri: lsconv.FileNameToDocumentURI(project.ConfigFileName()), Diagnostics: lspDiagnostics, }); err != nil && s.options.LoggingEnabled { s.logger.Logf("Error publishing diagnostics: %v", err) From 2082a7b247b3c82b970c1af273871d8213b4ad21 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 20:32:43 -0800 Subject: [PATCH 05/13] Testing --- internal/fourslash/fourslash.go | 8 +- internal/ls/lsconv/converters.go | 8 +- internal/lsp/server.go | 21 ++++- internal/project/project_test.go | 134 +++++++++++++++++++++++++++ internal/project/refcounting_test.go | 3 +- internal/project/session.go | 71 +++++++++----- internal/project/snapshot_test.go | 22 ++++- 7 files changed, 233 insertions(+), 34 deletions(-) diff --git a/internal/fourslash/fourslash.go b/internal/fourslash/fourslash.go index 1c982f10f3..8c7cd9cf09 100644 --- a/internal/fourslash/fourslash.go +++ b/internal/fourslash/fourslash.go @@ -250,8 +250,14 @@ func (f *FourslashTest) nextID() int32 { } func (f *FourslashTest) initialize(t *testing.T, capabilities *lsproto.ClientCapabilities) { + initOptions := map[string]any{ + // Hack: disable push diagnostics entirely, since the fourslash runner does not + // yet gracefully handle non-request messages. + "disablePushDiagnostics": true, + } params := &lsproto.InitializeParams{ - Locale: ptrTo("en-US"), + Locale: ptrTo("en-US"), + InitializationOptions: ptrTo[any](initOptions), } params.Capabilities = getCapabilitiesWithDefaults(capabilities) // !!! check for errors? diff --git a/internal/ls/lsconv/converters.go b/internal/ls/lsconv/converters.go index a5beb0bb5c..152acfdeb7 100644 --- a/internal/ls/lsconv/converters.go +++ b/internal/ls/lsconv/converters.go @@ -264,8 +264,14 @@ func diagnosticToLSP(converters *Converters, diagnostic *ast.Diagnostic, caps di } } + // For diagnostics without a file (e.g., program diagnostics), use a zero range + var lspRange lsproto.Range + if diagnostic.File() != nil { + lspRange = converters.ToLSPRange(diagnostic.File(), diagnostic.Loc()) + } + return &lsproto.Diagnostic{ - Range: converters.ToLSPRange(diagnostic.File(), diagnostic.Loc()), + Range: lspRange, Code: &lsproto.IntegerOrString{ Integer: ptrTo(diagnostic.Code()), }, diff --git a/internal/lsp/server.go b/internal/lsp/server.go index ea5d67bef5..d4813fca3c 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -723,6 +723,16 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali cwd = s.cwd } + var disablePushDiagnostics bool + if s.initializeParams != nil && s.initializeParams.InitializationOptions != nil && *s.initializeParams.InitializationOptions != nil { + // Check for disablePushDiagnostics option + if initOpts, ok := (*s.initializeParams.InitializationOptions).(map[string]any); ok { + if disable, ok := initOpts["disablePushDiagnostics"].(bool); ok { + disablePushDiagnostics = disable + } + } + } + s.session = project.NewSession(&project.SessionInit{ Options: &project.SessionOptions{ CurrentDirectory: cwd, @@ -733,11 +743,12 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali LoggingEnabled: true, DebounceDelay: 500 * time.Millisecond, }, - FS: s.fs, - Logger: s.logger, - Client: s, - NpmExecutor: s, - ParseCache: s.parseCache, + FS: s.fs, + Logger: s.logger, + Client: s, + NpmExecutor: s, + ParseCache: s.parseCache, + DisablePushDiagnostics: disablePushDiagnostics, }) if s.initializeParams != nil && s.initializeParams.InitializationOptions != nil && *s.initializeParams.InitializationOptions != nil { diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 90ad9cda64..795f3dd1ce 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -175,3 +175,137 @@ func TestProject(t *testing.T) { assert.NilError(t, err) }) } + +func TestPushDiagnostics(t *testing.T) { + t.Parallel() + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + t.Run("publishes program diagnostics on initial program creation", func(t *testing.T) { + t.Parallel() + files := map[string]any{ + "/src/tsconfig.json": `{"compilerOptions": {"baseUrl": "."}}`, + "/src/index.ts": "export const x = 1;", + } + session, utils := projecttestutil.Setup(files) + session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, files["/src/index.ts"].(string), lsproto.LanguageKindTypeScript) + _, err := session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts")) + assert.NilError(t, err) + + session.WaitForBackgroundTasks() + + calls := utils.Client().PublishDiagnosticsCalls() + assert.Assert(t, len(calls) > 0, "expected at least one PublishDiagnostics call") + + // Find the call for tsconfig.json + var tsconfigCall *struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams + } + for i := range calls { + if calls[i].Params.Uri == "file:///src/tsconfig.json" { + tsconfigCall = &calls[i] + break + } + } + assert.Assert(t, tsconfigCall != nil, "expected PublishDiagnostics call for tsconfig.json") + assert.Assert(t, len(tsconfigCall.Params.Diagnostics) > 0, "expected at least one diagnostic") + }) + + t.Run("clears diagnostics when config fixed", func(t *testing.T) { + t.Parallel() + files := map[string]any{ + "/src/tsconfig.json": `{"compilerOptions": {"baseUrl": "."}}`, + "/src/index.ts": "export const x = 1;", + "/src2/tsconfig.json": `{"compilerOptions": {}}`, + "/src2/index.ts": "export const y = 2;", + } + session, utils := projecttestutil.Setup(files) + session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, files["/src/index.ts"].(string), lsproto.LanguageKindTypeScript) + _, err := session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts")) + assert.NilError(t, err) + session.WaitForBackgroundTasks() + + // Open a file in a different project to trigger cleanup of the first + session.DidCloseFile(context.Background(), "file:///src/index.ts") + session.DidOpenFile(context.Background(), "file:///src2/index.ts", 1, files["/src2/index.ts"].(string), lsproto.LanguageKindTypeScript) + _, err = session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src2/index.ts")) + assert.NilError(t, err) + session.WaitForBackgroundTasks() + + calls := utils.Client().PublishDiagnosticsCalls() + // Should have at least one call for the first project with diagnostics, + // and one clearing it after switching projects + var firstProjectCalls []struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams + } + for i := range calls { + if calls[i].Params.Uri == "file:///src/tsconfig.json" { + firstProjectCalls = append(firstProjectCalls, calls[i]) + } + } + assert.Assert(t, len(firstProjectCalls) >= 2, "expected at least 2 PublishDiagnostics calls for first project") + // Last call should clear diagnostics + lastCall := firstProjectCalls[len(firstProjectCalls)-1] + assert.Equal(t, len(lastCall.Params.Diagnostics), 0, "expected empty diagnostics after project cleanup") + }) + + t.Run("updates diagnostics when program changes", func(t *testing.T) { + t.Parallel() + files := map[string]any{ + "/src/tsconfig.json": `{"compilerOptions": {"baseUrl": "."}}`, + "/src/index.ts": "export const x = 1;", + } + session, utils := projecttestutil.Setup(files) + session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, files["/src/index.ts"].(string), lsproto.LanguageKindTypeScript) + _, err := session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts")) + assert.NilError(t, err) + session.WaitForBackgroundTasks() + + initialCallCount := len(utils.Client().PublishDiagnosticsCalls()) + + // Change the tsconfig to remove baseUrl + err = utils.FS().WriteFile("/src/tsconfig.json", `{"compilerOptions": {}}`, false) + assert.NilError(t, err) + session.DidChangeWatchedFiles(context.Background(), []*lsproto.FileEvent{{Uri: lsproto.DocumentUri("file:///src/tsconfig.json"), Type: lsproto.FileChangeTypeChanged}}) + _, err = session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts")) + assert.NilError(t, err) + session.WaitForBackgroundTasks() + + calls := utils.Client().PublishDiagnosticsCalls() + assert.Assert(t, len(calls) > initialCallCount, "expected additional PublishDiagnostics call after change") + + // Find the last call for tsconfig.json + var lastTsconfigCall *struct { + Ctx context.Context + Params *lsproto.PublishDiagnosticsParams + } + for i := len(calls) - 1; i >= 0; i-- { + if calls[i].Params.Uri == "file:///src/tsconfig.json" { + lastTsconfigCall = &calls[i] + break + } + } + assert.Assert(t, lastTsconfigCall != nil, "expected PublishDiagnostics call for tsconfig.json") + // After fixing the error, there should be no program diagnostics + assert.Equal(t, len(lastTsconfigCall.Params.Diagnostics), 0, "expected no diagnostics after removing baseUrl option") + }) + + t.Run("does not publish for inferred projects", func(t *testing.T) { + t.Parallel() + files := map[string]any{ + "/src/index.ts": "let x: number = 'not a number';", + } + session, utils := projecttestutil.Setup(files) + session.DidOpenFile(context.Background(), "file:///src/index.ts", 1, files["/src/index.ts"].(string), lsproto.LanguageKindTypeScript) + _, err := session.GetLanguageService(context.Background(), lsproto.DocumentUri("file:///src/index.ts")) + assert.NilError(t, err) + session.WaitForBackgroundTasks() + + calls := utils.Client().PublishDiagnosticsCalls() + // Should not have any calls since inferred projects don't have tsconfig.json + assert.Equal(t, len(calls), 0, "expected no PublishDiagnostics calls for inferred projects") + }) +} diff --git a/internal/project/refcounting_test.go b/internal/project/refcounting_test.go index eef05a7898..d0a5df44fe 100644 --- a/internal/project/refcounting_test.go +++ b/internal/project/refcounting_test.go @@ -28,7 +28,8 @@ func TestRefCountingCaches(t *testing.T) { WatchEnabled: false, LoggingEnabled: false, }, - FS: fs, + FS: fs, + Client: &noopClient{}, }) return session } diff --git a/internal/project/session.go b/internal/project/session.go index 153c15c1ea..cd9ccd9777 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -48,12 +48,13 @@ type SessionOptions struct { } type SessionInit struct { - Options *SessionOptions - FS vfs.FS - Client Client - Logger logging.Logger - NpmExecutor ata.NpmExecutor - ParseCache *ParseCache + Options *SessionOptions + FS vfs.FS + Client Client + Logger logging.Logger + NpmExecutor ata.NpmExecutor + ParseCache *ParseCache + DisablePushDiagnostics bool } // Session manages the state of an LSP session. It receives textDocument @@ -87,6 +88,7 @@ type Session struct { compilerOptionsForInferredProjects *core.CompilerOptions typingsInstaller *ata.TypingsInstaller backgroundQueue *background.Queue + disablePushDiagnostics bool // snapshotID is the counter for snapshot IDs. It does not necessarily // equal the `snapshot.ID`. It is stored on Session instead of globally @@ -124,6 +126,9 @@ type Session struct { } func NewSession(init *SessionInit) *Session { + if init.Client == nil { + panic("NewSession: Client cannot be nil") + } currentDirectory := init.Options.CurrentDirectory useCaseSensitiveFileNames := init.FS.UseCaseSensitiveFileNames() toPath := func(fileName string) tspath.Path { @@ -137,17 +142,17 @@ func NewSession(init *SessionInit) *Session { extendedConfigCache := &ExtendedConfigCache{} session := &Session{ - options: init.Options, - toPath: toPath, - client: init.Client, - logger: init.Logger, - npmExecutor: init.NpmExecutor, - fs: overlayFS, - parseCache: parseCache, - extendedConfigCache: extendedConfigCache, - programCounter: &programCounter{}, - backgroundQueue: background.NewQueue(), - snapshotID: atomic.Uint64{}, + options: init.Options, + toPath: toPath, + client: init.Client, + logger: init.Logger, + npmExecutor: init.NpmExecutor, + fs: overlayFS, + parseCache: parseCache, + extendedConfigCache: extendedConfigCache, + programCounter: &programCounter{}, + backgroundQueue: background.NewQueue(), + disablePushDiagnostics: init.DisablePushDiagnostics, snapshot: NewSnapshot( uint64(0), &SnapshotFS{ @@ -692,39 +697,55 @@ func (s *Session) NpmInstall(cwd string, npmInstallArgs []string) ([]byte, error } func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot *Snapshot) { + if s.disablePushDiagnostics { + return + } + ctx := context.Background() collections.DiffOrderedMaps( oldSnapshot.ProjectCollection.ProjectsByPath(), newSnapshot.ProjectCollection.ProjectsByPath(), - func(_ tspath.Path, addedProject *Project) { + func(configFilePath tspath.Path, addedProject *Project) { if addedProject.Kind != KindConfigured || addedProject.Program == nil || addedProject.ProgramLastUpdate != newSnapshot.ID() { return } - s.publishProjectDiagnostics(ctx, addedProject, addedProject.Program.GetProgramDiagnostics(), newSnapshot.converters) + s.publishProjectDiagnostics(ctx, string(configFilePath), addedProject.Program.GetProgramDiagnostics(), newSnapshot.converters) }, - func(_ tspath.Path, removedProject *Project) { + func(configFilePath tspath.Path, removedProject *Project) { if removedProject.Kind != KindConfigured { return } - s.publishProjectDiagnostics(ctx, removedProject, nil, newSnapshot.converters) + s.publishProjectDiagnostics(ctx, string(configFilePath), nil, oldSnapshot.converters) }, - func(_ tspath.Path, oldProject, newProject *Project) { + func(configFilePath tspath.Path, oldProject, newProject *Project) { if newProject.Kind != KindConfigured || newProject.Program == nil || newProject.ProgramLastUpdate != newSnapshot.ID() { return } - s.publishProjectDiagnostics(ctx, newProject, newProject.Program.GetProgramDiagnostics(), newSnapshot.converters) + s.publishProjectDiagnostics(ctx, string(configFilePath), newProject.Program.GetProgramDiagnostics(), newSnapshot.converters) }, ) } -func (s *Session) publishProjectDiagnostics(ctx context.Context, project *Project, diagnostics []*ast.Diagnostic, converters *lsconv.Converters) { +func (s *Session) publishProjectDiagnostics(ctx context.Context, configFilePath string, diagnostics []*ast.Diagnostic, converters *lsconv.Converters) { + if s.client == nil { + // Client not set (shouldn't happen in normal operation) + return + } + if converters == nil { + // This shouldn't happen, but be defensive + if s.options.LoggingEnabled { + s.logger.Logf("publishProjectDiagnostics called with nil converters for %s", configFilePath) + } + return + } + lspDiagnostics := make([]*lsproto.Diagnostic, 0, len(diagnostics)) for _, diag := range diagnostics { lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPush(ctx, converters, diag)) } if err := s.client.PublishDiagnostics(ctx, &lsproto.PublishDiagnosticsParams{ - Uri: lsconv.FileNameToDocumentURI(project.ConfigFileName()), + Uri: lsconv.FileNameToDocumentURI(configFilePath), Diagnostics: lspDiagnostics, }); err != nil && s.options.LoggingEnabled { s.logger.Logf("Error publishing diagnostics: %v", err) diff --git a/internal/project/snapshot_test.go b/internal/project/snapshot_test.go index ded5e16b5e..d9962721a7 100644 --- a/internal/project/snapshot_test.go +++ b/internal/project/snapshot_test.go @@ -11,6 +11,25 @@ import ( "gotest.tools/v3/assert" ) +// noopClient is a minimal Client implementation for snapshot tests +type noopClient struct{} + +func (n *noopClient) WatchFiles(ctx context.Context, id WatcherID, watchers []*lsproto.FileSystemWatcher) error { + return nil +} + +func (n *noopClient) UnwatchFiles(ctx context.Context, id WatcherID) error { + return nil +} + +func (n *noopClient) RefreshDiagnostics(ctx context.Context) error { + return nil +} + +func (n *noopClient) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { + return nil +} + func TestSnapshot(t *testing.T) { t.Parallel() if !bundled.Embedded { @@ -28,7 +47,8 @@ func TestSnapshot(t *testing.T) { WatchEnabled: false, LoggingEnabled: false, }, - FS: fs, + FS: fs, + Client: &noopClient{}, }) return session } From acee2d1d8f1b081046887c3b8ca7710dc5d7f347 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 20:48:28 -0800 Subject: [PATCH 06/13] oops --- internal/ls/diagnostics.go | 18 ------------ internal/ls/lsconv/converters.go | 12 +++++++- internal/lsp/server.go | 48 +++++++++++++------------------- 3 files changed, 31 insertions(+), 47 deletions(-) diff --git a/internal/ls/diagnostics.go b/internal/ls/diagnostics.go index 844fd8ece8..39ee512a22 100644 --- a/internal/ls/diagnostics.go +++ b/internal/ls/diagnostics.go @@ -2,10 +2,8 @@ package ls import ( "context" - "strings" "github.com/microsoft/typescript-go/internal/ast" - "github.com/microsoft/typescript-go/internal/diagnosticwriter" "github.com/microsoft/typescript-go/internal/ls/lsconv" "github.com/microsoft/typescript-go/internal/lsp/lsproto" ) @@ -44,19 +42,3 @@ func (l *LanguageService) toLSPDiagnostics(ctx context.Context, diagnostics ...[ } return lspDiagnostics } - -func messageChainToString(diagnostic *ast.Diagnostic) string { - if len(diagnostic.MessageChain()) == 0 { - return diagnostic.Message() - } - var b strings.Builder - diagnosticwriter.WriteFlattenedASTDiagnosticMessage(&b, diagnostic, "\n") - return b.String() -} - -func ptrToSliceIfNonEmpty[T any](s []T) *[]T { - if len(s) == 0 { - return nil - } - return &s -} diff --git a/internal/ls/lsconv/converters.go b/internal/ls/lsconv/converters.go index 152acfdeb7..4370c2b056 100644 --- a/internal/ls/lsconv/converters.go +++ b/internal/ls/lsconv/converters.go @@ -12,6 +12,7 @@ import ( "github.com/microsoft/typescript-go/internal/ast" "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/diagnostics" + "github.com/microsoft/typescript-go/internal/diagnosticwriter" "github.com/microsoft/typescript-go/internal/lsp/lsproto" "github.com/microsoft/typescript-go/internal/tspath" ) @@ -276,13 +277,22 @@ func diagnosticToLSP(converters *Converters, diagnostic *ast.Diagnostic, caps di Integer: ptrTo(diagnostic.Code()), }, Severity: &severity, - Message: diagnostic.Message(), + Message: messageChainToString(diagnostic), Source: ptrTo("ts"), RelatedInformation: ptrToSliceIfNonEmpty(relatedInformation), Tags: ptrToSliceIfNonEmpty(tags), } } +func messageChainToString(diagnostic *ast.Diagnostic) string { + if len(diagnostic.MessageChain()) == 0 { + return diagnostic.Message() + } + var b strings.Builder + diagnosticwriter.WriteFlattenedASTDiagnosticMessage(&b, diagnostic, "\n") + return b.String() +} + func ptrToSliceIfNonEmpty[T any](s []T) *[]T { if len(s) == 0 { return nil diff --git a/internal/lsp/server.go b/internal/lsp/server.go index d4813fca3c..89508b4709 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -751,36 +751,28 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali DisablePushDiagnostics: disablePushDiagnostics, }) - if s.initializeParams != nil && s.initializeParams.InitializationOptions != nil && *s.initializeParams.InitializationOptions != nil { - // handle userPreferences from initializationOptions - userPreferences := s.session.NewUserPreferences() - userPreferences.Parse(*s.initializeParams.InitializationOptions) - s.session.InitializeWithConfig(userPreferences) - } else { - // request userPreferences if not provided at initialization - userPreferences, err := s.RequestConfiguration(ctx) - if err != nil { - return err - } - s.session.InitializeWithConfig(userPreferences) + userPreferences, err := s.RequestConfiguration(ctx) + if err != nil { + return err + } + s.session.InitializeWithConfig(userPreferences) - _, err = sendClientRequest(ctx, s, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{ - Registrations: []*lsproto.Registration{ - { - Id: "typescript-config-watch-id", - Method: string(lsproto.MethodWorkspaceDidChangeConfiguration), - RegisterOptions: ptrTo(any(lsproto.DidChangeConfigurationRegistrationOptions{ - Section: &lsproto.StringOrStrings{ - // !!! Both the 'javascript' and 'js/ts' scopes need to be watched for settings as well. - Strings: &[]string{"typescript"}, - }, - })), - }, + _, err = sendClientRequest(ctx, s, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{ + Registrations: []*lsproto.Registration{ + { + Id: "typescript-config-watch-id", + Method: string(lsproto.MethodWorkspaceDidChangeConfiguration), + RegisterOptions: ptrTo(any(lsproto.DidChangeConfigurationRegistrationOptions{ + Section: &lsproto.StringOrStrings{ + // !!! Both the 'javascript' and 'js/ts' scopes need to be watched for settings as well. + Strings: &[]string{"typescript"}, + }, + })), }, - }) - if err != nil { - return fmt.Errorf("failed to register configuration change watcher: %w", err) - } + }, + }) + if err != nil { + return fmt.Errorf("failed to register configuration change watcher: %w", err) } // !!! temporary. From a32814b15bda94a018fb7a44cf5845a6c9ef0cf4 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 17 Nov 2025 20:49:11 -0800 Subject: [PATCH 07/13] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/project/project_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 795f3dd1ce..d4042f5d57 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -213,7 +213,7 @@ func TestPushDiagnostics(t *testing.T) { assert.Assert(t, len(tsconfigCall.Params.Diagnostics) > 0, "expected at least one diagnostic") }) - t.Run("clears diagnostics when config fixed", func(t *testing.T) { + t.Run("clears diagnostics when project is removed", func(t *testing.T) { t.Parallel() files := map[string]any{ "/src/tsconfig.json": `{"compilerOptions": {"baseUrl": "."}}`, From 528629f3bd754442dc4891f8c41e82c897f8ba02 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 09:40:43 -0800 Subject: [PATCH 08/13] PR feedback --- internal/project/session.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/project/session.go b/internal/project/session.go index cd9ccd9777..e805a3816c 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -706,7 +706,7 @@ func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot * oldSnapshot.ProjectCollection.ProjectsByPath(), newSnapshot.ProjectCollection.ProjectsByPath(), func(configFilePath tspath.Path, addedProject *Project) { - if addedProject.Kind != KindConfigured || addedProject.Program == nil || addedProject.ProgramLastUpdate != newSnapshot.ID() { + if !shouldPublishProgramDiagnostics(addedProject, newSnapshot.ID()) { return } s.publishProjectDiagnostics(ctx, string(configFilePath), addedProject.Program.GetProgramDiagnostics(), newSnapshot.converters) @@ -718,7 +718,7 @@ func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot * s.publishProjectDiagnostics(ctx, string(configFilePath), nil, oldSnapshot.converters) }, func(configFilePath tspath.Path, oldProject, newProject *Project) { - if newProject.Kind != KindConfigured || newProject.Program == nil || newProject.ProgramLastUpdate != newSnapshot.ID() { + if !shouldPublishProgramDiagnostics(newProject, newSnapshot.ID()) { return } s.publishProjectDiagnostics(ctx, string(configFilePath), newProject.Program.GetProgramDiagnostics(), newSnapshot.converters) @@ -726,6 +726,13 @@ func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot * ) } +func shouldPublishProgramDiagnostics(p *Project, snapshotID uint64) bool { + if p.Kind != KindConfigured || p.Program == nil || p.ProgramLastUpdate != snapshotID { + return false + } + return p.ProgramUpdateKind > ProgramUpdateKindCloned +} + func (s *Session) publishProjectDiagnostics(ctx context.Context, configFilePath string, diagnostics []*ast.Diagnostic, converters *lsconv.Converters) { if s.client == nil { // Client not set (shouldn't happen in normal operation) From f1117f39d7016f51b0309f7a8345992e0364dad3 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 09:41:55 -0800 Subject: [PATCH 09/13] Drop old workarounds --- internal/project/session.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/internal/project/session.go b/internal/project/session.go index e805a3816c..b315bc3241 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -734,18 +734,6 @@ func shouldPublishProgramDiagnostics(p *Project, snapshotID uint64) bool { } func (s *Session) publishProjectDiagnostics(ctx context.Context, configFilePath string, diagnostics []*ast.Diagnostic, converters *lsconv.Converters) { - if s.client == nil { - // Client not set (shouldn't happen in normal operation) - return - } - if converters == nil { - // This shouldn't happen, but be defensive - if s.options.LoggingEnabled { - s.logger.Logf("publishProjectDiagnostics called with nil converters for %s", configFilePath) - } - return - } - lspDiagnostics := make([]*lsproto.Diagnostic, 0, len(diagnostics)) for _, diag := range diagnostics { lspDiagnostics = append(lspDiagnostics, lsconv.DiagnosticToLSPPush(ctx, converters, diag)) From a8f9833da03827bcdf5e3d5ae6e6ac612f7ceeb9 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 09:43:40 -0800 Subject: [PATCH 10/13] Flip the condition I guess --- internal/lsp/server.go | 26 +++++++++---------- internal/project/session.go | 52 ++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 89508b4709..2c3eafa3a7 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -735,20 +735,20 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali s.session = project.NewSession(&project.SessionInit{ Options: &project.SessionOptions{ - CurrentDirectory: cwd, - DefaultLibraryPath: s.defaultLibraryPath, - TypingsLocation: s.typingsLocation, - PositionEncoding: s.positionEncoding, - WatchEnabled: s.watchEnabled, - LoggingEnabled: true, - DebounceDelay: 500 * time.Millisecond, + CurrentDirectory: cwd, + DefaultLibraryPath: s.defaultLibraryPath, + TypingsLocation: s.typingsLocation, + PositionEncoding: s.positionEncoding, + WatchEnabled: s.watchEnabled, + LoggingEnabled: true, + DebounceDelay: 500 * time.Millisecond, + PushDiagnosticsEnabled: !disablePushDiagnostics, }, - FS: s.fs, - Logger: s.logger, - Client: s, - NpmExecutor: s, - ParseCache: s.parseCache, - DisablePushDiagnostics: disablePushDiagnostics, + FS: s.fs, + Logger: s.logger, + Client: s, + NpmExecutor: s, + ParseCache: s.parseCache, }) userPreferences, err := s.RequestConfiguration(ctx) diff --git a/internal/project/session.go b/internal/project/session.go index b315bc3241..0144b3865f 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -38,23 +38,23 @@ const ( // SessionOptions are the immutable initialization options for a session. // Snapshots may reference them as a pointer since they never change. type SessionOptions struct { - CurrentDirectory string - DefaultLibraryPath string - TypingsLocation string - PositionEncoding lsproto.PositionEncodingKind - WatchEnabled bool - LoggingEnabled bool - DebounceDelay time.Duration + CurrentDirectory string + DefaultLibraryPath string + TypingsLocation string + PositionEncoding lsproto.PositionEncodingKind + WatchEnabled bool + LoggingEnabled bool + PushDiagnosticsEnabled bool + DebounceDelay time.Duration } type SessionInit struct { - Options *SessionOptions - FS vfs.FS - Client Client - Logger logging.Logger - NpmExecutor ata.NpmExecutor - ParseCache *ParseCache - DisablePushDiagnostics bool + Options *SessionOptions + FS vfs.FS + Client Client + Logger logging.Logger + NpmExecutor ata.NpmExecutor + ParseCache *ParseCache } // Session manages the state of an LSP session. It receives textDocument @@ -88,7 +88,6 @@ type Session struct { compilerOptionsForInferredProjects *core.CompilerOptions typingsInstaller *ata.TypingsInstaller backgroundQueue *background.Queue - disablePushDiagnostics bool // snapshotID is the counter for snapshot IDs. It does not necessarily // equal the `snapshot.ID`. It is stored on Session instead of globally @@ -142,17 +141,16 @@ func NewSession(init *SessionInit) *Session { extendedConfigCache := &ExtendedConfigCache{} session := &Session{ - options: init.Options, - toPath: toPath, - client: init.Client, - logger: init.Logger, - npmExecutor: init.NpmExecutor, - fs: overlayFS, - parseCache: parseCache, - extendedConfigCache: extendedConfigCache, - programCounter: &programCounter{}, - backgroundQueue: background.NewQueue(), - disablePushDiagnostics: init.DisablePushDiagnostics, + options: init.Options, + toPath: toPath, + client: init.Client, + logger: init.Logger, + npmExecutor: init.NpmExecutor, + fs: overlayFS, + parseCache: parseCache, + extendedConfigCache: extendedConfigCache, + programCounter: &programCounter{}, + backgroundQueue: background.NewQueue(), snapshot: NewSnapshot( uint64(0), &SnapshotFS{ @@ -697,7 +695,7 @@ func (s *Session) NpmInstall(cwd string, npmInstallArgs []string) ([]byte, error } func (s *Session) publishProgramDiagnostics(oldSnapshot *Snapshot, newSnapshot *Snapshot) { - if s.disablePushDiagnostics { + if !s.options.PushDiagnosticsEnabled { return } From 4c8b04473a96009dec40ad89cab54d6b20e397eb Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 09:51:07 -0800 Subject: [PATCH 11/13] noop client --- internal/project/client.go | 18 ++++++++++++++++++ internal/project/refcounting_test.go | 3 +-- internal/project/session.go | 7 ++++--- internal/project/snapshot_test.go | 22 +--------------------- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/internal/project/client.go b/internal/project/client.go index 46b5393a05..1c1fe5f46c 100644 --- a/internal/project/client.go +++ b/internal/project/client.go @@ -12,3 +12,21 @@ type Client interface { RefreshDiagnostics(ctx context.Context) error PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error } + +type noopClient struct{} + +func (n noopClient) WatchFiles(ctx context.Context, id WatcherID, watchers []*lsproto.FileSystemWatcher) error { + return nil +} + +func (n noopClient) UnwatchFiles(ctx context.Context, id WatcherID) error { + return nil +} + +func (n noopClient) RefreshDiagnostics(ctx context.Context) error { + return nil +} + +func (n noopClient) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { + return nil +} diff --git a/internal/project/refcounting_test.go b/internal/project/refcounting_test.go index d0a5df44fe..eef05a7898 100644 --- a/internal/project/refcounting_test.go +++ b/internal/project/refcounting_test.go @@ -28,8 +28,7 @@ func TestRefCountingCaches(t *testing.T) { WatchEnabled: false, LoggingEnabled: false, }, - FS: fs, - Client: &noopClient{}, + FS: fs, }) return session } diff --git a/internal/project/session.go b/internal/project/session.go index 0144b3865f..7772a5bfb5 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -125,8 +125,9 @@ type Session struct { } func NewSession(init *SessionInit) *Session { - if init.Client == nil { - panic("NewSession: Client cannot be nil") + client := init.Client + if client == nil { + client = noopClient{} } currentDirectory := init.Options.CurrentDirectory useCaseSensitiveFileNames := init.FS.UseCaseSensitiveFileNames() @@ -143,7 +144,7 @@ func NewSession(init *SessionInit) *Session { session := &Session{ options: init.Options, toPath: toPath, - client: init.Client, + client: client, logger: init.Logger, npmExecutor: init.NpmExecutor, fs: overlayFS, diff --git a/internal/project/snapshot_test.go b/internal/project/snapshot_test.go index d9962721a7..ded5e16b5e 100644 --- a/internal/project/snapshot_test.go +++ b/internal/project/snapshot_test.go @@ -11,25 +11,6 @@ import ( "gotest.tools/v3/assert" ) -// noopClient is a minimal Client implementation for snapshot tests -type noopClient struct{} - -func (n *noopClient) WatchFiles(ctx context.Context, id WatcherID, watchers []*lsproto.FileSystemWatcher) error { - return nil -} - -func (n *noopClient) UnwatchFiles(ctx context.Context, id WatcherID) error { - return nil -} - -func (n *noopClient) RefreshDiagnostics(ctx context.Context) error { - return nil -} - -func (n *noopClient) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { - return nil -} - func TestSnapshot(t *testing.T) { t.Parallel() if !bundled.Embedded { @@ -47,8 +28,7 @@ func TestSnapshot(t *testing.T) { WatchEnabled: false, LoggingEnabled: false, }, - FS: fs, - Client: &noopClient{}, + FS: fs, }) return session } From 1a19ca749ef04927ebfed178ed8d778d64f6f138 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 09:52:21 -0800 Subject: [PATCH 12/13] Just rely on the bool --- internal/project/client.go | 18 ------------------ internal/project/session.go | 6 +----- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/internal/project/client.go b/internal/project/client.go index 1c1fe5f46c..46b5393a05 100644 --- a/internal/project/client.go +++ b/internal/project/client.go @@ -12,21 +12,3 @@ type Client interface { RefreshDiagnostics(ctx context.Context) error PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error } - -type noopClient struct{} - -func (n noopClient) WatchFiles(ctx context.Context, id WatcherID, watchers []*lsproto.FileSystemWatcher) error { - return nil -} - -func (n noopClient) UnwatchFiles(ctx context.Context, id WatcherID) error { - return nil -} - -func (n noopClient) RefreshDiagnostics(ctx context.Context) error { - return nil -} - -func (n noopClient) PublishDiagnostics(ctx context.Context, params *lsproto.PublishDiagnosticsParams) error { - return nil -} diff --git a/internal/project/session.go b/internal/project/session.go index 7772a5bfb5..4adaf13e58 100644 --- a/internal/project/session.go +++ b/internal/project/session.go @@ -125,10 +125,6 @@ type Session struct { } func NewSession(init *SessionInit) *Session { - client := init.Client - if client == nil { - client = noopClient{} - } currentDirectory := init.Options.CurrentDirectory useCaseSensitiveFileNames := init.FS.UseCaseSensitiveFileNames() toPath := func(fileName string) tspath.Path { @@ -144,7 +140,7 @@ func NewSession(init *SessionInit) *Session { session := &Session{ options: init.Options, toPath: toPath, - client: client, + client: init.Client, logger: init.Logger, npmExecutor: init.NpmExecutor, fs: overlayFS, From 28221e8bbce1198616bff04630a65926f7c8f925 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 18 Nov 2025 10:09:36 -0800 Subject: [PATCH 13/13] Reenable --- .../testutil/projecttestutil/projecttestutil.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/testutil/projecttestutil/projecttestutil.go b/internal/testutil/projecttestutil/projecttestutil.go index 169709d42c..882a582f70 100644 --- a/internal/testutil/projecttestutil/projecttestutil.go +++ b/internal/testutil/projecttestutil/projecttestutil.go @@ -209,12 +209,13 @@ func SetupWithOptionsAndTypingsInstaller(files map[string]any, options *project. // Use provided options or create default ones if options == nil { options = &project.SessionOptions{ - CurrentDirectory: "/", - DefaultLibraryPath: bundled.LibPath(), - TypingsLocation: TestTypingsLocation, - PositionEncoding: lsproto.PositionEncodingKindUTF8, - WatchEnabled: true, - LoggingEnabled: true, + CurrentDirectory: "/", + DefaultLibraryPath: bundled.LibPath(), + TypingsLocation: TestTypingsLocation, + PositionEncoding: lsproto.PositionEncodingKindUTF8, + WatchEnabled: true, + LoggingEnabled: true, + PushDiagnosticsEnabled: true, } }