From b9584148efcb10d08933a70e48b8f7b0732a1b47 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Wed, 15 May 2019 12:24:49 -0400 Subject: [PATCH] internal/lsp: add structured layers to the cache This is primarily to separate the levels because they have different cache lifetimes and sharability. This will allow us to share results between views and even between servers. Change-Id: I280ca19d17a6ea8a15e48637d4445e2b6cf04769 Reviewed-on: https://go-review.googlesource.com/c/tools/+/177518 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- cmd/gopls/main.go | 2 +- internal/lsp/cache/cache.go | 24 ++++ internal/lsp/cache/check.go | 10 +- internal/lsp/cache/file.go | 2 +- internal/lsp/cache/parse.go | 2 +- internal/lsp/cache/session.go | 153 +++++++++++++++++++++++ internal/lsp/cache/view.go | 40 ++---- internal/lsp/cmd/cmd.go | 20 ++- internal/lsp/cmd/cmd_test.go | 4 +- internal/lsp/cmd/definition_test.go | 2 +- internal/lsp/cmd/format_test.go | 2 +- internal/lsp/cmd/serve.go | 6 +- internal/lsp/code_action.go | 2 +- internal/lsp/completion.go | 8 +- internal/lsp/definition.go | 4 +- internal/lsp/diagnostics.go | 4 +- internal/lsp/format.go | 2 +- internal/lsp/general.go | 12 +- internal/lsp/highlight.go | 2 +- internal/lsp/hover.go | 2 +- internal/lsp/link.go | 2 +- internal/lsp/lsp_test.go | 15 +-- internal/lsp/server.go | 25 ++-- internal/lsp/signature_help.go | 4 +- internal/lsp/source/completion_format.go | 2 +- internal/lsp/source/diagnostics.go | 10 +- internal/lsp/source/source_test.go | 6 +- internal/lsp/source/view.go | 40 +++++- internal/lsp/symbols.go | 2 +- internal/lsp/text_synchronization.go | 6 +- internal/lsp/workspace.go | 78 ++---------- 31 files changed, 314 insertions(+), 179 deletions(-) create mode 100644 internal/lsp/cache/cache.go create mode 100644 internal/lsp/cache/session.go diff --git a/cmd/gopls/main.go b/cmd/gopls/main.go index 7b708ab9..26d32d06 100644 --- a/cmd/gopls/main.go +++ b/cmd/gopls/main.go @@ -17,5 +17,5 @@ import ( ) func main() { - tool.Main(context.Background(), &cmd.Application{}, os.Args[1:]) + tool.Main(context.Background(), cmd.New(nil), os.Args[1:]) } diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go new file mode 100644 index 00000000..71db06b6 --- /dev/null +++ b/internal/lsp/cache/cache.go @@ -0,0 +1,24 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cache + +import ( + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/xlog" +) + +func New() source.Cache { + return &cache{} +} + +type cache struct { +} + +func (c *cache) NewSession(log xlog.Logger) source.Session { + return &session{ + cache: c, + log: log, + } +} diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 90eb5a55..b2d12526 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -138,7 +138,7 @@ func (v *view) link(ctx context.Context, pkgPath string, pkg *packages.Package, if f, _ := v.getFile(span.FileURI(filename)); f != nil { gof, ok := f.(*goFile) if !ok { - v.Logger().Errorf(ctx, "not a go file: %v", f.URI()) + v.Session().Logger().Errorf(ctx, "not a go file: %v", f.URI()) continue } gof.meta = m @@ -270,23 +270,23 @@ func (v *view) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) { for _, file := range pkg.GetSyntax() { // TODO: If a file is in multiple packages, which package do we store? if !file.Pos().IsValid() { - v.Logger().Errorf(ctx, "invalid position for file %v", file.Name) + v.Session().Logger().Errorf(ctx, "invalid position for file %v", file.Name) continue } tok := v.config.Fset.File(file.Pos()) if tok == nil { - v.Logger().Errorf(ctx, "no token.File for %v", file.Name) + v.Session().Logger().Errorf(ctx, "no token.File for %v", file.Name) continue } fURI := span.FileURI(tok.Name()) f, err := v.getFile(fURI) if err != nil { - v.Logger().Errorf(ctx, "no file: %v", err) + v.Session().Logger().Errorf(ctx, "no file: %v", err) continue } gof, ok := f.(*goFile) if !ok { - v.Logger().Errorf(ctx, "not a go file: %v", f.URI()) + v.Session().Logger().Errorf(ctx, "not a go file: %v", f.URI()) continue } gof.token = tok diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 3711e96e..9aecfa4a 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -147,7 +147,7 @@ func (f *fileBase) read(ctx context.Context) { // We don't know the content yet, so read it. content, err := ioutil.ReadFile(f.filename()) if err != nil { - f.view.Logger().Errorf(ctx, "unable to read file %s: %v", f.filename(), err) + f.view.Session().Logger().Errorf(ctx, "unable to read file %s: %v", f.filename(), err) return } f.content = content diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index b8264b50..c9876ba6 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -152,7 +152,7 @@ func (v *view) fix(ctx context.Context, file *ast.File, tok *token.File, src []b switch n := n.(type) { case *ast.BadStmt: if err := v.parseDeferOrGoStmt(n, parent, tok, src); err != nil { - v.log.Debugf(ctx, "unable to parse defer or go from *ast.BadStmt: %v", err) + v.Session().Logger().Debugf(ctx, "unable to parse defer or go from *ast.BadStmt: %v", err) } return false default: diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go new file mode 100644 index 00000000..7335b617 --- /dev/null +++ b/internal/lsp/cache/session.go @@ -0,0 +1,153 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cache + +import ( + "context" + "fmt" + "strings" + "sync" + + "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/lsp/xlog" + "golang.org/x/tools/internal/span" +) + +type session struct { + cache *cache + // the logger to use to communicate back with the client + log xlog.Logger + + viewMu sync.Mutex + views []*view + viewMap map[span.URI]source.View +} + +func (s *session) Shutdown(ctx context.Context) { + s.viewMu.Lock() + defer s.viewMu.Unlock() + for _, view := range s.views { + view.shutdown(ctx) + } + s.views = nil + s.viewMap = nil +} + +func (s *session) Cache() source.Cache { + return s.cache +} + +func (s *session) NewView(name string, folder span.URI, config *packages.Config) source.View { + s.viewMu.Lock() + defer s.viewMu.Unlock() + ctx := context.Background() + backgroundCtx, cancel := context.WithCancel(ctx) + v := &view{ + session: s, + baseCtx: ctx, + backgroundCtx: backgroundCtx, + builtinPkg: builtinPkg(*config), + cancel: cancel, + config: *config, + name: name, + folder: folder, + filesByURI: make(map[span.URI]viewFile), + filesByBase: make(map[string][]viewFile), + contentChanges: make(map[span.URI]func()), + mcache: &metadataCache{ + packages: make(map[string]*metadata), + }, + pcache: &packageCache{ + packages: make(map[string]*entry), + }, + } + s.views = append(s.views, v) + // we always need to drop the view map + s.viewMap = make(map[span.URI]source.View) + return v +} + +// View returns the view by name. +func (s *session) View(name string) source.View { + s.viewMu.Lock() + defer s.viewMu.Unlock() + for _, view := range s.views { + if view.Name() == name { + return view + } + } + return nil +} + +// ViewOf returns a view corresponding to the given URI. +// If the file is not already associated with a view, pick one using some heuristics. +func (s *session) ViewOf(uri span.URI) source.View { + s.viewMu.Lock() + defer s.viewMu.Unlock() + + // check if we already know this file + if v, found := s.viewMap[uri]; found { + return v + } + + // pick the best view for this file and memoize the result + v := s.bestView(uri) + s.viewMap[uri] = v + return v +} + +func (s *session) Views() []source.View { + s.viewMu.Lock() + defer s.viewMu.Unlock() + result := make([]source.View, len(s.views)) + for i, v := range s.views { + result[i] = v + } + return result +} + +// bestView finds the best view to associate a given URI with. +// viewMu must be held when calling this method. +func (s *session) bestView(uri span.URI) source.View { + // we need to find the best view for this file + var longest source.View + for _, view := range s.views { + if longest != nil && len(longest.Folder()) > len(view.Folder()) { + continue + } + if strings.HasPrefix(string(uri), string(view.Folder())) { + longest = view + } + } + if longest != nil { + return longest + } + //TODO: are there any more heuristics we can use? + return s.views[0] +} + +func (s *session) removeView(ctx context.Context, view *view) error { + s.viewMu.Lock() + defer s.viewMu.Unlock() + // we always need to drop the view map + s.viewMap = make(map[span.URI]source.View) + for i, v := range s.views { + if view == v { + // delete this view... we don't care about order but we do want to make + // sure we can garbage collect the view + s.views[i] = s.views[len(s.views)-1] + s.views[len(s.views)-1] = nil + s.views = s.views[:len(s.views)-1] + v.shutdown(ctx) + return nil + } + } + return fmt.Errorf("view %s for %v not found", view.Name(), view.Folder()) +} + +func (s *session) Logger() xlog.Logger { + return s.log +} diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 15c054e7..cf4227c8 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -16,11 +16,12 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/xlog" "golang.org/x/tools/internal/span" ) type view struct { + session *session + // mu protects all mutable state of the view. mu sync.Mutex @@ -36,9 +37,6 @@ type view struct { // should be stopped. cancel context.CancelFunc - // the logger to use to communicate back with the client - log xlog.Logger - // Name is the user visible name of this view. name string @@ -94,28 +92,8 @@ type entry struct { ready chan struct{} // closed to broadcast ready condition } -func NewView(ctx context.Context, log xlog.Logger, name string, folder span.URI, config *packages.Config) source.View { - backgroundCtx, cancel := context.WithCancel(ctx) - v := &view{ - baseCtx: ctx, - backgroundCtx: backgroundCtx, - builtinPkg: builtinPkg(*config), - cancel: cancel, - log: log, - config: *config, - name: name, - folder: folder, - filesByURI: make(map[span.URI]viewFile), - filesByBase: make(map[string][]viewFile), - contentChanges: make(map[span.URI]func()), - mcache: &metadataCache{ - packages: make(map[string]*metadata), - }, - pcache: &packageCache{ - packages: make(map[string]*entry), - }, - } - return v +func (v *view) Session() source.Session { + return v.session } // Name returns the user visible name of this view. @@ -138,7 +116,11 @@ func (v *view) SetEnv(env []string) { v.config.Env = env } -func (v *view) Shutdown(context.Context) { +func (v *view) Shutdown(ctx context.Context) { + v.session.removeView(ctx, v) +} + +func (v *view) shutdown(context.Context) { v.mu.Lock() defer v.mu.Unlock() if v.cancel != nil { @@ -393,7 +375,3 @@ func (v *view) mapFile(uri span.URI, f viewFile) { v.filesByBase[basename] = append(v.filesByBase[basename], f) } } - -func (v *view) Logger() xlog.Logger { - return v.log -} diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index 8be04fde..00ccd029 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -24,7 +24,9 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp" + "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/tool" ) @@ -45,6 +47,9 @@ type Application struct { // An initial, common go/packages configuration Config packages.Config + // The base cache to use for sessions from this application. + Cache source.Cache + // Support for remote lsp server Remote string `flag:"remote" help:"*EXPERIMENTAL* - forward all commands to a remote lsp"` @@ -52,6 +57,17 @@ type Application struct { Verbose bool `flag:"v" help:"Verbose output"` } +// Returns a new Application ready to run. +func New(config *packages.Config) *Application { + app := &Application{ + Cache: cache.New(), + } + if config != nil { + app.Config = *config + } + return app +} + // Name implements tool.Application returning the binary name. func (app *Application) Name() string { return "gopls" } @@ -135,7 +151,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) { switch app.Remote { case "": connection := newConnection(app) - connection.Server = lsp.NewClientServer(connection.Client) + connection.Server = lsp.NewClientServer(app.Cache, connection.Client) return connection, connection.initialize(ctx) case "internal": internalMu.Lock() @@ -150,7 +166,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) { var jc *jsonrpc2.Conn jc, connection.Server, _ = protocol.NewClient(jsonrpc2.NewHeaderStream(cr, cw), connection.Client) go jc.Run(ctx) - go lsp.NewServer(jsonrpc2.NewHeaderStream(sr, sw)).Run(ctx) + go lsp.NewServer(app.Cache, jsonrpc2.NewHeaderStream(sr, sw)).Run(ctx) if err := connection.initialize(ctx); err != nil { return nil, err } diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index 316b46e7..5b8b4c50 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -37,9 +37,7 @@ func testCommandLine(t *testing.T, exporter packagestest.Exporter) { r := &runner{ exporter: exporter, data: data, - app: &cmd.Application{ - Config: *data.Exported.Config, - }, + app: cmd.New(data.Exported.Config), } tests.Run(t, r, data) } diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index 0f298c4f..033c4185 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/internal/lsp/cmd/definition_test.go @@ -54,7 +54,7 @@ func TestDefinitionHelpExample(t *testing.T) { fmt.Sprintf("%v:#%v", thisFile, cmd.ExampleOffset)} { args := append(baseArgs, query) got := captureStdOut(t, func() { - tool.Main(context.Background(), &cmd.Application{}, args) + tool.Main(context.Background(), cmd.New(nil), args) }) if !expect.MatchString(got) { t.Errorf("test with %v\nexpected:\n%s\ngot:\n%s", args, expect, got) diff --git a/internal/lsp/cmd/format_test.go b/internal/lsp/cmd/format_test.go index 5674fa4c..981ebdb6 100644 --- a/internal/lsp/cmd/format_test.go +++ b/internal/lsp/cmd/format_test.go @@ -41,7 +41,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { //TODO: our error handling differs, for now just skip unformattable files continue } - app := &cmd.Application{} + app := cmd.New(nil) app.Config = r.data.Config got := captureStdOut(t, func() { tool.Main(context.Background(), app, append([]string{"-remote=internal", "format"}, args...)) diff --git a/internal/lsp/cmd/serve.go b/internal/lsp/cmd/serve.go index 2cc1de20..ddac3568 100644 --- a/internal/lsp/cmd/serve.go +++ b/internal/lsp/cmd/serve.go @@ -79,13 +79,13 @@ func (s *Serve) Run(ctx context.Context, args ...string) error { go srv.Conn.Run(ctx) } if s.Address != "" { - return lsp.RunServerOnAddress(ctx, s.Address, run) + return lsp.RunServerOnAddress(ctx, s.app.Cache, s.Address, run) } if s.Port != 0 { - return lsp.RunServerOnPort(ctx, s.Port, run) + return lsp.RunServerOnPort(ctx, s.app.Cache, s.Port, run) } stream := jsonrpc2.NewHeaderStream(os.Stdin, os.Stdout) - srv := lsp.NewServer(stream) + srv := lsp.NewServer(s.app.Cache, stream) srv.Conn.Logger = logger(s.Trace, out) return srv.Conn.Run(ctx) } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 7518e9a7..2b7e1cc5 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -16,7 +16,7 @@ import ( func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) _, m, err := getSourceFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index b9d82f43..71834624 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -17,7 +17,7 @@ import ( func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err @@ -32,19 +32,19 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara } items, prefix, err := source.Completion(ctx, f, rng.Start) if err != nil { - s.log.Infof(ctx, "no completions found for %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) + s.session.Logger().Infof(ctx, "no completions found for %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) } // We might need to adjust the position to account for the prefix. pos := params.Position if prefix.Pos().IsValid() { spn, err := span.NewRange(view.FileSet(), prefix.Pos(), 0).Span() if err != nil { - s.log.Infof(ctx, "failed to get span for prefix position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) + s.session.Logger().Infof(ctx, "failed to get span for prefix position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) } if prefixPos, err := m.Position(spn.Start()); err == nil { pos = prefixPos } else { - s.log.Infof(ctx, "failed to convert prefix position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) + s.session.Logger().Infof(ctx, "failed to convert prefix position: %s:%v:%v: %v", uri, int(params.Position.Line), int(params.Position.Character), err) } } return &protocol.CompletionList{ diff --git a/internal/lsp/definition.go b/internal/lsp/definition.go index b0f996cc..1453aa80 100644 --- a/internal/lsp/definition.go +++ b/internal/lsp/definition.go @@ -14,7 +14,7 @@ import ( func (s *Server) definition(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err @@ -48,7 +48,7 @@ func (s *Server) definition(ctx context.Context, params *protocol.TextDocumentPo func (s *Server) typeDefinition(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 1d0ac830..851d9bdc 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -14,12 +14,12 @@ import ( func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI) { if ctx.Err() != nil { - s.log.Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err()) + s.session.Logger().Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err()) return } reports, err := source.Diagnostics(ctx, view, uri) if err != nil { - s.log.Errorf(ctx, "failed to compute diagnostics for %s: %v", uri, err) + s.session.Logger().Errorf(ctx, "failed to compute diagnostics for %s: %v", uri, err) return } diff --git a/internal/lsp/format.go b/internal/lsp/format.go index 0c672e83..f7fa0655 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -15,7 +15,7 @@ import ( func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormattingParams) ([]protocol.TextEdit, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) spn := span.New(uri, span.Point{}, span.Point{}) return formatRange(ctx, view, spn) } diff --git a/internal/lsp/general.go b/internal/lsp/general.go index b23d7d5d..78d14f15 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -125,7 +125,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa }}, }) } - for _, view := range s.views { + for _, view := range s.session.Views() { config, err := s.client.Configuration(ctx, &protocol.ConfigurationParams{ Items: []protocol.ConfigurationItem{{ ScopeURI: protocol.NewURI(view.Folder()), @@ -142,7 +142,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa } buf := &bytes.Buffer{} PrintVersionInfo(buf, true, false) - s.log.Infof(ctx, "%s", buf) + s.session.Logger().Infof(ctx, "%s", buf) return nil } @@ -195,13 +195,7 @@ func (s *Server) shutdown(ctx context.Context) error { return jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server not initialized") } // drop all the active views - s.viewMu.Lock() - defer s.viewMu.Unlock() - for _, v := range s.views { - v.Shutdown(ctx) - } - s.views = nil - s.viewMap = nil + s.session.Shutdown(ctx) s.isInitialized = false return nil } diff --git a/internal/lsp/highlight.go b/internal/lsp/highlight.go index 22bb1699..2473d529 100644 --- a/internal/lsp/highlight.go +++ b/internal/lsp/highlight.go @@ -14,7 +14,7 @@ import ( func (s *Server) documentHighlight(ctx context.Context, params *protocol.TextDocumentPositionParams) ([]protocol.DocumentHighlight, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/hover.go b/internal/lsp/hover.go index f053b9d4..bdc295b1 100644 --- a/internal/lsp/hover.go +++ b/internal/lsp/hover.go @@ -15,7 +15,7 @@ import ( func (s *Server) hover(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.Hover, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/link.go b/internal/lsp/link.go index ecf6e3eb..d437fb12 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -14,7 +14,7 @@ import ( func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 04eaa814..c550ada6 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -33,19 +33,20 @@ type runner struct { data *tests.Data } -func testLSP(t *testing.T, exporter packagestest.Exporter) { - ctx := context.Background() +const viewName = "lsp_test" +func testLSP(t *testing.T, exporter packagestest.Exporter) { data := tests.Load(t, exporter, "testdata") defer data.Exported.Cleanup() log := xlog.New(xlog.StdSink{}) + cache := cache.New() + session := cache.NewSession(log) + session.NewView(viewName, span.FileURI(data.Config.Dir), &data.Config) r := &runner{ server: &Server{ - views: []source.View{cache.NewView(ctx, log, "lsp_test", span.FileURI(data.Config.Dir), &data.Config)}, - viewMap: make(map[span.URI]source.View), + session: session, undelivered: make(map[span.URI][]source.Diagnostic), - log: log, }, data: data, } @@ -53,7 +54,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { } func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { - v := r.server.views[0] + v := r.server.session.View(viewName) for uri, want := range data { results, err := source.Diagnostics(context.Background(), v, uri) if err != nil { @@ -293,7 +294,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { } continue } - _, m, err := getSourceFile(ctx, r.server.findView(ctx, uri), uri) + _, m, err := getSourceFile(ctx, r.server.session.ViewOf(uri), uri) if err != nil { t.Error(err) } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 7424ba34..bfedab49 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -18,30 +18,32 @@ import ( ) // NewClientServer -func NewClientServer(client protocol.Client) *Server { +func NewClientServer(cache source.Cache, client protocol.Client) *Server { return &Server{ - client: client, - log: xlog.New(protocol.NewLogger(client)), + client: client, + session: cache.NewSession(xlog.New(protocol.NewLogger(client))), } } // NewServer starts an LSP server on the supplied stream, and waits until the // stream is closed. -func NewServer(stream jsonrpc2.Stream) *Server { +func NewServer(cache source.Cache, stream jsonrpc2.Stream) *Server { s := &Server{} - s.Conn, s.client, s.log = protocol.NewServer(stream, s) + var log xlog.Logger + s.Conn, s.client, log = protocol.NewServer(stream, s) + s.session = cache.NewSession(log) return s } // RunServerOnPort starts an LSP server on the given port and does not exit. // This function exists for debugging purposes. -func RunServerOnPort(ctx context.Context, port int, h func(s *Server)) error { - return RunServerOnAddress(ctx, fmt.Sprintf(":%v", port), h) +func RunServerOnPort(ctx context.Context, cache source.Cache, port int, h func(s *Server)) error { + return RunServerOnAddress(ctx, cache, fmt.Sprintf(":%v", port), h) } // RunServerOnPort starts an LSP server on the given port and does not exit. // This function exists for debugging purposes. -func RunServerOnAddress(ctx context.Context, addr string, h func(s *Server)) error { +func RunServerOnAddress(ctx context.Context, cache source.Cache, addr string, h func(s *Server)) error { ln, err := net.Listen("tcp", addr) if err != nil { return err @@ -52,7 +54,7 @@ func RunServerOnAddress(ctx context.Context, addr string, h func(s *Server)) err return err } stream := jsonrpc2.NewHeaderStream(conn, conn) - s := NewServer(stream) + s := NewServer(cache, stream) h(s) go s.Run(ctx) } @@ -65,7 +67,6 @@ func (s *Server) Run(ctx context.Context) error { type Server struct { Conn *jsonrpc2.Conn client protocol.Client - log xlog.Logger initializedMu sync.Mutex isInitialized bool // set once the server has received "initialize" request @@ -81,9 +82,7 @@ type Server struct { textDocumentSyncKind protocol.TextDocumentSyncKind - viewMu sync.Mutex - views []source.View - viewMap map[span.URI]source.View + session source.Session // undelivered is a cache of any diagnostics that the server // failed to deliver for some reason. diff --git a/internal/lsp/signature_help.go b/internal/lsp/signature_help.go index b28f860b..20fe7b4a 100644 --- a/internal/lsp/signature_help.go +++ b/internal/lsp/signature_help.go @@ -14,7 +14,7 @@ import ( func (s *Server) signatureHelp(ctx context.Context, params *protocol.TextDocumentPositionParams) (*protocol.SignatureHelp, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err @@ -29,7 +29,7 @@ func (s *Server) signatureHelp(ctx context.Context, params *protocol.TextDocumen } info, err := source.SignatureHelp(ctx, f, rng.Start) if err != nil { - s.log.Infof(ctx, "no signature help for %s:%v:%v : %s", uri, int(params.Position.Line), int(params.Position.Character), err) + s.session.Logger().Infof(ctx, "no signature help for %s:%v:%v : %s", uri, int(params.Position.Line), int(params.Position.Character), err) } return toProtocolSignatureHelp(info), nil } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 24b24ce6..ad7f15be 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -163,7 +163,7 @@ func formatFieldList(ctx context.Context, v View, list *ast.FieldList) ([]string cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4} b := &bytes.Buffer{} if err := cfg.Fprint(b, v.FileSet(), p.Type); err != nil { - v.Logger().Errorf(ctx, "unable to print type %v", p.Type) + v.Session().Logger().Errorf(ctx, "unable to print type %v", p.Type) continue } typ := replacer.Replace(b.String()) diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 23b33b65..667f07df 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -157,29 +157,29 @@ func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span { // Don't set a range if it's anything other than a type error. f, err := v.GetFile(ctx, spn.URI()) if err != nil { - v.Logger().Errorf(ctx, "Could find file for diagnostic: %v", spn.URI()) + v.Session().Logger().Errorf(ctx, "Could find file for diagnostic: %v", spn.URI()) return spn } diagFile, ok := f.(GoFile) if !ok { - v.Logger().Errorf(ctx, "Not a go file: %v", spn.URI()) + v.Session().Logger().Errorf(ctx, "Not a go file: %v", spn.URI()) return spn } tok := diagFile.GetToken(ctx) if tok == nil { - v.Logger().Errorf(ctx, "Could not find tokens for diagnostic: %v", spn.URI()) + v.Session().Logger().Errorf(ctx, "Could not find tokens for diagnostic: %v", spn.URI()) return spn } content := diagFile.GetContent(ctx) if content == nil { - v.Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI()) + v.Session().Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI()) return spn } c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok) s, err := spn.WithOffset(c) //we just don't bother producing an error if this failed if err != nil { - v.Logger().Errorf(ctx, "invalid span for diagnostic: %v: %v", spn.URI(), err) + v.Session().Logger().Errorf(ctx, "invalid span for diagnostic: %v: %v", spn.URI(), err) return spn } start := s.Start() diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index a0e83087..263ba080 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -32,14 +32,14 @@ type runner struct { } func testSource(t *testing.T, exporter packagestest.Exporter) { - ctx := context.Background() - data := tests.Load(t, exporter, "../testdata") defer data.Exported.Cleanup() log := xlog.New(xlog.StdSink{}) + cache := cache.New() + session := cache.NewSession(log) r := &runner{ - view: cache.NewView(ctx, log, "source_test", span.FileURI(data.Config.Dir), &data.Config), + view: session.NewView("source_test", span.FileURI(data.Config.Dir), &data.Config), data: data, } tests.Run(t, r, data) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index fbbf91f3..57a87071 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -18,13 +18,47 @@ import ( "golang.org/x/tools/internal/span" ) -// View abstracts the underlying architecture of the package using the source -// package. The view provides access to files and their contents, so the source +// Cache abstracts the core logic of dealing with the environment from the +// higher level logic that processes the information to produce results. +// The cache provides access to files and their contents, so the source // package does not directly access the file system. +// A single cache is intended to be process wide, and is the primary point of +// sharing between all consumers. +// A cache may have many active sessions at any given time. +type Cache interface { + // NewSession creates a new Session manager and returns it. + NewSession(log xlog.Logger) Session +} + +// Session represents a single connection from a client. +// This is the level at which things like open files are maintained on behalf +// of the client. +// A session may have many active views at any given time. +type Session interface { + // NewView creates a new View and returns it. + NewView(name string, folder span.URI, config *packages.Config) View + + // Cache returns the cache that created this session. + Cache() Cache + + // Returns the logger in use for this session. + Logger() xlog.Logger + + View(name string) View + ViewOf(uri span.URI) View + Views() []View + + Shutdown(ctx context.Context) +} + +// View represents a single workspace. +// This is the level at which we maintain configuration like working directory +// and build tags. type View interface { + // Session returns the session that created this view. + Session() Session Name() string Folder() span.URI - Logger() xlog.Logger FileSet() *token.FileSet BuiltinPackage() *ast.Package GetFile(ctx context.Context, uri span.URI) (File, error) diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go index 2ee526aa..1b1eb22a 100644 --- a/internal/lsp/symbols.go +++ b/internal/lsp/symbols.go @@ -14,7 +14,7 @@ import ( func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSymbolParams) ([]protocol.DocumentSymbol, error) { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) f, m, err := getGoFile(ctx, view, uri) if err != nil { return nil, err diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 5402f23c..d8c219e7 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -42,7 +42,7 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo } func (s *Server) cacheAndDiagnose(ctx context.Context, uri span.URI, content []byte) error { - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) if err := view.SetContent(ctx, uri, content); err != nil { return err } @@ -64,7 +64,7 @@ func (s *Server) applyChanges(ctx context.Context, params *protocol.DidChangeTex } uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) file, m, err := getSourceFile(ctx, view, uri) if err != nil { return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "file not found") @@ -97,6 +97,6 @@ func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocume func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) + view := s.session.ViewOf(uri) return view.SetContent(ctx, uri, nil) } diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 938450a1..687e2c8f 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -11,20 +11,19 @@ import ( "go/parser" "go/token" "os" - "strings" "golang.org/x/tools/go/packages" - "golang.org/x/tools/internal/lsp/cache" "golang.org/x/tools/internal/lsp/protocol" - "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" ) func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFoldersChangeEvent) error { - s.log.Infof(ctx, "change folders") for _, folder := range event.Removed { - if err := s.removeView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil { - return err + view := s.session.View(folder.Name) + if view != nil { + view.Shutdown(ctx) + } else { + return fmt.Errorf("view %s for %v not found", folder.Name, folder.URI) } } @@ -37,17 +36,14 @@ func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFold } func (s *Server) addView(ctx context.Context, name string, uri span.URI) error { - s.viewMu.Lock() - defer s.viewMu.Unlock() // We need a "detached" context so it does not get timeout cancelled. // TODO(iancottrell): Do we need to copy any values across? viewContext := context.Background() - s.log.Infof(viewContext, "add view %v as %v", name, uri) folderPath, err := uri.Filename() if err != nil { return err } - s.views = append(s.views, cache.NewView(viewContext, s.log, name, uri, &packages.Config{ + s.session.NewView(name, uri, &packages.Config{ Context: viewContext, Dir: folderPath, Env: os.Environ(), @@ -58,65 +54,7 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) error { return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments) }, Tests: true, - })) - // we always need to drop the view map - s.viewMap = make(map[span.URI]source.View) + }) + return nil } - -func (s *Server) removeView(ctx context.Context, name string, uri span.URI) error { - s.viewMu.Lock() - defer s.viewMu.Unlock() - // we always need to drop the view map - s.viewMap = make(map[span.URI]source.View) - s.log.Infof(ctx, "drop view %v as %v", name, uri) - for i, view := range s.views { - if view.Name() == name { - // delete this view... we don't care about order but we do want to make - // sure we can garbage collect the view - s.views[i] = s.views[len(s.views)-1] - s.views[len(s.views)-1] = nil - s.views = s.views[:len(s.views)-1] - view.Shutdown(ctx) - return nil - } - } - return fmt.Errorf("view %s for %v not found", name, uri) -} - -// findView returns the view corresponding to the given URI. -// If the file is not already associated with a view, pick one using some heuristics. -func (s *Server) findView(ctx context.Context, uri span.URI) source.View { - s.viewMu.Lock() - defer s.viewMu.Unlock() - - // check if we already know this file - if v, found := s.viewMap[uri]; found { - return v - } - - // pick the best view for this file and memoize the result - v := s.bestView(ctx, uri) - s.viewMap[uri] = v - return v -} - -// bestView finds the best view to associate a given URI with. -// viewMu must be held when calling this method. -func (s *Server) bestView(ctx context.Context, uri span.URI) source.View { - // we need to find the best view for this file - var longest source.View - for _, view := range s.views { - if longest != nil && len(longest.Folder()) > len(view.Folder()) { - continue - } - if strings.HasPrefix(string(uri), string(view.Folder())) { - longest = view - } - } - if longest != nil { - return longest - } - //TODO: are there any more heuristics we can use? - return s.views[0] -}