diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 2c58d18bec..72323f5859 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -272,7 +272,7 @@ func (p *pass) loadPackageNames(imports []*importInfo) error { 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 @@ type ProcessEnv struct { // 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 @@ func (e *ProcessEnv) env() []string { return env } -func (e *ProcessEnv) getResolver() resolver { +func (e *ProcessEnv) GetResolver() Resolver { if e.resolver != nil { return e.resolver } @@ -631,7 +631,7 @@ func (e *ProcessEnv) getResolver() resolver { 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 @@ func addStdlibCandidates(pass *pass, refs references) { } } -// 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 (r *goPackagesResolver) scan(refs references) ([]*pkg, error) { } 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 a5f7f7fe35..d15b6dd248 100644 --- a/internal/imports/fix_test.go +++ b/internal/imports/fix_test.go @@ -1855,7 +1855,7 @@ func TestImportPathToNameGoPathParse(t *testing.T) { 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 a0ebd0737c..3d685332d1 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -18,37 +18,37 @@ import ( "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 @@ func (r *moduleResolver) init() error { 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 @@ func (r *moduleResolver) init() error { // 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 @@ func (r *moduleResolver) findPackage(importPath string) (*moduleJSON, string) { // 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 @@ func (r *moduleResolver) findModuleByDir(dir string) *moduleJSON { // - 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 @@ func (r *moduleResolver) findModuleByDir(dir string) *moduleJSON { // 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 findModFile(dir string) string { } } -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 @@ func (r *moduleResolver) loadPackageNames(importPaths []string, srcDir string) ( 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 @@ func (r *moduleResolver) scan(_ references) ([]*pkg, error) { 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 @@ func (r *moduleResolver) scan(_ references) ([]*pkg, error) { } 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 97ef715cd9..e1d9c8ceec 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -140,7 +140,7 @@ import _ "rsc.io/quote" 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 @@ var proxyDir string type modTest struct { *testing.T env *ProcessEnv - resolver *moduleResolver + resolver *ModuleResolver cleanup func() } @@ -538,7 +538,7 @@ func setup(t *testing.T, main, wd string) *modTest { 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 e0547b968c..8496b15fdd 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -12,9 +12,11 @@ import ( "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 @@ type view struct { // 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) Config() *packages.Config { } } +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 @@ func (v *view) SetEnv(env []string) { 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 e8884e6a07..145ad0a164 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -10,7 +10,6 @@ import ( "context" "fmt" "go/format" - "strings" "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/go/packages" @@ -68,8 +67,17 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]TextEd 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 @@ func hasListErrors(errors []packages.Error) bool { 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 d0db2642ca..2ed1887923 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -14,6 +14,7 @@ import ( "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 @@ type View interface { 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.