internal/lsp: call load in (*session).NewView

Add a source.Scope type that can be used to refer to directories or
files, and modify (*snapshot).load to take source.Scope.
Then call load in NewView.

Change-Id: I8f03c7b271d700b162100d2890d23219ef9578c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/204822
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/gofile.go

@@ -39,7 +39,7 @@ func (s *snapshot) CheckPackageHandles(ctx context.Context, f source.File) ([]so
 	// We only need to this if it has been invalidated, and is therefore unvailable.
 	if load {
 		var err error
-		m, err = s.load(ctx, f.URI())
+		m, err = s.load(ctx, source.FileURI(f.URI()))
 		if err != nil {
 			return nil, err
 		}
@@ -61,7 +61,7 @@ func (s *snapshot) CheckPackageHandles(ctx context.Context, f source.File) ([]so
 		cphs = results
 	}
 	if len(cphs) == 0 {
-		return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI())
+		return nil, errors.Errorf("no CheckPackageHandles for %s", f)
 	}
 	return cphs, nil
 }
@@ -86,7 +86,7 @@ func (s *snapshot) shouldCheck(fh source.FileHandle) (m []*metadata, cphs []sour
 	}
 	// We expect to see a checked package for each package ID,
 	// and it should be parsed in full mode.
-	cphs = s.getPackages(fh.Identity().URI, source.ParseFull)
+	cphs = s.getPackages(source.FileURI(fh.Identity().URI), source.ParseFull)
 	if len(cphs) < len(m) {
 		return m, nil, load, true
 	}

internal/lsp/cache/load.go

@@ -33,27 +33,43 @@ type metadata struct {
 	config *packages.Config
 }
 
-func (s *snapshot) load(ctx context.Context, uri span.URI) ([]*metadata, error) {
+func (s *snapshot) load(ctx context.Context, scope source.Scope) ([]*metadata, error) {
+	uri := scope.URI()
+	var query string
+	switch scope.(type) {
+	case source.FileURI:
+		query = fmt.Sprintf("file=%s", scope.URI().Filename())
+	case source.DirectoryURI:
+		query = fmt.Sprintf("%s/...", scope.URI().Filename())
+		// Simplify the query if it will be run in the requested directory.
+		// This ensures compatibility with Go 1.12 that doesn't allow
+		// <directory>/... in GOPATH mode.
+		if s.view.folder.Filename() == scope.URI().Filename() {
+			query = "./..."
+		}
+	default:
+		panic(fmt.Errorf("unsupported scope type %T", scope))
+	}
 	ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(uri))
 	defer done()
 
 	cfg := s.view.Config(ctx)
-	pkgs, err := packages.Load(cfg, fmt.Sprintf("file=%s", uri.Filename()))
+	pkgs, err := packages.Load(cfg, query)
 
 	// If the context was canceled, return early.
 	// Otherwise, we might be type-checking an incomplete result.
 	if err == context.Canceled {
-		return nil, errors.Errorf("no metadata for %s: %v", uri.Filename(), err)
+		return nil, errors.Errorf("no metadata for %s: %v", uri, err)
 	}
 	log.Print(ctx, "go/packages.Load", tag.Of("packages", len(pkgs)))
 	if len(pkgs) == 0 {
 		if err == nil {
-			err = errors.Errorf("go/packages.Load: no packages found for %s", uri)
+			err = errors.Errorf("go/packages.Load: no packages found for %s", query)
 		}
 		// Return this error as a diagnostic to the user.
 		return nil, err
 	}
-	m, prevMissingImports, err := s.updateMetadata(ctx, uri, pkgs, cfg)
+	m, prevMissingImports, err := s.updateMetadata(ctx, scope, pkgs, cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -65,7 +81,7 @@ func (s *snapshot) load(ctx context.Context, uri span.URI) ([]*metadata, error)
 }
 
 func validateMetadata(ctx context.Context, metadata []*metadata, prevMissingImports map[packageID]map[packagePath]struct{}) ([]*metadata, error) {
-	// If we saw incorrect metadata for this package previously, don't both rechecking it.
+	// If we saw incorrect metadata for this package previously, don't bother rechecking it.
 	for _, m := range metadata {
 		if len(m.missingDeps) > 0 {
 			prev, ok := prevMissingImports[m.id]
@@ -132,11 +148,22 @@ func (c *cache) shouldLoad(ctx context.Context, s *snapshot, originalFH, current
 	return false
 }
 
-func (s *snapshot) updateMetadata(ctx context.Context, uri span.URI, pkgs []*packages.Package, cfg *packages.Config) ([]*metadata, map[packageID]map[packagePath]struct{}, error) {
+func (s *snapshot) updateMetadata(ctx context.Context, uri source.Scope, pkgs []*packages.Package, cfg *packages.Config) ([]*metadata, map[packageID]map[packagePath]struct{}, error) {
 	// Clear metadata since we are re-running go/packages.
+	var m []*metadata
+	switch uri.(type) {
+	case source.FileURI:
+		m = s.getMetadataForURI(uri.URI())
+	case source.DirectoryURI:
+		for _, pkg := range pkgs {
+			if pkgMetadata := s.getMetadata(packageID(pkg.ID)); pkgMetadata != nil {
+				m = append(m, pkgMetadata)
+			}
+		}
+	default:
+		panic(fmt.Errorf("unsupported Scope type %T", uri))
+	}
 	prevMissingImports := make(map[packageID]map[packagePath]struct{})
-	m := s.getMetadataForURI(uri)
-
 	for _, m := range m {
 		if len(m.missingDeps) > 0 {
 			prevMissingImports[m.id] = m.missingDeps

internal/lsp/cache/session.go

@@ -78,7 +78,7 @@ func (s *session) Cache() source.Cache {
 	return s.cache
 }
 
-func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) source.View {
+func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) (source.View, error) {
 	index := atomic.AddInt64(&viewIndex, 1)
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()
@@ -118,11 +118,32 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt
 	// so we immediately add builtin.go to the list of ignored files.
 	v.buildBuiltinPackage(ctx)
 
+	// Preemptively load everything in this directory.
+	// TODO(matloob): Determine if this can be done in parallel with something else.
+	// Perhaps different calls to NewView can be run in parallel?
+	// TODO(matloob): By default when a new file is opened, its data is invalidated
+	// and it's loaded again. Determine if the redundant reload can be avoided.
+	v.snapshotMu.Lock()
+	defer v.snapshotMu.Unlock() // The code after the snapshot is used isn't expensive.
+	m, err := v.snapshot.load(ctx, source.DirectoryURI(folder))
+	if err != nil {
+		return nil, err
+	}
+	// Prepare CheckPackageHandles for every package that's been loaded.
+	// (*snapshot).CheckPackageHandle makes the assumption that every package that's
+	// been loaded has an existing checkPackageHandle.
+	for _, m := range m {
+		_, err := v.snapshot.checkPackageHandle(ctx, m.id, source.ParseFull)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	s.views = append(s.views, v)
 	// we always need to drop the view map
 	s.viewMap = make(map[span.URI]source.View)
 	debug.AddView(debugView{v})
-	return v
+	return v, nil
 }
 
 // View returns the view by name.

internal/lsp/cache/snapshot.go

@@ -79,11 +79,11 @@ func (s *snapshot) addPackage(cph *checkPackageHandle) {
 	s.packages[cph.packageKey()] = cph
 }
 
-func (s *snapshot) getPackages(uri span.URI, m source.ParseMode) (cphs []source.CheckPackageHandle) {
+func (s *snapshot) getPackages(uri source.FileURI, m source.ParseMode) (cphs []source.CheckPackageHandle) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if ids, ok := s.ids[uri]; ok {
+	if ids, ok := s.ids[uri.URI()]; ok {
 		for _, id := range ids {
 			key := packageKey{
 				id:   id,
@@ -178,6 +178,8 @@ func (s *snapshot) addAction(ah *actionHandle) {
 }
 
 func (s *snapshot) getMetadataForURI(uri span.URI) (metadata []*metadata) {
+	// TODO(matloob): uri can be a file or directory. Should we update the mappings
+	// to map directories to their contained packages?
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -340,6 +342,9 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source
 		ids[id] = struct{}{}
 	}
 
+	// Get the original FileHandle for the URI, if it exists.
+	originalFH := v.snapshot.getFile(f.URI())
+
 	switch changeType {
 	case protocol.Created:
 		// If this is a file we don't yet know about,
@@ -358,6 +363,8 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source
 				}
 			}
 		}
+		// If a file has been explicitly created, make sure that its original file handle is nil.
+		originalFH = nil
 	}
 
 	if len(ids) == 0 {
@@ -369,9 +376,6 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source
 		v.snapshot.reverseDependencies(id, withoutTypes, map[packageID]struct{}{})
 	}
 
-	// Get the original FileHandle for the URI, if it exists.
-	originalFH := v.snapshot.getFile(f.URI())
-
 	// Make sure to clear out the content if there has been a deletion.
 	if changeType == protocol.Deleted {
 		v.session.clearOverlay(f.URI())

internal/lsp/general.go

@@ -167,7 +167,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 	log.Print(ctx, buf.String())
 
 	for _, folder := range s.pendingFolders {
-		if err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
+		if _, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
 			return err
 		}
 	}

internal/lsp/lsp_test.go

@@ -53,7 +53,10 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	options := tests.DefaultOptions()
 	session.SetOptions(options)
 	options.Env = data.Config.Env
-	session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), options)
+	_, err := session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), options)
+	if err != nil {
+		t.Fatal(err)
+	}
 	for filename, content := range data.Config.Overlay {
 		session.SetOverlay(span.FileURI(filename), source.DetectLanguage("", filename), content)
 	}

internal/lsp/source/source_test.go

@@ -51,8 +51,12 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
 	session := cache.NewSession(ctx)
 	options := tests.DefaultOptions()
 	options.Env = data.Config.Env
+	view, err := session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), options)
+	if err != nil {
+		t.Fatal(err)
+	}
 	r := &runner{
-		view: session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), options),
+		view: view,
 		data: data,
 		ctx:  ctx,
 	}

internal/lsp/source/view.go

@@ -142,7 +142,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, options Options) View
+	NewView(ctx context.Context, name string, folder span.URI, options Options) (View, error)
 
 	// Cache returns the cache that created this session.
 	Cache() Cache
@@ -287,6 +287,22 @@ type File interface {
 	Kind() FileKind
 }
 
+type FileURI span.URI
+
+func (f FileURI) URI() span.URI {
+	return span.URI(f)
+}
+
+type DirectoryURI span.URI
+
+func (d DirectoryURI) URI() span.URI {
+	return span.URI(d)
+}
+
+type Scope interface {
+	URI() span.URI
+}
+
 // Package represents a Go package that has been type-checked. It maintains
 // only the relevant fields of a *go/packages.Package.
 type Package interface {

internal/lsp/telemetry/telemetry.go

@@ -20,6 +20,7 @@ const (
 	RPCID         = tag.Key("id")
 	RPCDirection  = tag.Key("direction")
 	File          = tag.Key("file")
+	Directory     = tag.Key("directory")
 	URI           = tag.Key("URI")
 	Package       = tag.Key("package")
 	PackagePath   = tag.Key("package_path")

internal/lsp/workspace.go

@@ -8,6 +8,7 @@ import (
 	"context"
 
 	"golang.org/x/tools/internal/lsp/protocol"
+	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
 	errors "golang.org/x/xerrors"
 )
@@ -23,23 +24,24 @@ func (s *Server) changeFolders(ctx context.Context, event protocol.WorkspaceFold
 	}
 
 	for _, folder := range event.Added {
-		if err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
+		if _, err := s.addView(ctx, folder.Name, span.NewURI(folder.URI)); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (s *Server) addView(ctx context.Context, name string, uri span.URI) error {
+func (s *Server) addView(ctx context.Context, name string, uri span.URI) (source.View, error) {
 	s.stateMu.Lock()
 	state := s.state
 	s.stateMu.Unlock()
 	if state < serverInitialized {
-		return errors.Errorf("addView called before server initialized")
+		return nil, errors.Errorf("addView called before server initialized")
 	}
 
 	options := s.session.Options()
 	s.fetchConfig(ctx, name, uri, &options)
-	s.session.NewView(ctx, name, uri, options)
+
-	return nil
+	return s.session.NewView(ctx, name, uri, options)
+
 }