From 7927dbab1be7d178ed14189b31e26c604b8baba8 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Fri, 17 May 2019 10:51:19 -0400 Subject: [PATCH] internal/lsp: build the packages config on demand from proper configuration This moves the fileset down to the base cache, the overlays down to the session and stores the environment on the view. packages.Config is no longer part of any public API, and the config is build on demand by combining all the layers of cache. Also added some documentation to the main source pacakge interfaces. Change-Id: I058092ad2275d433864d1f58576fc55e194607a6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/178017 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- cmd/gopls/main.go | 2 +- internal/lsp/cache/cache.go | 17 +++++- internal/lsp/cache/check.go | 21 ++++--- internal/lsp/cache/file.go | 8 ++- internal/lsp/cache/parse.go | 45 ++++++--------- internal/lsp/cache/session.go | 34 ++++++++--- internal/lsp/cache/view.go | 70 ++++++++++++++++------- internal/lsp/cmd/cmd.go | 52 +++++++---------- internal/lsp/cmd/cmd_test.go | 2 +- internal/lsp/cmd/definition_test.go | 2 +- internal/lsp/cmd/format_test.go | 3 +- internal/lsp/cmd/serve.go | 6 +- internal/lsp/general.go | 17 +----- internal/lsp/link.go | 2 +- internal/lsp/lsp_test.go | 7 ++- internal/lsp/source/analysis.go | 2 +- internal/lsp/source/completion.go | 2 +- internal/lsp/source/completion_format.go | 2 +- internal/lsp/source/completion_snippet.go | 2 +- internal/lsp/source/diagnostics.go | 4 +- internal/lsp/source/format.go | 2 +- internal/lsp/source/highlight.go | 2 +- internal/lsp/source/hover.go | 2 +- internal/lsp/source/identifier.go | 12 ++-- internal/lsp/source/source_test.go | 12 +++- internal/lsp/source/symbols.go | 2 +- internal/lsp/source/view.go | 37 ++++++++++-- internal/lsp/text_synchronization.go | 2 +- internal/lsp/util.go | 2 +- internal/lsp/workspace.go | 26 +-------- 30 files changed, 224 insertions(+), 175 deletions(-) diff --git a/cmd/gopls/main.go b/cmd/gopls/main.go index 26d32d06fd..dcd108d9fa 100644 --- a/cmd/gopls/main.go +++ b/cmd/gopls/main.go @@ -17,5 +17,5 @@ import ( ) func main() { - tool.Main(context.Background(), cmd.New(nil), 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 index 71db06b638..4f0e3e68d0 100644 --- a/internal/lsp/cache/cache.go +++ b/internal/lsp/cache/cache.go @@ -5,20 +5,31 @@ package cache import ( + "go/token" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/xlog" + "golang.org/x/tools/internal/span" ) func New() source.Cache { - return &cache{} + return &cache{ + fset: token.NewFileSet(), + } } type cache struct { + fset *token.FileSet } func (c *cache) NewSession(log xlog.Logger) source.Session { return &session{ - cache: c, - log: log, + cache: c, + log: log, + overlays: make(map[span.URI][]byte), } } + +func (c *cache) FileSet() *token.FileSet { + return c.fset +} diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index b2d12526d1..a81f6902e3 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -10,6 +10,7 @@ import ( "go/ast" "go/parser" "go/scanner" + "go/token" "go/types" "golang.org/x/tools/go/analysis" @@ -49,6 +50,7 @@ func (v *view) parse(ctx context.Context, file source.File) ([]packages.Error, e view: v, seen: make(map[string]struct{}), ctx: ctx, + fset: f.FileSet(), } // Start prefetching direct imports. for importPath := range f.meta.children { @@ -69,12 +71,11 @@ func (v *view) parse(ctx context.Context, file source.File) ([]packages.Error, e func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error, error) { if v.reparseImports(ctx, f, f.filename()) { - cfg := v.config - cfg.Mode = packages.LoadImports | packages.NeedTypesSizes - pkgs, err := packages.Load(&cfg, fmt.Sprintf("file=%s", f.filename())) + cfg := v.buildConfig() + pkgs, err := packages.Load(cfg, fmt.Sprintf("file=%s", f.filename())) if len(pkgs) == 0 { if err == nil { - err = fmt.Errorf("no packages found for %s", f.filename()) + err = fmt.Errorf("%s: no packages found", f.filename()) } // Return this error as a diagnostic to the user. return []packages.Error{ @@ -104,7 +105,7 @@ func (v *view) reparseImports(ctx context.Context, f *goFile, filename string) b } // Get file content in case we don't already have it? f.read(ctx) - parsed, _ := parser.ParseFile(v.config.Fset, filename, f.content, parser.ImportsOnly) + parsed, _ := parser.ParseFile(f.FileSet(), filename, f.content, parser.ImportsOnly) if parsed == nil { return true } @@ -173,7 +174,8 @@ type importer struct { // If we have seen a package that is already in this map, we have a circular import. seen map[string]struct{} - ctx context.Context + ctx context.Context + fset *token.FileSet } func (imp *importer) Import(pkgPath string) (*types.Package, error) { @@ -255,9 +257,10 @@ func (imp *importer) typeCheck(pkgPath string) (*pkg, error) { view: imp.view, seen: seen, ctx: imp.ctx, + fset: imp.fset, }, } - check := types.NewChecker(cfg, imp.view.config.Fset, pkg.types, pkg.typesInfo) + check := types.NewChecker(cfg, imp.fset, pkg.types, pkg.typesInfo) check.Files(pkg.syntax) // Add every file in this package to our cache. @@ -273,7 +276,7 @@ func (v *view) cachePackage(ctx context.Context, pkg *pkg, meta *metadata) { v.Session().Logger().Errorf(ctx, "invalid position for file %v", file.Name) continue } - tok := v.config.Fset.File(file.Pos()) + tok := v.Session().Cache().FileSet().File(file.Pos()) if tok == nil { v.Session().Logger().Errorf(ctx, "no token.File for %v", file.Name) continue @@ -341,7 +344,7 @@ func (v *view) appendPkgError(pkg *pkg, err error) { } case types.Error: errs = append(errs, packages.Error{ - Pos: v.config.Fset.Position(err.Pos).String(), + Pos: v.Session().Cache().FileSet().Position(err.Pos).String(), Msg: err.Msg, Kind: packages.TypeError, }) diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 14bb7f54e8..5b69dec1d7 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -80,8 +80,8 @@ func (f *fileBase) GetContent(ctx context.Context) []byte { return f.content } -func (f *fileBase) GetFileSet(ctx context.Context) *token.FileSet { - return f.view.config.Fset +func (f *fileBase) FileSet() *token.FileSet { + return f.view.Session().Cache().FileSet() } func (f *goFile) GetToken(ctx context.Context) *token.File { @@ -144,7 +144,9 @@ func (f *fileBase) read(ctx context.Context) { } } // We might have the content saved in an overlay. - if content, ok := f.view.config.Overlay[f.filename()]; ok { + f.view.session.overlayMu.Lock() + defer f.view.session.overlayMu.Unlock() + if content, ok := f.view.session.overlays[f.URI()]; ok { f.content = content return } diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index c9876ba6cb..d8329ffb37 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -11,7 +11,6 @@ import ( "go/parser" "go/scanner" "go/token" - "io/ioutil" "os" "path/filepath" "strings" @@ -20,6 +19,10 @@ import ( "golang.org/x/tools/internal/span" ) +func parseFile(fset *token.FileSet, filename string, src []byte) (*ast.File, error) { + return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments) +} + // We use a counting semaphore to limit // the number of parallel I/O calls per process. var ioLimit = make(chan bool, 20) @@ -37,53 +40,43 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) { parsed := make([]*ast.File, n) errors := make([]error, n) for i, filename := range filenames { - if imp.view.config.Context.Err() != nil { + if imp.ctx.Err() != nil { parsed[i] = nil - errors[i] = imp.view.config.Context.Err() + errors[i] = imp.ctx.Err() continue } // First, check if we have already cached an AST for this file. f, err := imp.view.findFile(span.FileURI(filename)) - if err != nil { + if err != nil || f == nil { parsed[i], errors[i] = nil, err + continue } - var fAST *ast.File - if f != nil { - if gof, ok := f.(*goFile); ok { - fAST = gof.ast - } + gof, ok := f.(*goFile) + if !ok { + parsed[i], errors[i] = nil, fmt.Errorf("Non go file in parse call: %v", filename) + continue } wg.Add(1) go func(i int, filename string) { ioLimit <- true // wait - if fAST != nil { - parsed[i], errors[i] = fAST, nil + if gof.ast != nil { + parsed[i], errors[i] = gof.ast, nil } else { // We don't have a cached AST for this file. - var src []byte - // Check for an available overlay. - for f, contents := range imp.view.config.Overlay { - if sameFile(f, filename) { - src = contents - } - } - var err error - // We don't have an overlay, so we must read the file's contents. + gof.read(imp.ctx) + src := gof.content if src == nil { - src, err = ioutil.ReadFile(filename) - } - if err != nil { - parsed[i], errors[i] = nil, err + parsed[i], errors[i] = nil, fmt.Errorf("No source for %v", filename) } else { // ParseFile may return both an AST and an error. - parsed[i], errors[i] = imp.view.config.ParseFile(imp.view.config.Fset, filename, src) + parsed[i], errors[i] = parseFile(imp.fset, filename, src) // Fix any badly parsed parts of the AST. if file := parsed[i]; file != nil { - tok := imp.view.config.Fset.File(file.Pos()) + tok := imp.fset.File(file.Pos()) imp.view.fix(imp.ctx, parsed[i], tok, src) } } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 4ebe57eceb..c4c7a9b978 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -10,7 +10,6 @@ import ( "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" @@ -24,6 +23,9 @@ type session struct { viewMu sync.Mutex views []*view viewMap map[span.URI]source.View + + overlayMu sync.Mutex + overlays map[span.URI][]byte } func (s *session) Shutdown(ctx context.Context) { @@ -40,7 +42,7 @@ func (s *session) Cache() source.Cache { return s.cache } -func (s *session) NewView(name string, folder span.URI, config *packages.Config) source.View { +func (s *session) NewView(name string, folder span.URI) source.View { s.viewMu.Lock() defer s.viewMu.Unlock() ctx := context.Background() @@ -49,9 +51,7 @@ func (s *session) NewView(name string, folder span.URI, config *packages.Config) session: s, baseCtx: ctx, backgroundCtx: backgroundCtx, - builtinPkg: builtinPkg(*config), cancel: cancel, - config: *config, name: name, folder: folder, filesByURI: make(map[span.URI]viewFile), @@ -65,9 +65,6 @@ func (s *session) NewView(name string, folder span.URI, config *packages.Config) }, ignoredURIs: make(map[span.URI]struct{}), } - for filename := range v.builtinPkg.Files { - v.ignoredURIs[span.NewURI(filename)] = struct{}{} - } s.views = append(s.views, v) // we always need to drop the view map s.viewMap = make(map[span.URI]source.View) @@ -155,3 +152,26 @@ func (s *session) removeView(ctx context.Context, view *view) error { func (s *session) Logger() xlog.Logger { return s.log } + +func (s *session) setOverlay(uri span.URI, content []byte) { + s.overlayMu.Lock() + defer s.overlayMu.Unlock() + //TODO: we also need to invalidate anything that depended on this "file" + if content == nil { + delete(s.overlays, uri) + return + } + s.overlays[uri] = content +} + +func (s *session) buildOverlay() map[string][]byte { + s.overlayMu.Lock() + defer s.overlayMu.Unlock() + overlay := make(map[string][]byte) + for uri, content := range s.overlays { + if filename, err := uri.Filename(); err == nil { + overlay[filename] = content + } + } + return overlay +} diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index f64f7a3712..f2573605a4 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -8,7 +8,6 @@ import ( "context" "go/ast" "go/parser" - "go/token" "go/types" "os" "sync" @@ -42,9 +41,8 @@ type view struct { // Folder is the root of this view. folder span.URI - // Config is the configuration used for the view's interaction with the - // go/packages API. It is shared across all views. - config packages.Config + // env is the environment to use when invoking underlying tools. + env []string // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files @@ -110,12 +108,40 @@ func (v *view) Folder() span.URI { // Config returns the configuration used for the view's interaction with the // go/packages API. It is shared across all views. -func (v *view) Config() packages.Config { - return v.config +func (v *view) buildConfig() *packages.Config { + //TODO:should we cache the config and/or overlay somewhere? + folderPath, err := v.folder.Filename() + if err != nil { + folderPath = "" + } + return &packages.Config{ + Context: v.backgroundCtx, + Dir: folderPath, + Env: v.env, + Mode: packages.NeedName | + packages.NeedFiles | + packages.NeedCompiledGoFiles | + packages.NeedImports | + packages.NeedDeps | + packages.NeedTypesSizes, + Fset: v.session.cache.fset, + Overlay: v.session.buildOverlay(), + ParseFile: parseFile, + Tests: true, + } +} + +func (v *view) Env() []string { + v.mu.Lock() + defer v.mu.Unlock() + return v.env } func (v *view) SetEnv(env []string) { - v.config.Env = env + v.mu.Lock() + defer v.mu.Unlock() + //TODO: this should invalidate the entire view + v.env = env } func (v *view) Shutdown(ctx context.Context) { @@ -139,33 +165,33 @@ func (v *view) BackgroundContext() context.Context { } func (v *view) BuiltinPackage() *ast.Package { + if v.builtinPkg == nil { + v.buildBuiltinPkg() + } return v.builtinPkg } -func builtinPkg(cfg packages.Config) *ast.Package { - var bpkg *ast.Package - cfg.Mode = packages.LoadFiles +func (v *view) buildBuiltinPkg() { + v.mu.Lock() + defer v.mu.Unlock() + cfg := *v.buildConfig() pkgs, _ := packages.Load(&cfg, "builtin") if len(pkgs) != 1 { - bpkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil) - return bpkg + v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil) + return } pkg := pkgs[0] files := make(map[string]*ast.File) for _, filename := range pkg.GoFiles { file, err := parser.ParseFile(cfg.Fset, filename, nil, parser.ParseComments) if err != nil { - bpkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil) - return bpkg + v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil) + return } files[filename] = file + v.ignoredURIs[span.NewURI(filename)] = struct{}{} } - bpkg, _ = ast.NewPackage(cfg.Fset, files, nil, nil) - return bpkg -} - -func (v *view) FileSet() *token.FileSet { - return v.config.Fset + v.builtinPkg, _ = ast.NewPackage(cfg.Fset, files, nil, nil) } // SetContent sets the overlay contents for a file. @@ -230,12 +256,12 @@ func (f *goFile) setContent(content []byte) { case f.active && content == nil: // The file was active, so we need to forget its content. f.active = false - delete(f.view.config.Overlay, f.filename()) + f.view.session.setOverlay(f.URI(), nil) f.content = nil case content != nil: // This is an active overlay, so we update the map. f.active = true - f.view.config.Overlay[f.filename()] = f.content + f.view.session.setOverlay(f.URI(), f.content) } } diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index 6c9ab2d8cb..b66039c5d8 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -11,8 +11,6 @@ import ( "context" "flag" "fmt" - "go/ast" - "go/parser" "go/token" "io/ioutil" "log" @@ -21,7 +19,6 @@ import ( "strings" "sync" - "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" @@ -44,11 +41,14 @@ type Application struct { // TODO: Remove this when we stop allowing the serve verb by default. Serve Serve - // An initial, common go/packages configuration - Config packages.Config - // The base cache to use for sessions from this application. - Cache source.Cache + cache source.Cache + + // The working directory to run commands in. + wd string + + // The environment variables to use. + env []string // Support for remote lsp server Remote string `flag:"remote" help:"*EXPERIMENTAL* - forward all commands to a remote lsp"` @@ -58,12 +58,14 @@ type Application struct { } // Returns a new Application ready to run. -func New(config *packages.Config) *Application { - app := &Application{ - Cache: cache.New(), +func New(wd string, env []string) *Application { + if wd == "" { + wd, _ = os.Getwd() } - if config != nil { - app.Config = *config + app := &Application{ + cache: cache.New(), + wd: wd, + env: env, } return app } @@ -104,20 +106,6 @@ func (app *Application) Run(ctx context.Context, args ...string) error { tool.Main(ctx, &app.Serve, args) return nil } - if app.Config.Dir == "" { - if wd, err := os.Getwd(); err == nil { - app.Config.Dir = wd - } - } - app.Config.Mode = packages.LoadSyntax - app.Config.Tests = true - if app.Config.Fset == nil { - app.Config.Fset = token.NewFileSet() - } - app.Config.Context = ctx - app.Config.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) { - return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments) - } command, args := args[0], args[1:] for _, c := range app.commands() { if c.Name() == command { @@ -151,12 +139,12 @@ func (app *Application) connect(ctx context.Context) (*connection, error) { switch app.Remote { case "": connection := newConnection(app) - connection.Server = lsp.NewClientServer(app.Cache, connection.Client) + connection.Server = lsp.NewClientServer(app.cache, connection.Client) return connection, connection.initialize(ctx) case "internal": internalMu.Lock() defer internalMu.Unlock() - if c := internalConnections[app.Config.Dir]; c != nil { + if c := internalConnections[app.wd]; c != nil { return c, nil } connection := newConnection(app) @@ -166,11 +154,11 @@ 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(app.Cache, 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 } - internalConnections[app.Config.Dir] = connection + internalConnections[app.wd] = connection return connection, nil default: connection := newConnection(app) @@ -188,7 +176,7 @@ func (app *Application) connect(ctx context.Context) (*connection, error) { func (c *connection) initialize(ctx context.Context) error { params := &protocol.InitializeParams{} - params.RootURI = string(span.FileURI(c.Client.app.Config.Dir)) + params.RootURI = string(span.FileURI(c.Client.app.wd)) params.Capabilities.Workspace.Configuration = true params.Capabilities.TextDocument.Hover.ContentFormat = []protocol.MarkupKind{protocol.PlainText} if _, err := c.Server.Initialize(ctx, params); err != nil { @@ -283,7 +271,7 @@ func (c *cmdClient) Configuration(ctx context.Context, p *protocol.Configuration continue } env := map[string]interface{}{} - for _, value := range c.app.Config.Env { + for _, value := range c.app.env { l := strings.SplitN(value, "=", 2) if len(l) != 2 { continue diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index 5b8b4c5086..611d78a825 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -37,7 +37,7 @@ func testCommandLine(t *testing.T, exporter packagestest.Exporter) { r := &runner{ exporter: exporter, data: data, - app: cmd.New(data.Exported.Config), + app: cmd.New(data.Config.Dir, data.Exported.Config.Env), } tests.Run(t, r, data) } diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index 6eacc18d9f..36b61eff5c 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.New(nil), 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 981ebdb60c..3e83d8d70f 100644 --- a/internal/lsp/cmd/format_test.go +++ b/internal/lsp/cmd/format_test.go @@ -41,8 +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.New(nil) - app.Config = r.data.Config + app := cmd.New(r.data.Config.Dir, r.data.Config.Env) 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 ddac3568ab..28b7398f3a 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.app.Cache, s.Address, run) + return lsp.RunServerOnAddress(ctx, s.app.cache, s.Address, run) } if s.Port != 0 { - return lsp.RunServerOnPort(ctx, s.app.Cache, s.Port, run) + return lsp.RunServerOnPort(ctx, s.app.cache, s.Port, run) } stream := jsonrpc2.NewHeaderStream(os.Stdin, os.Stdout) - srv := lsp.NewServer(s.app.Cache, 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/general.go b/internal/lsp/general.go index 78d14f1517..6091519a68 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "path" - "strings" "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" @@ -161,9 +160,11 @@ func (s *Server) processConfig(view source.View, config interface{}) error { if !ok { return fmt.Errorf("invalid config gopls.env type %T", env) } + env := view.Env() for k, v := range menv { - view.SetEnv(applyEnv(view.Config().Env, k, v)) + env = append(env, fmt.Sprintf("%s=%s", k, v)) } + view.SetEnv(env) } // Check if placeholders are enabled. if usePlaceholders, ok := c["usePlaceholders"].(bool); ok { @@ -176,18 +177,6 @@ func (s *Server) processConfig(view source.View, config interface{}) error { return nil } -func applyEnv(env []string, k string, v interface{}) []string { - prefix := k + "=" - value := prefix + fmt.Sprint(v) - for i, s := range env { - if strings.HasPrefix(s, prefix) { - env[i] = value - return env - } - } - return append(env, value) -} - func (s *Server) shutdown(ctx context.Context) error { s.initializedMu.Lock() defer s.initializedMu.Unlock() diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 155787ff93..e5ab123f02 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -27,7 +27,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink // Add a Godoc link for each imported package. var result []protocol.DocumentLink for _, imp := range file.Imports { - spn, err := span.NewRange(f.GetFileSet(ctx), imp.Pos(), imp.End()).Span() + spn, err := span.NewRange(f.FileSet(), imp.Pos(), imp.End()).Span() if err != nil { return nil, err } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index e6cffa075c..7d20a5d300 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -36,13 +36,18 @@ type runner struct { const viewName = "lsp_test" func testLSP(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) - session.NewView(viewName, span.FileURI(data.Config.Dir), &data.Config) + view := session.NewView(viewName, span.FileURI(data.Config.Dir)) + view.SetEnv(data.Config.Env) + for filename, content := range data.Config.Overlay { + view.SetContent(ctx, span.FileURI(filename), content) + } r := &runner{ server: &Server{ session: session, diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go index a1bb2e3b06..f7021b20b1 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/source/analysis.go @@ -42,7 +42,7 @@ func analyze(ctx context.Context, v View, pkgs []Package, analyzers []*analysis. } // Execute the graph in parallel. - if err := execAll(ctx, v.FileSet(), roots); err != nil { + if err := execAll(ctx, v.Session().Cache().FileSet(), roots); err != nil { return nil, err } return roots, nil diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index e1d03dcb3b..990cfdd352 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -189,7 +189,7 @@ func (c *completer) setSurrounding(ident *ast.Ident) { c.surrounding = &Selection{ Content: ident.Name, - Range: span.NewRange(c.view.FileSet(), ident.Pos(), ident.End()), + Range: span.NewRange(c.view.Session().Cache().FileSet(), ident.Pos(), ident.End()), Cursor: c.pos, } } diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index e5d069863e..c399afdc0a 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -143,7 +143,7 @@ func formatFieldList(ctx context.Context, v View, list *ast.FieldList) ([]string p := list.List[i] cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4} b := &bytes.Buffer{} - if err := cfg.Fprint(b, v.FileSet(), p.Type); err != nil { + if err := cfg.Fprint(b, v.Session().Cache().FileSet(), p.Type); err != nil { v.Session().Logger().Errorf(ctx, "unable to print type %v", p.Type) continue } diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go index 8ac4fab3a1..9b1841fe6e 100644 --- a/internal/lsp/source/completion_snippet.go +++ b/internal/lsp/source/completion_snippet.go @@ -46,7 +46,7 @@ func (c *completer) structFieldSnippets(label, detail string) (*snippet.Builder, // If the cursor position is on a different line from the literal's opening brace, // we are in a multiline literal. - if c.view.FileSet().Position(c.pos).Line != c.view.FileSet().Position(clInfo.cl.Lbrace).Line { + if c.view.Session().Cache().FileSet().Position(c.pos).Line != c.view.Session().Cache().FileSet().Position(clInfo.cl.Lbrace).Line { plain.WriteText(",") placeholder.WriteText(",") } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index f66e278637..5cbea3ccbd 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -141,7 +141,7 @@ func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI] func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]Diagnostic) error { // Type checking and parsing succeeded. Run analyses. if err := runAnalyses(ctx, v, pkg, func(a *analysis.Analyzer, diag analysis.Diagnostic) error { - r := span.NewRange(v.FileSet(), diag.Pos, 0) + r := span.NewRange(v.Session().Cache().FileSet(), diag.Pos, 0) s, err := r.Span() if err != nil { // The diagnostic has an invalid position, so we don't have a valid span. @@ -209,7 +209,7 @@ func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span { v.Session().Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI()) return spn } - c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok) + c := span.NewTokenConverter(diagFile.FileSet(), tok) s, err := spn.WithOffset(c) //we just don't bother producing an error if this failed if err != nil { diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index ba85ff52fb..ed26bee734 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -37,7 +37,7 @@ func Format(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) { // of Go used to build the LSP server will determine how it formats code. // This should be acceptable for all users, who likely be prompted to rebuild // the LSP server on each Go release. - fset := f.GetFileSet(ctx) + fset := f.FileSet() buf := &bytes.Buffer{} if err := format.Node(buf, fset, node); err != nil { return nil, err diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index bf3cb46f68..f8db00fba5 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -18,7 +18,7 @@ func Highlight(ctx context.Context, f GoFile, pos token.Pos) []span.Span { if file == nil { return nil } - fset := f.GetFileSet(ctx) + fset := f.FileSet() path, _ := astutil.PathEnclosingInterval(file, pos, pos) if len(path) == 0 { return nil diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index d4bb08ecd9..4d00527a85 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -29,7 +29,7 @@ func (i *IdentifierInfo) Hover(ctx context.Context, qf types.Qualifier, markdown if !wantComments { c = nil } - return writeHover(x, i.File.GetFileSet(ctx), &b, c, markdownSupported, qf) + return writeHover(x, i.File.FileSet(), &b, c, markdownSupported, qf) } obj := i.Declaration.Object switch node := i.Declaration.Node.(type) { diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 763494ff7a..affa130998 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -90,7 +90,7 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi } } result.Name = result.ident.Name - result.Range = span.NewRange(v.FileSet(), result.ident.Pos(), result.ident.End()) + result.Range = span.NewRange(f.FileSet(), result.ident.Pos(), result.ident.End()) result.Declaration.Object = pkg.GetTypesInfo().ObjectOf(result.ident) if result.Declaration.Object == nil { return nil, fmt.Errorf("no object for ident %v", result.Name) @@ -105,7 +105,7 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi return nil, fmt.Errorf("no declaration for %s", result.Name) } result.Declaration.Node = decl - if result.Declaration.Range, err = posToRange(ctx, v.FileSet(), result.Name, decl.Pos()); err != nil { + if result.Declaration.Range, err = posToRange(ctx, f.FileSet(), result.Name, decl.Pos()); err != nil { return nil, err } return result, nil @@ -121,7 +121,7 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi } } - if result.Declaration.Range, err = objToRange(ctx, v.FileSet(), result.Declaration.Object); err != nil { + if result.Declaration.Range, err = objToRange(ctx, f.FileSet(), result.Declaration.Object); err != nil { return nil, err } if result.Declaration.Node, err = objToNode(ctx, v, result.Declaration.Object, result.Declaration.Range); err != nil { @@ -137,7 +137,7 @@ func identifier(ctx context.Context, v View, f GoFile, pos token.Pos) (*Identifi if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.Range, err = objToRange(ctx, v.FileSet(), result.Type.Object); err != nil { + if result.Type.Range, err = objToRange(ctx, f.FileSet(), result.Type.Object); err != nil { return nil, err } } @@ -219,7 +219,7 @@ func importSpec(f GoFile, fAST *ast.File, pkg Package, pos token.Pos) (*Identifi result := &IdentifierInfo{ File: f, Name: importPath, - Range: span.NewRange(f.View().FileSet(), imp.Pos(), imp.End()), + Range: span.NewRange(f.FileSet(), imp.Pos(), imp.End()), } // Consider the "declaration" of an import spec to be the imported package. importedPkg := pkg.GetImport(importPath) @@ -239,7 +239,7 @@ func importSpec(f GoFile, fAST *ast.File, pkg Package, pos token.Pos) (*Identifi if dest == nil { return nil, fmt.Errorf("package %q has no files", importPath) } - result.Declaration.Range = span.NewRange(f.View().FileSet(), dest.Name.Pos(), dest.Name.End()) + result.Declaration.Range = span.NewRange(f.FileSet(), dest.Name.Pos(), dest.Name.End()) return result, nil } return nil, nil diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 475d9ace8e..dd979ef050 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -32,6 +32,7 @@ type runner struct { } func testSource(t *testing.T, exporter packagestest.Exporter) { + ctx := context.Background() data := tests.Load(t, exporter, "../testdata") defer data.Exported.Cleanup() @@ -39,9 +40,13 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { cache := cache.New() session := cache.NewSession(log) r := &runner{ - view: session.NewView("source_test", span.FileURI(data.Config.Dir), &data.Config), + view: session.NewView("source_test", span.FileURI(data.Config.Dir)), data: data, } + r.view.SetEnv(data.Config.Env) + for filename, content := range data.Config.Overlay { + r.view.SetContent(ctx, span.FileURI(filename), content) + } tests.Run(t, r, data) } @@ -134,6 +139,9 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests t.Fatalf("failed for %v: %v", src, err) } tok := f.(source.GoFile).GetToken(ctx) + if tok == nil { + t.Fatalf("failed to get token for %v", src) + } pos := tok.Pos(src.Start().Offset()) list, surrounding, err := source.Completion(ctx, f.(source.GoFile), pos) if err != nil { @@ -273,7 +281,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - rng, err := spn.Range(span.NewTokenConverter(f.GetFileSet(ctx), f.GetToken(ctx))) + rng, err := spn.Range(span.NewTokenConverter(f.FileSet(), f.GetToken(ctx))) if err != nil { t.Fatalf("failed for %v: %v", spn, err) } diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index aba8f81466..bb765cd649 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -41,7 +41,7 @@ type Symbol struct { } func DocumentSymbols(ctx context.Context, f GoFile) []Symbol { - fset := f.GetFileSet(ctx) + fset := f.FileSet() file := f.GetAST(ctx) if file == nil { return nil diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index f9802c1703..cef3011c64 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -28,6 +28,9 @@ import ( type Cache interface { // NewSession creates a new Session manager and returns it. NewSession(log xlog.Logger) Session + + // FileSet returns the shared fileset used by all files in the system. + FileSet() *token.FileSet } // Session represents a single connection from a client. @@ -36,7 +39,7 @@ type Cache interface { // 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 + NewView(name string, folder span.URI) View // Cache returns the cache that created this session. Cache() Cache @@ -44,10 +47,16 @@ type Session interface { // Returns the logger in use for this session. Logger() xlog.Logger + // View returns a view with a mathing name, if the session has one. View(name string) View + + // ViewOf returns a view corresponding to the given URI. ViewOf(uri span.URI) View + + // Views returns the set of active views built by this session. Views() []View + // Shutdown the session and all views it has created. Shutdown(ctx context.Context) } @@ -57,16 +66,36 @@ type Session interface { type View interface { // Session returns the session that created this view. Session() Session + + // Name returns the name this view was constructed with. Name() string + + // Folder returns the root folder for this view. Folder() span.URI - FileSet() *token.FileSet + + // BuiltinPackage returns the ast for the special "builtin" package. BuiltinPackage() *ast.Package + + // GetFile returns the file object for a given uri. GetFile(ctx context.Context, uri span.URI) (File, error) + + // Called to set the effective contents of a file from this view. SetContent(ctx context.Context, uri span.URI, content []byte) error + + // BackgroundContext returns a context used for all background processing + // on behalf of this view. BackgroundContext() context.Context - Config() packages.Config + + // Env returns the current set of environment overrides on this view. + Env() []string + + // SetEnv is used to adjust the environment applied to the view. SetEnv([]string) + + // Shutdown closes this view, and detaches it from it's session. Shutdown(ctx context.Context) + + // Ignore returns true if this file should be ignored by this view. Ignore(span.URI) bool } @@ -75,7 +104,7 @@ type File interface { URI() span.URI View() View GetContent(ctx context.Context) []byte - GetFileSet(ctx context.Context) *token.FileSet + FileSet() *token.FileSet GetToken(ctx context.Context) *token.File } diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 20a0ffa963..e273cd3b70 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -69,7 +69,7 @@ func (s *Server) applyChanges(ctx context.Context, params *protocol.DidChangeTex if err != nil { return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "file not found") } - fset := f.GetFileSet(ctx) + fset := f.FileSet() filename, err := f.URI().Filename() if err != nil { return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "no filename for %s", uri) diff --git a/internal/lsp/util.go b/internal/lsp/util.go index a41ba56516..5955882561 100644 --- a/internal/lsp/util.go +++ b/internal/lsp/util.go @@ -55,7 +55,7 @@ func getSourceFile(ctx context.Context, v source.View, uri span.URI) (source.Fil if err != nil { return nil, nil, err } - m := protocol.NewColumnMapper(f.URI(), filename, f.GetFileSet(ctx), f.GetToken(ctx), f.GetContent(ctx)) + m := protocol.NewColumnMapper(f.URI(), filename, f.FileSet(), f.GetToken(ctx), f.GetContent(ctx)) return f, m, nil } diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 687e2c8f69..4c761b48f1 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -7,12 +7,7 @@ package lsp import ( "context" "fmt" - "go/ast" - "go/parser" - "go/token" - "os" - "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" ) @@ -36,25 +31,6 @@ func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFold } func (s *Server) addView(ctx context.Context, name string, uri span.URI) error { - // 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() - folderPath, err := uri.Filename() - if err != nil { - return err - } - s.session.NewView(name, uri, &packages.Config{ - Context: viewContext, - Dir: folderPath, - Env: os.Environ(), - Mode: packages.LoadImports, - Fset: token.NewFileSet(), - Overlay: make(map[string][]byte), - ParseFile: func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) { - return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments) - }, - Tests: true, - }) - + s.session.NewView(name, uri) return nil }