internal/lsp: support go mod vendor as a command

In addition to adding a `go mod vendor` command option, which can be
exposed via an editor client frontend, we show a suggestion to users who
experience the "inconsistent vendoring" error message.

The main change made here is that we save the view initialization error,
and we return it if the view has absolutely no metadata. This seems
reasonable enough, but my fear is that it may lead to us showing
outdated error messages. I will spend some time improving the handling
of initialization errors in follow-up CLs.

Updates golang/go#39100

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

internal/lsp/cache/load.go

@@ -93,6 +93,11 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 		return ctx.Err()
 	}
 	if err != nil {
+		// Match on common error messages. This is really hacky, but I'm not sure
+		// of any better way. This can be removed when golang/go#39164 is resolved.
+		if strings.Contains(err.Error(), "inconsistent vendoring") {
+			return source.InconsistentVendoring
+		}
 		event.Error(ctx, "go/packages.Load", err, tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
 	} else {
 		event.Log(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
@@ -100,6 +105,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
 	if len(pkgs) == 0 {
 		return err
 	}
+
 	for _, pkg := range pkgs {
 		if !containsDir || s.view.Options().VerboseOutput {
 			event.Log(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.PackagePath.Of(pkg.PkgPath), tag.Files.Of(pkg.CompiledGoFiles))

internal/lsp/cache/snapshot.go

@@ -560,7 +560,18 @@ func (s *snapshot) awaitLoaded(ctx context.Context) error {
 	if err := s.reloadWorkspace(ctx); err != nil {
 		return err
 	}
-	return s.reloadOrphanedFiles(ctx)
+	if err := s.reloadOrphanedFiles(ctx); err != nil {
+		return err
+	}
+	// If we still have absolutely no metadata, check if the view failed to
+	// initialize and return any errors.
+	// TODO(rstambler): Should we clear the error after we return it?
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if len(s.metadata) == 0 {
+		return s.view.initializedErr
+	}
+	return nil
 }
 
 // reloadWorkspace reloads the metadata for all invalidated workspace packages.

internal/lsp/cache/view.go

@@ -93,6 +93,10 @@ type view struct {
 	initializeOnce sync.Once
 	initialized    chan struct{}
 
+	// initializedErr needs no mutex, since any access to it happens after it
+	// has been set.
+	initializedErr error
+
 	// builtin pins the AST and package for builtin.go in memory.
 	builtin *builtinPackageHandle
 
@@ -571,6 +575,7 @@ func (v *view) initialize(ctx context.Context, s *snapshot) {
 		defer close(v.initialized)
 
 		if err := s.load(ctx, viewLoadScope("LOAD_VIEW"), packagePath("builtin")); err != nil {
+			v.initializedErr = err
 			event.Error(ctx, "initial workspace load failed", err)
 		}
 	})

internal/lsp/command.go

@@ -29,17 +29,14 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 				Message: "could not run tests, there are unsaved files in the view",
 			})
 		}
-
 		funcName, uri, err := getRunTestArguments(params.Arguments)
 		if err != nil {
 			return nil, err
 		}
-
 		snapshot, fh, ok, err := s.beginFileRequest(protocol.DocumentURI(uri), source.Go)
 		if !ok {
 			return nil, err
 		}
-
 		dir := filepath.Dir(fh.Identity().URI.Filename())
 		go s.runTest(ctx, funcName, dir, snapshot)
 	case source.CommandGenerate:
@@ -55,53 +52,50 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
 		}
 		_, err := s.didModifyFiles(ctx, []source.FileModification{mod}, FromRegenerateCgo)
 		return nil, err
-	case source.CommandTidy:
+	case source.CommandTidy, source.CommandVendor:
 		if len(params.Arguments) == 0 || len(params.Arguments) > 1 {
-			return nil, errors.Errorf("expected one file URI for call to `go mod tidy`, got %v", params.Arguments)
+			return nil, errors.Errorf("expected 1 argument, got %v", params.Arguments)
 		}
 		uri := protocol.DocumentURI(params.Arguments[0].(string))
-		snapshot, _, ok, err := s.beginFileRequest(uri, source.Mod)
+
-		if !ok {
+		// The flow for `go mod tidy` and `go mod vendor` is almost identical,
-			return nil, err
+		// so we combine them into one case for convenience.
-		}
+		verb := "tidy"
-		cfg := snapshot.Config(ctx)
+		if params.Command == source.CommandVendor {
-		// Run go.mod tidy on the view.
+			verb = "vendor"
-		inv := gocommand.Invocation{
-			Verb:       "mod",
-			Args:       []string{"tidy"},
-			Env:        cfg.Env,
-			WorkingDir: snapshot.View().Folder().Filename(),
-		}
-		gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
-		if _, err := gocmdRunner.Run(ctx, inv); err != nil {
-			return nil, err
 		}
+		err := s.goModCommand(ctx, uri, verb)
+		return nil, err
 	case source.CommandUpgradeDependency:
 		if len(params.Arguments) < 2 {
-			return nil, errors.Errorf("expected one file URI and one dependency for call to `go get`, got %v", params.Arguments)
+			return nil, errors.Errorf("expected 2 arguments, got %v", params.Arguments)
 		}
 		uri := protocol.DocumentURI(params.Arguments[0].(string))
 		deps := params.Arguments[1].(string)
-		snapshot, _, ok, err := s.beginFileRequest(uri, source.UnknownKind)
+		err := s.goModCommand(ctx, uri, "get", strings.Split(deps, " ")...)
-		if !ok {
+		return nil, err
-			return nil, err
-		}
-		cfg := snapshot.Config(ctx)
-		// Run "go get" on the dependency to upgrade it to the latest version.
-		inv := gocommand.Invocation{
-			Verb:       "get",
-			Args:       strings.Split(deps, " "),
-			Env:        cfg.Env,
-			WorkingDir: snapshot.View().Folder().Filename(),
-		}
-		gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
-		if _, err := gocmdRunner.Run(ctx, inv); err != nil {
-			return nil, err
-		}
 	}
 	return nil, nil
 }
 
+func (s *Server) goModCommand(ctx context.Context, uri protocol.DocumentURI, verb string, args ...string) error {
+	view, err := s.session.ViewOf(uri.SpanURI())
+	if err != nil {
+		return err
+	}
+	snapshot := view.Snapshot()
+	cfg := snapshot.Config(ctx)
+	inv := gocommand.Invocation{
+		Verb:       "mod",
+		Args:       append([]string{verb}, args...),
+		Env:        cfg.Env,
+		WorkingDir: view.Folder().Filename(),
+	}
+	gocmdRunner := packagesinternal.GetGoCmdRunner(cfg)
+	_, err = gocmdRunner.Run(ctx, inv)
+	return err
+}
+
 func (s *Server) runTest(ctx context.Context, funcName string, dir string, snapshot source.Snapshot) {
 	args := []string{"-run", funcName, dir}
 	inv := gocommand.Invocation{

internal/lsp/diagnostics.go

@@ -6,6 +6,7 @@ package lsp
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"sync"
 
@@ -84,7 +85,26 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 
 	// Diagnose all of the packages in the workspace.
 	wsPackages, err := snapshot.WorkspacePackages(ctx)
-	if err != nil {
+	if err == source.InconsistentVendoring {
+		item, err := s.client.ShowMessageRequest(ctx, &protocol.ShowMessageRequestParams{
+			Type: protocol.Error,
+			Message: `Inconsistent vendoring detected. Please re-run "go mod vendor".
+See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
+			Actions: []protocol.MessageActionItem{
+				{Title: "go mod vendor"},
+			},
+		})
+		if item == nil || err != nil {
+			return nil, nil
+		}
+		if err := s.goModCommand(ctx, protocol.URIFromSpanURI(modURI), "vendor"); err != nil {
+			return nil, &protocol.ShowMessageParams{
+				Type:    protocol.Error,
+				Message: fmt.Sprintf(`"go mod vendor" failed with %v`, err),
+			}
+		}
+		return nil, nil
+	} else if err != nil {
 		event.Error(ctx, "failed to load workspace packages, skipping diagnostics", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
 		return nil, nil
 	}

internal/lsp/fake/client.go

@@ -6,6 +6,7 @@ package fake
 
 import (
 	"context"
+	"fmt"
 
 	"golang.org/x/tools/internal/lsp/protocol"
 )
@@ -17,6 +18,7 @@ type ClientHooks struct {
 	OnWorkDoneProgressCreate func(context.Context, *protocol.WorkDoneProgressCreateParams) error
 	OnProgress               func(context.Context, *protocol.ProgressParams) error
 	OnShowMessage            func(context.Context, *protocol.ShowMessageParams) error
+	OnShowMessageRequest     func(context.Context, *protocol.ShowMessageRequestParams) error
 }
 
 // Client is an adapter that converts an *Editor into an LSP Client. It mosly
@@ -34,7 +36,15 @@ func (c *Client) ShowMessage(ctx context.Context, params *protocol.ShowMessagePa
 }
 
 func (c *Client) ShowMessageRequest(ctx context.Context, params *protocol.ShowMessageRequestParams) (*protocol.MessageActionItem, error) {
-	return nil, nil
+	if c.hooks.OnShowMessageRequest != nil {
+		if err := c.hooks.OnShowMessageRequest(ctx, params); err != nil {
+			return nil, err
+		}
+	}
+	if len(params.Actions) == 0 || len(params.Actions) > 1 {
+		return nil, fmt.Errorf("fake editor cannot handle multiple action items")
+	}
+	return &params.Actions[0], nil
 }
 
 func (c *Client) LogMessage(ctx context.Context, params *protocol.LogMessageParams) error {

internal/lsp/regtest/env.go

@@ -43,9 +43,10 @@ type Env struct {
 // State encapsulates the server state TODO: explain more
 type State struct {
 	// diagnostics are a map of relative path->diagnostics params
-	diagnostics map[string]*protocol.PublishDiagnosticsParams
+	diagnostics        map[string]*protocol.PublishDiagnosticsParams
-	logs        []*protocol.LogMessageParams
+	logs               []*protocol.LogMessageParams
-	showMessage []*protocol.ShowMessageParams
+	showMessage        []*protocol.ShowMessageParams
+	showMessageRequest []*protocol.ShowMessageRequestParams
 	// outstandingWork is a map of token->work summary. All tokens are assumed to
 	// be string, though the spec allows for numeric tokens as well.  When work
 	// completes, it is deleted from this map.
@@ -125,6 +126,7 @@ func NewEnv(ctx context.Context, t *testing.T, scratch *fake.Sandbox, ts servert
 		OnWorkDoneProgressCreate: env.onWorkDoneProgressCreate,
 		OnProgress:               env.onProgress,
 		OnShowMessage:            env.onShowMessage,
+		OnShowMessageRequest:     env.onShowMessageRequest,
 	}
 	editor, err := fake.NewEditor(scratch, editorConfig).Connect(ctx, conn, hooks)
 	if err != nil {
@@ -153,6 +155,15 @@ func (e *Env) onShowMessage(_ context.Context, m *protocol.ShowMessageParams) er
 	return nil
 }
 
+func (e *Env) onShowMessageRequest(_ context.Context, m *protocol.ShowMessageRequestParams) error {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	e.state.showMessageRequest = append(e.state.showMessageRequest, m)
+	e.checkConditionsLocked()
+	return nil
+}
+
 func (e *Env) onLogMessage(_ context.Context, m *protocol.LogMessageParams) error {
 	e.mu.Lock()
 	defer e.mu.Unlock()
@@ -367,6 +378,29 @@ func SomeShowMessage(title string) SimpleExpectation {
 	}
 }
 
+// ShowMessageRequest asserts that the editor has received a ShowMessageRequest
+// with an action item that has the given title.
+func ShowMessageRequest(title string) SimpleExpectation {
+	check := func(s State) (Verdict, interface{}) {
+		if len(s.showMessageRequest) == 0 {
+			return Unmet, nil
+		}
+		// Only check the most recent one.
+		m := s.showMessageRequest[len(s.showMessageRequest)-1]
+		if len(m.Actions) == 0 || len(m.Actions) > 1 {
+			return Unmet, nil
+		}
+		if m.Actions[0].Title == title {
+			return Met, m.Actions[0]
+		}
+		return Unmet, nil
+	}
+	return SimpleExpectation{
+		check:       check,
+		description: "received ShowMessageRequest",
+	}
+}
+
 // CompletedWork expects a work item to have been completed >= atLeast times.
 //
 // Since the Progress API doesn't include any hidden metadata, we must use the

internal/lsp/regtest/vendor_test.go

@@ -0,0 +1,62 @@
+package regtest
+
+import (
+	"testing"
+
+	"golang.org/x/tools/internal/lsp"
+	"golang.org/x/tools/internal/testenv"
+)
+
+const basicProxy = `
+-- golang.org/x/hello@v1.2.3/go.mod --
+module golang.org/x/hello
+
+go 1.14
+-- golang.org/x/hello@v1.2.3/hi/hi.go --
+package hi
+
+var Goodbye error
+`
+
+func TestInconsistentVendoring(t *testing.T) {
+	testenv.NeedsGo1Point(t, 14)
+	const pkgThatUsesVendoring = `
+-- go.mod --
+module mod.com
+
+go 1.14
+
+require golang.org/x/hello v1.2.3
+-- vendor/modules.txt --
+-- a/a1.go --
+package a
+
+import "golang.org/x/hello/hi"
+
+func _() {
+	_ = hi.Goodbye
+	var q int // hardcode a diagnostic
+}
+`
+	runner.Run(t, pkgThatUsesVendoring, func(t *testing.T, env *Env) {
+		env.OpenFile("a/a1.go")
+		env.Await(
+			// The editor should pop up a message suggesting that the user
+			// run `go mod vendor`, along with a button to do so.
+			// By default, the fake editor always accepts such suggestions,
+			// so once we see the request, we can assume that `go mod vendor`
+			// will be executed.
+			OnceMet(
+				CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1),
+				ShowMessageRequest("go mod vendor"),
+			),
+		)
+		env.CheckForFileChanges()
+		env.Await(
+			OnceMet(
+				CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1),
+				DiagnosticAt("a/a1.go", 6, 5),
+			),
+		)
+	}, WithProxy(basicProxy))
+}

internal/lsp/source/options.go

@@ -62,6 +62,9 @@ const (
 	// CommandTidy is a gopls command to run `go mod tidy` for a module.
 	CommandTidy = "tidy"
 
+	// CommandVendor is a gopls command to run `go mod vendor` for a module.
+	CommandVendor = "vendor"
+
 	// CommandUpgradeDependency is a gopls command to upgrade a dependency.
 	CommandUpgradeDependency = "upgrade_dependency"
 

internal/lsp/source/view.go

@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/internal/imports"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/span"
+	errors "golang.org/x/xerrors"
 )
 
 // Snapshot represents the current state for the given view.
@@ -439,3 +440,5 @@ const (
 func (e *Error) Error() string {
 	return fmt.Sprintf("%s:%s: %s", e.URI, e.Range, e.Message)
 }
+
+var InconsistentVendoring = errors.New("inconsistent vendoring")