From 1e6483c8d4f62410778f4b03e7d02361a0621da2 Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Mon, 21 Oct 2019 20:29:50 +0200 Subject: [PATCH 1/6] tools/gopls: add command line support for suggestedfix This commit adds support for calling suggestedfix from the gopls command line, e.g. $ gopls suggestedfix ~/tmp/foo/main.go Optional arguments are: - -w, which writes the changes back to the original file; and - -d, which prints a unified diff to stdout With no arguments, the changed files are printed to stdout. --- internal/lsp/cmd/cmd.go | 1 + internal/lsp/cmd/suggested_fix.go | 108 +++++++++++++++++++++++++ internal/lsp/cmd/test/cmdtest.go | 4 - internal/lsp/cmd/test/suggested_fix.go | 25 ++++++ 4 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 internal/lsp/cmd/suggested_fix.go create mode 100644 internal/lsp/cmd/test/suggested_fix.go diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index d74a5af6ebe..daf4833fa97 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -145,6 +145,7 @@ func (app *Application) commands() []tool.Application { &format{app: app}, &query{app: app}, &rename{app: app}, + &suggestedfix{app: app}, &version{app: app}, } } diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go new file mode 100644 index 00000000000..0957318d541 --- /dev/null +++ b/internal/lsp/cmd/suggested_fix.go @@ -0,0 +1,108 @@ +package cmd + +import ( + "context" + "flag" + "fmt" + "io/ioutil" + "time" + + "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/tool" + errors "golang.org/x/xerrors" +) + +// suggestedfix implements the suggestedfix verb for gopls. +type suggestedfix struct { + Diff bool `flag:"d" help:"display diffs instead of rewriting files"` + Write bool `flag:"w" help:"write result to (source) file instead of stdout"` + + app *Application +} + +func (s *suggestedfix) Name() string { return "suggestedfix" } +func (s *suggestedfix) Usage() string { return "" } +func (s *suggestedfix) ShortHelp() string { return "apply suggested fixes" } +func (s *suggestedfix) DetailedHelp(f *flag.FlagSet) { + fmt.Fprintf(f.Output(), ` +Example: apply all suggested fixes for this file: + +  $ gopls suggestedfix -w internal/lsp/cmd/check.go + +gopls suggestedfix flags are: +`) + f.PrintDefaults() +} + +// Run performs diagnostic checks on the file specified and either; +// - if -w is specified, updates the file in place; +// - if -d is specified, prints out unified diffs of the changes; or +// - otherwise, prints the new versions to stdout. +func (s *suggestedfix) Run(ctx context.Context, args ...string) error { + if len(args) != 1 { + return tool.CommandLineErrorf("suggestedfix expects 1 argument") + } + conn, err := s.app.connect(ctx) + if err != nil { + return err + } + defer conn.terminate(ctx) + + from := span.Parse(args[0]) + uri := from.URI() + file := conn.AddFile(ctx, uri) + if file.err != nil { + return file.err + } + + // Wait for diagnostics results + select { + case <-file.hasDiagnostics: + case <-time.After(30 * time.Second): + return errors.Errorf("timed out waiting for results from %v", file.uri) + } + + file.diagnosticsMu.Lock() + defer file.diagnosticsMu.Unlock() + + p := protocol.CodeActionParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: protocol.NewURI(uri), + }, + Context: protocol.CodeActionContext{ + Only: []protocol.CodeActionKind{protocol.QuickFix}, + Diagnostics: file.diagnostics, + }, + } + actions, err := conn.CodeAction(ctx, &p) + if err != nil { + return err + } + var edits []protocol.TextEdit + for _, a := range actions { + edits = (*a.Edit.Changes)[string(uri)] + } + + sedits, err := source.FromProtocolEdits(file.mapper, edits) + if err != nil { + return errors.Errorf("%v: %v", edits, err) + } + newContent := diff.ApplyEdits(string(file.mapper.Content), sedits) + + filename := file.uri.Filename() + switch { + case s.Write: + if len(edits) > 0 { + ioutil.WriteFile(filename, []byte(newContent), 0644) + } + case s.Diff: + diffs := diff.ToUnified(filename+".orig", filename, string(file.mapper.Content), sedits) + fmt.Print(diffs) + default: + fmt.Print(string(newContent)) + } + return nil +} diff --git a/internal/lsp/cmd/test/cmdtest.go b/internal/lsp/cmd/test/cmdtest.go index 8aa8638f6bf..f6becff1099 100644 --- a/internal/lsp/cmd/test/cmdtest.go +++ b/internal/lsp/cmd/test/cmdtest.go @@ -98,10 +98,6 @@ func (r *runner) Import(t *testing.T, spn span.Span) { //TODO: add command line imports tests when it works } -func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { - //TODO: add suggested fix tests when it works -} - func CaptureStdOut(t testing.TB, f func()) string { r, out, err := os.Pipe() if err != nil { diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go new file mode 100644 index 00000000000..dd399998c09 --- /dev/null +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -0,0 +1,25 @@ +package cmdtest + +import ( + "testing" + + "golang.org/x/tools/internal/lsp/cmd" + "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/tool" +) + +func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { + uri := spn.URI() + filename := uri.Filename() + args := []string{"suggestedfix", filename} + app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) + got := CaptureStdOut(t, func() { + _ = tool.Run(r.ctx, app, args) + }) + want := string(r.data.Golden("suggestedfix", filename, func() ([]byte, error) { + return []byte(got), nil + })) + if want != got { + t.Errorf("suggested fixes failed for %s, expected:\n%v\ngot:\n%v", filename, want, got) + } +} From c078249d9a1df12086f05d22f7ffac2d76b40541 Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Mon, 21 Oct 2019 20:38:26 +0200 Subject: [PATCH 2/6] Add file Copyrights --- internal/lsp/cmd/suggested_fix.go | 3 +++ internal/lsp/cmd/test/suggested_fix.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 0957318d541..b87e669a2c2 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -1,3 +1,6 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. package cmd import ( diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index dd399998c09..6ecf8279121 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -1,3 +1,6 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. package cmdtest import ( From a0a0341d856482bc8afba1f020f313d320aaf5e3 Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Mon, 21 Oct 2019 20:53:20 +0200 Subject: [PATCH 3/6] Update error string --- internal/lsp/cmd/suggested_fix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index b87e669a2c2..aa5c1a8a103 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -82,7 +82,7 @@ func (s *suggestedfix) Run(ctx context.Context, args ...string) error { } actions, err := conn.CodeAction(ctx, &p) if err != nil { - return err + return errors.Errorf("%v: %v", spn, err) } var edits []protocol.TextEdit for _, a := range actions { From 79e10a4a2911f0f73c3b74328d8833b19d2147b3 Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Mon, 21 Oct 2019 21:42:48 +0200 Subject: [PATCH 4/6] Preferred fixes are now applied by default Add a flag for all fixes --- internal/lsp/cmd/suggested_fix.go | 9 ++++++--- internal/lsp/cmd/test/suggested_fix.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index aa5c1a8a103..3bfe123364a 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -22,6 +22,7 @@ import ( type suggestedfix struct { Diff bool `flag:"d" help:"display diffs instead of rewriting files"` Write bool `flag:"w" help:"write result to (source) file instead of stdout"` + All bool `flag:"a" help:"apply all fixes, not just preferred fixes"` app *Application } @@ -31,7 +32,7 @@ func (s *suggestedfix) Usage() string { return "" } func (s *suggestedfix) ShortHelp() string { return "apply suggested fixes" } func (s *suggestedfix) DetailedHelp(f *flag.FlagSet) { fmt.Fprintf(f.Output(), ` -Example: apply all suggested fixes for this file: +Example: apply suggested fixes for this file:   $ gopls suggestedfix -w internal/lsp/cmd/check.go @@ -82,11 +83,13 @@ func (s *suggestedfix) Run(ctx context.Context, args ...string) error { } actions, err := conn.CodeAction(ctx, &p) if err != nil { - return errors.Errorf("%v: %v", spn, err) + return errors.Errorf("%v: %v", from, err) } var edits []protocol.TextEdit for _, a := range actions { - edits = (*a.Edit.Changes)[string(uri)] + if a.IsPreferred || s.All { + edits = (*a.Edit.Changes)[string(uri)] + } } sedits, err := source.FromProtocolEdits(file.mapper, edits) diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index 6ecf8279121..c8d55efe533 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -14,7 +14,7 @@ import ( func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { uri := spn.URI() filename := uri.Filename() - args := []string{"suggestedfix", filename} + args := []string{"suggestedfix", "-a", filename} app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) got := CaptureStdOut(t, func() { _ = tool.Run(r.ctx, app, args) From d344403ccaace2002b7ff5941f3881e573d37bfe Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Mon, 21 Oct 2019 21:45:53 +0200 Subject: [PATCH 5/6] Rename from suggestedfix to fix --- internal/lsp/cmd/suggested_fix.go | 10 +++++----- internal/lsp/cmd/test/suggested_fix.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 3bfe123364a..7c19920f39e 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -18,7 +18,7 @@ import ( errors "golang.org/x/xerrors" ) -// suggestedfix implements the suggestedfix verb for gopls. +// suggestedfix implements the fix verb for gopls. type suggestedfix struct { Diff bool `flag:"d" help:"display diffs instead of rewriting files"` Write bool `flag:"w" help:"write result to (source) file instead of stdout"` @@ -27,16 +27,16 @@ type suggestedfix struct { app *Application } -func (s *suggestedfix) Name() string { return "suggestedfix" } +func (s *suggestedfix) Name() string { return "fix" } func (s *suggestedfix) Usage() string { return "" } func (s *suggestedfix) ShortHelp() string { return "apply suggested fixes" } func (s *suggestedfix) DetailedHelp(f *flag.FlagSet) { fmt.Fprintf(f.Output(), ` Example: apply suggested fixes for this file: -  $ gopls suggestedfix -w internal/lsp/cmd/check.go +  $ gopls fix -w internal/lsp/cmd/check.go -gopls suggestedfix flags are: +gopls fix flags are: `) f.PrintDefaults() } @@ -47,7 +47,7 @@ gopls suggestedfix flags are: // - otherwise, prints the new versions to stdout. func (s *suggestedfix) Run(ctx context.Context, args ...string) error { if len(args) != 1 { - return tool.CommandLineErrorf("suggestedfix expects 1 argument") + return tool.CommandLineErrorf("fix expects 1 argument") } conn, err := s.app.connect(ctx) if err != nil { diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index c8d55efe533..6770603ee00 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -14,7 +14,7 @@ import ( func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { uri := spn.URI() filename := uri.Filename() - args := []string{"suggestedfix", "-a", filename} + args := []string{"fix", "-a", filename} app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) got := CaptureStdOut(t, func() { _ = tool.Run(r.ctx, app, args) From 070fcda33ac3494bfe8f19c2cd78c089c713ed98 Mon Sep 17 00:00:00 2001 From: Kalman Bekesi Date: Tue, 29 Oct 2019 09:38:17 +0100 Subject: [PATCH 6/6] Fix spacing --- internal/lsp/cmd/suggested_fix.go | 1 + internal/lsp/cmd/test/suggested_fix.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 7c19920f39e..25ee8b62baf 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -1,6 +1,7 @@ // Copyright 2019 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. + package cmd import ( diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index 6770603ee00..9be0f8ede5e 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -1,6 +1,7 @@ // Copyright 2019 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. + package cmdtest import (