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>
This commit is contained in:
Heschi Kreinick 2021-02-05 22:18:47 -05:00
parent 8316e56472
commit 706a59cbbb
7 changed files with 186 additions and 54 deletions

View File

@ -692,7 +692,7 @@ func main() {
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 @@ func main() {
),
)
env.ApplyQuickFixes("main.go", d.Diagnostics)
env.CheckForFileChanges()
env.Await(
EmptyDiagnostics("main.go"),
)

View File

@ -48,31 +48,22 @@ func _() {
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"),
)
})
}

View File

@ -323,7 +323,7 @@ go 1.14
),
)
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 @@ require (
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 @@ func main() {
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 @@ func main() {
),
)
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 @@ func main() {
),
)
env.ApplyQuickFixes("go.mod", d.Diagnostics)
env.SaveBuffer("go.mod") // Save to trigger diagnostics.
env.Await(
EmptyDiagnostics("go.mod"),
)
@ -925,6 +927,7 @@ func main() {}
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 @@ func main() {}
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))
}
})

View File

@ -12,6 +12,7 @@ import (
"go/token"
"go/types"
"io"
"io/ioutil"
"os"
"path/filepath"
"sort"
@ -255,6 +256,44 @@ func (s *snapshot) RunGoCommandPiped(ctx context.Context, mode source.Invocation
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 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
}
}
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.

View File

@ -11,6 +11,7 @@ import (
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"strings"
@ -174,20 +175,19 @@ func (c *commandHandler) UpgradeDependency(ctx context.Context, args command.Dep
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 @@ func (c *commandHandler) UpdateGoSum(ctx context.Context, args command.URIArgs)
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 @@ func (c *commandHandler) Tidy(ctx context.Context, args command.URIArgs) error {
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 @@ func (c *commandHandler) Vendor(ctx context.Context, args command.URIArg) error
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 @@ func (c *commandHandler) RemoveDependency(ctx context.Context, args command.Remo
// 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 @@ func (c *commandHandler) GoGetPackage(ctx context.Context, args command.GoGetPac
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
}

View File

@ -7,6 +7,7 @@ package fake
import (
"context"
"fmt"
"os"
"golang.org/x/tools/internal/lsp/protocol"
)
@ -110,8 +111,7 @@ func (c *Client) WorkDoneProgressCreate(ctx context.Context, params *protocol.Wo
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 @@ func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceE
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
}

View File

@ -94,6 +94,10 @@ type Snapshot interface {
// 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 @@ const (
// 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 {