diff --git a/internal/lsp/cache/builtin.go b/internal/lsp/cache/builtin.go deleted file mode 100644 index 42a1a065bc..0000000000 --- a/internal/lsp/cache/builtin.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package cache - -import ( - "context" - "go/ast" - "strings" - - "golang.org/x/tools/go/packages" - "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/span" - errors "golang.org/x/xerrors" -) - -type builtinPkg struct { - pkg *ast.Package - files []source.ParseGoHandle -} - -func (v *view) LookupBuiltin(name string) (*ast.Object, error) { - if v.builtin == nil || v.builtin.pkg == nil || v.builtin.pkg.Scope == nil { - return nil, errors.Errorf("no builtin package") - } - astObj := v.builtin.pkg.Scope.Lookup(name) - if astObj == nil { - return nil, errors.Errorf("no builtin object for %s", name) - } - return astObj, nil -} - -func (b *builtinPkg) CompiledGoFiles() []source.ParseGoHandle { - return b.files -} - -// buildBuiltinPkg builds the view's builtin package. -// It assumes that the view is not active yet, -// i.e. it has not been added to the session's list of views. -func (v *view) buildBuiltinPackage(ctx context.Context) error { - if v.builtin != nil { - return errors.Errorf("rebuilding builtin package") - } - cfg := v.Config(ctx) - pkgs, err := packages.Load(cfg, "builtin") - // If the error is related to a go.mod parse error, we want to continue loading. - if err != nil && strings.Contains(err.Error(), ".mod:") { - return nil - } - if err != nil { - return err - } - if len(pkgs) != 1 { - return errors.Errorf("expected 1 (got %v) packages for builtin", len(pkgs)) - } - files := make(map[string]*ast.File) - var pghs []source.ParseGoHandle - for _, filename := range pkgs[0].GoFiles { - fh := v.session.GetFile(span.FileURI(filename)) - pgh := v.session.cache.ParseGoHandle(fh, source.ParseFull) - pghs = append(pghs, pgh) - file, _, _, err := pgh.Parse(ctx) - if err != nil { - return err - } - files[filename] = file - - v.ignoredURIsMu.Lock() - v.ignoredURIs[span.NewURI(filename)] = struct{}{} - v.ignoredURIsMu.Unlock() - } - pkg, err := ast.NewPackage(cfg.Fset, files, nil, nil) - if err != nil { - return err - } - v.builtin = &builtinPkg{ - files: pghs, - pkg: pkg, - } - return nil -} diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 92ca3eb2af..19fa7c0e62 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -35,29 +35,30 @@ type metadata struct { config *packages.Config } -func (s *snapshot) load(ctx context.Context, scope interface{}) ([]*metadata, error) { +func (s *snapshot) load(ctx context.Context, scopes ...interface{}) ([]*metadata, error) { var query []string - switch scope := scope.(type) { - case []packagePath: - for _, p := range scope { - query = append(query, string(p)) + for _, scope := range scopes { + switch scope := scope.(type) { + case []packagePath: + for _, p := range scope { + query = append(query, string(p)) + } + case packagePath: + query = append(query, string(scope)) + case fileURI: + query = append(query, fmt.Sprintf("file=%s", span.URI(scope).Filename())) + case directoryURI: + filename := span.URI(scope).Filename() + q := fmt.Sprintf("%s/...", 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() == filename { + q = "./..." + } + query = append(query, q) } - case packagePath: - query = append(query, string(scope)) - case fileURI: - query = append(query, fmt.Sprintf("file=%s", span.URI(scope).Filename())) - case directoryURI: - filename := span.URI(scope).Filename() - q := fmt.Sprintf("%s/...", 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() == filename { - q = "./..." - } - query = append(query, q) } - ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.Query.Of(query)) defer done() @@ -78,7 +79,7 @@ func (s *snapshot) load(ctx context.Context, scope interface{}) ([]*metadata, er } return nil, err } - return s.updateMetadata(ctx, scope, pkgs, cfg) + return s.updateMetadata(ctx, scopes, pkgs, cfg) } // shouldLoad reparses a file's package and import declarations to @@ -127,10 +128,18 @@ func (c *cache) shouldLoad(ctx context.Context, s *snapshot, originalFH, current return false } -func (s *snapshot) updateMetadata(ctx context.Context, scope interface{}, pkgs []*packages.Package, cfg *packages.Config) ([]*metadata, error) { +func (s *snapshot) updateMetadata(ctx context.Context, scopes []interface{}, pkgs []*packages.Package, cfg *packages.Config) ([]*metadata, error) { var results []*metadata for _, pkg := range pkgs { - if _, isDir := scope.(directoryURI); !isDir || s.view.Options().VerboseOutput { + // Don't log output for full workspace packages.Loads. + var containsDir bool + for _, scope := range scopes { + if _, ok := scope.(directoryURI); ok { + containsDir = true + break + } + } + if !containsDir || s.view.Options().VerboseOutput { log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles)) } // golang/go#36292: Ignore packages with no sources and no errors. @@ -154,7 +163,7 @@ func (s *snapshot) updateMetadata(ctx context.Context, scope interface{}, pkgs [ s.clearAndRebuildImportGraph() if len(results) == 0 { - return nil, errors.Errorf("no metadata for %s", scope) + return nil, errors.Errorf("no metadata for %s", scopes) } return results, nil } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 7915643976..b6a529c88f 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -18,11 +18,11 @@ import ( "sync" "time" - "golang.org/x/sync/errgroup" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/imports" "golang.org/x/tools/internal/lsp/debug" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/memoize" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/tag" @@ -97,8 +97,20 @@ type view struct { initialized chan struct{} initializationError error - // builtin is used to resolve builtin types. - builtin *builtinPkg + // builtin pins the AST and package for builtin.go in memory. + builtin *builtinPackageHandle +} + +type builtinPackageHandle struct { + handle *memoize.Handle + file source.ParseGoHandle +} + +type builtinPackageData struct { + memoize.NoCopy + + pkg *ast.Package + err error } // fileBase holds the common functionality for all files. @@ -169,6 +181,60 @@ func (v *view) SetOptions(ctx context.Context, options source.Options) (source.V return newView, err } +func (v *view) LookupBuiltin(name string) (*ast.Object, error) { + data := v.builtin.handle.Get(context.Background()) + if data == nil { + return nil, errors.Errorf("unexpected nil builtin package") + } + d, ok := data.(*builtinPackageData) + if !ok { + return nil, errors.Errorf("unexpected type %T", data) + } + if d.err != nil { + return nil, d.err + } + if d.pkg == nil || d.pkg.Scope == nil { + return nil, errors.Errorf("no builtin package") + } + astObj := d.pkg.Scope.Lookup(name) + if astObj == nil { + return nil, errors.Errorf("no builtin object for %s", name) + } + return astObj, nil +} + +func (v *view) buildBuiltinPackage(ctx context.Context, m *metadata) error { + if len(m.goFiles) != 1 { + return errors.Errorf("only expected 1 file, got %v", len(m.goFiles)) + } + uri := m.goFiles[0] + v.addIgnoredFile(uri) // to avoid showing diagnostics for builtin.go + + // Get the FileHandle through the session to avoid adding it to the snapshot. + pgh := v.session.cache.ParseGoHandle(v.session.GetFile(uri), source.ParseFull) + fset := v.session.cache.fset + h := v.session.cache.store.Bind(pgh.File().Identity(), func(ctx context.Context) interface{} { + data := &builtinPackageData{} + file, _, _, err := pgh.Parse(ctx) + if err != nil { + data.err = err + return data + } + data.pkg, data.err = ast.NewPackage(fset, map[string]*ast.File{ + pgh.File().Identity().URI.Filename(): file, + }, nil, nil) + if err != nil { + return err + } + return data + }) + v.builtin = &builtinPackageHandle{ + handle: h, + file: pgh, + } + return nil +} + // 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(ctx context.Context) *packages.Config { @@ -445,6 +511,13 @@ func (v *view) Ignore(uri span.URI) bool { return ok } +func (v *view) addIgnoredFile(uri span.URI) { + v.ignoredURIsMu.Lock() + defer v.ignoredURIsMu.Unlock() + + v.ignoredURIs[uri] = struct{}{} +} + func (v *view) BackgroundContext() context.Context { v.mu.Lock() defer v.mu.Unlock() @@ -467,35 +540,31 @@ func (v *view) initialize(ctx context.Context, s *snapshot) { v.initializeOnce.Do(func() { defer close(v.initialized) - g, ctx := errgroup.WithContext(ctx) - - // Load all of the packages in the workspace. - g.Go(func() error { + v.initializationError = func() error { // Do not cancel the call to go/packages.Load for the entire workspace. - meta, err := s.load(ctx, directoryURI(v.folder)) + meta, err := s.load(ctx, directoryURI(v.folder), packagePath("builtin")) if err != nil { return err } - // A test variant of a package can only be loaded directly by loading - // the non-test variant with -test. Track the import path of the non-test variant. + // Keep track of the workspace packages. for _, m := range meta { + // Make sure to handle the builtin package separately + // Don't set it as a workspace package. + if m.pkgPath == "builtin" { + if err := s.view.buildBuiltinPackage(ctx, m); err != nil { + return err + } + continue + } + // A test variant of a package can only be loaded directly by loading + // the non-test variant with -test. Track the import path of the non-test variant. s.setWorkspacePackage(m.id, m.pkgPath) if _, err := s.packageHandle(ctx, m.id); err != nil { return err } } return nil - }) - - // Build the builtin package on initialization. - g.Go(func() error { - return v.buildBuiltinPackage(ctx) - }) - - // Wait for all initialization tasks to complete. - if err := g.Wait(); err != nil { - v.initializationError = err - } + }() }) } diff --git a/internal/lsp/mod/mod_test.go b/internal/lsp/mod/mod_test.go index a72054617e..05c6169464 100644 --- a/internal/lsp/mod/mod_test.go +++ b/internal/lsp/mod/mod_test.go @@ -155,7 +155,7 @@ func TestDiagnostics(t *testing.T) { t.Fatal(err) } if len(reports) != 1 { - t.Errorf("expected 1 fileHandle, got %d", len(reports)) + t.Errorf("expected 1 diagnostic, got %d", len(reports)) } for fh, got := range reports { if diff := tests.DiffDiagnostics(fh.URI, tt.want, got); diff != "" {