Skip to content

Commit d062f15

Browse files
Offer workspace diagnostics through dynamic registration
The capability cannot be advertised at initialize. The setting that turns workspace diagnostics on arrives after it, and a server that claimed the capability while the setting was off would have clients pulling a workspace nobody asked it to check. It is registered and unregistered dynamically instead, as the setting changes, and only ever by the one provider. The client runs a workspace pull per provider that asks for it, into that provider's own collection, so a second provider carrying the capability would report every problem twice; the content mapper's registration deliberately leaves it off, and the provider that does carry it covers content-mapped files too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 659826c commit d062f15

3 files changed

Lines changed: 160 additions & 0 deletions

File tree

‎tsc/internal/lsp/server.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,9 @@ type Server struct {
256256
// produced the result id a client holds for each file.
257257
workspaceDiagnostics *workspaceDiagnosticsCache
258258

259+
workspaceDiagnosticsRegistrationMu sync.Mutex
260+
workspaceDiagnosticsRegistered bool
261+
259262
// workspaceDiagnosticsPull is the `workspace/diagnostic` request currently running, if any. A
260263
// newer pull supersedes it; see supersedeWorkspaceDiagnostics.
261264
workspaceDiagnosticsMu sync.Mutex
@@ -525,6 +528,10 @@ func (s *Server) RegisterContentMapperExtensions(ctx context.Context, extensions
525528
{
526529
Id: contentMapperDiagnosticRegistrationID,
527530
RegisterOptions: &lsproto.RegisterOptions{
531+
// Must not set WorkspaceDiagnostics: the client runs one workspace pull per provider
532+
// that asks for it, into that provider's own collection, so a second one would report
533+
// every problem twice. workspaceDiagnosticsRegistrationID is the only provider that
534+
// carries it, and it covers content-mapped files too.
528535
TextDocumentDiagnostic: &lsproto.DiagnosticRegistrationOptions{
529536
DocumentSelector: selector,
530537
Identifier: new("typescript"),
@@ -1780,6 +1787,7 @@ func (s *Server) handleInitialized(ctx context.Context, params *lsproto.Initiali
17801787
return err
17811788
}
17821789
s.session.InitializeWithUserConfig(userPreferences)
1790+
s.syncWorkspaceDiagnosticsRegistration(ctx, userPreferences)
17831791

17841792
_, err = sendClientRequest(ctx, s, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{
17851793
Registrations: []*lsproto.Registration{
@@ -1830,6 +1838,7 @@ func (s *Server) handleDidChangeWorkspaceConfiguration(ctx context.Context, para
18301838
} else if settings, ok := params.Settings.(map[string]any); ok {
18311839
preferences := lsutil.ParseUserPreferences(settings)
18321840
s.session.Configure(preferences)
1841+
s.syncWorkspaceDiagnosticsRegistration(ctx, preferences)
18331842
}
18341843
return nil
18351844
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package lsp_test
2+
3+
import (
4+
"slices"
5+
"testing"
6+
7+
"github.com/microsoft/TypeScript/tsc/internal/bundled"
8+
"github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto"
9+
"github.com/microsoft/TypeScript/tsc/internal/testutil/lsptestutil"
10+
"gotest.tools/v3/assert"
11+
)
12+
13+
// The capability is withheld at initialize and only offered once the setting asks for it, then
14+
// withdrawn when it is turned back off.
15+
func TestWorkspaceDiagnosticsCapabilityFollowsScope(t *testing.T) {
16+
t.Parallel()
17+
18+
if !bundled.Embedded {
19+
t.Skip("bundled files are not embedded")
20+
}
21+
22+
client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles)
23+
24+
// Nothing is offered while the setting sits at its default.
25+
assert.Assert(t, !slices.Contains(progress.registrationIDs(), "workspace-diagnostics"),
26+
"workspace diagnostics should not be registered by default, got %v", progress.registrationIDs())
27+
28+
setWorkspaceDiagnosticsScope(t, client, "allProjects")
29+
openWorkspaceDiagnosticsProject(t, client)
30+
pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}})
31+
assert.Assert(t, slices.Contains(progress.registrationIDs(), "workspace-diagnostics"),
32+
"expected a workspace diagnostics registration, got %v", progress.registrationIDs())
33+
34+
setWorkspaceDiagnosticsScope(t, client, "off")
35+
pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}})
36+
assert.Assert(t, slices.Contains(progress.unregistrationIDs(), "workspace-diagnostics"),
37+
"expected the registration to be withdrawn, got %v", progress.unregistrationIDs())
38+
}
39+
40+
// Moving between two enabled scopes must not churn the registration.
41+
func TestWorkspaceDiagnosticsCapabilityRegisteredOnce(t *testing.T) {
42+
t.Parallel()
43+
44+
if !bundled.Embedded {
45+
t.Skip("bundled files are not embedded")
46+
}
47+
48+
client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles)
49+
setWorkspaceDiagnosticsScope(t, client, "openProjects")
50+
setWorkspaceDiagnosticsScope(t, client, "allProjects")
51+
setWorkspaceDiagnosticsScope(t, client, "openProjectsAndDependents")
52+
openWorkspaceDiagnosticsProject(t, client)
53+
pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}})
54+
55+
progress.mu.Lock()
56+
defer progress.mu.Unlock()
57+
assert.Equal(t, progress.workspaceRegos, 1, "expected exactly one workspace diagnostics registration")
58+
assert.Assert(t, !slices.Contains(progress.unregistered, "workspace-diagnostics"))
59+
}
60+
61+
// Turning validation off silences diagnostics entirely, so the capability is withdrawn rather than
62+
// left in place for a client that would keep pulling the workspace every couple of seconds.
63+
func TestWorkspaceDiagnosticsCapabilityWithdrawnWhenValidationDisabled(t *testing.T) {
64+
t.Parallel()
65+
66+
if !bundled.Embedded {
67+
t.Skip("bundled files are not embedded")
68+
}
69+
70+
client, progress := startWorkspaceDiagnosticsClient(t, workspaceDiagnosticsFiles)
71+
setWorkspaceDiagnosticsScope(t, client, "allProjects")
72+
openWorkspaceDiagnosticsProject(t, client)
73+
pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}})
74+
assert.Assert(t, slices.Contains(progress.registrationIDs(), "workspace-diagnostics"))
75+
76+
// The scope still asks for every project, but validation is off.
77+
lsptestutil.SendNotification(t, client, lsproto.WorkspaceDidChangeConfigurationInfo, &lsproto.DidChangeConfigurationParams{
78+
Settings: map[string]any{"typescript": map[string]any{
79+
"validate": map[string]any{"enabled": false},
80+
"experimental": map[string]any{"workspaceDiagnostics": map[string]any{"scope": "allProjects"}},
81+
}},
82+
})
83+
pullWorkspaceDiagnostics(t, client, &lsproto.WorkspaceDiagnosticParams{PreviousResultIds: []lsproto.PreviousResultId{}})
84+
assert.Assert(t, slices.Contains(progress.unregistrationIDs(), "workspace-diagnostics"),
85+
"expected the registration to be withdrawn, got %v", progress.unregistrationIDs())
86+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package lsp
2+
3+
import (
4+
"context"
5+
6+
"github.com/microsoft/TypeScript/tsc/internal/ls/lsutil"
7+
"github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto"
8+
)
9+
10+
const workspaceDiagnosticsRegistrationID = "workspace-diagnostics"
11+
12+
// syncWorkspaceDiagnosticsRegistration offers workspace diagnostics to the client, or withdraws the
13+
// offer, to match the current settings. Workspace support is a property of a diagnostic provider,
14+
// so it is offered by registering one. A client that holds the capability re-pulls on a timer, so
15+
// withdrawing it matters as much as offering it.
16+
func (s *Server) syncWorkspaceDiagnosticsRegistration(ctx context.Context, preferences lsutil.UserPreferences) {
17+
if !s.clientCapabilities.TextDocument.Diagnostic.DynamicRegistration {
18+
return
19+
}
20+
21+
s.workspaceDiagnosticsRegistrationMu.Lock()
22+
defer s.workspaceDiagnosticsRegistrationMu.Unlock()
23+
24+
// Validation off silences diagnostics whatever the scope says.
25+
wanted := preferences.WorkspaceDiagnosticsScope.Enabled() && !preferences.EnableValidation.IsFalse()
26+
if wanted == s.workspaceDiagnosticsRegistered {
27+
return
28+
}
29+
30+
if !wanted {
31+
if _, err := sendClientRequest(ctx, s, lsproto.ClientUnregisterCapabilityInfo, &lsproto.UnregistrationParams{
32+
Unregisterations: []*lsproto.Unregistration{
33+
{Id: workspaceDiagnosticsRegistrationID, Method: string(lsproto.MethodTextDocumentDiagnostic)},
34+
},
35+
}); err != nil {
36+
s.logger.Error("failed to unregister workspace diagnostics: ", err)
37+
return
38+
}
39+
s.workspaceDiagnosticsRegistered = false
40+
return
41+
}
42+
43+
// The empty document selector is deliberate: document diagnostics are served by the provider
44+
// advertised at initialize, and matching no document keeps this one from pulling them twice.
45+
if _, err := sendClientRequest(ctx, s, lsproto.ClientRegisterCapabilityInfo, &lsproto.RegistrationParams{
46+
Registrations: []*lsproto.Registration{
47+
{
48+
Id: workspaceDiagnosticsRegistrationID,
49+
RegisterOptions: &lsproto.RegisterOptions{
50+
TextDocumentDiagnostic: &lsproto.DiagnosticRegistrationOptions{
51+
DocumentSelector: lsproto.DocumentSelectorOrNull{DocumentSelector: &[]lsproto.TextDocumentFilterLanguageOrSchemeOrPattern{}},
52+
Identifier: new("typescript-workspace"),
53+
InterFileDependencies: true,
54+
WorkspaceDiagnostics: true,
55+
Id: new(workspaceDiagnosticsRegistrationID),
56+
},
57+
},
58+
},
59+
},
60+
}); err != nil {
61+
s.logger.Error("failed to register workspace diagnostics: ", err)
62+
return
63+
}
64+
s.workspaceDiagnosticsRegistered = true
65+
}

0 commit comments

Comments
 (0)