Skip to content

Commit

Permalink
internal/lsp: only build a view when we have its configuration
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@ianthehat
ianthehat committed Sep 10, 2019
1 parent 238129a commit 3d22a3c
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 81 deletions.
7 changes: 3 additions & 4 deletions internal/lsp/cache/session.go
View file
Expand Up @@ -6,7 +6,6 @@ package cache

import (
"context"
"os"
"path/filepath"
"sort"
"strconv"
Expand Down Expand Up @@ -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),
Expand Down
30 changes: 2 additions & 28 deletions internal/lsp/cache/view.go
View file
Expand Up @@ -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.
//
Expand All @@ -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
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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)
}
Expand Down
48 changes: 22 additions & 26 deletions internal/lsp/general.go
View file
Expand Up @@ -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),
}}
Expand All @@ -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 {
Expand Down Expand Up @@ -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{},
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions internal/lsp/lsp_test.go
View file
Expand Up @@ -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: {
Expand All @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions internal/lsp/server.go
View file
Expand Up @@ -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
Expand Down
17 changes: 15 additions & 2 deletions internal/lsp/source/options.go
View file
Expand Up @@ -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{
Expand All @@ -24,8 +28,10 @@ var (
Deep: true,
FuzzyMatching: true,
},
DefaultViewOptions: ViewOptions{
Env: os.Environ(),
},
}
DefaultViewOptions = ViewOptions{}
)

type SessionOptions struct {
Expand All @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions internal/lsp/source/source_test.go
View file
Expand Up @@ -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)
}
Expand Down
11 changes: 1 addition & 10 deletions internal/lsp/source/view.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
12 changes: 8 additions & 4 deletions internal/lsp/workspace.go
View file
Expand Up @@ -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()
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) }()
if state >= serverInitialized {
s.fetchConfig(ctx, view, &options)
}
s.fetchConfig(ctx, name, uri, &options, &viewOptions)
s.session.NewView(ctx, name, uri, viewOptions)
return nil
}

0 comments on commit 3d22a3c

Please sign in to comment.