internal/lsp: re-enable upgrades for individual dependencies In CL 271297, I disabled the constantly-running upgrade check, which removed the upgrade commands for individual dependencies. This seems to have been a relatively popular feature. Re-introduce it, but requiring explicit user interaction. We now run an upgrade check when the user clicks "Check for upgrades". Those results are stored on the View and used to show diagnostics on any requires they apply to. Right now we only check the go.mod the user has open; in multi-module workspaces it might make sense to check all of them, but I'm not sure. Fixes golang/go#42969. Change-Id: I65205dc99a4fa9daafdb83145b0294b6f3be5336 Reviewed-on: https://go-review.googlesource.com/c/tools/+/286474 Trust: Heschi Kreinick <heschi@google.com> Run-TryBot: Heschi Kreinick <heschi@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index f545551..0fa1e86 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md
@@ -53,6 +53,12 @@ go_get_package runs `go get` to fetch a package. +### **Check for upgrades** +Identifier: `gopls.check_upgrades` + +check_upgrades checks for module upgrades. + + ### **Add dependency** Identifier: `gopls.add_dependency`
diff --git a/gopls/internal/regtest/codelens/codelens_test.go b/gopls/internal/regtest/codelens/codelens_test.go index 305047f..52a7e11 100644 --- a/gopls/internal/regtest/codelens/codelens_test.go +++ b/gopls/internal/regtest/codelens/codelens_test.go
@@ -116,6 +116,14 @@ _ = hi.Goodbye } ` + + const wantGoMod = `module mod.com + +go 1.12 + +require golang.org/x/hello v1.3.3 +` + for _, commandTitle := range []string{ "Upgrade transitive dependencies", "Upgrade direct dependencies", @@ -143,19 +151,26 @@ t.Fatal(err) } env.Await(env.DoneWithChangeWatchedFiles()) - got := env.Editor.BufferText("go.mod") - const wantGoMod = `module mod.com - -go 1.12 - -require golang.org/x/hello v1.3.3 -` - if got != wantGoMod { + if got := env.Editor.BufferText("go.mod"); got != wantGoMod { t.Fatalf("go.mod upgrade failed:\n%s", tests.Diff(t, wantGoMod, got)) } }) }) } + t.Run("Upgrade individual dependency", func(t *testing.T) { + WithOptions(ProxyFiles(proxyWithLatest)).Run(t, shouldUpdateDep, func(t *testing.T, env *Env) { + env.OpenFile("go.mod") + env.ExecuteCodeLensCommand("go.mod", source.CommandCheckUpgrades) + d := &protocol.PublishDiagnosticsParams{} + env.Await(OnceMet(env.DiagnosticAtRegexpWithMessage("go.mod", `require`, "can be upgraded"), + ReadDiagnostics("go.mod", d))) + env.ApplyQuickFixes("go.mod", d.Diagnostics) + env.Await(env.DoneWithChangeWatchedFiles()) + if got := env.Editor.BufferText("go.mod"); got != wantGoMod { + t.Fatalf("go.mod upgrade failed:\n%s", tests.Diff(t, wantGoMod, got)) + } + }) + }) } func TestUnusedDependenciesCodelens(t *testing.T) {
diff --git a/internal/gocommand/vendor.go b/internal/gocommand/vendor.go index 1cd8d84..5e75bd6 100644 --- a/internal/gocommand/vendor.go +++ b/internal/gocommand/vendor.go
@@ -12,6 +12,7 @@ "path/filepath" "regexp" "strings" + "time" "golang.org/x/mod/semver" ) @@ -19,11 +20,15 @@ // ModuleJSON holds information about a module. type ModuleJSON struct { Path string // module path + Version string // module version + Versions []string // available module versions (with -versions) Replace *ModuleJSON // replaced by this module + Time *time.Time // time version was created + 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 + GoMod string // path to go.mod file used when loading this module, if any GoVersion string // go version used in module }
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index fb1790c..1102808 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go
@@ -200,8 +200,9 @@ baseCtx: baseCtx, name: name, folder: folder, - filesByURI: make(map[span.URI]*fileBase), - filesByBase: make(map[string][]*fileBase), + moduleUpgrades: map[string]string{}, + filesByURI: map[span.URI]*fileBase{}, + filesByBase: map[string][]*fileBase{}, rootURI: root, workspaceInformation: *ws, tempWorkspace: tempWorkspace,
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index ec35561..5b767d1 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go
@@ -60,6 +60,9 @@ importsState *importsState + // moduleUpgrades tracks known upgrades for module paths. + moduleUpgrades map[string]string + // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files filesByURI map[span.URI]*fileBase @@ -863,6 +866,26 @@ return globsMatchPath(v.goprivate, target) } +func (v *View) ModuleUpgrades() map[string]string { + v.mu.Lock() + defer v.mu.Unlock() + + upgrades := map[string]string{} + for mod, ver := range v.moduleUpgrades { + upgrades[mod] = ver + } + return upgrades +} + +func (v *View) RegisterModuleUpgrades(upgrades map[string]string) { + v.mu.Lock() + defer v.mu.Unlock() + + for mod, ver := range upgrades { + v.moduleUpgrades[mod] = ver + } +} + // Copied from // https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/str/path.go;l=58;drc=2910c5b4a01a573ebc97744890a07c1a3122c67a func globsMatchPath(globs, target string) bool {
diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 79c8663..a3bc21d 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go
@@ -217,6 +217,25 @@ return err } return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri.SpanURI(), "list", []string{"all"}) + case source.CommandCheckUpgrades: + var uri protocol.DocumentURI + var modules []string + if err := source.UnmarshalArgs(args, &uri, &modules); err != nil { + return err + } + snapshot, _, ok, release, err := s.beginFileRequest(ctx, uri, source.UnknownKind) + defer release() + if !ok { + return err + } + upgrades, err := s.getUpgrades(ctx, snapshot, uri.SpanURI(), modules) + if err != nil { + return err + } + snapshot.View().RegisterModuleUpgrades(upgrades) + // Re-diagnose the snapshot to publish the new module diagnostics. + s.diagnoseSnapshot(snapshot, nil, false) + return nil case source.CommandAddDependency, source.CommandUpgradeDependency: var uri protocol.DocumentURI var goCmdArgs []string @@ -511,3 +530,27 @@ }) return err } + +func (s *Server) getUpgrades(ctx context.Context, snapshot source.Snapshot, uri span.URI, modules []string) (map[string]string, error) { + stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal|source.AllowNetwork, &gocommand.Invocation{ + Verb: "list", + Args: append([]string{"-m", "-u", "-json"}, modules...), + WorkingDir: filepath.Dir(uri.Filename()), + }) + if err != nil { + return nil, err + } + + upgrades := map[string]string{} + for dec := json.NewDecoder(stdout); dec.More(); { + mod := &gocommand.ModuleJSON{} + if err := dec.Decode(mod); err != nil { + return nil, err + } + if mod.Update == nil { + continue + } + upgrades[mod.Path] = mod.Update.Version + } + return upgrades, nil +}
diff --git a/internal/lsp/mod/code_lens.go b/internal/lsp/mod/code_lens.go index 4465637..88ffe84 100644 --- a/internal/lsp/mod/code_lens.go +++ b/internal/lsp/mod/code_lens.go
@@ -42,6 +42,10 @@ for _, req := range pm.File.Require { requires = append(requires, req.Mod.Path) } + checkUpgradeArgs, err := source.MarshalArgs(fh.URI(), requires) + if err != nil { + return nil, err + } upgradeDirectArgs, err := source.MarshalArgs(fh.URI(), false, requires) if err != nil { return nil, err @@ -51,10 +55,19 @@ if err != nil { return nil, err } + return []protocol.CodeLens{ { Range: rng, Command: protocol.Command{ + Title: "Check for upgrades", + Command: source.CommandCheckUpgrades.ID(), + Arguments: checkUpgradeArgs, + }, + }, + { + Range: rng, + Command: protocol.Command{ Title: "Upgrade transitive dependencies", Command: source.CommandUpgradeDependency.ID(), Arguments: upgradeTransitiveArgs, @@ -69,7 +82,6 @@ }, }, }, nil - } func tidyLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]protocol.CodeLens, error) {
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index f57a743..b5da560 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go
@@ -8,6 +8,7 @@ import ( "context" + "fmt" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/lsp/debug/tag" @@ -36,9 +37,12 @@ Range: e.Range, Source: e.Category, } - if e.Category == "syntax" || e.Kind == source.ListError { + switch { + case e.Category == "syntax", e.Kind == source.ListError: d.Severity = protocol.SeverityError - } else { + case e.Kind == source.UpgradeNotification: + d.Severity = protocol.SeverityInformation + default: d.Severity = protocol.SeverityWarning } fh, err := snapshot.GetVersionedFile(ctx, e.URI) @@ -59,13 +63,49 @@ } return pm.ParseErrors, nil } + + var errors []*source.Error + + // Add upgrade quick fixes for individual modules if we know about them. + upgrades := snapshot.View().ModuleUpgrades() + for _, req := range pm.File.Require { + ver, ok := upgrades[req.Mod.Path] + if !ok || req.Mod.Version == ver { + continue + } + rng, err := lineToRange(pm.Mapper, fh.URI(), req.Syntax.Start, req.Syntax.End) + if err != nil { + return nil, err + } + // Upgrade to the exact version we offer the user, not the most recent. + args, err := source.MarshalArgs(fh.URI(), false, []string{req.Mod.Path + "@" + ver}) + if err != nil { + return nil, err + } + errors = append(errors, &source.Error{ + URI: fh.URI(), + Range: rng, + Kind: source.UpgradeNotification, + Message: fmt.Sprintf("%v can be upgraded", req.Mod.Path), + SuggestedFixes: []source.SuggestedFix{{ + Title: fmt.Sprintf("Upgrade to %v", ver), + Command: &protocol.Command{ + Title: fmt.Sprintf("Upgrade to %v", ver), + Command: source.CommandUpgradeDependency.ID(), + Arguments: args, + }, + }}, + }) + } + tidied, err := snapshot.ModTidy(ctx, pm) if source.IsNonFatalGoModError(err) { - return nil, nil + return errors, nil } if err != nil { return nil, err } - return tidied.Errors, nil + errors = append(errors, tidied.Errors...) + return errors, nil }
diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index da164ed..79c0ab9 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go
@@ -713,6 +713,11 @@ Doc: "go_get_package runs `go get` to fetch a package.\n", }, { + Command: "gopls.check_upgrades", + Title: "Check for upgrades", + Doc: "check_upgrades checks for module upgrades.\n", + }, + { Command: "gopls.add_dependency", Title: "Add dependency", Doc: "add_dependency adds a dependency.\n",
diff --git a/internal/lsp/source/command.go b/internal/lsp/source/command.go index 16d57ff..f014e13 100644 --- a/internal/lsp/source/command.go +++ b/internal/lsp/source/command.go
@@ -65,6 +65,7 @@ CommandUpdateGoSum, CommandUndeclaredName, CommandGoGetPackage, + CommandCheckUpgrades, CommandAddDependency, CommandUpgradeDependency, CommandRemoveDependency, @@ -113,6 +114,12 @@ Title: "Update go.sum", } + // CommandCheckUpgrades checks for module upgrades. + CommandCheckUpgrades = &Command{ + Name: "check_upgrades", + Title: "Check for upgrades", + } + // CommandAddDependency adds a dependency. CommandAddDependency = &Command{ Name: "add_dependency",
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 62ca995..c07e31b 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go
@@ -227,6 +227,12 @@ // IsGoPrivatePath reports whether target is a private import path, as identified // by the GOPRIVATE environment variable. IsGoPrivatePath(path string) bool + + // ModuleUpgrades returns known module upgrades. + ModuleUpgrades() map[string]string + + // RegisterModuleUpgrades registers that upgrades exist for the given modules. + RegisterModuleUpgrades(upgrades map[string]string) } // A FileSource maps uris to FileHandles. This abstraction exists both for @@ -592,6 +598,7 @@ TypeError ModTidyError Analysis + UpgradeNotification ) func (e *Error) Error() string {