Skip to content

Commit 7e146a6

Browse files
committed
gopls/internal/lsp/cmd: simplify connection type
This change is a refactoring of cmd.connection so that all fields are set once at construction, the Client field is no longer exported, and the dead cmd.Client.Server field is removed. Change-Id: I7b0e8bde6cab37bff2db17415a13492a71b33fef Reviewed-on: https://go-review.googlesource.com/c/tools/+/497496 Reviewed-by: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Alan Donovan <[email protected]>
1 parent 1e60668 commit 7e146a6

File tree

4 files changed

+47
-37
lines changed

4 files changed

+47
-37
lines changed

gopls/internal/lsp/cmd/capabilities_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,16 @@ func TestCapabilities(t *testing.T) {
4141
defer os.RemoveAll(tmpDir)
4242

4343
app := New("gopls-test", tmpDir, os.Environ(), nil)
44-
c := newConnection(app, nil)
45-
ctx := context.Background()
46-
defer c.terminate(ctx)
4744

4845
params := &protocol.ParamInitialize{}
49-
params.RootURI = protocol.URIFromPath(c.Client.app.wd)
46+
params.RootURI = protocol.URIFromPath(app.wd)
5047
params.Capabilities.Workspace.Configuration = true
5148

5249
// Send an initialize request to the server.
53-
c.Server = lsp.NewServer(cache.NewSession(ctx, cache.New(nil), app.options), c.Client)
54-
result, err := c.Server.Initialize(ctx, params)
50+
ctx := context.Background()
51+
client := newClient(app, nil)
52+
server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil), app.options), client)
53+
result, err := server.Initialize(ctx, params)
5554
if err != nil {
5655
t.Fatal(err)
5756
}
@@ -60,10 +59,13 @@ func TestCapabilities(t *testing.T) {
6059
t.Error(err)
6160
}
6261
// Complete initialization of server.
63-
if err := c.Server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
62+
if err := server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
6463
t.Fatal(err)
6564
}
6665

66+
c := newConnection(server, client)
67+
defer c.terminate(ctx)
68+
6769
// Open the file on the server side.
6870
uri := protocol.URIFromPath(tmpFile)
6971
if err := c.Server.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{

gopls/internal/lsp/cmd/check.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ func (c *check) Run(ctx context.Context, args ...string) error {
5757
if err := conn.diagnoseFiles(ctx, uris); err != nil {
5858
return err
5959
}
60-
conn.Client.filesMu.Lock()
61-
defer conn.Client.filesMu.Unlock()
60+
conn.client.filesMu.Lock()
61+
defer conn.client.filesMu.Unlock()
6262

6363
for _, file := range checking {
6464
for _, d := range file.diagnostics {

gopls/internal/lsp/cmd/cmd.go

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,14 @@ var (
293293
func (app *Application) connect(ctx context.Context, onProgress func(*protocol.ProgressParams)) (*connection, error) {
294294
switch {
295295
case app.Remote == "":
296-
connection := newConnection(app, onProgress)
297-
connection.Server = lsp.NewServer(cache.NewSession(ctx, cache.New(nil), app.options), connection.Client)
298-
ctx = protocol.WithClient(ctx, connection.Client)
299-
return connection, connection.initialize(ctx, app.options)
296+
client := newClient(app, onProgress)
297+
server := lsp.NewServer(cache.NewSession(ctx, cache.New(nil), app.options), client)
298+
conn := newConnection(server, client)
299+
if err := conn.initialize(protocol.WithClient(ctx, client), app.options); err != nil {
300+
return nil, err
301+
}
302+
return conn, nil
303+
300304
case strings.HasPrefix(app.Remote, "internal@"):
301305
internalMu.Lock()
302306
defer internalMu.Unlock()
@@ -331,19 +335,19 @@ func CloseTestConnections(ctx context.Context) {
331335
}
332336

333337
func (app *Application) connectRemote(ctx context.Context, remote string) (*connection, error) {
334-
connection := newConnection(app, nil)
335338
conn, err := lsprpc.ConnectToRemote(ctx, remote)
336339
if err != nil {
337340
return nil, err
338341
}
339342
stream := jsonrpc2.NewHeaderStream(conn)
340343
cc := jsonrpc2.NewConn(stream)
341-
connection.Server = protocol.ServerDispatcher(cc)
342-
ctx = protocol.WithClient(ctx, connection.Client)
344+
server := protocol.ServerDispatcher(cc)
345+
client := newClient(app, nil)
346+
connection := newConnection(server, client)
347+
ctx = protocol.WithClient(ctx, connection.client)
343348
cc.Go(ctx,
344349
protocol.Handlers(
345-
protocol.ClientHandler(connection.Client,
346-
jsonrpc2.MethodNotFound)))
350+
protocol.ClientHandler(client, jsonrpc2.MethodNotFound)))
347351
return connection, connection.initialize(ctx, app.options)
348352
}
349353

@@ -355,7 +359,7 @@ var matcherString = map[source.SymbolMatcher]string{
355359

356360
func (c *connection) initialize(ctx context.Context, options func(*source.Options)) error {
357361
params := &protocol.ParamInitialize{}
358-
params.RootURI = protocol.URIFromPath(c.Client.app.wd)
362+
params.RootURI = protocol.URIFromPath(c.client.app.wd)
359363
params.Capabilities.Workspace.Configuration = true
360364

361365
// Make sure to respect configured options when sending initialize request.
@@ -377,7 +381,7 @@ func (c *connection) initialize(ctx context.Context, options func(*source.Option
377381

378382
// If the subcommand has registered a progress handler, report the progress
379383
// capability.
380-
if c.Client.onProgress != nil {
384+
if c.client.onProgress != nil {
381385
params.Capabilities.Window.WorkDoneProgress = true
382386
}
383387

@@ -395,11 +399,10 @@ func (c *connection) initialize(ctx context.Context, options func(*source.Option
395399

396400
type connection struct {
397401
protocol.Server
398-
Client *cmdClient
402+
client *cmdClient
399403
}
400404

401405
type cmdClient struct {
402-
protocol.Server
403406
app *Application
404407
onProgress func(*protocol.ProgressParams)
405408

@@ -417,13 +420,18 @@ type cmdFile struct {
417420
diagnostics []protocol.Diagnostic
418421
}
419422

420-
func newConnection(app *Application, onProgress func(*protocol.ProgressParams)) *connection {
423+
func newClient(app *Application, onProgress func(*protocol.ProgressParams)) *cmdClient {
424+
return &cmdClient{
425+
app: app,
426+
onProgress: onProgress,
427+
files: make(map[span.URI]*cmdFile),
428+
}
429+
}
430+
431+
func newConnection(server protocol.Server, client *cmdClient) *connection {
421432
return &connection{
422-
Client: &cmdClient{
423-
app: app,
424-
onProgress: onProgress,
425-
files: make(map[span.URI]*cmdFile),
426-
},
433+
Server: server,
434+
client: client,
427435
}
428436
}
429437

@@ -611,7 +619,7 @@ func (c *cmdClient) openFile(ctx context.Context, uri span.URI) *cmdFile {
611619
// - map a (URI, protocol.Range) to a MappedRange;
612620
// - parse a command-line argument to a MappedRange.
613621
func (c *connection) openFile(ctx context.Context, uri span.URI) (*cmdFile, error) {
614-
file := c.Client.openFile(ctx, uri)
622+
file := c.client.openFile(ctx, uri)
615623
if file.err != nil {
616624
return nil, file.err
617625
}
@@ -646,22 +654,22 @@ func (c *connection) diagnoseFiles(ctx context.Context, files []span.URI) error
646654
for _, file := range files {
647655
untypedFiles = append(untypedFiles, string(file))
648656
}
649-
c.Client.diagnosticsMu.Lock()
650-
defer c.Client.diagnosticsMu.Unlock()
657+
c.client.diagnosticsMu.Lock()
658+
defer c.client.diagnosticsMu.Unlock()
651659

652-
c.Client.diagnosticsDone = make(chan struct{})
660+
c.client.diagnosticsDone = make(chan struct{})
653661
_, err := c.Server.NonstandardRequest(ctx, "gopls/diagnoseFiles", map[string]interface{}{"files": untypedFiles})
654662
if err != nil {
655-
close(c.Client.diagnosticsDone)
663+
close(c.client.diagnosticsDone)
656664
return err
657665
}
658666

659-
<-c.Client.diagnosticsDone
667+
<-c.client.diagnosticsDone
660668
return nil
661669
}
662670

663671
func (c *connection) terminate(ctx context.Context) {
664-
if strings.HasPrefix(c.Client.app.Remote, "internal@") {
672+
if strings.HasPrefix(c.client.app.Remote, "internal@") {
665673
// internal connections need to be left alive for the next test
666674
return
667675
}

gopls/internal/lsp/cmd/suggested_fix.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ func (s *suggestedFix) Run(ctx context.Context, args ...string) error {
7171
return err
7272
}
7373
diagnostics := []protocol.Diagnostic{} // LSP wants non-nil slice
74-
conn.Client.filesMu.Lock()
74+
conn.client.filesMu.Lock()
7575
diagnostics = append(diagnostics, file.diagnostics...)
76-
conn.Client.filesMu.Unlock()
76+
conn.client.filesMu.Unlock()
7777

7878
// Request code actions
7979
codeActionKinds := []protocol.CodeActionKind{protocol.QuickFix}

0 commit comments

Comments
 (0)