Skip to content

Commit 5657a42

Browse files
committed
pkg/lsp: refactor diagnostics
1 parent 9d57ec0 commit 5657a42

4 files changed

Lines changed: 101 additions & 38 deletions

File tree

cmd/run.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ var runCmd = &cobra.Command{
9898

9999
fileManager := files.NewFileManager(gitignore.NewMatcherFactory())
100100
languageDetector := lsp.NewLanguageDetector()
101-
diagnosticsStore := lsp.NewDiagnosticsStore()
101+
diagnosticsService := lsp.NewDiagnosticsService()
102102
clientPool := lsp.NewClientPool()
103-
lspService := lsp.NewService(languageDetector, lsp.LspServerExecutables, diagnosticsStore, clientPool)
103+
lspService := lsp.NewService(languageDetector, lsp.LspServerExecutables, diagnosticsService, clientPool)
104104
projectManager := project.NewProjectManager(containerRunner, projectStore, projectsDir, fileManager, lspService, languageDetector, random.String)
105105
validator := validator.New(validator.WithRequiredStructEnabled())
106106

pkg/lsp/client.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66

7+
"github.com/rs/zerolog/log"
78
"github.com/sourcegraph/jsonrpc2"
89

910
protocol "github.com/tliron/glsp/protocol_3_16"
@@ -44,6 +45,7 @@ type ClientImpl struct {
4445
func NewClient(server Process, diagnosticsChannel chan protocol.PublishDiagnosticsParams) Client {
4546
handler := &lspHandler{
4647
diagnosticsHandler: func(params protocol.PublishDiagnosticsParams) {
48+
log.Debug().Msgf("Handling diagnostics: %+v", params)
4749
diagnosticsChannel <- params
4850
},
4951
}
@@ -83,6 +85,5 @@ func (c *ClientImpl) NotifyDidClose(ctx context.Context, params protocol.DidClos
8385

8486
func (c *ClientImpl) Shutdown(ctx context.Context) error {
8587
err := c.server.Stop()
86-
close(c.diagnosticsChannel)
8788
return err
8889
}

pkg/lsp/diagnostics.go

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,36 @@
11
package lsp
22

33
import (
4+
"context"
45
"sync"
56

7+
"github.com/rs/zerolog/log"
68
protocol "github.com/tliron/glsp/protocol_3_16"
79
)
810

11+
// TODO: update docs
912
// In memory store for diagnostics. Applies mutex locking for concurrent access.
10-
type DiagnosticsStore struct {
13+
type DiagnosticsService struct {
1114
diagnostics map[ProjectId]map[protocol.DocumentUri][]protocol.Diagnostic
12-
mu sync.Mutex
15+
listeners map[ProjectId]map[LanguageId]*listenerInfo
16+
mu sync.RWMutex
1317
}
1418

15-
func (d *DiagnosticsStore) Get(projectId ProjectId, uri protocol.DocumentUri) ([]protocol.Diagnostic, bool) {
16-
d.mu.Lock()
17-
defer d.mu.Unlock()
19+
type listenerInfo struct {
20+
cancel context.CancelFunc
21+
channel chan protocol.PublishDiagnosticsParams
22+
}
23+
24+
func NewDiagnosticsService() *DiagnosticsService {
25+
return &DiagnosticsService{
26+
diagnostics: make(map[ProjectId]map[protocol.DocumentUri][]protocol.Diagnostic),
27+
listeners: make(map[ProjectId]map[LanguageId]*listenerInfo),
28+
}
29+
}
30+
31+
func (d *DiagnosticsService) Get(projectId ProjectId, uri protocol.DocumentUri) ([]protocol.Diagnostic, bool) {
32+
d.mu.RLock()
33+
defer d.mu.RUnlock()
1834

1935
if diagnostics, ok := d.diagnostics[projectId]; ok {
2036
if diagnostics, ok := diagnostics[uri]; ok {
@@ -25,7 +41,7 @@ func (d *DiagnosticsStore) Get(projectId ProjectId, uri protocol.DocumentUri) ([
2541
return nil, false
2642
}
2743

28-
func (d *DiagnosticsStore) Set(projectId ProjectId, uri protocol.DocumentUri, diagnostics []protocol.Diagnostic) {
44+
func (d *DiagnosticsService) set(projectId ProjectId, uri protocol.DocumentUri, diagnostics []protocol.Diagnostic) {
2945
d.mu.Lock()
3046
defer d.mu.Unlock()
3147

@@ -36,15 +52,76 @@ func (d *DiagnosticsStore) Set(projectId ProjectId, uri protocol.DocumentUri, di
3652
d.diagnostics[projectId][uri] = diagnostics
3753
}
3854

39-
func (d *DiagnosticsStore) DeleteAllForProject(projectId ProjectId) {
55+
func (d *DiagnosticsService) StartListener(projectId ProjectId, languageId LanguageId, channel chan protocol.PublishDiagnosticsParams) {
4056
d.mu.Lock()
4157
defer d.mu.Unlock()
4258

43-
delete(d.diagnostics, projectId)
59+
if _, exists := d.listeners[projectId]; !exists {
60+
d.listeners[projectId] = make(map[LanguageId]*listenerInfo)
61+
}
62+
63+
// TODO: should we cancel or return error?
64+
// Cancel existing listener if any
65+
if info, exists := d.listeners[projectId][languageId]; exists {
66+
info.cancel()
67+
close(info.channel)
68+
}
69+
70+
ctx, cancel := context.WithCancel(context.Background())
71+
d.listeners[projectId][languageId] = &listenerInfo{
72+
cancel: cancel,
73+
channel: channel,
74+
}
75+
76+
go d.listen(ctx, projectId, languageId, channel)
4477
}
4578

46-
func NewDiagnosticsStore() *DiagnosticsStore {
47-
return &DiagnosticsStore{
48-
diagnostics: make(map[ProjectId]map[protocol.DocumentUri][]protocol.Diagnostic),
79+
func (d *DiagnosticsService) StopListener(projectId ProjectId, languageId LanguageId) {
80+
d.mu.Lock()
81+
defer d.mu.Unlock()
82+
83+
if projectListeners, exists := d.listeners[projectId]; exists {
84+
if info, exists := projectListeners[languageId]; exists {
85+
info.cancel()
86+
close(info.channel)
87+
delete(projectListeners, languageId)
88+
}
89+
if len(projectListeners) == 0 {
90+
delete(d.listeners, projectId)
91+
}
4992
}
5093
}
94+
95+
func (d *DiagnosticsService) listen(ctx context.Context, projectId ProjectId, languageId LanguageId, channel chan protocol.PublishDiagnosticsParams) {
96+
for {
97+
select {
98+
case <-ctx.Done():
99+
return
100+
case diagnostics, ok := <-channel:
101+
if !ok {
102+
return
103+
}
104+
d.set(projectId, diagnostics.URI, diagnostics.Diagnostics)
105+
log.Debug().
106+
Str("projectId", string(projectId)).
107+
Str("languageId", string(languageId)).
108+
Str("uri", string(diagnostics.URI)).
109+
Msgf("Received diagnostics: %d", len(diagnostics.Diagnostics))
110+
}
111+
}
112+
}
113+
114+
func (d *DiagnosticsService) DeleteAllForProject(projectId ProjectId) {
115+
d.mu.Lock()
116+
defer d.mu.Unlock()
117+
118+
delete(d.diagnostics, projectId)
119+
if projectListeners, exists := d.listeners[projectId]; exists {
120+
for _, info := range projectListeners {
121+
info.cancel()
122+
close(info.channel)
123+
}
124+
delete(d.listeners, projectId)
125+
}
126+
}
127+

pkg/lsp/service.go

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type Service interface {
3939
type ServiceImpl struct {
4040
languageDetector LanguageDetector
4141
clientPool ClientPool
42-
diagnosticsStore *DiagnosticsStore
42+
diagnosticsService *DiagnosticsService
4343
lspServerExecutables map[LanguageId]Command
4444
}
4545

@@ -115,9 +115,7 @@ func (s *ServiceImpl) StartServer(ctx context.Context, languageId LanguageId) er
115115
}
116116

117117
s.clientPool.Set(projectId, languageId, client)
118-
119-
// TODO: kill this goroutine when the project is deleted
120-
go s.listenForDiagnostics(projectId, diagnosticsChannel)
118+
s.diagnosticsService.StartListener(projectId, languageId, diagnosticsChannel)
121119
return nil
122120
}
123121

@@ -141,6 +139,9 @@ func (s *ServiceImpl) StopServer(ctx context.Context, languageId LanguageId) err
141139
}
142140

143141
s.clientPool.Delete(project.Id, languageId)
142+
s.diagnosticsService.StopListener(project.Id, languageId)
143+
// TODO: do we need this?
144+
// s.diagnosticsManager.DeleteAllForLanguage(project.Id, languageId)
144145

145146
return nil
146147
}
@@ -280,7 +281,7 @@ func (s *ServiceImpl) GetDiagnostics(ctx context.Context, file model.File) ([]pr
280281
}
281282

282283
uri := PathToURI(filepath.Join(project.Path, file.Path))
283-
if diagnostics, ok := s.diagnosticsStore.Get(project.Id, uri); ok {
284+
if diagnostics, ok := s.diagnosticsService.Get(project.Id, uri); ok {
284285
return diagnostics, nil
285286
}
286287

@@ -300,7 +301,7 @@ func (s *ServiceImpl) CleanupProject(ctx context.Context, projectId ProjectId) e
300301
}
301302

302303
s.clientPool.DeleteAllForProject(projectId)
303-
s.diagnosticsStore.DeleteAllForProject(projectId)
304+
s.diagnosticsService.DeleteAllForProject(projectId)
304305
return nil
305306
}
306307

@@ -336,31 +337,15 @@ func (s *ServiceImpl) getClients(ctx context.Context) []Client {
336337
return clients
337338
}
338339

339-
func (s *ServiceImpl) listenForDiagnostics(projectId ProjectId, channel chan protocol.PublishDiagnosticsParams) {
340-
for {
341-
select {
342-
case diagnostics := <-channel:
343-
log.Debug().Str("projectId", projectId).Str("uri", diagnostics.URI).Msg("Received diagnostics")
344-
log.Debug().Str("projectId", projectId).Str("uri", diagnostics.URI).Msgf("Diagnostics: %+v", diagnostics.Diagnostics)
345-
346-
s.updateDiagnostics(projectId, diagnostics)
347-
}
348-
}
349-
}
350-
351-
func (s *ServiceImpl) updateDiagnostics(projectId ProjectId, diagnostics protocol.PublishDiagnosticsParams) {
352-
s.diagnosticsStore.Set(projectId, diagnostics.URI, diagnostics.Diagnostics)
353-
}
354-
355340
func PathToURI(path string) protocol.DocumentUri {
356341
return protocol.DocumentUri("file://" + path)
357342
}
358343

359-
func NewService(languageDetector LanguageDetector, lspServerExecutables map[LanguageId]Command, diagnosticsStore *DiagnosticsStore, clientPool ClientPool) Service {
344+
func NewService(languageDetector LanguageDetector, lspServerExecutables map[LanguageId]Command, diagnosticsService *DiagnosticsService, clientPool ClientPool) Service {
360345
return &ServiceImpl{
361346
languageDetector: languageDetector,
362347
clientPool: clientPool,
363-
diagnosticsStore: diagnosticsStore,
348+
diagnosticsService: diagnosticsService,
364349
lspServerExecutables: lspServerExecutables,
365350
}
366351
}

0 commit comments

Comments
 (0)