diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 70153dfe89..8c5a06d26f 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -194,7 +194,7 @@ Args: ``` { // The file URI. - "URI": string, + "URIs": []string, } ``` @@ -222,7 +222,7 @@ Args: ``` { // The file URI. - "URI": string, + "URIs": []string, } ``` diff --git a/gopls/doc/generate.go b/gopls/doc/generate.go index 90014a8c89..d6bd1ab6a8 100644 --- a/gopls/doc/generate.go +++ b/gopls/doc/generate.go @@ -410,6 +410,10 @@ func argDoc(arg *commandmeta.Field, level int) string { return "{ ... }" } under := arg.Type.Underlying() + switch u := under.(type) { + case *types.Slice: + return fmt.Sprintf("[]%s", u.Elem().Underlying().String()) + } return types.TypeString(under, nil) } diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go index 98eea27743..f3f1a77a93 100644 --- a/gopls/internal/regtest/workspace/workspace_test.go +++ b/gopls/internal/regtest/workspace/workspace_test.go @@ -782,3 +782,61 @@ package exclude env.Await(env.DiagnosticAtRegexp("include/include.go", `exclude.(X)`)) }) } + +// Confirm that a fix for a tidy module will correct all modules in the +// workspace. +func TestMultiModule_OneBrokenModule(t *testing.T) { + testenv.NeedsGo1Point(t, 15) + + const mod = ` +-- a/go.mod -- +module a.com + +go 1.12 +-- a/main.go -- +package main +-- b/go.mod -- +module b.com + +go 1.12 + +require ( + example.com v1.2.3 +) +-- b/go.sum -- +-- b/main.go -- +package b + +import "example.com/blah" + +func main() { + blah.Hello() +} +` + WithOptions( + ProxyFiles(workspaceProxy), + Modes(Experimental), + ).Run(t, mod, func(t *testing.T, env *Env) { + params := &protocol.PublishDiagnosticsParams{} + env.OpenFile("a/go.mod") + env.Await( + ReadDiagnostics("a/go.mod", params), + ) + for _, d := range params.Diagnostics { + if d.Message != `go.sum is out of sync with go.mod. Please update it by applying the quick fix.` { + continue + } + actions, err := env.GetQuickFixes("a/go.mod", []protocol.Diagnostic{d}) + if err != nil { + t.Fatal(err) + } + if len(actions) != 2 { + t.Fatalf("expected 2 code actions, got %v", len(actions)) + } + env.ApplyQuickFixes("a/go.mod", []protocol.Diagnostic{d}) + } + env.Await( + EmptyDiagnostics("a/go.mod"), + ) + }) +} diff --git a/gopls/internal/regtest/wrappers.go b/gopls/internal/regtest/wrappers.go index fa9367e129..12ac7f9746 100644 --- a/gopls/internal/regtest/wrappers.go +++ b/gopls/internal/regtest/wrappers.go @@ -200,6 +200,12 @@ func (e *Env) ApplyQuickFixes(path string, diagnostics []protocol.Diagnostic) { } } +// GetQuickFixes returns the available quick fix code actions. +func (e *Env) GetQuickFixes(path string, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { + e.T.Helper() + return e.Editor.GetQuickFixes(e.Ctx, path, nil, diagnostics) +} + // Hover in the editor, calling t.Fatal on any error. func (e *Env) Hover(name string, pos fake.Pos) (*protocol.MarkupContent, fake.Pos) { e.T.Helper() diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index 99f9a22969..9112bfdeb8 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -214,17 +214,23 @@ func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string // extractGoCommandError tries to parse errors that come from the go command // and shape them into go.mod diagnostics. -func (s *snapshot) extractGoCommandErrors(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, goCmdError string) []*source.Diagnostic { +func (s *snapshot) extractGoCommandErrors(ctx context.Context, snapshot source.Snapshot, goCmdError string) []*source.Diagnostic { var srcErrs []*source.Diagnostic - if srcErr := s.parseModError(ctx, fh, goCmdError); srcErr != nil { - srcErrs = append(srcErrs, srcErr) + if srcErr := s.parseModError(ctx, goCmdError); srcErr != nil { + srcErrs = append(srcErrs, srcErr...) } - // If the error message contains a position, use that. Don't pass a file - // handle in, as it might not be the file associated with the error. if srcErr := extractErrorWithPosition(ctx, goCmdError, s); srcErr != nil { srcErrs = append(srcErrs, srcErr) - } else if srcErr := s.matchErrorToModule(ctx, fh, goCmdError); srcErr != nil { - srcErrs = append(srcErrs, srcErr) + } else { + for _, uri := range s.ModFiles() { + fh, err := s.GetFile(ctx, uri) + if err != nil { + continue + } + if srcErr := s.matchErrorToModule(ctx, fh, goCmdError); srcErr != nil { + srcErrs = append(srcErrs, srcErr) + } + } } return srcErrs } diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index e8a85478c7..1b7ef5262f 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/internal/lsp/cache/mod_tidy.go @@ -151,8 +151,7 @@ func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*sourc return mth.tidy(ctx, s) } - -func (s *snapshot) parseModError(ctx context.Context, fh source.FileHandle, errText string) *source.Diagnostic { +func (s *snapshot) parseModError(ctx context.Context, errText string) []*source.Diagnostic { // 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. isInconsistentVendor := strings.Contains(errText, "inconsistent vendoring") @@ -162,58 +161,78 @@ func (s *snapshot) parseModError(ctx context.Context, fh source.FileHandle, errT return nil } - pmf, err := s.ParseMod(ctx, fh) - if err != nil { - return nil - } - if pmf.File.Module == nil || pmf.File.Module.Syntax == nil { - return nil - } - rng, err := rangeFromPositions(pmf.Mapper, pmf.File.Module.Syntax.Start, pmf.File.Module.Syntax.End) - if err != nil { - return nil - } - // TODO(rFindley): we shouldn't really need to call three constructors here. - // Reconsider this. - args := command.URIArg{protocol.URIFromSpanURI(fh.URI())} switch { case isInconsistentVendor: + uri := s.ModFiles()[0] + args := command.URIArg{URI: protocol.URIFromSpanURI(uri)} + rng, err := s.uriToModDecl(ctx, uri) + if err != nil { + return nil + } cmd, err := command.NewVendorCommand("Run go mod vendor", args) if err != nil { return nil } - return &source.Diagnostic{ - URI: fh.URI(), + return []*source.Diagnostic{{ + URI: uri, Range: rng, Severity: protocol.SeverityError, Source: source.ListError, Message: `Inconsistent vendoring detected. Please re-run "go mod vendor". See https://github.com/golang/go/issues/39164 for more detail on this issue.`, SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, - } + }} case isGoSumUpdates: - tidyCmd, err := command.NewTidyCommand("Run go mod tidy", args) - if err != nil { - return nil + var args []protocol.DocumentURI + for _, uri := range s.ModFiles() { + args = append(args, protocol.URIFromSpanURI(uri)) } - updateCmd, err := command.NewUpdateGoSumCommand("Update go.sum", args) - if err != nil { - return nil - } - return &source.Diagnostic{ - URI: fh.URI(), - Range: rng, - Severity: protocol.SeverityError, - Source: source.ListError, - Message: `go.sum is out of sync with go.mod. Please update it or run "go mod tidy".`, - SuggestedFixes: []source.SuggestedFix{ - source.SuggestedFixFromCommand(tidyCmd), - source.SuggestedFixFromCommand(updateCmd), - }, + var diagnostics []*source.Diagnostic + for _, uri := range s.ModFiles() { + rng, err := s.uriToModDecl(ctx, uri) + if err != nil { + return nil + } + tidyCmd, err := command.NewTidyCommand("Run go mod tidy", command.URIArgs{URIs: args}) + if err != nil { + return nil + } + updateCmd, err := command.NewUpdateGoSumCommand("Update go.sum", command.URIArgs{URIs: args}) + if err != nil { + return nil + } + diagnostics = append(diagnostics, &source.Diagnostic{ + URI: uri, + Range: rng, + Severity: protocol.SeverityError, + Source: source.ListError, + Message: `go.sum is out of sync with go.mod. Please update it by applying the quick fix.`, + SuggestedFixes: []source.SuggestedFix{ + source.SuggestedFixFromCommand(tidyCmd), + source.SuggestedFixFromCommand(updateCmd), + }, + }) } + return diagnostics + default: + return nil } - return nil +} + +func (s *snapshot) uriToModDecl(ctx context.Context, uri span.URI) (protocol.Range, error) { + fh, err := s.GetFile(ctx, uri) + if err != nil { + return protocol.Range{}, nil + } + pmf, err := s.ParseMod(ctx, fh) + if err != nil { + return protocol.Range{}, nil + } + if pmf.File.Module == nil || pmf.File.Module.Syntax == nil { + return protocol.Range{}, nil + } + return rangeFromPositions(pmf.Mapper, pmf.File.Module.Syntax.Start, pmf.File.Module.Syntax.End) } func hashImports(ctx context.Context, wsPackages []source.Package) (string, error) { diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 019a7e63b0..2c90492c0a 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -1032,14 +1032,7 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError { } criticalErr := &source.CriticalError{ MainError: loadErr, - } - // Attempt to place diagnostics in the relevant go.mod files, if any. - for _, uri := range s.ModFiles() { - fh, err := s.GetFile(ctx, uri) - if err != nil { - continue - } - criticalErr.DiagList = append(criticalErr.DiagList, s.extractGoCommandErrors(ctx, s, fh, loadErr.Error())...) + DiagList: s.extractGoCommandErrors(ctx, s, loadErr.Error()), } return criticalErr } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 1ab91fb5c9..6c41dc3539 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -553,6 +553,7 @@ func quickFixesForDiagnostics(ctx context.Context, snapshot source.Snapshot, pdi } return quickFixes, nil } + func sameDiagnostic(pd protocol.Diagnostic, sd *source.Diagnostic) bool { return pd.Message == sd.Message && protocol.CompareRange(pd.Range, sd.Range) == 0 && pd.Source == string(sd.Source) } diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 487e18212a..a959d1cf8d 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -184,23 +184,41 @@ func (c *commandHandler) GoGetModule(ctx context.Context, args command.Dependenc // TODO(rFindley): UpdateGoSum, Tidy, and Vendor could probably all be one command. -func (c *commandHandler) UpdateGoSum(ctx context.Context, args command.URIArg) error { +func (c *commandHandler) UpdateGoSum(ctx context.Context, args command.URIArgs) error { return c.run(ctx, commandConfig{ requireSave: true, progress: "Updating go.sum", - forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { - return runSimpleGoCommand(ctx, deps.snapshot, source.UpdateUserModFile|source.AllowNetwork, args.URI.SpanURI(), "list", []string{"all"}) + for _, uri := range args.URIs { + snapshot, fh, ok, release, err := c.s.beginFileRequest(ctx, uri, source.UnknownKind) + defer release() + if !ok { + return err + } + if err := runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, fh.URI(), "list", []string{"all"}); err != nil { + return err + } + } + return nil }) } -func (c *commandHandler) Tidy(ctx context.Context, args command.URIArg) error { +func (c *commandHandler) Tidy(ctx context.Context, args command.URIArgs) error { return c.run(ctx, commandConfig{ requireSave: true, progress: "Running go mod tidy", - forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { - return runSimpleGoCommand(ctx, deps.snapshot, source.UpdateUserModFile|source.AllowNetwork, args.URI.SpanURI(), "mod", []string{"tidy"}) + for _, uri := range args.URIs { + snapshot, fh, ok, release, err := c.s.beginFileRequest(ctx, uri, source.UnknownKind) + defer release() + if !ok { + return err + } + if err := runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, fh.URI(), "mod", []string{"tidy"}); err != nil { + return err + } + } + return nil }) } diff --git a/internal/lsp/command/command_gen.go b/internal/lsp/command/command_gen.go index 7fda594597..3e7280c632 100644 --- a/internal/lsp/command/command_gen.go +++ b/internal/lsp/command/command_gen.go @@ -137,7 +137,7 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte err := s.Test(ctx, a0, a1, a2) return nil, err case "gopls.tidy": - var a0 URIArg + var a0 URIArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } @@ -151,7 +151,7 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte err := s.ToggleGCDetails(ctx, a0) return nil, err case "gopls.update_go_sum": - var a0 URIArg + var a0 URIArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } @@ -307,7 +307,7 @@ func NewTestCommand(title string, a0 protocol.DocumentURI, a1 []string, a2 []str }, nil } -func NewTidyCommand(title string, a0 URIArg) (protocol.Command, error) { +func NewTidyCommand(title string, a0 URIArgs) (protocol.Command, error) { args, err := MarshalArgs(a0) if err != nil { return protocol.Command{}, err @@ -331,7 +331,7 @@ func NewToggleGCDetailsCommand(title string, a0 URIArg) (protocol.Command, error }, nil } -func NewUpdateGoSumCommand(title string, a0 URIArg) (protocol.Command, error) { +func NewUpdateGoSumCommand(title string, a0 URIArgs) (protocol.Command, error) { args, err := MarshalArgs(a0) if err != nil { return protocol.Command{}, err diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index 9de4b32b2d..3d258a8638 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -61,7 +61,7 @@ type Interface interface { // Tidy: Run go mod tidy // // Runs `go mod tidy` for a module. - Tidy(context.Context, URIArg) error + Tidy(context.Context, URIArgs) error // Vendor: Run go mod vendor // @@ -71,7 +71,7 @@ type Interface interface { // UpdateGoSum: Update go.sum // // Updates the go.sum file for a module. - UpdateGoSum(context.Context, URIArg) error + UpdateGoSum(context.Context, URIArgs) error // CheckUpgrades: Check for upgrades // @@ -151,6 +151,11 @@ type URIArg struct { URI protocol.DocumentURI } +type URIArgs struct { + // The file URI. + URIs []protocol.DocumentURI +} + type CheckUpgradesArgs struct { // The go.mod file URI. URI protocol.DocumentURI diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index ea77ffa2f5..0764e6e626 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -754,22 +754,15 @@ func (e *Editor) ApplyQuickFixes(ctx context.Context, path string, rng *protocol return e.codeAction(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll) } +// GetQuickFixes returns the available quick fix code actions. +func (e *Editor) GetQuickFixes(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { + return e.getCodeActions(ctx, path, rng, diagnostics, protocol.QuickFix, protocol.SourceFixAll) +} + func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) error { - if e.Server == nil { - return nil - } - params := &protocol.CodeActionParams{} - params.TextDocument.URI = e.sandbox.Workdir.URI(path) - params.Context.Only = only - if diagnostics != nil { - params.Context.Diagnostics = diagnostics - } - if rng != nil { - params.Range = *rng - } - actions, err := e.Server.CodeAction(ctx, params) + actions, err := e.getCodeActions(ctx, path, rng, diagnostics, only...) if err != nil { - return errors.Errorf("textDocument/codeAction: %w", err) + return err } for _, action := range actions { if action.Title == "" { @@ -814,6 +807,22 @@ func (e *Editor) codeAction(ctx context.Context, path string, rng *protocol.Rang return nil } +func (e *Editor) getCodeActions(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic, only ...protocol.CodeActionKind) ([]protocol.CodeAction, error) { + if e.Server == nil { + return nil, nil + } + params := &protocol.CodeActionParams{} + params.TextDocument.URI = e.sandbox.Workdir.URI(path) + params.Context.Only = only + if diagnostics != nil { + params.Context.Diagnostics = diagnostics + } + if rng != nil { + params.Range = *rng + } + return e.Server.CodeAction(ctx, params) +} + func (e *Editor) ExecuteCommand(ctx context.Context, params *protocol.ExecuteCommandParams) (interface{}, error) { if e.Server == nil { return nil, nil diff --git a/internal/lsp/mod/code_lens.go b/internal/lsp/mod/code_lens.go index f66a3e17f9..1598ed53f2 100644 --- a/internal/lsp/mod/code_lens.go +++ b/internal/lsp/mod/code_lens.go @@ -86,7 +86,7 @@ func tidyLens(ctx context.Context, snapshot source.Snapshot, fh source.FileHandl return nil, nil } uri := protocol.URIFromSpanURI(fh.URI()) - cmd, err := command.NewTidyCommand("Run go mod tidy", command.URIArg{URI: uri}) + cmd, err := command.NewTidyCommand("Run go mod tidy", command.URIArgs{URIs: []protocol.DocumentURI{uri}}) if err != nil { return nil, err } diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index 041bf72e6d..e515adae10 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -105,7 +105,12 @@ func DiagnosticsForMod(ctx context.Context, snapshot source.Snapshot, fh source. event.Error(ctx, "diagnosing go.mod", err) } if err == nil { - diagnostics = append(diagnostics, tidied.Diagnostics...) + for _, d := range tidied.Diagnostics { + if d.URI != fh.URI() { + continue + } + diagnostics = append(diagnostics, d) + } } return diagnostics, nil } diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index dc9ce6fb39..1942cf3f29 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -742,7 +742,7 @@ var GeneratedAPIJSON = &APIJSON{ Command: "gopls.tidy", Title: "Run go mod tidy", Doc: "Runs `go mod tidy` for a module.", - ArgDoc: "{\n\t// The file URI.\n\t\"URI\": string,\n}", + ArgDoc: "{\n\t// The file URI.\n\t\"URIs\": []string,\n}", }, { Command: "gopls.toggle_gc_details", @@ -754,7 +754,7 @@ var GeneratedAPIJSON = &APIJSON{ Command: "gopls.update_go_sum", Title: "Update go.sum", Doc: "Updates the go.sum file for a module.", - ArgDoc: "{\n\t// The file URI.\n\t\"URI\": string,\n}", + ArgDoc: "{\n\t// The file URI.\n\t\"URIs\": []string,\n}", }, { Command: "gopls.upgrade_dependency",