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>
diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go
index 0aa19e9..f55a818 100644
--- a/internal/lsp/cache/gofile.go
+++ b/internal/lsp/cache/gofile.go
@@ -39,7 +39,7 @@
// 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 @@
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 @@
}
// 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
}
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 3408d3f..285f6a0 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -33,27 +33,43 @@
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 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 @@
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
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index f2eaa7a..d2b3b13 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -78,7 +78,7 @@
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 @@
// 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.
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index f409d73..766877f 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -79,11 +79,11 @@
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) 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 @@
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 @@
}
}
}
+ // 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 @@
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())
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 42b7472..a18fdd2 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -167,7 +167,7 @@
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
}
}
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 36be0fd..de92703 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -53,7 +53,10 @@
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)
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 65a7371..b124674 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -51,8 +51,12 @@
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,
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 747c923..dcbd1b8 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -142,7 +142,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, 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 @@
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 {
diff --git a/internal/lsp/telemetry/telemetry.go b/internal/lsp/telemetry/telemetry.go
index ae4c246..d0c0f2e 100644
--- a/internal/lsp/telemetry/telemetry.go
+++ b/internal/lsp/telemetry/telemetry.go
@@ -20,6 +20,7 @@
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")
diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go
index 42a252a..d20b9d3 100644
--- a/internal/lsp/workspace.go
+++ b/internal/lsp/workspace.go
@@ -8,6 +8,7 @@
"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 @@
}
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)
+
}
