internal/lsp: apply go.sum fixes to all modules in multi-module module
When go.sum updates are needed in experimental workspace module mode, we
don't necessarily know which module needs the correction. As a fix,
apply all of these fixes to each module in the multi-module workspace.
The "add dependency" quick fix also seems to be broken, but I'll fix
that in a separate CL.
Fixes golang/go#44097
Change-Id: Id4a6013f2aceb6b902dff3118b027f6cb99eb3c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289772
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md
index 70153df..8c5a06d 100644
--- a/gopls/doc/commands.md
+++ b/gopls/doc/commands.md
@@ -194,7 +194,7 @@
```
{
// The file URI.
- "URI": string,
+ "URIs": []string,
}
```
@@ -222,7 +222,7 @@
```
{
// The file URI.
- "URI": string,
+ "URIs": []string,
}
```
diff --git a/gopls/doc/generate.go b/gopls/doc/generate.go
index 90014a8..d6bd1ab 100644
--- a/gopls/doc/generate.go
+++ b/gopls/doc/generate.go
@@ -410,6 +410,10 @@
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 98eea27..f3f1a77 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -782,3 +782,61 @@
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 fa9367e..12ac7f9 100644
--- a/gopls/internal/regtest/wrappers.go
+++ b/gopls/internal/regtest/wrappers.go
@@ -200,6 +200,12 @@
}
}
+// 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 99f9a22..9112bfd 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -214,17 +214,23 @@
// 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 e8a8547..1b7ef52 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -151,8 +151,7 @@
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 @@
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
+ 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 &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),
- },
- }
+ 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 019a7e6..2c90492 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -1032,14 +1032,7 @@
}
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 1ab91fb..6c41dc3 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -553,6 +553,7 @@
}
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 487e182..a959d1c 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -184,23 +184,41 @@
// 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 7fda594..3e7280c 100644
--- a/internal/lsp/command/command_gen.go
+++ b/internal/lsp/command/command_gen.go
@@ -137,7 +137,7 @@
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 @@
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 @@
}, 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 @@
}, 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 9de4b32..3d258a8 100644
--- a/internal/lsp/command/interface.go
+++ b/internal/lsp/command/interface.go
@@ -61,7 +61,7 @@
// 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 @@
// 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 @@
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 ea77ffa..0764e6e 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -754,22 +754,15 @@
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 @@
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 f66a3e1..1598ed5 100644
--- a/internal/lsp/mod/code_lens.go
+++ b/internal/lsp/mod/code_lens.go
@@ -86,7 +86,7 @@
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 041bf72..e515ada 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -105,7 +105,12 @@
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 dc9ce6f..1942cf3 100755
--- a/internal/lsp/source/api_json.go
+++ b/internal/lsp/source/api_json.go
@@ -742,7 +742,7 @@
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 @@
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",
