internal/lsp: add regtest for golang/go#38211

A few more updates to the regtest framework - add support for all
codeActions, not just organize imports. Also, return diagnostics from
env.Await to pass into code actions. Not sure if that's the correct way
to do it, so please let me know if there's a better way.

The test is for 1.14 only, since -modfile is only supported after 1.14.

Fixes golang/go#38211

Change-Id: I5cdd35e2ec06e2b2487d1f3491197030008be9c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226958
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/lsp/code_action.go

@@ -49,7 +49,11 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 	switch fh.Identity().Kind {
 	case source.Mod:
 		if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 {
-			codeActions = append(codeActions, mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)...)
+			modFixes, err := mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)
+			if err != nil {
+				return nil, err
+			}
+			codeActions = append(codeActions, modFixes...)
 		}
 		if wanted[protocol.SourceOrganizeImports] {
 			codeActions = append(codeActions, protocol.CodeAction{

internal/lsp/fake/editor.go

@@ -494,12 +494,24 @@ func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (stri
 
 // OrganizeImports requests and performs the source.organizeImports codeAction.
 func (e *Editor) OrganizeImports(ctx context.Context, path string) error {
+	return e.codeAction(ctx, path, nil, protocol.SourceOrganizeImports)
+}
+
+// ApplyQuickFixes requests and performs the quickfix codeAction.
+func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, diagnostics []protocol.Diagnostic) error {
+	return e.codeAction(ctx, path, diagnostics, protocol.QuickFix)
+}
+
+func (e *Editor) codeAction(ctx context.Context, path string, diagnostics []protocol.Diagnostic, only protocol.CodeActionKind) error {
 	if e.server == nil {
 		return nil
 	}
 	params := &protocol.CodeActionParams{}
 	params.TextDocument.URI = e.ws.URI(path)
-
+	params.Context.Only = []protocol.CodeActionKind{only}
+	if diagnostics != nil {
+		params.Context.Diagnostics = diagnostics
+	}
 	actions, err := e.server.CodeAction(ctx, params)
 	if err != nil {
 		return fmt.Errorf("textDocument/codeAction: %v", err)
@@ -507,7 +519,7 @@ func (e *Editor) OrganizeImports(ctx context.Context, path string) error {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 	for _, action := range actions {
-		if action.Kind == protocol.SourceOrganizeImports {
+		if action.Kind == only {
 			for _, change := range action.Edit.DocumentChanges {
 				path := e.ws.URIToPath(change.TextDocument.URI)
 				if float64(e.buffers[path].version) != change.TextDocument.Version {

internal/lsp/mod/diagnostics.go

@@ -59,16 +59,15 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File
 	return reports, missingDeps, nil
 }
 
-func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source.FileHandle, diags []protocol.Diagnostic) []protocol.CodeAction {
+func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source.FileHandle, diags []protocol.Diagnostic) ([]protocol.CodeAction, error) {
 	mth, err := snapshot.ModTidyHandle(ctx, realfh)
 	if err != nil {
-		return nil
+		return nil, err
 	}
 	_, _, _, parseErrors, err := mth.Tidy(ctx)
 	if err != nil {
-		return nil
+		return nil, err
 	}
-
 	errorsMap := make(map[string][]source.Error)
 	for _, e := range parseErrors {
 		if errorsMap[e.Message] == nil {
@@ -76,7 +75,6 @@ func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source
 		}
 		errorsMap[e.Message] = append(errorsMap[e.Message], e)
 	}
-
 	var actions []protocol.CodeAction
 	for _, diag := range diags {
 		for _, e := range errorsMap[diag.Message] {
@@ -93,8 +91,7 @@ func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source
 				for uri, edits := range fix.Edits {
 					fh, err := snapshot.GetFile(uri)
 					if err != nil {
-						event.Error(ctx, "no file", err, tag.URI.Of(uri))
-						continue
+						return nil, err
 					}
 					action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, protocol.TextDocumentEdit{
 						TextDocument: protocol.VersionedTextDocumentIdentifier{
@@ -110,7 +107,7 @@ func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source
 			}
 		}
 	}
-	return actions
+	return actions, nil
 }
 
 func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot) (map[string]protocol.TextDocumentEdit, error) {

internal/lsp/regtest/diagnostics_114_test.go

@@ -0,0 +1,88 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build 1.14
+
+package regtest
+
+import (
+	"testing"
+
+	"golang.org/x/tools/internal/lsp/protocol"
+)
+
+// -modfile flag that is used to provide modfile diagnostics is only available
+// with 1.14.
+func Test_Issue38211(t *testing.T) {
+	const ardanLabs = `
+-- go.mod --
+module mod.com
+
+go 1.14
+-- main.go --
+package main
+
+import "github.com/ardanlabs/conf"
+
+func main() {
+	_ = conf.ErrHelpWanted
+}
+`
+	const proxy = `
+-- github.com/ardanlabs/conf@v1.2.3/go.mod --
+module github.com/ardanlabs/conf
+
+go 1.12
+-- github.com/ardanlabs/conf@v1.2.3/conf.go --
+package conf
+
+var ErrHelpWanted error
+`
+
+	runner.Run(t, ardanLabs, func(t *testing.T, env *Env) {
+		// Expect a diagnostic with a suggested fix to add
+		// "github.com/ardanlabs/conf" to the go.mod file.
+		env.OpenFile("go.mod")
+		env.OpenFile("main.go")
+		metBy := env.Await(
+			env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+		)
+		d, ok := metBy[0].(*protocol.PublishDiagnosticsParams)
+		if !ok {
+			t.Fatalf("unexpected type for metBy (%T)", metBy)
+		}
+		env.ApplyQuickFixes("main.go", d.Diagnostics)
+		env.SaveBuffer("go.mod")
+		env.Await(
+			EmptyDiagnostics("main.go"),
+		)
+		// Comment out the line that depends on conf and expect a
+		// diagnostic and a fix to remove the import.
+		env.RegexpReplace("main.go", "_ = conf.ErrHelpWanted", "//_ = conf.ErrHelpWanted")
+		env.Await(
+			env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+		)
+		env.SaveBuffer("main.go")
+		// Expect a diagnostic and fix to remove the dependency in the go.mod.
+		metBy = env.Await(
+			EmptyDiagnostics("main.go"),
+			env.DiagnosticAtRegexp("go.mod", "require github.com/ardanlabs/conf"),
+		)
+		d, ok = metBy[1].(*protocol.PublishDiagnosticsParams)
+		if !ok {
+			t.Fatalf("unexpected type for metBy (%T)", metBy)
+		}
+		env.ApplyQuickFixes("go.mod", d.Diagnostics)
+		env.SaveBuffer("go.mod")
+		env.Await(
+			EmptyDiagnostics("go.mod"),
+		)
+		// Uncomment the lines and expect a new diagnostic for the import.
+		env.RegexpReplace("main.go", "//_ = conf.ErrHelpWanted", "_ = conf.ErrHelpWanted")
+		env.SaveBuffer("main.go")
+		env.Await(
+			env.DiagnosticAtRegexp("main.go", `"github.com/ardanlabs/conf"`),
+		)
+	}, WithProxy(proxy))
+}

internal/lsp/regtest/env.go

@@ -418,7 +418,7 @@ func (e *Env) onLogMessage(_ context.Context, m *protocol.LogMessageParams) erro
 
 func (e *Env) checkConditionsLocked() {
 	for id, condition := range e.waiters {
-		if v, _ := checkExpectations(e.state, condition.expectations); v != Unmet {
+		if v, _, _ := checkExpectations(e.state, condition.expectations); v != Unmet {
 			delete(e.waiters, id)
 			condition.verdict <- v
 		}
@@ -442,23 +442,27 @@ func (e *Env) ExpectNow(expectations ...Expectation) {
 	e.T.Helper()
 	e.mu.Lock()
 	defer e.mu.Unlock()
-	if verdict, summary := checkExpectations(e.state, expectations); verdict != Met {
+	if verdict, summary, _ := checkExpectations(e.state, expectations); verdict != Met {
 		e.T.Fatalf("expectations unmet:\n%s\ncurrent state:\n%v", summary, e.state)
 	}
 }
 
 // checkExpectations reports whether s meets all expectations.
-func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
+func checkExpectations(s State, expectations []Expectation) (Verdict, string, []interface{}) {
 	finalVerdict := Met
+	var metBy []interface{}
 	var summary strings.Builder
 	for _, e := range expectations {
-		v := e.Check(s)
+		v, mb := e.Check(s)
+		if v == Met {
+			metBy = append(metBy, mb)
+		}
 		if v > finalVerdict {
 			finalVerdict = v
 		}
 		summary.WriteString(fmt.Sprintf("\t%v: %s\n", v, e.Description()))
 	}
-	return finalVerdict, summary.String()
+	return finalVerdict, summary.String(), metBy
 }
 
 // An Expectation asserts that the state of the editor at a point in time
@@ -466,8 +470,8 @@ func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
 // certain conditions in the editor are met.
 type Expectation interface {
 	// Check determines whether the state of the editor satisfies the
-	// expectation.
-	Check(State) Verdict
+	// expectation, returning the results that met the condition.
+	Check(State) (Verdict, interface{})
 	// Description is a human-readable description of the expectation.
 	Description() string
 }
@@ -504,12 +508,12 @@ func (v Verdict) String() string {
 // LogExpectation is an expectation on the log messages received by the editor
 // from gopls.
 type LogExpectation struct {
-	check       func([]*protocol.LogMessageParams) Verdict
+	check       func([]*protocol.LogMessageParams) (Verdict, interface{})
 	description string
 }
 
 // Check implements the Expectation interface.
-func (e LogExpectation) Check(s State) Verdict {
+func (e LogExpectation) Check(s State) (Verdict, interface{}) {
 	return e.check(s.logs)
 }
 
@@ -521,13 +525,13 @@ func (e LogExpectation) Description() string {
 // NoErrorLogs asserts that the client has not received any log messages of
 // error severity.
 func NoErrorLogs() LogExpectation {
-	check := func(msgs []*protocol.LogMessageParams) Verdict {
+	check := func(msgs []*protocol.LogMessageParams) (Verdict, interface{}) {
 		for _, msg := range msgs {
 			if msg.Type == protocol.Error {
-				return Unmeetable
+				return Unmeetable, nil
 			}
 		}
-		return Met
+		return Met, nil
 	}
 	return LogExpectation{
 		check:       check,
@@ -542,13 +546,13 @@ func LogMatching(typ protocol.MessageType, re string) LogExpectation {
 	if err != nil {
 		panic(err)
 	}
-	check := func(msgs []*protocol.LogMessageParams) Verdict {
+	check := func(msgs []*protocol.LogMessageParams) (Verdict, interface{}) {
 		for _, msg := range msgs {
 			if msg.Type == typ && rec.Match([]byte(msg.Message)) {
-				return Met
+				return Met, msg
 			}
 		}
-		return Unmet
+		return Unmet, nil
 	}
 	return LogExpectation{
 		check:       check,
@@ -569,11 +573,11 @@ type DiagnosticExpectation struct {
 }
 
 // Check implements the Expectation interface.
-func (e DiagnosticExpectation) Check(s State) Verdict {
+func (e DiagnosticExpectation) Check(s State) (Verdict, interface{}) {
 	if diags, ok := s.diagnostics[e.path]; ok && e.isMet(diags) {
-		return Met
+		return Met, diags
 	}
-	return Unmet
+	return Unmet, nil
 }
 
 // Description implements the Expectation interface.
@@ -639,15 +643,15 @@ func DiagnosticAt(name string, line, col int) DiagnosticExpectation {
 
 // Await waits for all expectations to simultaneously be met. It should only be
 // called from the main test goroutine.
-func (e *Env) Await(expectations ...Expectation) {
+func (e *Env) Await(expectations ...Expectation) []interface{} {
 	e.T.Helper()
 	e.mu.Lock()
 	// Before adding the waiter, we check if the condition is currently met or
 	// failed to avoid a race where the condition was realized before Await was
 	// called.
-	switch verdict, summary := checkExpectations(e.state, expectations); verdict {
+	switch verdict, summary, metBy := checkExpectations(e.state, expectations); verdict {
 	case Met:
-		return
+		return metBy
 	case Unmeetable:
 		e.mu.Unlock()
 		e.T.Fatalf("unmeetable expectations:\n%s\nstate:\n%v", summary, e.state)
@@ -669,13 +673,14 @@ func (e *Env) Await(expectations ...Expectation) {
 			err = fmt.Errorf("condition has final verdict %v", v)
 		}
 	}
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	_, summary, metBy := checkExpectations(e.state, expectations)
 
+	// Debugging an unmet expectation can be tricky, so we put some effort into
+	// nicely formatting the failure.
 	if err != nil {
-		// Debugging an unmet expectation can be tricky, so we put some effort into
-		// nicely formatting the failure.
-		e.mu.Lock()
-		defer e.mu.Unlock()
-		_, summary := checkExpectations(e.state, expectations)
 		e.T.Fatalf("waiting on:\n%s\nerr:%v\nstate:\n%v", err, summary, e.state)
 	}
+	return metBy
 }

internal/lsp/regtest/wrappers.go

@@ -6,6 +6,7 @@ package regtest
 
 import (
 	"golang.org/x/tools/internal/lsp/fake"
+	"golang.org/x/tools/internal/lsp/protocol"
 )
 
 // RemoveFileFromWorkspace deletes a file on disk but does nothing in the
@@ -121,6 +122,14 @@ func (e *Env) OrganizeImports(name string) {
 	}
 }
 
+// ApplyQuickFixes processes the quickfix codeAction, calling t.Fatal on any error.
+func (e *Env) ApplyQuickFixes(path string, diagnostics []protocol.Diagnostic) {
+	e.T.Helper()
+	if err := e.E.ApplyQuickFixes(e.Ctx, path, diagnostics); err != nil {
+		e.T.Fatal(err)
+	}
+}
+
 // CloseEditor shuts down the editor, calling t.Fatal on any error.
 func (e *Env) CloseEditor() {
 	e.T.Helper()