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 <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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:])
 }

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,
+		cache:    c,
-		log:   log,
+		log:      log,
+		overlays: make(map[span.URI][]byte),
 	}
 }
+
+func (c *cache) FileSet() *token.FileSet {
+	return c.fset
+}

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 := v.buildConfig()
-		cfg.Mode = packages.LoadImports | packages.NeedTypesSizes
+		pkgs, err := packages.Load(cfg, fmt.Sprintf("file=%s", f.filename()))
-		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,
 		})

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 {
+func (f *fileBase) FileSet() *token.FileSet {
-	return f.view.config.Fset
+	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
 	}

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
+		gof, ok := f.(*goFile)
-		if f != nil {
+		if !ok {
-			if gof, ok := f.(*goFile); ok {
+			parsed[i], errors[i] = nil, fmt.Errorf("Non go file in parse call: %v", filename)
-				fAST = gof.ast
+			continue
-			}
 		}
 
 		wg.Add(1)
 		go func(i int, filename string) {
 			ioLimit <- true // wait
 
-			if fAST != nil {
+			if gof.ast != nil {
-				parsed[i], errors[i] = fAST, nil
+				parsed[i], errors[i] = gof.ast, nil
 			} else {
 				// We don't have a cached AST for this file.
-				var src []byte
+				gof.read(imp.ctx)
-				// Check for an available overlay.
+				src := gof.content
-				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.
 				if src == nil {
-					src, err = ioutil.ReadFile(filename)
+					parsed[i], errors[i] = nil, fmt.Errorf("No source for %v", filename)
-				}
-				if err != nil {
-					parsed[i], errors[i] = nil, err
 				} 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)
 					}
 				}

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
+}

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
+	// env is the environment to use when invoking underlying tools.
-	// go/packages API. It is shared across all views.
+	env []string
-	config packages.Config
 
 	// 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 {
+func (v *view) buildConfig() *packages.Config {
-	return v.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 {
+func (v *view) buildBuiltinPkg() {
-	var bpkg *ast.Package
+	v.mu.Lock()
-	cfg.Mode = packages.LoadFiles
+	defer v.mu.Unlock()
+	cfg := *v.buildConfig()
 	pkgs, _ := packages.Load(&cfg, "builtin")
 	if len(pkgs) != 1 {
-		bpkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)
+		v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)
-		return bpkg
+		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)
+			v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)
-			return bpkg
+			return
 		}
 		files[filename] = file
+		v.ignoredURIs[span.NewURI(filename)] = struct{}{}
 	}
-	bpkg, _ = ast.NewPackage(cfg.Fset, files, nil, nil)
+	v.builtinPkg, _ = ast.NewPackage(cfg.Fset, files, nil, nil)
-	return bpkg
-}
-
-func (v *view) FileSet() *token.FileSet {
-	return v.config.Fset
 }
 
 // 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)
 	}
 }
 

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 {
+func New(wd string, env []string) *Application {
-	app := &Application{
+	if wd == "" {
-		Cache: cache.New(),
+		wd, _ = os.Getwd()
 	}
-	if config != nil {
+	app := &Application{
-		app.Config = *config
+		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

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)
 }

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)

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 := cmd.New(r.data.Config.Dir, r.data.Config.Env)
-			app.Config = r.data.Config
 			got := captureStdOut(t, func() {
 				tool.Main(context.Background(), app, append([]string{"-remote=internal", "format"}, args...))
 			})

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)
 }

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()

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
 		}

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,

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

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,
 	}
 }

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
 		}

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(",")
 	}

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 {

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

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

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) {

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

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)
 		}

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

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
 }
 

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)

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
 }

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.
+	s.session.NewView(name, uri)
-	// 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,
-	})
-
 	return nil
 }