diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go index 542991a2e2..3a731527ad 100644 --- a/gopls/internal/regtest/diagnostics/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go @@ -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"), ) diff --git a/gopls/internal/regtest/misc/vendor_test.go b/gopls/internal/regtest/misc/vendor_test.go index 8e76dd82ca..1c9a4eca1f 100644 --- a/gopls/internal/regtest/misc/vendor_test.go +++ b/gopls/internal/regtest/misc/vendor_test.go @@ -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"), ) }) } diff --git a/gopls/internal/regtest/modfile/modfile_test.go b/gopls/internal/regtest/modfile/modfile_test.go index 8c167cc2c5..0f897e772c 100644 --- a/gopls/internal/regtest/modfile/modfile_test.go +++ b/gopls/internal/regtest/modfile/modfile_test.go @@ -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)) } }) diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 2c90492c0a..3bc9be3de7 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -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. diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 469ef925d5..8bbb684451 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -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 } diff --git a/internal/lsp/fake/client.go b/internal/lsp/fake/client.go index 21336216e1..acc4ea5d00 100644 --- a/internal/lsp/fake/client.go +++ b/internal/lsp/fake/client.go @@ -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 } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 964d631140..5c96908ecd 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -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 {