diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 4757a1ee1d..f645ebfd73 100644 --- a/internal/lsp/code_action.go +++ b/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{ diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index 1fff155085..b6a4493c98 100644 --- a/internal/lsp/fake/editor.go +++ b/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 { diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index ba7730d4e5..df407ee3c7 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/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) { diff --git a/internal/lsp/regtest/diagnostics_114_test.go b/internal/lsp/regtest/diagnostics_114_test.go new file mode 100644 index 0000000000..1e297e0167 --- /dev/null +++ b/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)) +} diff --git a/internal/lsp/regtest/env.go b/internal/lsp/regtest/env.go index 7cee9acd56..3d805ecd38 100644 --- a/internal/lsp/regtest/env.go +++ b/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 } diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go index aee14822b0..cbc018d754 100644 --- a/internal/lsp/regtest/wrappers.go +++ b/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()