From db2fa46ec33c618b29a7c6935439e6800b3078a8 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Fri, 12 Jul 2019 15:54:06 -0700 Subject: [PATCH] internal/lsp: compare mod file versions used in imports The results of running 'go list -m' are only valid as long as the current module and the modules in its replace directives do not change their go.mod files. Store the 'go.mod' versions that are used in the imports call, and reinitialize the module resolver if they change. Change-Id: Idb73c92b9e4dc243a276885e5333fafd2315134d Reviewed-on: https://go-review.googlesource.com/c/tools/+/186597 Run-TryBot: Suzy Mueller TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/view.go | 73 +++++++++++++++++++++++++++++++++-- internal/lsp/source/format.go | 18 ++++----- internal/lsp/source/view.go | 11 ++---- 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 14687d27f1..ef65e64394 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -55,8 +55,17 @@ type view struct { // process is the process env for this view. // Note: this contains cached module and filesystem state. + // + // 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 + // modFileVersions stores the last seen versions of the module files that are used + // by processEnvs resolver. + // TODO(suzmue): These versions may not actually be on disk. + modFileVersions map[string]string + // buildFlags is the build flags to use when invoking underlying tools. buildFlags []string @@ -144,14 +153,37 @@ func (v *view) Config(ctx context.Context) *packages.Config { } } -func (v *view) ProcessEnv(ctx context.Context) *imports.ProcessEnv { +func (v *view) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error, opts *imports.Options) error { v.mu.Lock() defer v.mu.Unlock() - if v.processEnv == nil { v.processEnv = v.buildProcessEnv(ctx) } - return v.processEnv + + // Before running the user provided function, clear caches in the resolver. + if v.modFilesChanged() { + if r, ok := v.processEnv.GetResolver().(*imports.ModuleResolver); ok { + // Clear the resolver cache and set Initialized to false. + r.Initialized = false + r.Main = nil + r.ModsByModPath = nil + r.ModsByDir = nil + // Reset the modFileVersions. + v.modFileVersions = nil + } + } + + // Run the user function. + opts.Env = v.processEnv + if err := fn(opts); err != nil { + return err + } + + // If applicable, store the file versions of the 'go.mod' files that are + // looked at by the resolver. + v.storeModFileVersions() + + return nil } func (v *view) buildProcessEnv(ctx context.Context) *imports.ProcessEnv { @@ -185,6 +217,41 @@ func (v *view) buildProcessEnv(ctx context.Context) *imports.ProcessEnv { return env } +func (v *view) modFilesChanged() bool { + // Check the versions of the 'go.mod' files of the main module + // and modules included by a replace directive. Return true if + // any of these file versions do not match. + for filename, version := range v.modFileVersions { + if version != v.fileVersion(filename) { + return true + } + } + return false +} + +func (v *view) storeModFileVersions() { + // Store the mod files versions, if we are using a ModuleResolver. + r, moduleMode := v.processEnv.GetResolver().(*imports.ModuleResolver) + if !moduleMode || !r.Initialized { + return + } + v.modFileVersions = make(map[string]string) + + // Get the file versions of the 'go.mod' files of the main module + // and modules included by a replace directive in the resolver. + for _, mod := range r.ModsByModPath { + if (mod.Main || mod.Replace != nil) && mod.GoMod != "" { + v.modFileVersions[mod.GoMod] = v.fileVersion(mod.GoMod) + } + } +} + +func (v *view) fileVersion(filename string) string { + uri := span.FileURI(filename) + f := v.session.GetFile(uri) + return f.Identity().Version +} + func (v *view) Env() []string { v.mu.Lock() defer v.mu.Unlock() diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 7a52cbcb71..b4e24b4b34 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -86,17 +86,7 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]TextEd return nil, fmt.Errorf("%s has list errors, not running goimports", f.URI()) } - if resolver, ok := view.ProcessEnv(ctx).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 - resolver.ModCachePkgs = nil - } options := &imports.Options{ - Env: view.ProcessEnv(ctx), // Defaults. AllErrors: true, Comments: true, @@ -105,10 +95,16 @@ func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]TextEd TabIndent: true, TabWidth: 8, } - formatted, err := imports.Process(f.URI().Filename(), data, options) + var formatted []byte + importFn := func(opts *imports.Options) error { + formatted, err = imports.Process(f.URI().Filename(), data, opts) + return err + } + err = view.RunProcessEnvFunc(ctx, importFn, options) if err != nil { return nil, err } + return computeTextEdits(ctx, f, string(formatted)), nil } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 622794d7ec..826921e8fd 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -210,14 +210,9 @@ type View interface { Config(ctx context.Context) *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(ctx context.Context) *imports.ProcessEnv + // RunProcessEnvFunc runs fn with the process env for this view inserted into opts. + // Note: the process env contains cached module and filesystem state. + RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error, opts *imports.Options) error } // File represents a source file of any type.