Skip to content

Commit 78d0674

Browse files
committed
internal/lsp: remove the Context argument from NewSession
The passed-in Context is not used, and creates the illusion of a startup dependency problem: existing code is careful to pass in the context containing the correct Client instance. This allows passing in a source.Session, rather than a source.Cache, into lsp server constructors. Updates golang/go#34111 Change-Id: I081ad6fa800b846b63e04d7164577e3a32966704 Reviewed-on: https://go-review.googlesource.com/c/tools/+/215740 Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Ian Cottrell <iancottrell@google.com>
1 parent 29f64ef commit 78d0674

File tree

10 files changed

+19
-18
lines changed

10 files changed

+19
-18
lines changed

internal/lsp/cache/cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (c *cache) GetFile(uri span.URI) source.FileHandle {
7373
}
7474
}
7575

76-
func (c *cache) NewSession(ctx context.Context) source.Session {
76+
func (c *cache) NewSession() source.Session {
7777
index := atomic.AddInt64(&sessionIndex, 1)
7878
s := &session{
7979
cache: c,

internal/lsp/cmd/capabilities_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestCapabilities(t *testing.T) {
4040
params.Capabilities.Workspace.Configuration = true
4141

4242
// Send an initialize request to the server.
43-
ctx, c.Server = lsp.NewClientServer(ctx, cache.New(app.options), c.Client)
43+
ctx, c.Server = lsp.NewClientServer(ctx, cache.New(app.options).NewSession(), c.Client)
4444
result, err := c.Server.Initialize(ctx, params)
4545
if err != nil {
4646
t.Fatal(err)

internal/lsp/cmd/cmd.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) {
192192
switch app.Remote {
193193
case "":
194194
connection := newConnection(app)
195-
ctx, connection.Server = lsp.NewClientServer(ctx, cache.New(app.options), connection.Client)
195+
ctx, connection.Server = lsp.NewClientServer(ctx, cache.New(app.options).NewSession(), connection.Client)
196196
return connection, connection.initialize(ctx, app.options)
197197
case "internal":
198198
internalMu.Lock()
@@ -208,7 +208,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) {
208208
ctx, jc, connection.Server = protocol.NewClient(ctx, jsonrpc2.NewHeaderStream(cr, cw), connection.Client)
209209
go jc.Run(ctx)
210210
go func() {
211-
ctx, srv := lsp.NewServer(ctx, cache.New(app.options), jsonrpc2.NewHeaderStream(sr, sw))
211+
ctx, srv := lsp.NewServer(ctx, cache.New(app.options).NewSession(), jsonrpc2.NewHeaderStream(sr, sw))
212212
srv.Run(ctx)
213213
}()
214214
if err := connection.initialize(ctx, app.options); err != nil {

internal/lsp/cmd/serve.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (s *Serve) Run(ctx context.Context, args ...string) error {
9797
if s.Trace {
9898
stream = protocol.LoggingStream(stream, out)
9999
}
100-
ctx, srv := lsp.NewServer(ctx, cache.New(s.app.options), stream)
100+
ctx, srv := lsp.NewServer(ctx, cache.New(s.app.options).NewSession(), stream)
101101
return prepare(ctx, srv).Run(ctx)
102102
}
103103

internal/lsp/cmd/test/cmdtest.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ func (r *runner) RunGoplsCmd(t testing.TB, args ...string) (string, string) {
140140
return string(stdout), string(stderr)
141141
}
142142

143+
// NormalizeGoplsCmd runs the gopls command and normalizes its output.
143144
func (r *runner) NormalizeGoplsCmd(t testing.TB, args ...string) (string, string) {
144145
stdout, stderr := r.RunGoplsCmd(t, args...)
145146
return r.Normalize(stdout), r.Normalize(stderr)

internal/lsp/lsp_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
5252
defer data.Exported.Cleanup()
5353

5454
cache := cache.New(nil)
55-
session := cache.NewSession(ctx)
55+
session := cache.NewSession()
5656
options := tests.DefaultOptions()
5757
session.SetOptions(options)
5858
options.Env = data.Config.Env
@@ -928,7 +928,7 @@ func TestModfileSuggestedFixes(t *testing.T) {
928928

929929
ctx := tests.Context(t)
930930
cache := cache.New(nil)
931-
session := cache.NewSession(ctx)
931+
session := cache.NewSession()
932932
options := tests.DefaultOptions()
933933
options.TempModfile = true
934934
options.Env = append(os.Environ(), "GOPACKAGESDRIVER=off", "GOROOT=")

internal/lsp/mod/mod_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestMain(m *testing.M) {
2929
func TestModfileRemainsUnchanged(t *testing.T) {
3030
ctx := tests.Context(t)
3131
cache := cache.New(nil)
32-
session := cache.NewSession(ctx)
32+
session := cache.NewSession()
3333
options := tests.DefaultOptions()
3434
options.TempModfile = true
3535
options.Env = append(os.Environ(), "GOPACKAGESDRIVER=off", "GOROOT=")
@@ -67,7 +67,7 @@ func TestModfileRemainsUnchanged(t *testing.T) {
6767
func TestDiagnostics(t *testing.T) {
6868
ctx := tests.Context(t)
6969
cache := cache.New(nil)
70-
session := cache.NewSession(ctx)
70+
session := cache.NewSession()
7171
options := tests.DefaultOptions()
7272
options.TempModfile = true
7373
options.Env = append(os.Environ(), "GOPACKAGESDRIVER=off", "GOROOT=")

internal/lsp/server.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,23 @@ import (
1818
)
1919

2020
// NewClientServer
21-
func NewClientServer(ctx context.Context, cache source.Cache, client protocol.Client) (context.Context, *Server) {
21+
func NewClientServer(ctx context.Context, session source.Session, client protocol.Client) (context.Context, *Server) {
2222
ctx = protocol.WithClient(ctx, client)
2323
return ctx, &Server{
2424
client: client,
25-
session: cache.NewSession(ctx),
25+
session: session,
2626
delivered: make(map[span.URI]sentDiagnostics),
2727
}
2828
}
2929

30-
// NewServer starts an LSP server on the supplied stream, and waits until the
31-
// stream is closed.
32-
func NewServer(ctx context.Context, cache source.Cache, stream jsonrpc2.Stream) (context.Context, *Server) {
30+
// NewServer creates an LSP server and binds it to handle incoming client
31+
// messages on on the supplied stream.
32+
func NewServer(ctx context.Context, session source.Session, stream jsonrpc2.Stream) (context.Context, *Server) {
3333
s := &Server{
3434
delivered: make(map[span.URI]sentDiagnostics),
35+
session: session,
3536
}
3637
ctx, s.Conn, s.client = protocol.NewServer(ctx, stream, s)
37-
s.session = cache.NewSession(ctx)
3838
return ctx, s
3939
}
4040

@@ -56,7 +56,7 @@ func RunServerOnAddress(ctx context.Context, cache source.Cache, addr string, h
5656
if err != nil {
5757
return err
5858
}
59-
h(NewServer(ctx, cache, jsonrpc2.NewHeaderStream(conn, conn)))
59+
h(NewServer(ctx, cache.NewSession(), jsonrpc2.NewHeaderStream(conn, conn)))
6060
}
6161
}
6262

internal/lsp/source/source_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
4848
defer data.Exported.Cleanup()
4949

5050
cache := cache.New(nil)
51-
session := cache.NewSession(ctx)
51+
session := cache.NewSession()
5252
options := tests.DefaultOptions()
5353
options.Env = data.Config.Env
5454
view, _, err := session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), options)

internal/lsp/source/view.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ type Cache interface {
215215
FileSystem
216216

217217
// NewSession creates a new Session manager and returns it.
218-
NewSession(ctx context.Context) Session
218+
NewSession() Session
219219

220220
// FileSet returns the shared fileset used by all files in the system.
221221
FileSet() *token.FileSet

0 commit comments

Comments
 (0)