From 57610eddc97412e0df6bdb373051fed02932f22a Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 27 Sep 2019 13:17:59 -0400 Subject: [PATCH] internal/lsp: rework snapshots and cache FileHandles per-snapshot This change does not complete the work to handle snapshots correctly, but it does implement the behavior of re-building the snapshot on each file invalidation. It also moves to the approach of caching the FileHandles on the snapshot, rather than in the goFile object, which is now not necessary. Finally, this change shifts the logic of metadata invalidation into the content invalidation step, so there is less logic to decide if we should re-load a package or not. Change-Id: I18387c385fb070da4db1302bf97035ce6328b5c3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/197799 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/check.go | 63 ++-- internal/lsp/cache/file.go | 20 +- internal/lsp/cache/gofile.go | 171 ++++++--- internal/lsp/cache/load.go | 271 +++++++------- internal/lsp/cache/pkg.go | 8 + internal/lsp/cache/session.go | 2 + internal/lsp/cache/snapshot.go | 442 ++++++++++------------- internal/lsp/cache/view.go | 68 ++-- internal/lsp/code_action.go | 16 +- internal/lsp/command.go | 3 +- internal/lsp/completion.go | 2 +- internal/lsp/definition.go | 4 +- internal/lsp/diagnostics.go | 8 +- internal/lsp/folding_range.go | 2 +- internal/lsp/hover.go | 2 +- internal/lsp/link.go | 4 +- internal/lsp/lsp_test.go | 8 +- internal/lsp/references.go | 2 +- internal/lsp/rename.go | 4 +- internal/lsp/server.go | 1 + internal/lsp/signature_help.go | 2 +- internal/lsp/source/completion.go | 8 +- internal/lsp/source/completion_format.go | 2 +- internal/lsp/source/diagnostics.go | 13 +- internal/lsp/source/folding_range.go | 6 +- internal/lsp/source/format.go | 21 +- internal/lsp/source/highlight.go | 3 +- internal/lsp/source/identifier.go | 45 +-- internal/lsp/source/rename.go | 5 +- internal/lsp/source/signature_help.go | 8 +- internal/lsp/source/source_test.go | 32 +- internal/lsp/source/symbols.go | 4 +- internal/lsp/source/util.go | 2 +- internal/lsp/source/view.go | 40 +- internal/lsp/symbols.go | 2 +- internal/lsp/text_synchronization.go | 15 +- internal/lsp/util.go | 25 -- internal/lsp/watched_files.go | 12 +- 38 files changed, 654 insertions(+), 692 deletions(-) delete mode 100644 internal/lsp/util.go diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 2f8020869a..46ade5cbfc 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -23,9 +23,9 @@ import ( ) type importer struct { - view *view - ctx context.Context - config *packages.Config + snapshot *snapshot + ctx context.Context + config *packages.Config // seen maintains the set of previously imported packages. // If we have seen a package that is already in this map, we have a circular import. @@ -62,7 +62,7 @@ type checkPackageHandle struct { config *packages.Config } -func (cph *checkPackageHandle) mode() source.ParseMode { +func (cph *checkPackageHandle) Mode() source.ParseMode { if len(cph.files) == 0 { return -1 } @@ -83,19 +83,15 @@ type checkPackageData struct { err error } -func (pkg *pkg) GetImport(ctx context.Context, pkgPath string) (source.Package, error) { - if imp := pkg.imports[packagePath(pkgPath)]; imp != nil { - return imp, nil - } - // Don't return a nil pointer because that still satisfies the interface. - return nil, errors.Errorf("no imported package for %s", pkgPath) -} - // checkPackageHandle returns a source.CheckPackageHandle for a given package and config. -func (imp *importer) checkPackageHandle(ctx context.Context, m *metadata) (*checkPackageHandle, error) { +func (imp *importer) checkPackageHandle(ctx context.Context, id packageID, s *snapshot) (*checkPackageHandle, error) { + m := s.getMetadata(id) + if m == nil { + return nil, errors.Errorf("no metadata for %s", id) + } phs, err := imp.parseGoHandles(ctx, m) if err != nil { - log.Error(ctx, "no ParseGoHandles", err, telemetry.Package.Of(m.id)) + log.Error(ctx, "no ParseGoHandles", err, telemetry.Package.Of(id)) return nil, err } cph := &checkPackageHandle{ @@ -104,12 +100,19 @@ func (imp *importer) checkPackageHandle(ctx context.Context, m *metadata) (*chec config: imp.config, imports: make(map[packagePath]*checkPackageHandle), } - h := imp.view.session.cache.store.Bind(cph.key(), func(ctx context.Context) interface{} { + h := imp.snapshot.view.session.cache.store.Bind(cph.key(), func(ctx context.Context) interface{} { data := &checkPackageData{} data.pkg, data.err = imp.typeCheck(ctx, cph, m) return data }) + cph.handle = h + + // Cache the CheckPackageHandle in the snapshot. + for _, ph := range cph.files { + uri := ph.File().Identity().URI + s.addPackage(uri, cph) + } return cph, nil } @@ -190,16 +193,16 @@ func (cph *checkPackageHandle) key() checkPackageKey { func (imp *importer) parseGoHandles(ctx context.Context, m *metadata) ([]source.ParseGoHandle, error) { phs := make([]source.ParseGoHandle, 0, len(m.files)) for _, uri := range m.files { - f, err := imp.view.GetFile(ctx, uri) + f, err := imp.snapshot.view.GetFile(ctx, uri) if err != nil { return nil, err } - fh := f.Handle(ctx) + fh := imp.snapshot.Handle(ctx, f) mode := source.ParseExported if imp.topLevelPackageID == m.id { mode = source.ParseFull } - phs = append(phs, imp.view.session.cache.ParseGoHandle(fh, mode)) + phs = append(phs, imp.snapshot.view.session.cache.ParseGoHandle(fh, mode)) } return phs, nil } @@ -236,7 +239,7 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m * defer done() pkg := &pkg{ - view: imp.view, + view: imp.snapshot.view, id: m.id, pkgPath: m.pkgPath, files: cph.Files(), @@ -259,13 +262,17 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m * } // Set imports of package to correspond to cached packages. cimp := imp.child(ctx, pkg, cph) - for _, child := range m.children { - childHandle, err := cimp.checkPackageHandle(ctx, child) - if err != nil { - log.Error(ctx, "no check package handle", err, telemetry.Package.Of(child.id)) + for _, depID := range m.deps { + dep := imp.snapshot.getMetadata(depID) + if dep == nil { continue } - cph.imports[child.pkgPath] = childHandle + depHandle, err := cimp.checkPackageHandle(ctx, depID, imp.snapshot) + if err != nil { + log.Error(ctx, "no check package handle", err, telemetry.Package.Of(depID)) + continue + } + cph.imports[dep.pkgPath] = depHandle } var ( files = make([]*ast.File, len(pkg.files)) @@ -284,7 +291,7 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m * for _, err := range parseErrors { if err != nil { - imp.view.session.cache.appendPkgError(pkg, err) + imp.snapshot.view.session.cache.appendPkgError(pkg, err) } } @@ -308,11 +315,11 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle, m * cfg := &types.Config{ Error: func(err error) { - imp.view.session.cache.appendPkgError(pkg, err) + imp.snapshot.view.session.cache.appendPkgError(pkg, err) }, Importer: cimp, } - check := types.NewChecker(cfg, imp.view.session.cache.FileSet(), pkg.types, pkg.typesInfo) + check := types.NewChecker(cfg, imp.snapshot.view.session.cache.FileSet(), pkg.types, pkg.typesInfo) // Type checking errors are handled via the config, so ignore them here. _ = check.Files(files) @@ -328,7 +335,7 @@ func (imp *importer) child(ctx context.Context, pkg *pkg, cph *checkPackageHandl } seen[pkg.id] = struct{}{} return &importer{ - view: imp.view, + snapshot: imp.snapshot, ctx: ctx, config: imp.config, seen: seen, diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index a9748d5336..a40e0244dc 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -5,11 +5,9 @@ package cache import ( - "context" "go/token" "path/filepath" "strings" - "sync" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" @@ -31,9 +29,6 @@ type fileBase struct { kind source.FileKind view *view - - handleMu sync.Mutex - handle source.FileHandle } func basename(filename string) string { @@ -44,6 +39,10 @@ func (f *fileBase) URI() span.URI { return f.uris[0] } +func (f *fileBase) Kind() source.FileKind { + return f.kind +} + func (f *fileBase) filename() string { return f.fname } @@ -53,17 +52,6 @@ func (f *fileBase) View() source.View { return f.view } -// Content returns a handle for the contents of the file. -func (f *fileBase) Handle(ctx context.Context) source.FileHandle { - f.handleMu.Lock() - defer f.handleMu.Unlock() - - if f.handle == nil { - f.handle = f.view.session.GetFile(f.URI(), f.kind) - } - return f.handle -} - func (f *fileBase) FileSet() *token.FileSet { return f.view.Session().Cache().FileSet() } diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go index 1102cec943..e3ed8ec739 100644 --- a/internal/lsp/cache/gofile.go +++ b/internal/lsp/cache/gofile.go @@ -6,8 +6,6 @@ package cache import ( "context" - "go/ast" - "sync" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -15,68 +13,112 @@ import ( errors "golang.org/x/xerrors" ) -// goFile holds all of the information we know about a Go file. -type goFile struct { - fileBase +func (v *view) CheckPackageHandles(ctx context.Context, f source.File) (source.Snapshot, []source.CheckPackageHandle, error) { + // Get the snapshot that will be used for type-checking. + s := v.getSnapshot() - // mu protects all mutable state of the Go file, - // which can be modified during type-checking. - mu sync.Mutex - - imports []*ast.ImportSpec + cphs, err := s.checkPackageHandles(ctx, f) + if err != nil { + return nil, nil, err + } + return s, cphs, nil } -type packageKey struct { - id packageID - mode source.ParseMode -} - -func (f *goFile) CheckPackageHandles(ctx context.Context) (results []source.CheckPackageHandle, err error) { +func (s *snapshot) checkPackageHandles(ctx context.Context, f source.File) ([]source.CheckPackageHandle, error) { ctx = telemetry.File.With(ctx, f.URI()) - fh := f.Handle(ctx) - var dirty bool - meta, pkgs := f.view.getSnapshot(f.URI()) - if len(meta) == 0 { - dirty = true - } - for _, m := range meta { - if len(m.missingDeps) != 0 { - dirty = true - } - } - for _, cph := range pkgs { - // If we're explicitly checking if a file needs to be type-checked, - // we need it to be fully parsed. - if cph.mode() != source.ParseFull { - continue - } - // Check if there is a fully-parsed package to which this file belongs. - for _, file := range cph.Files() { - if file.File().Identity() == fh.Identity() { - results = append(results, cph) - } - } - } - if dirty || len(results) == 0 { - cphs, err := f.view.loadParseTypecheck(ctx, f, fh) + fh := s.Handle(ctx, f) + + // Determine if we need to type-check the package. + m, cphs, load, check := s.shouldCheck(fh) + cfg := s.view.Config(ctx) + + // We may need to re-load package metadata. + // 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(), cfg) if err != nil { return nil, err } - if cphs == nil { - return results, nil + // If load has explicitly returned nil metadata and no error, + // it means that we should not re-type-check the packages. + if m == nil { + return cphs, nil } - results = cphs } - if len(results) == 0 { + if check { + var results []source.CheckPackageHandle + for _, m := range m { + imp := &importer{ + config: cfg, + seen: make(map[packageID]struct{}), + topLevelPackageID: m.id, + snapshot: s, + } + cph, err := imp.checkPackageHandle(ctx, m.id, s) + if err != nil { + return nil, err + } + results = append(results, cph) + } + cphs = results + } + if len(cphs) == 0 { return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI()) } - return results, nil + return cphs, nil } -func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results []source.CheckPackageHandle) { +func (s *snapshot) shouldCheck(fh source.FileHandle) (m []*metadata, cphs []source.CheckPackageHandle, load, check bool) { + // Get the metadata for the given file. + m = s.getMetadataForURI(fh.Identity().URI) + + // If there is no metadata for the package, we definitely need to type-check again. + if len(m) == 0 { + return nil, nil, true, true + } + + // If the metadata for the package had missing dependencies, + // we _may_ need to re-check. If the missing dependencies haven't changed + // since previous load, we will not check again. + for _, m := range m { + if len(m.missingDeps) != 0 { + load = true + check = true + } + } + // We expect to see a checked package for each package ID, + // and it should be parsed in full mode. var ( - rdeps = v.reverseDependencies(ctx, uri) + expected = len(m) + cachedCPHs = s.getPackages(fh.Identity().URI) + ) + if len(cachedCPHs) < expected { + return m, nil, load, true + } + for _, cph := range cachedCPHs { + // The package may have been checked in the exported mode. + if cph.Mode() != source.ParseFull { + continue + } + // Confirm that the file belongs to this package. + for _, file := range cph.Files() { + if file.File().Identity() == fh.Identity() { + cphs = append(cphs, cph) + } + } + } + if len(cphs) < expected { + return m, nil, load, true + } + return m, cphs, load, check +} + +func (v *view) GetActiveReverseDeps(ctx context.Context, f source.File) (results []source.CheckPackageHandle) { + var ( + s = v.getSnapshot() + rdeps = transitiveReverseDependencies(ctx, f.URI(), s) files = v.openFiles(ctx, rdeps) seen = make(map[span.URI]struct{}) ) @@ -84,11 +126,7 @@ func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results if _, ok := seen[f.URI()]; ok { continue } - gof, ok := f.(source.GoFile) - if !ok { - continue - } - cphs, err := gof.CheckPackageHandles(ctx) + cphs, err := s.checkPackageHandles(ctx, f) if err != nil { continue } @@ -100,3 +138,28 @@ func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results } return results } + +func transitiveReverseDependencies(ctx context.Context, uri span.URI, s *snapshot) (result []span.URI) { + var ( + seen = make(map[packageID]struct{}) + uris = make(map[span.URI]struct{}) + topLevelURIs = make(map[span.URI]struct{}) + ) + + metadata := s.getMetadataForURI(uri) + + for _, m := range metadata { + for _, uri := range m.files { + topLevelURIs[uri] = struct{}{} + } + s.reverseDependencies(m.id, uris, seen) + } + // Filter out the URIs that belong to the original package. + for uri := range uris { + if _, ok := topLevelURIs[uri]; ok { + continue + } + result = append(result, uri) + } + return result +} diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 9d99eff9f8..f175b5f215 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -7,7 +7,7 @@ package cache import ( "context" "fmt" - "go/ast" + "go/types" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/source" @@ -19,122 +19,37 @@ import ( errors "golang.org/x/xerrors" ) -func (v *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.FileHandle) ([]source.CheckPackageHandle, error) { - ctx, done := trace.StartSpan(ctx, "cache.view.loadParseTypeCheck", telemetry.URI.Of(f.URI())) - defer done() - - meta, err := v.load(ctx, f, fh) - if err != nil { - return nil, err - } - // If load has explicitly returns nil metadata and no error, - // it means that we should not re-typecheck the packages. - if meta == nil { - return nil, nil - } - var ( - cphs []*checkPackageHandle - results []source.CheckPackageHandle - ) - for _, m := range meta { - imp := &importer{ - view: v, - config: v.Config(ctx), - seen: make(map[packageID]struct{}), - topLevelPackageID: m.id, - } - cph, err := imp.checkPackageHandle(ctx, m) - if err != nil { - return nil, err - } - for _, ph := range cph.files { - if err := v.cachePerFile(ctx, ph); err != nil { - return nil, err - } - } - cphs = append(cphs, cph) - results = append(results, cph) - } - // Cache the package type information for the top-level package. - v.updatePackages(cphs) - return results, nil +type packageKey struct { + mode source.ParseMode + id packageID } -func (v *view) cachePerFile(ctx context.Context, ph source.ParseGoHandle) error { - file, _, _, err := ph.Parse(ctx) - if err != nil { - return err - } - f, err := v.GetFile(ctx, ph.File().Identity().URI) - if err != nil { - return err - } - gof, ok := f.(*goFile) - if !ok { - return errors.Errorf("%s is not a Go file", ph.File().Identity().URI) - } - gof.mu.Lock() - gof.imports = file.Imports - gof.mu.Unlock() - return nil +type metadata struct { + id packageID + pkgPath packagePath + name string + files []span.URI + typesSizes types.Sizes + errors []packages.Error + deps []packageID + missingDeps map[packagePath]struct{} } -func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) { - ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(f.URI())) +func (s *snapshot) load(ctx context.Context, uri span.URI, cfg *packages.Config) ([]*metadata, error) { + ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(uri)) defer done() - // Get the metadata for the file. - meta, err := view.checkMetadata(ctx, f, fh) - if err != nil { - return nil, err - } - if len(meta) == 0 { - return nil, fmt.Errorf("no package metadata found for %s", f.URI()) - } - return meta, nil -} - -// checkMetadata determines if we should run go/packages.Load for this file. -// If yes, update the metadata for the file and its package. -func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) { - var shouldRunGopackages bool - - m := v.getMetadata(fh.Identity().URI) - if len(m) == 0 { - shouldRunGopackages = true - } - // Get file content in case we don't already have it. - parsed, _, _, err := v.session.cache.ParseGoHandle(fh, source.ParseHeader).Parse(ctx) - if err != nil { - return nil, err - } - // Check if we need to re-run go/packages before loading the package. - shouldRunGopackages = shouldRunGopackages || v.shouldRunGopackages(ctx, f, parsed, m) - - // The package metadata is correct as-is, so just return it. - if !shouldRunGopackages { - return m, nil - } - - // Don't bother running go/packages if the context has been canceled. - if ctx.Err() != nil { - return nil, ctx.Err() - } - - ctx, done := trace.StartSpan(ctx, "packages.Load", telemetry.File.Of(f.filename())) - defer done() - - pkgs, err := packages.Load(v.Config(ctx), fmt.Sprintf("file=%s", f.filename())) + pkgs, err := packages.Load(cfg, fmt.Sprintf("file=%s", uri.Filename())) 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", f.filename()) + err = errors.Errorf("go/packages.Load: no packages found for %s", uri) } // Return this error as a diagnostic to the user. return nil, err } - m, prevMissingImports, err := v.updateMetadata(ctx, f.URI(), pkgs) + m, prevMissingImports, err := s.updateMetadata(ctx, uri, pkgs) if err != nil { return nil, err } @@ -145,36 +60,6 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile, fh source.FileHandl return meta, nil } -// shouldRunGopackages reparses a file's package and import declarations to -// determine if they have changed. -// It assumes that the caller holds the lock on the f.mu lock. -func (v *view) shouldRunGopackages(ctx context.Context, f *goFile, file *ast.File, metadata []*metadata) bool { - f.mu.Lock() - defer f.mu.Unlock() - - // Check if the package's name has changed, by checking if this is a filename - // we already know about, and if so, check if its package name has changed. - for _, m := range metadata { - for _, uri := range m.files { - if span.CompareURI(uri, f.URI()) == 0 { - if m.name != file.Name.Name { - return true - } - } - } - } - // If the package's imports have changed, re-run `go list`. - if len(f.imports) != len(file.Imports) { - return true - } - for i, importSpec := range f.imports { - if importSpec.Path.Value != file.Imports[i].Path.Value { - return true - } - } - return false -} - 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. for _, m := range metadata { @@ -207,3 +92,123 @@ func sameSet(x, y map[packagePath]struct{}) bool { } return true } + +// shouldLoad reparses a file's package and import declarations to +// determine if they have changed. +func (c *cache) shouldLoad(ctx context.Context, s *snapshot, originalFH, currentFH source.FileHandle) bool { + if originalFH == nil { + return true + } + + // Get the original parsed file in order to check package name and imports. + original, _, _, err := c.ParseGoHandle(originalFH, source.ParseHeader).Parse(ctx) + if err != nil { + return false + } + + // Get the current parsed file in order to check package name and imports. + current, _, _, err := c.ParseGoHandle(currentFH, source.ParseHeader).Parse(ctx) + if err != nil { + return false + } + + // Check if the package's metadata has changed. The cases handled are: + // + // 1. A package's name has changed + // 2. A file's imports have changed + // + if original.Name.Name != current.Name.Name { + return true + } + // If the package's imports have changed, re-run `go list`. + if len(original.Imports) != len(current.Imports) { + return true + } + for i, importSpec := range original.Imports { + // TODO: Handle the case where the imports have just been re-ordered. + if importSpec.Path.Value != current.Imports[i].Path.Value { + return true + } + } + return false +} + +func (s *snapshot) updateMetadata(ctx context.Context, uri span.URI, pkgs []*packages.Package) ([]*metadata, map[packageID]map[packagePath]struct{}, error) { + // Clear metadata since we are re-running go/packages. + 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 + } + } + + var results []*metadata + for _, pkg := range pkgs { + log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles)) + + // Set the metadata for this package. + if err := s.updateImports(ctx, packagePath(pkg.PkgPath), pkg); err != nil { + return nil, nil, err + } + m := s.getMetadata(packageID(pkg.ID)) + if m != nil { + results = append(results, m) + } + } + + // Rebuild the import graph when the metadata is updated. + s.clearAndRebuildImportGraph() + + if len(results) == 0 { + return nil, nil, errors.Errorf("no metadata for %s", uri) + } + return results, prevMissingImports, nil +} + +func (s *snapshot) updateImports(ctx context.Context, pkgPath packagePath, pkg *packages.Package) error { + // Recreate the metadata rather than reusing it to avoid locking. + m := &metadata{ + id: packageID(pkg.ID), + pkgPath: pkgPath, + name: pkg.Name, + typesSizes: pkg.TypesSizes, + errors: pkg.Errors, + } + for _, filename := range pkg.CompiledGoFiles { + uri := span.FileURI(filename) + m.files = append(m.files, uri) + + s.addID(uri, m.id) + } + + // Add the metadata to the cache. + s.setMetadata(m) + + for importPath, importPkg := range pkg.Imports { + importPkgPath := packagePath(importPath) + importID := packageID(importPkg.ID) + + if importPkgPath == pkgPath { + return errors.Errorf("cycle detected in %s", importPath) + } + m.deps = append(m.deps, importID) + + // Don't remember any imports with significant errors. + if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 { + if m.missingDeps == nil { + m.missingDeps = make(map[packagePath]struct{}) + } + m.missingDeps[importPkgPath] = struct{}{} + continue + } + dep := s.getMetadata(importID) + if dep == nil { + if err := s.updateImports(ctx, importPkgPath, importPkg); err != nil { + log.Error(ctx, "error in dependency", err) + } + } + } + return nil +} diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index e0fc01ad92..32b8521811 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -191,6 +191,14 @@ func (pkg *pkg) IsIllTyped() bool { return pkg.types == nil || pkg.typesInfo == nil || pkg.typesSizes == nil } +func (pkg *pkg) GetImport(ctx context.Context, pkgPath string) (source.Package, error) { + if imp := pkg.imports[packagePath(pkgPath)]; imp != nil { + return imp, nil + } + // Don't return a nil pointer because that still satisfies the interface. + return nil, errors.Errorf("no imported package for %s", pkgPath) +} + func (pkg *pkg) SetDiagnostics(a *analysis.Analyzer, diags []source.Diagnostic) { pkg.diagMu.Lock() defer pkg.diagMu.Unlock() diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 6cd4a61f54..4e175b5a6a 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -102,10 +102,12 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt packages: make(map[span.URI]map[packageKey]*checkPackageHandle), ids: make(map[span.URI][]packageID), metadata: make(map[packageID]*metadata), + files: make(map[span.URI]source.FileHandle), }, ignoredURIs: make(map[span.URI]struct{}), builtin: &builtinPkg{}, } + v.snapshot.view = v v.analyzers = UpdateAnalyzers(v, defaultAnalyzers) // Preemptively build the builtin package, // so we immediately add builtin.go to the list of ignored files. diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index a998fa7fdd..972cad193b 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -2,286 +2,171 @@ package cache import ( "context" - "go/types" + "sync" - "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/telemetry/log" - "golang.org/x/tools/internal/telemetry/tag" - errors "golang.org/x/xerrors" ) type snapshot struct { - id uint64 + id uint64 + view *view - packages map[span.URI]map[packageKey]*checkPackageHandle - ids map[span.URI][]packageID + mu sync.Mutex + + // ids maps file URIs to package IDs. + // It may be invalidated on calls to go/packages. + ids map[span.URI][]packageID + + // metadata maps file IDs to their associated metadata. + // It may invalidated on calls to go/packages. metadata map[packageID]*metadata + + // importedBy maps package IDs to the list of packages that import them. + importedBy map[packageID][]packageID + + // files maps file URIs to their corresponding FileHandles. + // It may invalidated when a file's content changes. + files map[span.URI]source.FileHandle + + // packages maps a file URI to a set of CheckPackageHandles to which that file belongs. + // It may be invalidated when a file's content changes. + packages map[span.URI]map[packageKey]*checkPackageHandle } -type metadata struct { - id packageID - pkgPath packagePath - name string - files []span.URI - typesSizes types.Sizes - parents map[packageID]bool - children map[packageID]*metadata - errors []packages.Error - missingDeps map[packagePath]struct{} -} +func (s *snapshot) getImportedBy(id packageID) []packageID { + s.mu.Lock() + defer s.mu.Unlock() -func (v *view) getSnapshot(uri span.URI) ([]*metadata, []*checkPackageHandle) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - var m []*metadata - for _, id := range v.snapshot.ids[uri] { - m = append(m, v.snapshot.metadata[id]) + // If we haven't rebuilt the import graph since creating the snapshot. + if len(s.importedBy) == 0 { + s.rebuildImportGraph() } - var cphs []*checkPackageHandle - for _, cph := range v.snapshot.packages[uri] { + + return s.importedBy[id] +} + +func (s *snapshot) addPackage(uri span.URI, cph *checkPackageHandle) { + s.mu.Lock() + defer s.mu.Unlock() + + pkgs, ok := s.packages[uri] + if !ok { + pkgs = make(map[packageKey]*checkPackageHandle) + s.packages[uri] = pkgs + } + // TODO: Check that there isn't already something set here. + // This can't be done until we fix the cache keys for CheckPackageHandles. + pkgs[packageKey{ + id: cph.m.id, + mode: cph.Mode(), + }] = cph +} + +func (s *snapshot) getPackages(uri span.URI) (cphs []source.CheckPackageHandle) { + s.mu.Lock() + defer s.mu.Unlock() + + for _, cph := range s.packages[uri] { cphs = append(cphs, cph) } - return m, cphs + return cphs } -func (v *view) getMetadata(uri span.URI) []*metadata { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) getMetadataForURI(uri span.URI) (metadata []*metadata) { + s.mu.Lock() + defer s.mu.Unlock() - var m []*metadata - for _, id := range v.snapshot.ids[uri] { - m = append(m, v.snapshot.metadata[id]) - } - return m -} - -func (v *view) getPackages(uri span.URI) map[packageKey]*checkPackageHandle { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - return v.snapshot.packages[uri] -} - -func (v *view) updateMetadata(ctx context.Context, uri span.URI, pkgs []*packages.Package) ([]*metadata, map[packageID]map[packagePath]struct{}, error) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - // Clear metadata since we are re-running go/packages. - prevMissingImports := make(map[packageID]map[packagePath]struct{}) - for _, id := range v.snapshot.ids[uri] { - if m, ok := v.snapshot.metadata[id]; ok && len(m.missingDeps) > 0 { - prevMissingImports[id] = m.missingDeps + for _, id := range s.ids[uri] { + if m, ok := s.metadata[id]; ok { + metadata = append(metadata, m) } } - without := make(map[span.URI]struct{}) - for _, id := range v.snapshot.ids[uri] { - v.remove(id, without, map[packageID]struct{}{}) - } - v.snapshot = v.snapshot.cloneMetadata(without) - - var results []*metadata - for _, pkg := range pkgs { - log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles)) - - // Build the import graph for this package. - if err := v.updateImportGraph(ctx, &importGraph{ - pkgPath: packagePath(pkg.PkgPath), - pkg: pkg, - parent: nil, - }); err != nil { - return nil, nil, err - } - results = append(results, v.snapshot.metadata[packageID(pkg.ID)]) - } - if len(results) == 0 { - return nil, nil, errors.Errorf("no metadata for %s", uri) - } - return results, prevMissingImports, nil + return metadata } -type importGraph struct { - pkgPath packagePath - pkg *packages.Package - parent *metadata +func (s *snapshot) setMetadata(m *metadata) { + s.mu.Lock() + defer s.mu.Unlock() + + s.metadata[m.id] = m } -func (v *view) updateImportGraph(ctx context.Context, g *importGraph) error { - // Recreate the metadata rather than reusing it to avoid locking. - m := &metadata{ - id: packageID(g.pkg.ID), - pkgPath: g.pkgPath, - name: g.pkg.Name, - typesSizes: g.pkg.TypesSizes, - errors: g.pkg.Errors, - } - for _, filename := range g.pkg.CompiledGoFiles { - uri := span.FileURI(filename) - v.snapshot.ids[uri] = append(v.snapshot.ids[uri], m.id) - m.files = append(m.files, uri) - } - // Preserve the import graph. - if original, ok := v.snapshot.metadata[m.id]; ok { - m.children = original.children - m.parents = original.parents - } - if m.children == nil { - m.children = make(map[packageID]*metadata) - } - if m.parents == nil { - m.parents = make(map[packageID]bool) - } +func (s *snapshot) getMetadata(id packageID) *metadata { + s.mu.Lock() + defer s.mu.Unlock() - // Add the metadata to the cache. - v.snapshot.metadata[m.id] = m - - // Connect the import graph. - if g.parent != nil { - m.parents[g.parent.id] = true - g.parent.children[m.id] = m - } - for importPath, importPkg := range g.pkg.Imports { - importPkgPath := packagePath(importPath) - if importPkgPath == g.pkgPath { - return errors.Errorf("cycle detected in %s", importPath) - } - // Don't remember any imports with significant errors. - if importPkgPath != "unsafe" && len(importPkg.CompiledGoFiles) == 0 { - if m.missingDeps == nil { - m.missingDeps = make(map[packagePath]struct{}) - } - m.missingDeps[importPkgPath] = struct{}{} - continue - } - if _, ok := m.children[packageID(importPkg.ID)]; !ok { - if err := v.updateImportGraph(ctx, &importGraph{ - pkgPath: importPkgPath, - pkg: importPkg, - parent: m, - }); err != nil { - log.Error(ctx, "error in dependency", err) - } - } - } - // Clear out any imports that have been removed since the package was last loaded. - for importID := range m.children { - child, ok := v.snapshot.metadata[importID] - if !ok { - continue - } - importPath := string(child.pkgPath) - if _, ok := g.pkg.Imports[importPath]; ok { - continue - } - delete(m.children, importID) - delete(child.parents, m.id) - } - return nil + return s.metadata[id] } -func (v *view) updatePackages(cphs []*checkPackageHandle) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) addID(uri span.URI, id packageID) { + s.mu.Lock() + defer s.mu.Unlock() - for _, cph := range cphs { - for _, ph := range cph.files { - uri := ph.File().Identity().URI - if _, ok := v.snapshot.packages[uri]; !ok { - v.snapshot.packages[uri] = make(map[packageKey]*checkPackageHandle) - } - v.snapshot.packages[uri][packageKey{ - id: cph.m.id, - mode: ph.Mode(), - }] = cph - } - } + s.ids[uri] = append(s.ids[uri], id) } -// invalidateContent invalidates the content of a Go file, -// including any position and type information that depends on it. -func (v *view) invalidateContent(ctx context.Context, f *goFile) { - f.handleMu.Lock() - defer f.handleMu.Unlock() +func (s *snapshot) getIDs(uri span.URI) []packageID { + s.mu.Lock() + defer s.mu.Unlock() - without := make(map[span.URI]struct{}) - - // Remove the package and all of its reverse dependencies from the cache. - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - for _, id := range v.snapshot.ids[f.URI()] { - f.view.remove(id, without, map[packageID]struct{}{}) - } - v.snapshot = v.snapshot.clonePackages(without) - f.handle = nil + return s.ids[uri] } -// invalidateMeta invalidates package metadata for all files in f's -// package. This forces f's package's metadata to be reloaded next -// time the package is checked. -func (v *view) invalidateMetadata(uri span.URI) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) getFile(uri span.URI) source.FileHandle { + s.mu.Lock() + defer s.mu.Unlock() - without := make(map[span.URI]struct{}) - - for _, id := range v.snapshot.ids[uri] { - v.remove(id, without, map[packageID]struct{}{}) - } - v.snapshot = v.snapshot.cloneMetadata(without) + return s.files[uri] } -// remove invalidates a package and its reverse dependencies in the view's -// package cache. It is assumed that the caller has locked both the mutexes -// of both the mcache and the pcache. -func (v *view) remove(id packageID, toDelete map[span.URI]struct{}, seen map[packageID]struct{}) { - if _, ok := seen[id]; ok { - return - } - m, ok := v.snapshot.metadata[id] - if !ok { - return - } - seen[id] = struct{}{} - for parentID := range m.parents { - v.remove(parentID, toDelete, seen) - } - for _, uri := range m.files { - toDelete[uri] = struct{}{} +func (s *snapshot) Handle(ctx context.Context, f source.File) source.FileHandle { + s.mu.Lock() + defer s.mu.Unlock() + + if _, ok := s.files[f.URI()]; !ok { + s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.Kind()) } + return s.files[f.URI()] } -func (s *snapshot) clonePackages(without map[span.URI]struct{}) *snapshot { +func (s *snapshot) clone(withoutURI span.URI, withoutTypes, withoutMetadata map[span.URI]struct{}) *snapshot { + s.mu.Lock() + defer s.mu.Unlock() + result := &snapshot{ - id: s.id + 1, - packages: make(map[span.URI]map[packageKey]*checkPackageHandle), - ids: s.ids, - metadata: s.metadata, + id: s.id + 1, + view: s.view, + packages: make(map[span.URI]map[packageKey]*checkPackageHandle), + ids: make(map[span.URI][]packageID), + metadata: make(map[packageID]*metadata), + importedBy: make(map[packageID][]packageID), + files: make(map[span.URI]source.FileHandle), + } + // Copy all of the FileHandles except for the one that was invalidated. + for k, v := range s.files { + if k == withoutURI { + continue + } + result.files[k] = v } for k, v := range s.packages { - if _, ok := without[k]; ok { - continue + if withoutTypes != nil { + if _, ok := withoutTypes[k]; ok { + continue + } } result.packages[k] = v } - return result -} - -func (s *snapshot) cloneMetadata(without map[span.URI]struct{}) *snapshot { - result := &snapshot{ - id: s.id + 1, - packages: s.packages, - ids: make(map[span.URI][]packageID), - metadata: make(map[packageID]*metadata), - } withoutIDs := make(map[packageID]struct{}) for k, ids := range s.ids { - if _, ok := without[k]; ok { - for _, id := range ids { - withoutIDs[id] = struct{}{} + if withoutMetadata != nil { + if _, ok := withoutMetadata[k]; ok { + for _, id := range ids { + withoutIDs[id] = struct{}{} + } + continue } - continue } result.ids[k] = ids } @@ -294,34 +179,91 @@ func (s *snapshot) cloneMetadata(without map[span.URI]struct{}) *snapshot { return result } -func (v *view) reverseDependencies(ctx context.Context, uri span.URI) map[span.URI]struct{} { - seen := make(map[packageID]struct{}) - uris := make(map[span.URI]struct{}) +// invalidateContent invalidates the content of a Go file, +// including any position and type information that depends on it. +func (v *view) invalidateContent(ctx context.Context, uri span.URI, kind source.FileKind) { + withoutTypes := make(map[span.URI]struct{}) + withoutMetadata := make(map[span.URI]struct{}) + // This should be the only time we hold the view's snapshot lock for any period of time. v.snapshotMu.Lock() defer v.snapshotMu.Unlock() - for _, id := range v.snapshot.ids[uri] { - v.rdeps(id, seen, uris, id) + ids := v.snapshot.getIDs(uri) + + // Remove the package and all of its reverse dependencies from the cache. + for _, id := range ids { + v.snapshot.reverseDependencies(id, withoutTypes, map[packageID]struct{}{}) } - return uris + + // Get the original FileHandle for the URI, if it exists. + originalFH := v.snapshot.getFile(uri) + + // Get the current FileHandle for the URI. + currentFH := v.session.GetFile(uri, kind) + + // Check if the file's package name or imports have changed, + // and if so, invalidate metadata. + if v.session.cache.shouldLoad(ctx, v.snapshot, originalFH, currentFH) { + withoutMetadata = withoutTypes + + // TODO: If a package's name has changed, + // we should invalidate the metadata for the new package name (if it exists). + } + + v.snapshot = v.snapshot.clone(uri, withoutTypes, withoutMetadata) } -func (v *view) rdeps(topID packageID, seen map[packageID]struct{}, results map[span.URI]struct{}, id packageID) { +// invalidateMetadata invalidates package metadata for all files in f's +// package. This forces f's package's metadata to be reloaded next +// time the package is checked. +// +// TODO: This function shouldn't be necessary. +// We should be able to handle its use cases more efficiently. +func (v *view) invalidateMetadata(uri span.URI) { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + withoutMetadata := make(map[span.URI]struct{}) + for _, id := range v.snapshot.getIDs(uri) { + v.snapshot.reverseDependencies(id, withoutMetadata, map[packageID]struct{}{}) + } + v.snapshot = v.snapshot.clone(uri, nil, withoutMetadata) +} + +// reverseDependencies populates the uris map with file URIs belonging to the +// provided package and its transitive reverse dependencies. +func (s *snapshot) reverseDependencies(id packageID, uris map[span.URI]struct{}, seen map[packageID]struct{}) { if _, ok := seen[id]; ok { return } - seen[id] = struct{}{} - m, ok := v.snapshot.metadata[id] - if !ok { + m := s.getMetadata(id) + if m == nil { return } - if id != topID { - for _, uri := range m.files { - results[uri] = struct{}{} - } + seen[id] = struct{}{} + importedBy := s.getImportedBy(id) + for _, parentID := range importedBy { + s.reverseDependencies(parentID, uris, seen) } - for parentID := range m.parents { - v.rdeps(topID, seen, results, parentID) + for _, uri := range m.files { + uris[uri] = struct{}{} + } +} + +func (s *snapshot) clearAndRebuildImportGraph() { + s.mu.Lock() + defer s.mu.Unlock() + + // Completely invalidate the original map. + s.importedBy = make(map[packageID][]packageID) + s.rebuildImportGraph() +} + +func (s *snapshot) rebuildImportGraph() { + for id, m := range s.metadata { + for _, importID := range m.deps { + s.importedBy[importID] = append(s.importedBy[importID], id) + } } } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 34cebd4d6d..f9279dcc32 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -288,6 +288,17 @@ func (v *view) BuiltinPackage() source.BuiltinPackage { return v.builtin } +func (v *view) Snapshot() source.Snapshot { + return v.getSnapshot() +} + +func (v *view) getSnapshot() *snapshot { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + return v.snapshot +} + // SetContent sets the overlay contents for a file. func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bool, error) { v.mu.Lock() @@ -298,11 +309,12 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bo v.cancel() v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx) - if !v.Ignore(uri) { - kind := source.DetectLanguage("", uri.Filename()) - return v.session.SetOverlay(uri, kind, content), nil + if v.Ignore(uri) { + return false, nil } - return false, nil + + kind := source.DetectLanguage("", uri.Filename()) + return v.session.SetOverlay(uri, kind, content), nil } // FindFile returns the file if the given URI is already a part of the view. @@ -329,46 +341,20 @@ func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) { // getFile is the unlocked internal implementation of GetFile. func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (viewFile, error) { - if f, err := v.findFile(uri); err != nil { + f, err := v.findFile(uri) + if err != nil { return nil, err } else if f != nil { return f, nil } - var f viewFile - switch kind { - case source.Mod: - f = &modFile{ - fileBase: fileBase{ - view: v, - fname: uri.Filename(), - kind: source.Mod, - }, - } - case source.Sum: - f = &sumFile{ - fileBase: fileBase{ - view: v, - fname: uri.Filename(), - kind: source.Sum, - }, - } - default: - // Assume that all other files are Go files, regardless of extension. - f = &goFile{ - fileBase: fileBase{ - view: v, - fname: uri.Filename(), - kind: source.Go, - }, - } - v.session.filesWatchMap.Watch(uri, func() { - gof, ok := f.(*goFile) - if !ok { - return - } - v.invalidateContent(ctx, gof) - }) + f = &fileBase{ + view: v, + fname: uri.Filename(), + kind: source.Go, } + v.session.filesWatchMap.Watch(uri, func() { + v.invalidateContent(ctx, uri, kind) + }) v.mapFile(uri, f) return f, nil } @@ -425,11 +411,11 @@ func (v *view) mapFile(uri span.URI, f viewFile) { } } -func (v *view) openFiles(ctx context.Context, uris map[span.URI]struct{}) (results []source.File) { +func (v *view) openFiles(ctx context.Context, uris []span.URI) (results []source.File) { v.mu.Lock() defer v.mu.Unlock() - for uri := range uris { + for _, uri := range uris { // Call unlocked version of getFile since we hold the lock on the view. if f, err := v.getFile(ctx, uri, source.Go); err == nil && v.session.IsOpen(uri) { results = append(results, f) diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 0e6523cc0d..ea7bff628c 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -27,8 +27,10 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara return nil, err } + fh := view.Snapshot().Handle(ctx, f) + // Determine the supported actions for this file kind. - fileKind := f.Handle(ctx).Identity().Kind + fileKind := fh.Identity().Kind supportedCodeActions, ok := view.Options().SupportedCodeActions[fileKind] if !ok { return nil, fmt.Errorf("no supported code actions for %v file kind", fileKind) @@ -67,17 +69,13 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara }, }) case source.Go: - gof, ok := f.(source.GoFile) - if !ok { - return nil, errors.Errorf("%s is not a Go file", f.URI()) - } - edits, editsPerFix, err := source.AllImportsFixes(ctx, view, gof) + edits, editsPerFix, err := source.AllImportsFixes(ctx, view, f) if err != nil { return nil, err } if diagnostics := params.Context.Diagnostics; wanted[protocol.QuickFix] && len(diagnostics) > 0 { // First, add the quick fixes reported by go/analysis. - qf, err := quickFixes(ctx, view, gof, diagnostics) + qf, err := quickFixes(ctx, view, f, diagnostics) if err != nil { log.Error(ctx, "quick fixes failed", err, telemetry.File.Of(uri)) } @@ -207,9 +205,9 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic return results } -func quickFixes(ctx context.Context, view source.View, gof source.GoFile, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { +func quickFixes(ctx context.Context, view source.View, f source.File, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { var codeActions []protocol.CodeAction - cphs, err := gof.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index d6c4a03f4d..72e8647532 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -22,7 +22,8 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if err != nil { return nil, err } - if _, ok := f.(source.ModFile); !ok { + fh := view.Snapshot().Handle(ctx, f) + if fh.Identity().Kind != source.Mod { return nil, errors.Errorf("%s is not a mod file", uri) } // Run go.mod tidy on the view. diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index 7b64a99392..0f6b7f4ea4 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -20,7 +20,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) options := view.Options() - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/definition.go b/internal/lsp/definition.go index 8e49cd3119..f8f18df9ef 100644 --- a/internal/lsp/definition.go +++ b/internal/lsp/definition.go @@ -15,7 +15,7 @@ import ( func (s *Server) definition(ctx context.Context, params *protocol.DefinitionParams) ([]protocol.Location, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } @@ -38,7 +38,7 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefinitionParams) ([]protocol.Location, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 4f137b6cce..6b602cd3d1 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -14,7 +14,6 @@ import ( "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/trace" - errors "golang.org/x/xerrors" ) func (s *Server) diagnostics(view source.View, uri span.URI) error { @@ -28,12 +27,7 @@ func (s *Server) diagnostics(view source.View, uri span.URI) error { if err != nil { return err } - // For non-Go files, don't return any diagnostics. - gof, ok := f.(source.GoFile) - if !ok { - return errors.Errorf("%s is not a Go file", f.URI()) - } - reports, warningMsg, err := source.Diagnostics(ctx, view, gof, view.Options().DisabledAnalyses) + reports, warningMsg, err := source.Diagnostics(ctx, view, f, view.Options().DisabledAnalyses) if err != nil { return err } diff --git a/internal/lsp/folding_range.go b/internal/lsp/folding_range.go index 49de6033c8..d0eecc49a2 100644 --- a/internal/lsp/folding_range.go +++ b/internal/lsp/folding_range.go @@ -11,7 +11,7 @@ import ( func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRangeParams) ([]protocol.FoldingRange, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/hover.go b/internal/lsp/hover.go index c79ba6e0c7..ae51f46596 100644 --- a/internal/lsp/hover.go +++ b/internal/lsp/hover.go @@ -18,7 +18,7 @@ import ( func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*protocol.Hover, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/link.go b/internal/lsp/link.go index cea01ab955..4c79bdc707 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -23,11 +23,11 @@ import ( func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } - fh := f.Handle(ctx) + fh := view.Snapshot().Handle(ctx, f) file, m, _, err := view.Session().Cache().ParseGoHandle(fh, source.ParseFull).Parse(ctx) if err != nil { return nil, err diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 691621d71f..4b2856cf65 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -76,11 +76,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti if err != nil { t.Fatalf("no file for %s: %v", f, err) } - gof, ok := f.(source.GoFile) - if !ok { - t.Fatalf("%s is not a Go file: %v", uri, err) - } - results, _, err := source.Diagnostics(r.ctx, v, gof, nil) + results, _, err := source.Diagnostics(r.ctx, v, f, nil) if err != nil { t.Fatal(err) } @@ -323,7 +319,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { uri := spn.URI() filename := uri.Filename() view := r.server.session.ViewOf(uri) - f, err := getGoFile(r.ctx, view, uri) + f, err := view.GetFile(r.ctx, uri) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/references.go b/internal/lsp/references.go index 25977774ca..6930a6c174 100644 --- a/internal/lsp/references.go +++ b/internal/lsp/references.go @@ -17,7 +17,7 @@ import ( func (s *Server) references(ctx context.Context, params *protocol.ReferenceParams) ([]protocol.Location, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go index d4b4d0876a..9ce36729a1 100644 --- a/internal/lsp/rename.go +++ b/internal/lsp/rename.go @@ -15,7 +15,7 @@ import ( func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } @@ -38,7 +38,7 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRenameParams) (*protocol.Range, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 7b514fc547..fe096522f6 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// Package lsp implements LSP for gopls. package lsp import ( diff --git a/internal/lsp/signature_help.go b/internal/lsp/signature_help.go index 8f466b36df..5771ef16c0 100644 --- a/internal/lsp/signature_help.go +++ b/internal/lsp/signature_help.go @@ -17,7 +17,7 @@ import ( func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHelpParams) (*protocol.SignatureHelp, error) { uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 12d036e974..6a4b1c81e3 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -126,7 +126,8 @@ func (ipm insensitivePrefixMatcher) Score(candidateLabel string) float32 { // completer contains the necessary information for a single completion request. type completer struct { - pkg Package + snapshot Snapshot + pkg Package qf types.Qualifier opts CompletionOptions @@ -376,13 +377,13 @@ func (e ErrIsDefinition) Error() string { // The selection is computed based on the preceding identifier and can be used by // the client to score the quality of the completion. For instance, some clients // may tolerate imperfect matches as valid completion results, since users may make typos. -func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position, opts CompletionOptions) ([]CompletionItem, *Selection, error) { +func Completion(ctx context.Context, view View, f File, pos protocol.Position, opts CompletionOptions) ([]CompletionItem, *Selection, error) { ctx, done := trace.StartSpan(ctx, "source.Completion") defer done() startTime := time.Now() - cphs, err := f.CheckPackageHandles(ctx) + snapshot, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, nil, err } @@ -427,6 +428,7 @@ func Completion(ctx context.Context, view View, f GoFile, pos protocol.Position, clInfo := enclosingCompositeLiteral(path, rng.Start, pkg.GetTypesInfo()) c := &completer{ pkg: pkg, + snapshot: snapshot, qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), view: view, ctx: ctx, diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index 9ccb573322..199dde8e40 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -133,7 +133,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) { return CompletionItem{}, errors.Errorf("no file for %s", obj.Name()) } - ident, err := findIdentifier(c.ctx, c.view, pkg, file, obj.Pos()) + ident, err := findIdentifier(c.ctx, c.view, c.snapshot, pkg, file, obj.Pos()) if err != nil { return CompletionItem{}, err } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 5176ca5e28..04c4737a6b 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -17,7 +17,6 @@ import ( "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/trace" - errors "golang.org/x/xerrors" ) type Diagnostic struct { @@ -38,11 +37,11 @@ const ( SeverityError ) -func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[string]struct{}) (map[span.URI][]Diagnostic, string, error) { +func Diagnostics(ctx context.Context, view View, f File, disabledAnalyses map[string]struct{}) (map[span.URI][]Diagnostic, string, error) { ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(f.URI())) defer done() - cphs, err := f.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, "", err } @@ -85,7 +84,7 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[ } } // Updates to the diagnostics for this package may need to be propagated. - revDeps := view.GetActiveReverseDeps(ctx, f.URI()) + revDeps := view.GetActiveReverseDeps(ctx, f) for _, cph := range revDeps { pkg, err := cph.Check(ctx) if err != nil { @@ -215,13 +214,9 @@ func toDiagnostic(ctx context.Context, view View, diag analysis.Diagnostic, cate if err != nil { return Diagnostic{}, err } - gof, ok := f.(GoFile) - if !ok { - return Diagnostic{}, errors.Errorf("%s is not a Go file", f.URI()) - } // If the package has changed since these diagnostics were computed, // this may be incorrect. Should the package be associated with the diagnostic? - cphs, err := gof.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return Diagnostic{}, err } diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index f66d958416..e944e9b6d1 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -16,10 +16,12 @@ type FoldingRangeInfo struct { } // FoldingRange gets all of the folding range for f. -func FoldingRange(ctx context.Context, view View, f GoFile, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) { +func FoldingRange(ctx context.Context, view View, f File, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) { // TODO(suzmue): consider limiting the number of folding ranges returned, and // implement a way to prioritize folding ranges in that case. - ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull) + s := view.Snapshot() + fh := s.Handle(ctx, f) + ph := view.Session().Cache().ParseGoHandle(fh, ParseFull) file, m, _, err := ph.Parse(ctx) if err != nil { return nil, err diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 746de0ef5f..50563d39fc 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -24,11 +24,7 @@ func Format(ctx context.Context, view View, f File) ([]protocol.TextEdit, error) ctx, done := trace.StartSpan(ctx, "source.Format") defer done() - gof, ok := f.(GoFile) - if !ok { - return nil, errors.Errorf("formatting is not supported for non-Go files") - } - cphs, err := gof.CheckPackageHandles(ctx) + snapshot, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } @@ -55,7 +51,7 @@ func Format(ctx context.Context, view View, f File) ([]protocol.TextEdit, error) // have any parse errors and can still be formatted. Using format.Node // on an ast with errors may result in code being added or removed. // Attempt to format the source of this file instead. - formatted, err := formatSource(ctx, f) + formatted, err := formatSource(ctx, snapshot, f) if err != nil { return nil, err } @@ -75,10 +71,11 @@ func Format(ctx context.Context, view View, f File) ([]protocol.TextEdit, error) return computeTextEdits(ctx, ph.File(), m, buf.String()) } -func formatSource(ctx context.Context, file File) ([]byte, error) { +func formatSource(ctx context.Context, s Snapshot, f File) ([]byte, error) { ctx, done := trace.StartSpan(ctx, "source.formatSource") defer done() - data, _, err := file.Handle(ctx).Read(ctx) + + data, _, err := s.Handle(ctx, f).Read(ctx) if err != nil { return nil, err } @@ -86,11 +83,11 @@ func formatSource(ctx context.Context, file File) ([]byte, error) { } // Imports formats a file using the goimports tool. -func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]protocol.TextEdit, error) { +func Imports(ctx context.Context, view View, f File, rng span.Range) ([]protocol.TextEdit, error) { ctx, done := trace.StartSpan(ctx, "source.Imports") defer done() - cphs, err := f.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } @@ -149,11 +146,11 @@ type ImportFix struct { // In addition to returning the result of applying all edits, // it returns a list of fixes that could be applied to the file, with the // corresponding TextEdits that would be needed to apply that fix. -func AllImportsFixes(ctx context.Context, view View, f GoFile) (edits []protocol.TextEdit, editsPerFix []*ImportFix, err error) { +func AllImportsFixes(ctx context.Context, view View, f File) (edits []protocol.TextEdit, editsPerFix []*ImportFix, err error) { ctx, done := trace.StartSpan(ctx, "source.AllImportsFixes") defer done() - cphs, err := f.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, nil, err } diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index 992ada5e0b..64fa906e74 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -23,7 +23,8 @@ func Highlight(ctx context.Context, view View, uri span.URI, pos protocol.Positi if err != nil { return nil, err } - ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseFull) + fh := view.Snapshot().Handle(ctx, f) + ph := view.Session().Cache().ParseGoHandle(fh, ParseFull) file, m, _, err := ph.Parse(ctx) if err != nil { return nil, err diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index ea8bed1302..2aa0613c42 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -20,9 +20,10 @@ import ( // IdentifierInfo holds information about an identifier in Go source. type IdentifierInfo struct { - Name string - View View - File ParseGoHandle + Name string + View View + snapshot Snapshot + File ParseGoHandle mappedRange Type struct { @@ -47,8 +48,8 @@ type Declaration struct { // Identifier returns identifier information for a position // in a file, accounting for a potentially incomplete selector. -func Identifier(ctx context.Context, view View, f GoFile, pos protocol.Position) (*IdentifierInfo, error) { - cphs, err := f.CheckPackageHandles(ctx) +func Identifier(ctx context.Context, view View, f File, pos protocol.Position) (*IdentifierInfo, error) { + snapshot, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } @@ -73,17 +74,17 @@ func Identifier(ctx context.Context, view View, f GoFile, pos protocol.Position) if err != nil { return nil, err } - return findIdentifier(ctx, view, pkg, file, rng.Start) + return findIdentifier(ctx, view, snapshot, pkg, file, rng.Start) } -func findIdentifier(ctx context.Context, view View, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { - if result, err := identifier(ctx, view, pkg, file, pos); err != nil || result != nil { +func findIdentifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { + if result, err := identifier(ctx, view, snapshot, pkg, file, pos); err != nil || result != nil { return result, err } // If the position is not an identifier but immediately follows // an identifier or selector period (as is common when // requesting a completion), use the path to the preceding node. - ident, err := identifier(ctx, view, pkg, file, pos-1) + ident, err := identifier(ctx, view, snapshot, pkg, file, pos-1) if ident == nil && err == nil { err = errors.New("no identifier found") } @@ -91,14 +92,14 @@ func findIdentifier(ctx context.Context, view View, pkg Package, file *ast.File, } // identifier checks a single position for a potential identifier. -func identifier(ctx context.Context, view View, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { +func identifier(ctx context.Context, view View, snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) { ctx, done := trace.StartSpan(ctx, "source.identifier") defer done() var err error // Handle import specs separately, as there is no formal position for a package declaration. - if result, err := importSpec(ctx, view, file, pkg, pos); result != nil || err != nil { + if result, err := importSpec(ctx, view, snapshot, file, pkg, pos); result != nil || err != nil { return result, err } path, _ := astutil.PathEnclosingInterval(file, pos, pos) @@ -113,11 +114,12 @@ func identifier(ctx context.Context, view View, pkg Package, file *ast.File, pos } } result := &IdentifierInfo{ - View: view, - File: ph, - qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), - pkg: pkg, - ident: searchForIdent(path[0]), + View: view, + snapshot: snapshot, + File: ph, + qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), + pkg: pkg, + ident: searchForIdent(path[0]), } // No identifier at the given position. if result.ident == nil { @@ -273,7 +275,7 @@ func objToNode(ctx context.Context, view View, pkg Package, obj types.Object) (a } // importSpec handles positions inside of an *ast.ImportSpec. -func importSpec(ctx context.Context, view View, file *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) { +func importSpec(ctx context.Context, view View, snapshot Snapshot, file *ast.File, pkg Package, pos token.Pos) (*IdentifierInfo, error) { var imp *ast.ImportSpec for _, spec := range file.Imports { if spec.Path.Pos() <= pos && pos < spec.Path.End() { @@ -295,10 +297,11 @@ func importSpec(ctx context.Context, view View, file *ast.File, pkg Package, pos } } result := &IdentifierInfo{ - View: view, - File: ph, - Name: importPath, - pkg: pkg, + View: view, + snapshot: snapshot, + File: ph, + Name: importPath, + pkg: pkg, } if result.mappedRange, err = posToMappedRange(ctx, view, pkg, imp.Path.Pos(), imp.Path.End()); err != nil { return nil, err diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index bb4cebccc4..100b55c488 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -41,7 +41,7 @@ type PrepareItem struct { Text string } -func PrepareRename(ctx context.Context, view View, f GoFile, pos protocol.Position) (*PrepareItem, error) { +func PrepareRename(ctx context.Context, view View, f File, pos protocol.Position) (*PrepareItem, error) { ctx, done := trace.StartSpan(ctx, "source.PrepareRename") defer done() @@ -151,7 +151,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) if err != nil { return nil, err } - fh := f.Handle(ctx) + fh := i.snapshot.Handle(ctx, f) data, _, err := fh.Read(ctx) if err != nil { return nil, err @@ -227,6 +227,7 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t return &IdentifierInfo{ Name: pkgName.Name(), View: ident.View, + snapshot: ident.snapshot, mappedRange: decl.mappedRange, File: ident.File, Declaration: decl, diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index b582fb6bc2..de6b1b35c3 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -27,11 +27,11 @@ type ParameterInformation struct { Label string } -func SignatureHelp(ctx context.Context, view View, f GoFile, pos protocol.Position) (*SignatureInformation, error) { +func SignatureHelp(ctx context.Context, view View, f File, pos protocol.Position) (*SignatureInformation, error) { ctx, done := trace.StartSpan(ctx, "source.SignatureHelp") defer done() - cphs, err := f.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } @@ -94,7 +94,7 @@ FindCall: // Handle builtin functions separately. if obj, ok := obj.(*types.Builtin); ok { - return builtinSignature(ctx, f.View(), callExpr, obj.Name(), rng.Start) + return builtinSignature(ctx, view, callExpr, obj.Name(), rng.Start) } // Get the type information for the function being called. @@ -118,7 +118,7 @@ FindCall: comment *ast.CommentGroup ) if obj != nil { - node, err := objToNode(ctx, f.View(), pkg, obj) + node, err := objToNode(ctx, view, pkg, obj) if err != nil { return nil, err } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 640ebae268..161085d187 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -68,7 +68,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti if err != nil { t.Fatal(err) } - results, _, err := source.Diagnostics(r.ctx, r.view, f.(source.GoFile), nil) + results, _, err := source.Diagnostics(r.ctx, r.view, f, nil) if err != nil { t.Fatal(err) } @@ -243,7 +243,7 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options source.Comp if err != nil { t.Fatal(err) } - list, surrounding, err := source.Completion(r.ctx, r.view, f.(source.GoFile), protocol.Position{ + list, surrounding, err := source.Completion(r.ctx, r.view, f, protocol.Position{ Line: float64(src.Start().Line() - 1), Character: float64(src.Start().Column() - 1), }, options) @@ -284,14 +284,15 @@ func (r *runner) FoldingRange(t *testing.T, spn span.Span) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - data, _, err := f.Handle(r.ctx).Read(r.ctx) + fh := r.view.Snapshot().Handle(r.ctx, f) + data, _, err := fh.Read(r.ctx) if err != nil { t.Error(err) return } // Test all folding ranges. - ranges, err := source.FoldingRange(r.ctx, r.view, f.(source.GoFile), false) + ranges, err := source.FoldingRange(r.ctx, r.view, f, false) if err != nil { t.Error(err) return @@ -299,7 +300,7 @@ func (r *runner) FoldingRange(t *testing.T, spn span.Span) { r.foldingRanges(t, "foldingRange", uri, string(data), ranges) // Test folding ranges with lineFoldingOnly - ranges, err = source.FoldingRange(r.ctx, r.view, f.(source.GoFile), true) + ranges, err = source.FoldingRange(r.ctx, r.view, f, true) if err != nil { t.Error(err) return @@ -434,7 +435,8 @@ func (r *runner) Format(t *testing.T, spn span.Span) { } return } - data, _, err := f.Handle(ctx).Read(ctx) + fh := r.view.Snapshot().Handle(r.ctx, f) + data, _, err := fh.Read(ctx) if err != nil { t.Fatal(err) } @@ -465,7 +467,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - fh := f.Handle(ctx) + fh := r.view.Snapshot().Handle(r.ctx, f) tok, err := r.view.Session().Cache().TokenHandle(fh).Token(ctx) if err != nil { t.Fatal(err) @@ -474,7 +476,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - edits, err := source.Imports(ctx, r.view, f.(source.GoFile), rng) + edits, err := source.Imports(ctx, r.view, f, rng) if err != nil { if goimported != "" { t.Error(err) @@ -512,7 +514,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { if err != nil { t.Fatal(err) } - ident, err := source.Identifier(ctx, r.view, f.(source.GoFile), srcRng.Start) + ident, err := source.Identifier(ctx, r.view, f, srcRng.Start) if err != nil { t.Fatalf("failed for %v: %v", d.Src, err) } @@ -590,7 +592,7 @@ func (r *runner) Reference(t *testing.T, src span.Span, itemList []span.Span) { if err != nil { t.Fatal(err) } - ident, err := source.Identifier(ctx, r.view, f.(source.GoFile), srcRng.Start) + ident, err := source.Identifier(ctx, r.view, f, srcRng.Start) if err != nil { t.Fatalf("failed for %v: %v", src, err) } @@ -637,7 +639,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { if err != nil { t.Fatal(err) } - ident, err := source.Identifier(r.ctx, r.view, f.(source.GoFile), srcRng.Start) + ident, err := source.Identifier(r.ctx, r.view, f, srcRng.Start) if err != nil { t.Error(err) return @@ -659,7 +661,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - fh := f.Handle(ctx) + fh := r.view.Snapshot().Handle(r.ctx, f) data, _, err := fh.Read(ctx) if err != nil { t.Fatal(err) @@ -726,7 +728,7 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare t.Fatal(err) } // Find the identifier at the position. - item, err := source.PrepareRename(ctx, r.view, f.(source.GoFile), srcRng.Start) + item, err := source.PrepareRename(ctx, r.view, f, srcRng.Start) if err != nil { if want.Text != "" { // expected an ident. t.Errorf("prepare rename failed for %v: got error: %v", src, err) @@ -754,7 +756,7 @@ func (r *runner) Symbol(t *testing.T, uri span.URI, expectedSymbols []protocol.D if err != nil { t.Fatalf("failed for %v: %v", uri, err) } - symbols, err := source.DocumentSymbols(ctx, r.view, f.(source.GoFile)) + symbols, err := source.DocumentSymbols(ctx, r.view, f) if err != nil { t.Errorf("symbols failed for %s: %v", uri, err) } @@ -820,7 +822,7 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, expectedSignature *s if err != nil { t.Fatal(err) } - gotSignature, err := source.SignatureHelp(ctx, r.view, f.(source.GoFile), rng.Start) + gotSignature, err := source.SignatureHelp(ctx, r.view, f, rng.Start) if err != nil { // Only fail if we got an error we did not expect. if expectedSignature != nil { diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index 5d58b372e2..5f249b4fb3 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -14,11 +14,11 @@ import ( "golang.org/x/tools/internal/telemetry/trace" ) -func DocumentSymbols(ctx context.Context, view View, f GoFile) ([]protocol.DocumentSymbol, error) { +func DocumentSymbols(ctx context.Context, view View, f File) ([]protocol.DocumentSymbol, error) { ctx, done := trace.StartSpan(ctx, "source.DocumentSymbols") defer done() - cphs, err := f.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, err } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 238821f748..c65b8705c3 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -91,7 +91,7 @@ func IsGenerated(ctx context.Context, view View, uri span.URI) bool { if err != nil { return false } - ph := view.Session().Cache().ParseGoHandle(f.Handle(ctx), ParseHeader) + ph := view.Session().Cache().ParseGoHandle(view.Snapshot().Handle(ctx, f), ParseHeader) parsed, _, _, err := ph.Parse(ctx) if err != nil { return false diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 77556afc53..90bd1f069b 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -117,6 +117,10 @@ type CheckPackageHandle interface { // Config is the *packages.Config that the package metadata was loaded with. Config() *packages.Config + // Mode returns the ParseMode for all of the files in the CheckPackageHandle. + // The files should always have the same parse mode. + Mode() ParseMode + // Check returns the type-checked Package for the CheckPackageHandle. Check(ctx context.Context) (Package, error) @@ -240,6 +244,7 @@ type View interface { // Ignore returns true if this file should be ignored by this view. Ignore(span.URI) bool + // Config returns the configuration for the view. Config(ctx context.Context) *packages.Config // RunProcessEnvFunc runs fn with the process env for this view inserted into opts. @@ -257,33 +262,28 @@ type View interface { // Analyzers returns the set of Analyzers active for this view. Analyzers() []*analysis.Analyzer + // CheckPackageHandles returns the CheckPackageHandles for the packages + // that this file belongs to. + CheckPackageHandles(ctx context.Context, f File) (Snapshot, []CheckPackageHandle, error) + // GetActiveReverseDeps returns the active files belonging to the reverse // dependencies of this file's package. - GetActiveReverseDeps(ctx context.Context, uri span.URI) []CheckPackageHandle + GetActiveReverseDeps(ctx context.Context, f File) []CheckPackageHandle + + // Snapshot returns the current snapshot for the view. + Snapshot() Snapshot +} + +// Snapshot represents the current state for the given view. +type Snapshot interface { + // Handle returns the FileHandle for the given file. + Handle(ctx context.Context, f File) FileHandle } // File represents a source file of any type. type File interface { URI() span.URI - View() View - Handle(ctx context.Context) FileHandle -} - -// GoFile represents a Go source file that has been type-checked. -type GoFile interface { - File - - // GetCheckPackageHandles returns the CheckPackageHandles for the packages - // that this file belongs to. - CheckPackageHandles(ctx context.Context) ([]CheckPackageHandle, error) -} - -type ModFile interface { - File -} - -type SumFile interface { - File + Kind() FileKind } // Package represents a Go package that has been type-checked. It maintains diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go index addbe443e0..66f1654e68 100644 --- a/internal/lsp/symbols.go +++ b/internal/lsp/symbols.go @@ -19,7 +19,7 @@ func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSy uri := span.NewURI(params.TextDocument.URI) view := s.session.ViewOf(uri) - f, err := getGoFile(ctx, view, uri) + f, err := view.GetFile(ctx, uri) if err != nil { return nil, err } diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 201b1423b3..303f66f41f 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -136,7 +136,7 @@ func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocume func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { uri := span.NewURI(params.TextDocument.URI) - ctx = telemetry.File.With(ctx, uri) + ctx = telemetry.URI.With(ctx, uri) s.session.DidClose(uri) view := s.session.ViewOf(uri) if _, err := view.SetContent(ctx, uri, nil); err != nil { @@ -154,18 +154,11 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu // clear out all diagnostics for the package. f, err := view.GetFile(ctx, uri) if err != nil { - log.Error(ctx, "no file for %s: %v", err, telemetry.File) - return nil + log.Error(ctx, "no file", err, telemetry.URI) } - // For non-Go files, don't return any diagnostics. - gof, ok := f.(source.GoFile) - if !ok { - log.Error(ctx, "closing a non-Go file, no diagnostics to clear", nil, telemetry.File) - return nil - } - cphs, err := gof.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { - log.Error(ctx, "no CheckPackageHandles", err, telemetry.URI.Of(gof.URI())) + log.Error(ctx, "no CheckPackageHandles", err, telemetry.URI.Of(uri)) return nil } for _, cph := range cphs { diff --git a/internal/lsp/util.go b/internal/lsp/util.go deleted file mode 100644 index 60323b1ebd..0000000000 --- a/internal/lsp/util.go +++ /dev/null @@ -1,25 +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 lsp - -import ( - "context" - - "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/span" - errors "golang.org/x/xerrors" -) - -func getGoFile(ctx context.Context, view source.View, uri span.URI) (source.GoFile, error) { - f, err := view.GetFile(ctx, uri) - if err != nil { - return nil, err - } - gof, ok := f.(source.GoFile) - if !ok { - return nil, errors.Errorf("%s is not a Go file", uri) - } - return gof, nil -} diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index 281b7ab796..a923afeeda 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -27,10 +27,10 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did ctx := telemetry.File.With(ctx, uri) for _, view := range s.session.Views() { - gof, _ := view.FindFile(ctx, uri).(source.GoFile) + f := view.FindFile(ctx, uri) // If we have never seen this file before, there is nothing to do. - if gof == nil { + if f == nil { continue } @@ -53,14 +53,14 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did case protocol.Deleted: log.Print(ctx, "watched file deleted", telemetry.File) - cphs, err := gof.CheckPackageHandles(ctx) + _, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { log.Error(ctx, "didChangeWatchedFiles: GetPackage", err, telemetry.File) continue } // Find a different file in the same package we can use to trigger diagnostics. // TODO(rstambler): Allow diagnostics to be called per-package to avoid this. - var otherFile source.GoFile + var otherFile source.File sort.Slice(cphs, func(i, j int) bool { return len(cphs[i].Files()) > len(cphs[j].Files()) }) @@ -69,10 +69,10 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did continue } ident := ph.File().Identity() - if ident.URI == gof.URI() { + if ident.URI == f.URI() { continue } - otherFile, _ = view.FindFile(ctx, ident.URI).(source.GoFile) + otherFile := view.FindFile(ctx, ident.URI) if otherFile != nil { break }