internal/lsp: only build a view when we have its configuration
We now wait to build views until we have the options for that view,
and pass the options in to the view constructor.
The environment and build flags are now part of the view options.
Change-Id: I303c8ba1eefd01b66962ba9cadb4847d3d2e1d3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 517538d..5a469e0 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -6,7 +6,6 @@
import (
"context"
- "os"
"path/filepath"
"sort"
"strconv"
@@ -79,22 +78,22 @@
return s.cache
}
-func (s *session) NewView(ctx context.Context, name string, folder span.URI) source.View {
+func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.ViewOptions) source.View {
index := atomic.AddInt64(&viewIndex, 1)
s.viewMu.Lock()
defer s.viewMu.Unlock()
- // We want a true background context and not a detatched context here
+ // We want a true background context and not a detached context here
// the spans need to be unrelated and no tag values should pollute it.
baseCtx := trace.Detach(xcontext.Detach(ctx))
backgroundCtx, cancel := context.WithCancel(baseCtx)
v := &view{
session: s,
id: strconv.FormatInt(index, 10),
+ options: options,
baseCtx: baseCtx,
backgroundCtx: backgroundCtx,
cancel: cancel,
name: name,
- env: os.Environ(),
folder: folder,
filesByURI: make(map[span.URI]viewFile),
filesByBase: make(map[string][]viewFile),
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index f43bcb1..767bdbd 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -53,9 +53,6 @@
// Folder is the root of this view.
folder span.URI
- // env is the environment to use when invoking underlying tools.
- env []string
-
// process is the process env for this view.
// Note: this contains cached module and filesystem state.
//
@@ -69,9 +66,6 @@
// TODO(suzmue): These versions may not actually be on disk.
modFileVersions map[string]string
- // buildFlags is the build flags to use when invoking underlying tools.
- buildFlags []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
filesByURI map[span.URI]viewFile
@@ -130,8 +124,8 @@
// TODO: Should we cache the config and/or overlay somewhere?
return &packages.Config{
Dir: v.folder.Filename(),
- Env: v.env,
- BuildFlags: v.buildFlags,
+ Env: v.options.Env,
+ BuildFlags: v.options.BuildFlags,
Mode: packages.NeedName |
packages.NeedFiles |
packages.NeedCompiledGoFiles |
@@ -262,26 +256,6 @@
return f.Identity().Version
}
-func (v *view) Env() []string {
- v.mu.Lock()
- defer v.mu.Unlock()
- return v.env
-}
-
-func (v *view) SetEnv(env []string) {
- v.mu.Lock()
- defer v.mu.Unlock()
- //TODO: this should invalidate the entire view
- v.env = env
- v.processEnv = nil // recompute process env
-}
-
-func (v *view) SetBuildFlags(buildFlags []string) {
- v.mu.Lock()
- defer v.mu.Unlock()
- v.buildFlags = buildFlags
-}
-
func (v *view) Shutdown(ctx context.Context) {
v.session.removeView(ctx, v)
}
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index f485070..1e7ac99 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -60,10 +60,10 @@
s.setClientCapabilities(&options, params.Capabilities)
- folders := params.WorkspaceFolders
- if len(folders) == 0 {
+ s.pendingFolders = params.WorkspaceFolders
+ if len(s.pendingFolders) == 0 {
if params.RootURI != "" {
- folders = []protocol.WorkspaceFolder{{
+ s.pendingFolders = []protocol.WorkspaceFolder{{
URI: params.RootURI,
Name: path.Base(params.RootURI),
}}
@@ -75,12 +75,6 @@
}
}
- for _, folder := range folders {
- if err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
- return nil, err
- }
- }
-
var codeActionProvider interface{}
if params.Capabilities.TextDocument.CodeAction.CodeActionLiteralSupport != nil &&
len(params.Capabilities.TextDocument.CodeAction.CodeActionLiteralSupport.CodeActionKind.ValueSet) > 0 {
@@ -208,28 +202,32 @@
})
}
- if options.ConfigurationSupported {
- for _, view := range s.session.Views() {
- if err := s.fetchConfig(ctx, view, &options); err != nil {
- return err
- }
- }
- }
buf := &bytes.Buffer{}
debug.PrintVersionInfo(buf, true, debug.PlainText)
log.Print(ctx, buf.String())
+
+ for _, folder := range s.pendingFolders {
+ if err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
+ return err
+ }
+ }
+ s.pendingFolders = nil
+
return nil
}
-func (s *Server) fetchConfig(ctx context.Context, view source.View, options *source.SessionOptions) error {
+func (s *Server) fetchConfig(ctx context.Context, name string, folder span.URI, options *source.SessionOptions, vo *source.ViewOptions) error {
+ if !options.ConfigurationSupported {
+ return nil
+ }
v := protocol.ParamConfig{
protocol.ConfigurationParams{
Items: []protocol.ConfigurationItem{{
- ScopeURI: protocol.NewURI(view.Folder()),
+ ScopeURI: protocol.NewURI(folder),
Section: "gopls",
}, {
- ScopeURI: protocol.NewURI(view.Folder()),
- Section: view.Name(),
+ ScopeURI: protocol.NewURI(folder),
+ Section: name,
},
},
}, protocol.PartialResultParams{},
@@ -239,14 +237,14 @@
return err
}
for _, config := range configs {
- if err := s.processConfig(ctx, view, options, config); err != nil {
+ if err := s.processConfig(ctx, options, vo, config); err != nil {
return err
}
}
return nil
}
-func (s *Server) processConfig(ctx context.Context, view source.View, options *source.SessionOptions, config interface{}) error {
+func (s *Server) processConfig(ctx context.Context, options *source.SessionOptions, vo *source.ViewOptions, config interface{}) error {
// TODO: We should probably store and process more of the config.
if config == nil {
return nil // ignore error if you don't have a config
@@ -263,11 +261,9 @@
if !ok {
return errors.Errorf("invalid config gopls.env type %T", env)
}
- env := view.Env()
for k, v := range menv {
- env = append(env, fmt.Sprintf("%s=%s", k, v))
+ vo.Env = append(vo.Env, fmt.Sprintf("%s=%s", k, v))
}
- view.SetEnv(env)
}
// Get the build flags for the go/packages config.
@@ -280,7 +276,7 @@
for _, flag := range iflags {
flags = append(flags, fmt.Sprintf("%s", flag))
}
- view.SetBuildFlags(flags)
+ vo.BuildFlags = flags
}
// Set the hover kind.
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index b756ebf..4ca0b7d 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -50,11 +50,6 @@
cache := cache.New()
session := cache.NewSession(ctx)
- view := session.NewView(ctx, viewName, span.FileURI(data.Config.Dir))
- view.SetEnv(data.Config.Env)
- for filename, content := range data.Config.Overlay {
- session.SetOverlay(span.FileURI(filename), content)
- }
options := session.Options()
options.SupportedCodeActions = map[source.FileKind]map[protocol.CodeActionKind]bool{
source.Go: {
@@ -66,6 +61,12 @@
}
options.HoverKind = source.SynopsisDocumentation
session.SetOptions(options)
+ vo := options.DefaultViewOptions
+ vo.Env = data.Config.Env
+ session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), vo)
+ for filename, content := range data.Config.Overlay {
+ session.SetOverlay(span.FileURI(filename), content)
+ }
r := &runner{
server: &Server{
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index 5622522..423f734 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -82,6 +82,10 @@
// failed to deliver for some reason.
undeliveredMu sync.Mutex
undelivered map[span.URI][]source.Diagnostic
+
+ // folders is only valid between initialize and initialized, and holds the
+ // set of folders to build views for when we are ready
+ pendingFolders []protocol.WorkspaceFolder
}
// General
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 81596f8..a2b58d4 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -4,7 +4,11 @@
package source
-import "golang.org/x/tools/internal/lsp/protocol"
+import (
+ "os"
+
+ "golang.org/x/tools/internal/lsp/protocol"
+)
var (
DefaultSessionOptions = SessionOptions{
@@ -24,8 +28,10 @@
Deep: true,
FuzzyMatching: true,
},
+ DefaultViewOptions: ViewOptions{
+ Env: os.Environ(),
+ },
}
- DefaultViewOptions = ViewOptions{}
)
type SessionOptions struct {
@@ -47,9 +53,16 @@
TextDocumentSyncKind protocol.TextDocumentSyncKind
Completion CompletionOptions
+
+ DefaultViewOptions ViewOptions
}
type ViewOptions struct {
+ // Env is the current set of environment overrides on this view.
+ Env []string
+
+ // BuildFlags is used to adjust the build flags applied to the view.
+ BuildFlags []string
}
type CompletionOptions struct {
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index f0ec5bd..1b88f88 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -48,12 +48,14 @@
cache := cache.New()
session := cache.NewSession(ctx)
+ options := session.Options()
+ vo := options.DefaultViewOptions
+ vo.Env = data.Config.Env
r := &runner{
- view: session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir)),
+ view: session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), vo),
data: data,
ctx: ctx,
}
- r.view.SetEnv(data.Config.Env)
for filename, content := range data.Config.Overlay {
session.SetOverlay(span.FileURI(filename), content)
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 252c4c9..e3d8d03 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -151,7 +151,7 @@
// A session may have many active views at any given time.
type Session interface {
// NewView creates a new View and returns it.
- NewView(ctx context.Context, name string, folder span.URI) View
+ NewView(ctx context.Context, name string, folder span.URI, options ViewOptions) View
// Cache returns the cache that created this session.
Cache() Cache
@@ -229,15 +229,6 @@
// on behalf of this view.
BackgroundContext() context.Context
- // 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)
-
- // SetBuildFlags is used to adjust the build flags applied to the view.
- SetBuildFlags([]string)
-
// Shutdown closes this view, and detaches it from it's session.
Shutdown(ctx context.Context)
diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go
index e357fe3..bfbf065 100644
--- a/internal/lsp/workspace.go
+++ b/internal/lsp/workspace.go
@@ -31,14 +31,18 @@
}
func (s *Server) addView(ctx context.Context, name string, uri span.URI) error {
- view := s.session.NewView(ctx, name, uri)
s.stateMu.Lock()
state := s.state
s.stateMu.Unlock()
- options := s.session.Options()
- defer func() { s.session.SetOptions(options) }()
- if state >= serverInitialized {
- s.fetchConfig(ctx, view, &options)
+ if state < serverInitialized {
+ return errors.Errorf("addView called before server initialized")
}
+
+ options := s.session.Options()
+ viewOptions := options.DefaultViewOptions
+ //TODO: take this out, we only allow new session options here
+ defer func() { s.session.SetOptions(options) }()
+ s.fetchConfig(ctx, name, uri, &options, &viewOptions)
+ s.session.NewView(ctx, name, uri, viewOptions)
return nil
}
