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 | 61 ++-- internal/lsp/cache/file.go | 20 +- internal/lsp/cache/gofile.go | 159 ++++++--- internal/lsp/cache/load.go | 271 +++++++------- internal/lsp/cache/pkg.go | 8 + internal/lsp/cache/session.go | 2 + internal/lsp/cache/snapshot.go | 430 ++++++++++------------- 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, 641 insertions(+), 679 deletions(-) delete mode 100644 internal/lsp/util.go diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 2f8020869a5..46ade5cbfc7 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) + for _, depID := range m.deps { + dep := imp.snapshot.getMetadata(depID) + if dep == nil { + continue + } + depHandle, err := cimp.checkPackageHandle(ctx, depID, imp.snapshot) if err != nil { - log.Error(ctx, "no check package handle", err, telemetry.Package.Of(child.id)) + log.Error(ctx, "no check package handle", err, telemetry.Package.Of(depID)) continue } - cph.imports[child.pkgPath] = childHandle + 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 a9748d5336c..a40e0244dcb 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 1102cec9434..e3ed8ec739f 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 (s *snapshot) checkPackageHandles(ctx context.Context, f source.File) ([]source.CheckPackageHandle, error) { + ctx = telemetry.File.With(ctx, f.URI()) + + 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 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 + } + } + 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 cphs, nil } -func (f *goFile) CheckPackageHandles(ctx context.Context) (results []source.CheckPackageHandle, err error) { - ctx = telemetry.File.With(ctx, f.URI()) - fh := f.Handle(ctx) +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) - var dirty bool - meta, pkgs := f.view.getSnapshot(f.URI()) - if len(meta) == 0 { - dirty = true + // If there is no metadata for the package, we definitely need to type-check again. + if len(m) == 0 { + return nil, nil, true, true } - for _, m := range meta { + + // 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 { - dirty = true + load = true + check = 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 { + // We expect to see a checked package for each package ID, + // and it should be parsed in full mode. + var ( + 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 } - // Check if there is a fully-parsed package to which this file belongs. + // Confirm that the file belongs to this package. for _, file := range cph.Files() { if file.File().Identity() == fh.Identity() { - results = append(results, cph) + cphs = append(cphs, cph) } } } - if dirty || len(results) == 0 { - cphs, err := f.view.loadParseTypecheck(ctx, f, fh) - if err != nil { - return nil, err - } - if cphs == nil { - return results, nil - } - results = cphs - } - if len(results) == 0 { - return nil, errors.Errorf("no CheckPackageHandles for %s", f.URI()) + if len(cphs) < expected { + return m, nil, load, true } - return results, nil + return m, cphs, load, check } -func (v *view) GetActiveReverseDeps(ctx context.Context, uri span.URI) (results []source.CheckPackageHandle) { +func (v *view) GetActiveReverseDeps(ctx context.Context, f source.File) (results []source.CheckPackageHandle) { var ( - rdeps = v.reverseDependencies(ctx, uri) + 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 9d99eff9f8e..f175b5f215a 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 -} - -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 packageKey struct { + mode source.ParseMode + id packageID } -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())) - 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 +type metadata struct { + id packageID + pkgPath packagePath + name string + files []span.URI + typesSizes types.Sizes + errors []packages.Error + deps []packageID + missingDeps map[packagePath]struct{} } -// 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())) +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() - 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 e0fc01ad92c..32b85218116 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 6cd4a61f54f..4e175b5a6a0 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 a998fa7fddb..972cad193b9 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 -} -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{} -} + // importedBy maps package IDs to the list of packages that import them. + importedBy map[packageID][]packageID -func (v *view) getSnapshot(uri span.URI) ([]*metadata, []*checkPackageHandle) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() + // files maps file URIs to their corresponding FileHandles. + // It may invalidated when a file's content changes. + files map[span.URI]source.FileHandle - var m []*metadata - for _, id := range v.snapshot.ids[uri] { - m = append(m, v.snapshot.metadata[id]) - } - var cphs []*checkPackageHandle - for _, cph := range v.snapshot.packages[uri] { - cphs = append(cphs, cph) - } - return m, cphs + // 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 } -func (v *view) getMetadata(uri span.URI) []*metadata { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) getImportedBy(id packageID) []packageID { + s.mu.Lock() + defer s.mu.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() } - return m -} -func (v *view) getPackages(uri span.URI) map[packageKey]*checkPackageHandle { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() - - return v.snapshot.packages[uri] + return s.importedBy[id] } -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() +func (s *snapshot) addPackage(uri span.URI, cph *checkPackageHandle) { + s.mu.Lock() + defer s.mu.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 - } - } - 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 + 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 } -type importGraph struct { - pkgPath packagePath - pkg *packages.Package - parent *metadata -} +func (s *snapshot) getPackages(uri span.URI) (cphs []source.CheckPackageHandle) { + s.mu.Lock() + defer s.mu.Unlock() -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) + for _, cph := range s.packages[uri] { + cphs = append(cphs, cph) } + return cphs +} - // Add the metadata to the cache. - v.snapshot.metadata[m.id] = m +func (s *snapshot) getMetadataForURI(uri span.URI) (metadata []*metadata) { + s.mu.Lock() + defer s.mu.Unlock() - // 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 + for _, id := range s.ids[uri] { + if m, ok := s.metadata[id]; ok { + metadata = append(metadata, m) } - delete(m.children, importID) - delete(child.parents, m.id) } - return nil + return metadata } -func (v *view) updatePackages(cphs []*checkPackageHandle) { - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) setMetadata(m *metadata) { + 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.metadata[m.id] = m } -// 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) getMetadata(id packageID) *metadata { + s.mu.Lock() + defer s.mu.Unlock() - without := make(map[span.URI]struct{}) + return s.metadata[id] +} - // Remove the package and all of its reverse dependencies from the cache. - v.snapshotMu.Lock() - defer v.snapshotMu.Unlock() +func (s *snapshot) addID(uri span.URI, id packageID) { + s.mu.Lock() + defer s.mu.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 + s.ids[uri] = append(s.ids[uri], id) } -// 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) getIDs(uri span.URI) []packageID { + s.mu.Lock() + defer s.mu.Unlock() + + return s.ids[uri] +} - without := make(map[span.URI]struct{}) +func (s *snapshot) getFile(uri span.URI) source.FileHandle { + s.mu.Lock() + defer s.mu.Unlock() - 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() + + 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{}{}) + } + + // 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) +} + +// 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() - for _, id := range v.snapshot.ids[uri] { - v.rdeps(id, seen, uris, id) + withoutMetadata := make(map[span.URI]struct{}) + for _, id := range v.snapshot.getIDs(uri) { + v.snapshot.reverseDependencies(id, withoutMetadata, map[packageID]struct{}{}) } - return uris + v.snapshot = v.snapshot.clone(uri, nil, withoutMetadata) } -func (v *view) rdeps(topID packageID, seen map[packageID]struct{}, results map[span.URI]struct{}, id packageID) { +// 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 _, uri := range m.files { + uris[uri] = struct{}{} } - for parentID := range m.parents { - v.rdeps(topID, seen, results, parentID) +} + +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 34cebd4d6d5..f9279dcc324 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 0e6523cc0dc..ea7bff628cf 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 d6c4a03f4dc..72e86475322 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 7b64a993920..0f6b7f4ea46 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 8e49cd3119a..f8f18df9ef3 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 4f137b6cce4..6b602cd3d16 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 49de6033c84..d0eecc49a22 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 c79ba6e0c75..ae51f46596e 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 cea01ab955f..4c79bdc707c 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 691621d71f0..4b2856cf655 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 25977774ca6..6930a6c1743 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 d4b4d0876ae..9ce36729a1a 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 7b514fc547c..fe096522f63 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 8f466b36df2..5771ef16c0e 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 12d036e9744..6a4b1c81e35 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 9ccb5733228..199dde8e408 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 5176ca5e28a..04c4737a6b2 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 f66d9584160..e944e9b6d19 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 746de0ef5ff..50563d39fca 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 992ada5e0be..64fa906e74f 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 ea8bed13026..2aa0613c420 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 bb4cebccc47..100b55c4888 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 b582fb6bc27..de6b1b35c33 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 640ebae2683..161085d1870 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 5d58b372e24..5f249b4fb30 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 238821f748d..c65b8705c3a 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 77556afc53c..90bd1f069b1 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 -// File represents a source file of any type. -type File interface { - URI() span.URI - View() View - Handle(ctx context.Context) FileHandle + // Snapshot returns the current snapshot for the view. + Snapshot() Snapshot } -// 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 +// 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 } -type SumFile interface { - File +// File represents a source file of any type. +type File interface { + URI() span.URI + 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 addbe443e01..66f1654e68d 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 201b1423b33..303f66f41f5 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 - } - // 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 + log.Error(ctx, "no file", err, telemetry.URI) } - 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 60323b1ebdb..00000000000 --- 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 281b7ab7969..a923afeeda1 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 }