From 5ae6a9745e44f90b85d14baaf7e4bd6419db11ab Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 4 Jun 2019 18:06:55 -0400 Subject: [PATCH] internal/lsp: track missing imports, re-running packages.Load This change marks any unresolved packages discovered as imports by go/packages.Load. It re-runs go/packages.Load for any package with missing imports, which will result in the LSP registering changes in dependencies. However, this means that we re-run go/packages.Load on package's with unresolved imports much more than we normally would, so it may result in a slowdown or unexpected behavior. I'm not sure if this is necessarily the correct approach here. Updates golang/go#32232 Change-Id: Id611fa1876e42c88ca2c3e4db30da66dc66945fc Reviewed-on: https://go-review.googlesource.com/c/tools/+/180537 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/cache/gofile.go | 2 +- internal/lsp/cache/load.go | 44 +++++++++++++++++++++++++++++------- internal/lsp/cache/view.go | 4 ++++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go index 43a3264e..a3e1249d 100644 --- a/internal/lsp/cache/gofile.go +++ b/internal/lsp/cache/gofile.go @@ -112,7 +112,7 @@ func unexpectedAST(ctx context.Context, f *goFile) bool { // isDirty is true if the file needs to be type-checked. // It assumes that the file's view's mutex is held by the caller. func (f *goFile) isDirty() bool { - return f.meta == nil || f.token == nil || f.ast == nil || f.pkg == nil + return f.meta == nil || len(f.meta.missingImports) > 0 || f.token == nil || f.ast == nil || f.pkg == nil } func (f *goFile) astIsTrimmed() bool { diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 6e669cef..56e22a88 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -19,13 +19,24 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er f.invalidateAST() } + // Save the metadata's current missing imports, if any. + var originalMissingImports map[string]struct{} + if f.meta != nil { + originalMissingImports = f.meta.missingImports + } // Check if we need to run go/packages.Load for this file's package. if errs, err := v.checkMetadata(ctx, f); err != nil { return errs, err } + // If `go list` failed to get data for the file in question (this should never happen). if f.meta == nil { return nil, fmt.Errorf("loadParseTypecheck: no metadata found for %v", f.filename()) } + // If we have already seen these missing imports before, and we still have type information, + // there is no need to continue. + if sameSet(originalMissingImports, f.meta.missingImports) && f.pkg != nil { + return nil, nil + } imp := &importer{ view: v, @@ -51,10 +62,22 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er return nil, nil } +func sameSet(x, y map[string]struct{}) bool { + if len(x) != len(y) { + return false + } + for k := range x { + if _, ok := y[k]; !ok { + return false + } + } + return true +} + // 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) ([]packages.Error, error) { - if !v.reparseImports(ctx, f) { + if !v.parseImports(ctx, f) { return nil, nil } pkgs, err := packages.Load(v.buildConfig(), fmt.Sprintf("file=%s", f.filename())) @@ -84,8 +107,8 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) ([]packages.Error, // reparseImports reparses a file's package and import declarations to // determine if they have changed. -func (v *view) reparseImports(ctx context.Context, f *goFile) bool { - if f.meta == nil { +func (v *view) parseImports(ctx context.Context, f *goFile) bool { + if f.meta == nil || len(f.meta.missingImports) > 0 { return true } // Get file content in case we don't already have it. @@ -97,6 +120,7 @@ func (v *view) reparseImports(ctx context.Context, f *goFile) bool { if parsed == nil { return true } + // If the package name has changed, re-run `go list`. if f.meta.name != parsed.Name.Name { return true @@ -117,11 +141,12 @@ func (v *view) link(ctx context.Context, pkgPath string, pkg *packages.Package, m, ok := v.mcache.packages[pkgPath] if !ok { m = &metadata{ - pkgPath: pkgPath, - id: pkg.ID, - typesSizes: pkg.TypesSizes, - parents: make(map[string]bool), - children: make(map[string]bool), + pkgPath: pkgPath, + id: pkg.ID, + typesSizes: pkg.TypesSizes, + parents: make(map[string]bool), + children: make(map[string]bool), + missingImports: make(map[string]struct{}), } v.mcache.packages[pkgPath] = m } @@ -143,6 +168,9 @@ func (v *view) link(ctx context.Context, pkgPath string, pkg *packages.Package, parent.children[pkgPath] = true } for importPath, importPkg := range pkg.Imports { + if len(importPkg.Errors) > 0 { + m.missingImports[pkg.PkgPath] = struct{}{} + } if _, ok := m.children[importPath]; !ok { v.link(ctx, importPath, importPkg, m) } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 9d2ee155..269aec77 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -79,6 +79,10 @@ type metadata struct { files []string typesSizes types.Sizes parents, children map[string]bool + + // missingImports is the set of unresolved imports for this package. + // It contains any packages with `go list` errors. + missingImports map[string]struct{} } type packageCache struct {