internal/lsp: correctly marshal arguments for upgrade code lens

I'm not sure how the regtest didn't catch this - is it possible that
it could unmarshal a single string a slice of string? Either way, I'd
like to get the fix in quicker - I'll try to add more regtests for this
later.

Also, validate the upgrade results more thoroughly.

Change-Id: I812a3fecd9f0642a1408c0a9c0376bb98d50b397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245065
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

Author: stamblerre

internal/lsp/cache/mod.go

@@ -350,12 +350,16 @@ func (s *snapshot) ModUpgradeHandle(ctx context.Context) (source.ModUpgradeHandl
 		for _, upgrade := range upgradesList[1:] {
 			// Example: "github.com/x/tools v1.1.0 [v1.2.0]"
 			info := strings.Split(upgrade, " ")
-			if len(info) < 3 {
+			if len(info) != 3 {
 				continue
 			}
 			dep, version := info[0], info[2]
-			latest := version[1:]                    // remove the "["
-			latest = strings.TrimSuffix(latest, "]") // remove the "]"
+
+			// Make sure that the format matches our expectation.
+			if version[0] != '[' || version[len(version)-1] != ']' {
+				continue
+			}
+			latest := version[1 : len(version)-1] // remove the "[" and "]"
 			upgrades[dep] = latest
 		}
 		return &modUpgradeData{

internal/lsp/command.go

@@ -137,11 +137,11 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 		return nil, err
 	case source.CommandUpgradeDependency:
 		var uri protocol.DocumentURI
-		var deps []string
-		if err := source.UnmarshalArgs(params.Arguments, &uri, &deps); err != nil {
+		var goCmdArgs []string
+		if err := source.UnmarshalArgs(params.Arguments, &uri, &goCmdArgs); err != nil {
 			return nil, err
 		}
-		err := s.directGoModCommand(ctx, uri, "get", deps...)
+		err := s.directGoModCommand(ctx, uri, "get", goCmdArgs...)
 		return nil, err
 	default:
 		return nil, fmt.Errorf("unknown command: %s", params.Command)

internal/lsp/mod/code_lens.go

@@ -59,7 +59,7 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr
 		if err != nil {
 			return nil, err
 		}
-		jsonArgs, err := source.MarshalArgs(uri, dep)
+		jsonArgs, err := source.MarshalArgs(uri, []string{dep})
 		if err != nil {
 			return nil, err
 		}

internal/lsp/regtest/codelens_test.go

@@ -10,6 +10,7 @@ import (
 	"golang.org/x/tools/internal/lsp/fake"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/testenv"
 )
 
@@ -100,16 +101,16 @@ func main() {
 `
 	runner.Run(t, shouldUpdateDep, func(t *testing.T, env *Env) {
 		env.OpenFile("go.mod")
-		before := env.ReadWorkspaceFile("go.mod")
 		lenses := env.CodeLens("go.mod")
 		want := "Upgrade dependency to v1.3.3"
-		var found *protocol.CodeLens
+		var found protocol.CodeLens
 		for _, lens := range lenses {
 			if lens.Command.Title == want {
-				found = &lens
+				found = lens
+				break
 			}
 		}
-		if found == nil {
+		if found.Command.Command == "" {
 			t.Fatalf("did not find lens %q, got %v", want, lenses)
 		}
 		if _, err := env.Editor.Server.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{
@@ -118,9 +119,15 @@ func main() {
 		}); err != nil {
 			t.Fatal(err)
 		}
-		after := env.ReadWorkspaceFile("go.mod")
-		if before == after {
-			t.Fatalf("go.mod file was unchanged by upgrade command")
+		got := env.ReadWorkspaceFile("go.mod")
+		const wantGoMod = `module mod.com
+
+go 1.14
+
+require golang.org/x/hello v1.3.3
+`
+		if got != wantGoMod {
+			t.Fatalf("go.mod upgrade failed:\n%s", tests.Diff(wantGoMod, got))
 		}
 	}, WithProxyFiles(proxyWithLatest))
 }