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 }