internal/lsp: apply go.mod/sum changes via workspace edits
We currently write directly to go.mod/sum via the go command, expecting
that editors will pick up the changes. While that's true for VS Code,
vim doesn't necessarily reload unchanged buffers. Change to send
explicit edits instead, but only if the file is open. Behavior when
using Go versions that don't support -modfile is unchanged.
Fixes golang/go#44035.
Change-Id: Ie4e5cf60673b860f5dfcbdb726bee0efe6aae405
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290189
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index 542991a..3a73152 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -692,7 +692,7 @@
env.Await(EmptyDiagnostics("main.go"))
env.Await(
OnceMet(
- env.DiagnosticAtRegexp("go.mod", "require github.com/ardanlabs/conf"),
+ env.DiagnosticAtRegexpWithMessage("go.mod", "require github.com/ardanlabs/conf", "not used in this module"),
ReadDiagnostics("go.mod", &d),
),
)
@@ -741,7 +741,6 @@
),
)
env.ApplyQuickFixes("main.go", d.Diagnostics)
- env.CheckForFileChanges()
env.Await(
EmptyDiagnostics("main.go"),
)
diff --git a/gopls/internal/regtest/misc/vendor_test.go b/gopls/internal/regtest/misc/vendor_test.go
index 8e76dd8..1c9a4ec 100644
--- a/gopls/internal/regtest/misc/vendor_test.go
+++ b/gopls/internal/regtest/misc/vendor_test.go
@@ -48,31 +48,22 @@
var q int // hardcode a diagnostic
}
`
- // TODO(rstambler): Remove this when golang/go#41819 is resolved.
WithOptions(
Modes(Singleton),
ProxyFiles(basicProxy),
).Run(t, pkgThatUsesVendoring, func(t *testing.T, env *Env) {
env.OpenFile("a/a1.go")
+ d := &protocol.PublishDiagnosticsParams{}
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(
- env.DoneWithOpen(),
- env.DiagnosticAtRegexp("go.mod", "module mod.com"),
+ env.DiagnosticAtRegexpWithMessage("go.mod", "module mod.com", "Inconsistent vendoring"),
+ ReadDiagnostics("go.mod", d),
),
)
- // Apply the quickfix associated with the diagnostic.
- d := &protocol.PublishDiagnosticsParams{}
- env.Await(ReadDiagnostics("go.mod", d))
env.ApplyQuickFixes("go.mod", d.Diagnostics)
- // Confirm that there is no longer any inconsistent vendoring.
env.Await(
- DiagnosticAt("a/a1.go", 6, 5),
+ env.DiagnosticAtRegexpWithMessage("a/a1.go", `q int`, "not used"),
)
})
}
diff --git a/gopls/internal/regtest/modfile/modfile_test.go b/gopls/internal/regtest/modfile/modfile_test.go
index 8c167cc..0f897e7 100644
--- a/gopls/internal/regtest/modfile/modfile_test.go
+++ b/gopls/internal/regtest/modfile/modfile_test.go
@@ -323,7 +323,7 @@
),
)
env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
- if got := env.ReadWorkspaceFile("a/go.mod"); got != want {
+ if got := env.Editor.BufferText("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
}
})
@@ -502,6 +502,7 @@
example.com/blah/v2 v2.0.0
)
`
+ env.SaveBuffer("a/go.mod")
env.Await(EmptyDiagnostics("a/main.go"))
if got := env.Editor.BufferText("a/go.mod"); got != want {
t.Fatalf("suggested fixes failed:\n%s", tests.Diff(t, want, got))
@@ -542,7 +543,7 @@
env.DiagnosticAtRegexp("a/go.mod", "example.com v1.2.2"),
)
env.RegexpReplace("a/go.mod", "v1.2.2", "v1.2.3")
- env.Editor.SaveBuffer(env.Ctx, "a/go.mod") // go.mod changes must be on disk
+ env.SaveBuffer("a/go.mod") // Save to trigger diagnostics.
d := protocol.PublishDiagnosticsParams{}
env.Await(
@@ -552,7 +553,7 @@
),
)
env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
-
+ env.SaveBuffer("a/go.mod") // Save to trigger diagnostics.
env.Await(
EmptyDiagnostics("a/go.mod"),
env.DiagnosticAtRegexp("a/main.go", "x = "),
@@ -826,6 +827,7 @@
),
)
env.ApplyQuickFixes("go.mod", d.Diagnostics)
+ env.SaveBuffer("go.mod") // Save to trigger diagnostics.
env.Await(
EmptyDiagnostics("go.mod"),
)
@@ -925,6 +927,7 @@
WithOptions(
ProxyFiles(proxy),
).Run(t, mod, func(t *testing.T, env *Env) {
+ env.OpenFile("go.mod")
d := &protocol.PublishDiagnosticsParams{}
env.Await(
OnceMet(
@@ -937,7 +940,7 @@
go 1.12
`
env.ApplyQuickFixes("go.mod", d.Diagnostics)
- if got := env.ReadWorkspaceFile("go.mod"); got != want {
+ if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("unexpected content in go.mod:\n%s", tests.Diff(t, want, got))
}
})
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 2c90492..3bc9be3 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -12,6 +12,7 @@
"go/token"
"go/types"
"io"
+ "io/ioutil"
"os"
"path/filepath"
"sort"
@@ -255,6 +256,44 @@
return s.view.session.gocmdRunner.RunPiped(ctx, *inv, stdout, stderr)
}
+func (s *snapshot) RunGoCommands(ctx context.Context, allowNetwork bool, wd string, run func(invoke func(...string) (*bytes.Buffer, error)) error) (bool, []byte, []byte, error) {
+ var flags source.InvocationFlags
+ if s.workspaceMode()&tempModfile != 0 {
+ flags = source.WriteTemporaryModFile
+ } else {
+ flags = source.Normal
+ }
+ if allowNetwork {
+ flags |= source.AllowNetwork
+ }
+ tmpURI, inv, cleanup, err := s.goCommandInvocation(ctx, flags, &gocommand.Invocation{WorkingDir: wd})
+ if err != nil {
+ return false, nil, nil, err
+ }
+ defer cleanup()
+ invoke := func(args ...string) (*bytes.Buffer, error) {
+ inv.Verb = args[0]
+ inv.Args = args[1:]
+ return s.view.session.gocmdRunner.Run(ctx, *inv)
+ }
+ if err := run(invoke); err != nil {
+ return false, nil, nil, err
+ }
+ if flags.Mode() != source.WriteTemporaryModFile {
+ return false, nil, nil, nil
+ }
+ var modBytes, sumBytes []byte
+ modBytes, err = ioutil.ReadFile(tmpURI.Filename())
+ if err != nil && !os.IsNotExist(err) {
+ return false, nil, nil, err
+ }
+ sumBytes, err = ioutil.ReadFile(strings.TrimSuffix(tmpURI.Filename(), ".mod") + ".sum")
+ if err != nil && !os.IsNotExist(err) {
+ return false, nil, nil, err
+ }
+ return true, modBytes, sumBytes, nil
+}
+
func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.InvocationFlags, inv *gocommand.Invocation) (tmpURI span.URI, updatedInv *gocommand.Invocation, cleanup func(), err error) {
s.view.optionsMu.Lock()
allowModfileModificationOption := s.view.options.AllowModfileModifications
@@ -1386,8 +1425,6 @@
}
}
if isGoMod(uri) {
- // If the view's go.mod file's contents have changed, invalidate
- // the metadata for every known package in the snapshot.
delete(result.parseModHandles, uri)
}
// Handle the invalidated file; it may have new contents or not exist.
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 469ef92..8bbb684 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -11,6 +11,7 @@
"fmt"
"io"
"io/ioutil"
+ "os"
"path/filepath"
"strings"
@@ -174,20 +175,19 @@
func (c *commandHandler) GoGetModule(ctx context.Context, args command.DependencyArgs) error {
return c.run(ctx, commandConfig{
- requireSave: true,
- progress: "Running go get",
- forURI: args.URI,
+ progress: "Running go get",
+ forURI: args.URI,
}, func(ctx context.Context, deps commandDeps) error {
- return runGoGetModule(ctx, deps.snapshot, args.URI.SpanURI(), args.AddRequire, args.GoCmdArgs)
+ return c.s.runGoModUpdateCommands(ctx, deps.snapshot, args.URI.SpanURI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
+ return runGoGetModule(invoke, args.AddRequire, args.GoCmdArgs)
+ })
})
}
// TODO(rFindley): UpdateGoSum, Tidy, and Vendor could probably all be one command.
-
func (c *commandHandler) UpdateGoSum(ctx context.Context, args command.URIArgs) error {
return c.run(ctx, commandConfig{
- requireSave: true,
- progress: "Updating go.sum",
+ progress: "Updating go.sum",
}, func(ctx context.Context, deps commandDeps) error {
for _, uri := range args.URIs {
snapshot, fh, ok, release, err := c.s.beginFileRequest(ctx, uri, source.UnknownKind)
@@ -195,7 +195,10 @@
if !ok {
return err
}
- if err := runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, fh.URI(), "list", []string{"all"}); err != nil {
+ if err := c.s.runGoModUpdateCommands(ctx, snapshot, fh.URI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
+ _, err := invoke("list", "all")
+ return err
+ }); err != nil {
return err
}
}
@@ -214,7 +217,10 @@
if !ok {
return err
}
- if err := runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, fh.URI(), "mod", []string{"tidy"}); err != nil {
+ if err := c.s.runGoModUpdateCommands(ctx, snapshot, fh.URI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
+ _, err := invoke("mod", "tidy")
+ return err
+ }); err != nil {
return err
}
}
@@ -228,15 +234,19 @@
progress: "Running go mod vendor",
forURI: args.URI,
}, func(ctx context.Context, deps commandDeps) error {
- return runSimpleGoCommand(ctx, deps.snapshot, source.UpdateUserModFile|source.AllowNetwork, args.URI.SpanURI(), "mod", []string{"vendor"})
+ _, err := deps.snapshot.RunGoCommandDirect(ctx, source.Normal|source.AllowNetwork, &gocommand.Invocation{
+ Verb: "mod",
+ Args: []string{"vendor"},
+ WorkingDir: filepath.Dir(args.URI.SpanURI().Filename()),
+ })
+ return err
})
}
func (c *commandHandler) RemoveDependency(ctx context.Context, args command.RemoveDependencyArgs) error {
return c.run(ctx, commandConfig{
- requireSave: true,
- progress: "Removing dependency",
- forURI: args.URI,
+ progress: "Removing dependency",
+ forURI: args.URI,
}, func(ctx context.Context, deps commandDeps) error {
// If the module is tidied apart from the one unused diagnostic, we can
// run `go get module@none`, and then run `go mod tidy`. Otherwise, we
@@ -244,10 +254,13 @@
// TODO(rstambler): In Go 1.17+, we will be able to use the go command
// without checking if the module is tidy.
if args.OnlyDiagnostic {
- if err := runGoGetModule(ctx, deps.snapshot, args.URI.SpanURI(), false, []string{args.ModulePath + "@none"}); err != nil {
+ return c.s.runGoModUpdateCommands(ctx, deps.snapshot, args.URI.SpanURI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
+ if err := runGoGetModule(invoke, false, []string{args.ModulePath + "@none"}); err != nil {
+ return err
+ }
+ _, err := invoke("mod", "tidy")
return err
- }
- return runSimpleGoCommand(ctx, deps.snapshot, source.UpdateUserModFile|source.AllowNetwork, args.URI.SpanURI(), "mod", []string{"tidy"})
+ })
}
pm, err := deps.snapshot.ParseMod(ctx, deps.fh)
if err != nil {
@@ -444,39 +457,114 @@
forURI: args.URI,
progress: "Running go get",
}, func(ctx context.Context, deps commandDeps) error {
- uri := args.URI.SpanURI()
+ // Run on a throwaway go.mod, otherwise it'll write to the real one.
stdout, err := deps.snapshot.RunGoCommandDirect(ctx, source.WriteTemporaryModFile|source.AllowNetwork, &gocommand.Invocation{
Verb: "list",
Args: []string{"-f", "{{.Module.Path}}@{{.Module.Version}}", args.Pkg},
- WorkingDir: filepath.Dir(uri.Filename()),
+ WorkingDir: filepath.Dir(args.URI.SpanURI().Filename()),
})
if err != nil {
return err
}
ver := strings.TrimSpace(stdout.String())
- return runGoGetModule(ctx, deps.snapshot, uri, args.AddRequire, []string{ver})
+ return c.s.runGoModUpdateCommands(ctx, deps.snapshot, args.URI.SpanURI(), func(invoke func(...string) (*bytes.Buffer, error)) error {
+ return runGoGetModule(invoke, args.AddRequire, []string{ver})
+ })
})
}
-func runGoGetModule(ctx context.Context, snapshot source.Snapshot, uri span.URI, addRequire bool, args []string) error {
+func (s *Server) runGoModUpdateCommands(ctx context.Context, snapshot source.Snapshot, uri span.URI, run func(invoke func(...string) (*bytes.Buffer, error)) error) error {
+ tmpModfile, newModBytes, newSumBytes, err := snapshot.RunGoCommands(ctx, true, filepath.Dir(uri.Filename()), run)
+ if err != nil {
+ return err
+ }
+ if !tmpModfile {
+ return nil
+ }
+ modURI := snapshot.GoModForFile(uri)
+ sumURI := span.URIFromPath(strings.TrimSuffix(modURI.Filename(), ".mod") + ".sum")
+ modEdits, err := applyFileEdits(ctx, snapshot, modURI, newModBytes)
+ if err != nil {
+ return err
+ }
+ sumEdits, err := applyFileEdits(ctx, snapshot, sumURI, newSumBytes)
+ if err != nil {
+ return err
+ }
+ changes := append(sumEdits, modEdits...)
+ if len(changes) == 0 {
+ return nil
+ }
+ response, err := s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{
+ Edit: protocol.WorkspaceEdit{
+ DocumentChanges: changes,
+ },
+ })
+ if err != nil {
+ return err
+ }
+ if !response.Applied {
+ return fmt.Errorf("edits not applied because of %s", response.FailureReason)
+ }
+ return nil
+}
+
+func applyFileEdits(ctx context.Context, snapshot source.Snapshot, uri span.URI, newContent []byte) ([]protocol.TextDocumentEdit, error) {
+ fh, err := snapshot.GetVersionedFile(ctx, uri)
+ if err != nil {
+ return nil, err
+ }
+ oldContent, err := fh.Read()
+ if err != nil && !os.IsNotExist(err) {
+ return nil, err
+ }
+ if bytes.Equal(oldContent, newContent) {
+ return nil, nil
+ }
+
+ // Sending a workspace edit to a closed file causes VS Code to open the
+ // file and leave it unsaved. We would rather apply the changes directly,
+ // especially to go.sum, which should be mostly invisible to the user.
+ if !snapshot.IsOpen(uri) {
+ err := ioutil.WriteFile(uri.Filename(), newContent, 0666)
+ return nil, err
+ }
+
+ m := &protocol.ColumnMapper{
+ URI: fh.URI(),
+ Converter: span.NewContentConverter(fh.URI().Filename(), oldContent),
+ Content: oldContent,
+ }
+ diff, err := snapshot.View().Options().ComputeEdits(uri, string(oldContent), string(newContent))
+ if err != nil {
+ return nil, err
+ }
+ edits, err := source.ToProtocolEdits(m, diff)
+ if err != nil {
+ return nil, err
+ }
+ return []protocol.TextDocumentEdit{{
+ TextDocument: protocol.OptionalVersionedTextDocumentIdentifier{
+ Version: fh.Version(),
+ TextDocumentIdentifier: protocol.TextDocumentIdentifier{
+ URI: protocol.URIFromSpanURI(uri),
+ },
+ },
+ Edits: edits,
+ }}, nil
+}
+
+func runGoGetModule(invoke func(...string) (*bytes.Buffer, error), addRequire bool, args []string) error {
if addRequire {
// Using go get to create a new dependency results in an
// `// indirect` comment we may not want. The only way to avoid it
// is to add the require as direct first. Then we can use go get to
// update go.sum and tidy up.
- if err := runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile, uri, "mod", append([]string{"edit", "-require"}, args...)); err != nil {
+ if _, err := invoke(append([]string{"mod", "edit", "-require"}, args...)...); err != nil {
return err
}
}
- return runSimpleGoCommand(ctx, snapshot, source.UpdateUserModFile|source.AllowNetwork, uri, "get", append([]string{"-d"}, args...))
-}
-
-func runSimpleGoCommand(ctx context.Context, snapshot source.Snapshot, mode source.InvocationFlags, uri span.URI, verb string, args []string) error {
- _, err := snapshot.RunGoCommandDirect(ctx, mode, &gocommand.Invocation{
- Verb: verb,
- Args: args,
- WorkingDir: filepath.Dir(uri.Filename()),
- })
+ _, err := invoke(append([]string{"get", "-d"}, args...)...)
return err
}
diff --git a/internal/lsp/fake/client.go b/internal/lsp/fake/client.go
index 2133621..acc4ea5 100644
--- a/internal/lsp/fake/client.go
+++ b/internal/lsp/fake/client.go
@@ -7,6 +7,7 @@
import (
"context"
"fmt"
+ "os"
"golang.org/x/tools/internal/lsp/protocol"
)
@@ -110,8 +111,7 @@
return nil
}
-// ApplyEdit applies edits sent from the server. Note that as of writing gopls
-// doesn't use this feature, so it is untested.
+// ApplyEdit applies edits sent from the server.
func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceEditParams) (*protocol.ApplyWorkspaceEditResponse, error) {
if len(params.Edit.Changes) != 0 {
return &protocol.ApplyWorkspaceEditResponse{FailureReason: "Edit.Changes is unsupported"}, nil
@@ -119,6 +119,16 @@
for _, change := range params.Edit.DocumentChanges {
path := c.editor.sandbox.Workdir.URIToPath(change.TextDocument.URI)
edits := convertEdits(change.Edits)
+ if !c.editor.HasBuffer(path) {
+ err := c.editor.OpenFile(ctx, path)
+ if os.IsNotExist(err) {
+ c.editor.CreateBuffer(ctx, path, "")
+ err = nil
+ }
+ if err != nil {
+ return nil, err
+ }
+ }
if err := c.editor.EditBuffer(ctx, path, edits); err != nil {
return nil, err
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 964d631..5c96908 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -94,6 +94,10 @@
// WorkingDir must be specified.
RunGoCommandDirect(ctx context.Context, mode InvocationFlags, inv *gocommand.Invocation) (*bytes.Buffer, error)
+ // RunGoCommands runs a series of `go` commands that updates the go.mod
+ // and go.sum file for wd, and returns their updated contents.
+ RunGoCommands(ctx context.Context, allowNetwork bool, wd string, run func(invoke func(...string) (*bytes.Buffer, error)) error) (bool, []byte, []byte, error)
+
// RunProcessEnvFunc runs fn with the process env for this snapshot's view.
// Note: the process env contains cached module and filesystem state.
RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error) error
@@ -185,7 +189,7 @@
// AllowNetwork is a flag bit that indicates the invocation should be
// allowed to access the network.
- AllowNetwork = 1 << 10
+ AllowNetwork InvocationFlags = 1 << 10
)
func (m InvocationFlags) Mode() InvocationFlags {
