diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 517538d7ad..5a469e0860 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -6,7 +6,6 @@ package cache import ( "context" - "os" "path/filepath" "sort" "strconv" @@ -79,22 +78,22 @@ func (s *session) Cache() source.Cache { 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 f43bcb16b8..767bdbd268 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -53,9 +53,6 @@ type view struct { // 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 @@ type view struct { // 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 @@ func (v *view) Config(ctx context.Context) *packages.Config { // 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 @@ func (v *view) fileVersion(filename string) string { 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 f48507090f..1e7ac99fa9 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -60,10 +60,10 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitia) ( 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 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitia) ( } } - 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 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa }) } - 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 @@ func (s *Server) fetchConfig(ctx context.Context, view source.View, options *sou 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 @@ func (s *Server) processConfig(ctx context.Context, view source.View, options *s 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 @@ func (s *Server) processConfig(ctx context.Context, view source.View, options *s 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 b756ebf745..4ca0b7dfc6 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -50,11 +50,6 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { 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 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { } 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 5622522c3e..423f734c22 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -82,6 +82,10 @@ type Server struct { // 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 81596f8284..a2b58d4e83 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 @@ var ( Deep: true, FuzzyMatching: true, }, + DefaultViewOptions: ViewOptions{ + Env: os.Environ(), + }, } - DefaultViewOptions = ViewOptions{} ) type SessionOptions struct { @@ -47,9 +53,16 @@ type SessionOptions struct { 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 f0ec5bdda6..1b88f88648 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -48,12 +48,14 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { 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 252c4c9284..e3d8d03c4a 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -151,7 +151,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(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 @@ type View interface { // 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 e357fe35f4..bfbf0652dc 100644 --- a/internal/lsp/workspace.go +++ b/internal/lsp/workspace.go @@ -31,14 +31,18 @@ func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFold } 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 }