internal/lsp: cache the imports ProcessEnv across imports calls
The imports ProcessEnv contains cached module and filesystem state. This change
allows gopls to use the same ProcessEnv and resolver across multiple calls to the
internal/imports library.
A ProcessEnv belongs to a view, because the cached module state depends
on the module that is open in the workspace.
Since we do not yet track whether the 'go.mod' file has changed, we
conservatively reset the cached state in the module resolver before
every call to imports.Process.
Change-Id: I27c8e212cb0b477ff425d5ed98a544b27b7d92ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184921
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/imports/fix.go b/internal/imports/fix.go
index 2c58d18..72323f5 100644
--- a/internal/imports/fix.go
+++ b/internal/imports/fix.go
@@ -272,7 +272,7 @@
unknown = append(unknown, imp.importPath)
}
- names, err := p.env.getResolver().loadPackageNames(unknown, p.srcDir)
+ names, err := p.env.GetResolver().loadPackageNames(unknown, p.srcDir)
if err != nil {
return err
}
@@ -595,7 +595,7 @@
// Logf is the default logger for the ProcessEnv.
Logf func(format string, args ...interface{})
- resolver resolver
+ resolver Resolver
}
func (e *ProcessEnv) env() []string {
@@ -617,7 +617,7 @@
return env
}
-func (e *ProcessEnv) getResolver() resolver {
+func (e *ProcessEnv) GetResolver() Resolver {
if e.resolver != nil {
return e.resolver
}
@@ -631,7 +631,7 @@
e.resolver = &gopathResolver{env: e}
return e.resolver
}
- e.resolver = &moduleResolver{env: e}
+ e.resolver = &ModuleResolver{env: e}
return e.resolver
}
@@ -700,15 +700,15 @@
}
}
-// A resolver does the build-system-specific parts of goimports.
-type resolver interface {
+// A Resolver does the build-system-specific parts of goimports.
+type Resolver interface {
// loadPackageNames loads the package names in importPaths.
loadPackageNames(importPaths []string, srcDir string) (map[string]string, error)
// scan finds (at least) the packages satisfying refs. The returned slice is unordered.
scan(refs references) ([]*pkg, error)
}
-// gopathResolver implements resolver for GOPATH and module workspaces using go/packages.
+// gopackagesResolver implements resolver for GOPATH and module workspaces using go/packages.
type goPackagesResolver struct {
env *ProcessEnv
}
@@ -758,7 +758,7 @@
}
func addExternalCandidates(pass *pass, refs references, filename string) error {
- dirScan, err := pass.env.getResolver().scan(refs)
+ dirScan, err := pass.env.GetResolver().scan(refs)
if err != nil {
return err
}
diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go
index a5f7f7f..d15b6dd 100644
--- a/internal/imports/fix_test.go
+++ b/internal/imports/fix_test.go
@@ -1855,7 +1855,7 @@
if strings.Contains(t.Name(), "GoPackages") {
t.Skip("go/packages does not ignore package main")
}
- r := t.env.getResolver()
+ r := t.env.GetResolver()
srcDir := filepath.Dir(t.exported.File("example.net/pkg", "z.go"))
names, err := r.loadPackageNames([]string{"example.net/pkg"}, srcDir)
if err != nil {
diff --git a/internal/imports/mod.go b/internal/imports/mod.go
index a0ebd07..3d68533 100644
--- a/internal/imports/mod.go
+++ b/internal/imports/mod.go
@@ -18,37 +18,37 @@
"golang.org/x/tools/internal/module"
)
-// moduleResolver implements resolver for modules using the go command as little
+// ModuleResolver implements resolver for modules using the go command as little
// as feasible.
-type moduleResolver struct {
+type ModuleResolver struct {
env *ProcessEnv
- initialized bool
- main *moduleJSON
- modsByModPath []*moduleJSON // All modules, ordered by # of path components in module Path...
- modsByDir []*moduleJSON // ...or Dir.
+ Initialized bool
+ Main *ModuleJSON
+ ModsByModPath []*ModuleJSON // All modules, ordered by # of path components in module Path...
+ ModsByDir []*ModuleJSON // ...or Dir.
}
-type moduleJSON struct {
+type ModuleJSON struct {
Path string // module path
Version string // module version
Versions []string // available module versions (with -versions)
- Replace *moduleJSON // replaced by this module
+ Replace *ModuleJSON // replaced by this module
Time *time.Time // time version was created
- Update *moduleJSON // available update, if any (with -u)
+ Update *ModuleJSON // available update, if any (with -u)
Main bool // is this the main module?
Indirect bool // is this module only an indirect dependency of main module?
Dir string // directory holding files for this module, if any
GoMod string // path to go.mod file for this module, if any
- Error *moduleErrorJSON // error loading module
+ Error *ModuleErrorJSON // error loading module
}
-type moduleErrorJSON struct {
+type ModuleErrorJSON struct {
Err string // the error itself
}
-func (r *moduleResolver) init() error {
- if r.initialized {
+func (r *ModuleResolver) init() error {
+ if r.Initialized {
return nil
}
stdout, err := r.env.invokeGo("list", "-m", "-json", "...")
@@ -56,7 +56,7 @@
return err
}
for dec := json.NewDecoder(stdout); dec.More(); {
- mod := &moduleJSON{}
+ mod := &ModuleJSON{}
if err := dec.Decode(mod); err != nil {
return err
}
@@ -67,34 +67,34 @@
// Can't do anything with a module that's not downloaded.
continue
}
- r.modsByModPath = append(r.modsByModPath, mod)
- r.modsByDir = append(r.modsByDir, mod)
+ r.ModsByModPath = append(r.ModsByModPath, mod)
+ r.ModsByDir = append(r.ModsByDir, mod)
if mod.Main {
- r.main = mod
+ r.Main = mod
}
}
- sort.Slice(r.modsByModPath, func(i, j int) bool {
+ sort.Slice(r.ModsByModPath, func(i, j int) bool {
count := func(x int) int {
- return strings.Count(r.modsByModPath[x].Path, "/")
+ return strings.Count(r.ModsByModPath[x].Path, "/")
}
return count(j) < count(i) // descending order
})
- sort.Slice(r.modsByDir, func(i, j int) bool {
+ sort.Slice(r.ModsByDir, func(i, j int) bool {
count := func(x int) int {
- return strings.Count(r.modsByDir[x].Dir, "/")
+ return strings.Count(r.ModsByDir[x].Dir, "/")
}
return count(j) < count(i) // descending order
})
- r.initialized = true
+ r.Initialized = true
return nil
}
// findPackage returns the module and directory that contains the package at
// the given import path, or returns nil, "" if no module is in scope.
-func (r *moduleResolver) findPackage(importPath string) (*moduleJSON, string) {
- for _, m := range r.modsByModPath {
+func (r *ModuleResolver) findPackage(importPath string) (*ModuleJSON, string) {
+ for _, m := range r.ModsByModPath {
if !strings.HasPrefix(importPath, m.Path) {
continue
}
@@ -123,7 +123,7 @@
// findModuleByDir returns the module that contains dir, or nil if no such
// module is in scope.
-func (r *moduleResolver) findModuleByDir(dir string) *moduleJSON {
+func (r *ModuleResolver) findModuleByDir(dir string) *ModuleJSON {
// This is quite tricky and may not be correct. dir could be:
// - a package in the main module.
// - a replace target underneath the main module's directory.
@@ -134,7 +134,7 @@
// - in /vendor/ in -mod=vendor mode.
// - nested module? Dunno.
// Rumor has it that replace targets cannot contain other replace targets.
- for _, m := range r.modsByDir {
+ for _, m := range r.ModsByDir {
if !strings.HasPrefix(dir, m.Dir) {
continue
}
@@ -150,7 +150,7 @@
// dirIsNestedModule reports if dir is contained in a nested module underneath
// mod, not actually in mod.
-func dirIsNestedModule(dir string, mod *moduleJSON) bool {
+func dirIsNestedModule(dir string, mod *ModuleJSON) bool {
if !strings.HasPrefix(dir, mod.Dir) {
return false
}
@@ -176,7 +176,7 @@
}
}
-func (r *moduleResolver) loadPackageNames(importPaths []string, srcDir string) (map[string]string, error) {
+func (r *ModuleResolver) loadPackageNames(importPaths []string, srcDir string) (map[string]string, error) {
if err := r.init(); err != nil {
return nil, err
}
@@ -195,7 +195,7 @@
return names, nil
}
-func (r *moduleResolver) scan(_ references) ([]*pkg, error) {
+func (r *ModuleResolver) scan(_ references) ([]*pkg, error) {
if err := r.init(); err != nil {
return nil, err
}
@@ -204,15 +204,15 @@
roots := []gopathwalk.Root{
{filepath.Join(r.env.GOROOT, "/src"), gopathwalk.RootGOROOT},
}
- if r.main != nil {
- roots = append(roots, gopathwalk.Root{r.main.Dir, gopathwalk.RootCurrentModule})
+ if r.Main != nil {
+ roots = append(roots, gopathwalk.Root{r.Main.Dir, gopathwalk.RootCurrentModule})
}
for _, p := range filepath.SplitList(r.env.GOPATH) {
roots = append(roots, gopathwalk.Root{filepath.Join(p, "/pkg/mod"), gopathwalk.RootModuleCache})
}
// Walk replace targets, just in case they're not in any of the above.
- for _, mod := range r.modsByModPath {
+ for _, mod := range r.ModsByModPath {
if mod.Replace != nil {
roots = append(roots, gopathwalk.Root{mod.Dir, gopathwalk.RootOther})
}
@@ -247,7 +247,7 @@
}
switch root.Type {
case gopathwalk.RootCurrentModule:
- importPath = path.Join(r.main.Path, filepath.ToSlash(subdir))
+ importPath = path.Join(r.Main.Path, filepath.ToSlash(subdir))
case gopathwalk.RootModuleCache:
matches := modCacheRegexp.FindStringSubmatch(subdir)
modPath, err := module.DecodePath(filepath.ToSlash(matches[1]))
diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go
index 97ef715..e1d9c8c 100644
--- a/internal/imports/mod_test.go
+++ b/internal/imports/mod_test.go
@@ -140,7 +140,7 @@
mt.env.GOFLAGS = ""
// Clear out the resolver's cache, since we've changed the environment.
- mt.resolver = &moduleResolver{env: mt.env}
+ mt.resolver = &ModuleResolver{env: mt.env}
mt.assertModuleFoundInDir("rsc.io/quote", "quote", `pkg.*mod.*/quote@.*$`)
}
@@ -486,7 +486,7 @@
type modTest struct {
*testing.T
env *ProcessEnv
- resolver *moduleResolver
+ resolver *ModuleResolver
cleanup func()
}
@@ -538,7 +538,7 @@
return &modTest{
T: t,
env: env,
- resolver: &moduleResolver{env: env},
+ resolver: &ModuleResolver{env: env},
cleanup: func() {
_ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
if err != nil {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index e0547b9..8496b15 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -12,9 +12,11 @@
"go/types"
"os"
"path/filepath"
+ "strings"
"sync"
"golang.org/x/tools/go/packages"
+ "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/debug"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/xlog"
@@ -49,6 +51,10 @@
// env is the environment to use when invoking underlying tools.
env []string
+ // process is the process env for this view.
+ // Note: this contains cached module and filesystem state.
+ processEnv *imports.ProcessEnv
+
// buildFlags is the build flags to use when invoking underlying tools.
buildFlags []string
@@ -133,6 +139,47 @@
}
}
+func (v *view) ProcessEnv() *imports.ProcessEnv {
+ v.mu.Lock()
+ defer v.mu.Unlock()
+
+ if v.processEnv == nil {
+ v.processEnv = v.buildProcessEnv()
+ }
+ return v.processEnv
+}
+
+func (v *view) buildProcessEnv() *imports.ProcessEnv {
+ cfg := v.Config()
+ env := &imports.ProcessEnv{
+ WorkingDir: cfg.Dir,
+ Logf: func(format string, u ...interface{}) {
+ xlog.Infof(v.backgroundCtx, format, u...)
+ },
+ }
+ for _, kv := range cfg.Env {
+ split := strings.Split(kv, "=")
+ if len(split) < 2 {
+ continue
+ }
+ switch split[0] {
+ case "GOPATH":
+ env.GOPATH = split[1]
+ case "GOROOT":
+ env.GOROOT = split[1]
+ case "GO111MODULE":
+ env.GO111MODULE = split[1]
+ case "GOPROXY":
+ env.GOROOT = split[1]
+ case "GOFLAGS":
+ env.GOFLAGS = split[1]
+ case "GOSUMDB":
+ env.GOSUMDB = split[1]
+ }
+ }
+ return env
+}
+
func (v *view) Env() []string {
v.mu.Lock()
defer v.mu.Unlock()
@@ -144,6 +191,7 @@
defer v.mu.Unlock()
//TODO: this should invalidate the entire view
v.env = env
+ v.processEnv = nil // recompute process env
}
func (v *view) SetBuildFlags(buildFlags []string) {
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index e8884e6..145ad0a 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -10,7 +10,6 @@
"context"
"fmt"
"go/format"
- "strings"
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages"
@@ -68,8 +67,17 @@
if hasListErrors(pkg.GetErrors()) {
return nil, fmt.Errorf("%s has list errors, not running goimports", f.URI())
}
+
+ if resolver, ok := view.ProcessEnv().GetResolver().(*imports.ModuleResolver); ok && resolver.Initialized {
+ // TODO(suzmue): only reset this state when necessary (eg when the go.mod files of this
+ // module or modules with replace directive changes).
+ resolver.Initialized = false
+ resolver.Main = nil
+ resolver.ModsByModPath = nil
+ resolver.ModsByDir = nil
+ }
options := &imports.Options{
- Env: buildProcessEnv(ctx, view),
+ Env: view.ProcessEnv(),
// Defaults.
AllErrors: true,
Comments: true,
@@ -103,37 +111,6 @@
return false
}
-func buildProcessEnv(ctx context.Context, view View) *imports.ProcessEnv {
- cfg := view.Config()
- env := &imports.ProcessEnv{
- WorkingDir: cfg.Dir,
- Logf: func(format string, v ...interface{}) {
- xlog.Infof(ctx, format, v...)
- },
- }
- for _, kv := range cfg.Env {
- split := strings.Split(kv, "=")
- if len(split) < 2 {
- continue
- }
- switch split[0] {
- case "GOPATH":
- env.GOPATH = split[1]
- case "GOROOT":
- env.GOROOT = split[1]
- case "GO111MODULE":
- env.GO111MODULE = split[1]
- case "GOPROXY":
- env.GOROOT = split[1]
- case "GOFLAGS":
- env.GOFLAGS = split[1]
- case "GOSUMDB":
- env.GOSUMDB = split[1]
- }
- }
- return env
-}
-
func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
ctx, done := trace.StartSpan(ctx, "source.computeTextEdits")
defer done()
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index d0db264..2ed1887 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
+ "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/span"
)
@@ -208,6 +209,15 @@
Ignore(span.URI) bool
Config() *packages.Config
+
+ // Process returns the process for this view.
+ // Note: this contains cached module and filesystem state, which must
+ // be invalidated after a 'go.mod' change.
+ //
+ // TODO(suzmue): the state cached in the process env is specific to each view,
+ // however, there is state that can be shared between views that is not currently
+ // cached, like the module cache.
+ ProcessEnv() *imports.ProcessEnv
}
// File represents a source file of any type.
