diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go index 0aa19e986b7..f55a8184665 100644 --- a/internal/lsp/cache/gofile.go +++ b/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 } diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 3408d3fef2c..285f6a0cc66 100644 --- a/internal/lsp/cache/load.go +++ b/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 + // /... 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 diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index f2eaa7aa511..d2b3b13d8c0 100644 --- a/internal/lsp/cache/session.go +++ b/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. diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index f409d73e1fc..766877f6613 100644 --- a/internal/lsp/cache/snapshot.go +++ b/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()) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 42b7472db9d..a18fdd283f9 100644 --- a/internal/lsp/general.go +++ b/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 } } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 36be0fde58a..de927030900 100644 --- a/internal/lsp/lsp_test.go +++ b/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) } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 65a7371d59f..b12467434cb 100644 --- a/internal/lsp/source/source_test.go +++ b/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, } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 747c9232ebe..dcbd1b8137f 100644 --- a/internal/lsp/source/view.go +++ b/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 { diff --git a/internal/lsp/telemetry/telemetry.go b/internal/lsp/telemetry/telemetry.go index ae4c2467454..d0c0f2e11e9 100644 --- a/internal/lsp/telemetry/telemetry.go +++ b/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") diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go index 42a252a9fab..d20b9d3dffe 100644 --- a/internal/lsp/workspace.go +++ b/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) + }