From 2ca718005c187b688247516b87f8da3b9d03e6b6 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 6 Sep 2019 14:55:14 -0400 Subject: [PATCH] internal/lsp: use protocol.TextEdits in suggested fixes Change-Id: I8b454dd8c59839468ecfcb19b7edfa659e386b57 Reviewed-on: https://go-review.googlesource.com/c/tools/+/193957 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/code_action.go | 18 +++------------- internal/lsp/lsp_test.go | 27 ++++------------------- internal/lsp/rename.go | 16 ++------------ internal/lsp/source/diagnostics.go | 9 ++++---- internal/lsp/source/rename.go | 20 ++++++++++++----- internal/lsp/source/source_test.go | 8 +++++-- internal/lsp/source/suggested_fix.go | 32 +++++++++++++++++----------- internal/lsp/util.go | 13 ----------- 8 files changed, 54 insertions(+), 89 deletions(-) diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index e7fb25bc90..5e00666be0 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -213,25 +213,13 @@ func quickFixes(ctx context.Context, view source.View, gof source.GoFile) ([]pro if err != nil { return nil, err } - for _, ca := range diag.SuggestedFixes { - f, err := view.GetFile(ctx, diag.URI) - if err != nil { - return nil, err - } - m, err := getMapper(ctx, f) - if err != nil { - return nil, err - } - edits, err := source.ToProtocolEdits(m, ca.Edits) - if err != nil { - return nil, err - } + for _, fix := range diag.SuggestedFixes { codeActions = append(codeActions, protocol.CodeAction{ - Title: ca.Title, + Title: fix.Title, Kind: protocol.QuickFix, // TODO(matloob): Be more accurate about these? Edit: &protocol.WorkspaceEdit{ Changes: &map[string][]protocol.TextEdit{ - protocol.NewURI(diag.URI): edits, + protocol.NewURI(diag.URI): fix.Edits, }, }, Diagnostics: []protocol.Diagnostic{pdiag}, diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 862eface56..bf7c452f93 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -302,15 +302,10 @@ func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) { } func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, ranges []protocol.FoldingRange) { - f, err := getGoFile(r.ctx, r.server.session.ViewOf(uri), uri) + m, err := r.mapper(uri) if err != nil { t.Fatal(err) } - m, err := getMapper(r.ctx, f) - if err != nil { - t.Fatal(err) - } - // Fold all ranges. nonOverlapping := nonOverlappingRanges(ranges) for i, rngs := range nonOverlapping { @@ -439,11 +434,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { } continue } - f, err := getGoFile(r.ctx, r.server.session.ViewOf(uri), uri) - if err != nil { - t.Fatal(err) - } - m, err := getMapper(r.ctx, f) + m, err := r.mapper(uri) if err != nil { t.Fatal(err) } @@ -479,11 +470,7 @@ func (r *runner) Import(t *testing.T, data tests.Imports) { } continue } - f, err := getGoFile(r.ctx, r.server.session.ViewOf(uri), uri) - if err != nil { - t.Fatal(err) - } - m, err := getMapper(r.ctx, f) + m, err := r.mapper(uri) if err != nil { t.Fatal(err) } @@ -669,12 +656,7 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { var res []string for uri, edits := range *workspaceEdits.Changes { - spnURI := span.URI(uri) - f, err := getGoFile(r.ctx, r.server.session.ViewOf(spnURI), spnURI) - if err != nil { - t.Fatal(err) - } - m, err := getMapper(r.ctx, f) + m, err := r.mapper(span.URI(uri)) if err != nil { t.Fatal(err) } @@ -682,7 +664,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { if err != nil { t.Error(err) } - filename := filepath.Base(m.URI.Filename()) contents := applyEdits(string(m.Content), sedits) res = append(res, fmt.Sprintf("%s:\n%s", filename, contents)) diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go index 92616143e2..439f27efb8 100644 --- a/internal/lsp/rename.go +++ b/internal/lsp/rename.go @@ -28,20 +28,8 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr return nil, err } changes := make(map[string][]protocol.TextEdit) - for uri, textEdits := range edits { - f, err := getGoFile(ctx, view, uri) - if err != nil { - return nil, err - } - m, err := getMapper(ctx, f) - if err != nil { - return nil, err - } - protocolEdits, err := source.ToProtocolEdits(m, textEdits) - if err != nil { - return nil, err - } - changes[string(uri)] = protocolEdits + for uri, e := range edits { + changes[protocol.NewURI(uri)] = e } return &protocol.WorkspaceEdit{Changes: &changes}, nil diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 58292096f4..4d0a8d56ac 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -35,7 +35,6 @@ import ( "golang.org/x/tools/go/analysis/passes/unsafeptr" "golang.org/x/tools/go/analysis/passes/unusedresult" "golang.org/x/tools/go/packages" - "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/telemetry" "golang.org/x/tools/internal/span" @@ -51,12 +50,12 @@ type Diagnostic struct { Source string Severity DiagnosticSeverity - SuggestedFixes []SuggestedFixes + SuggestedFixes []SuggestedFix } -type SuggestedFixes struct { +type SuggestedFix struct { Title string - Edits []diff.TextEdit + Edits []protocol.TextEdit } type DiagnosticSeverity int @@ -241,7 +240,7 @@ func toDiagnostic(ctx context.Context, view View, diag analysis.Diagnostic, cate if diag.Category != "" { category += "." + category } - ca, err := getCodeActions(view.Session().Cache().FileSet(), diag) + ca, err := getCodeActions(ctx, view, diag) if err != nil { return Diagnostic{}, err } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 7c49ce3316..5bd040d29d 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -77,7 +77,7 @@ func PrepareRename(ctx context.Context, view View, f GoFile, pos protocol.Positi } // Rename returns a map of TextEdits for each file modified when renaming a given identifier within a package. -func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) (map[span.URI][]diff.TextEdit, error) { +func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) (map[span.URI][]protocol.TextEdit, error) { ctx, done := trace.StartSpan(ctx, "source.Rename") defer done() @@ -143,12 +143,22 @@ func (i *IdentifierInfo) Rename(ctx context.Context, view View, newName string) if err != nil { return nil, err } - - // Sort edits for each file. - for _, edits := range changes { + result := make(map[span.URI][]protocol.TextEdit) + for uri, edits := range changes { + // Sort the edits first. diff.SortTextEdits(edits) + + _, m, err := cachedFileToMapper(ctx, view, uri) + if err != nil { + return nil, err + } + protocolEdits, err := ToProtocolEdits(m, edits) + if err != nil { + return nil, err + } + result[uri] = protocolEdits } - return changes, nil + return result, nil } // getPkgName gets the pkg name associated with an identifer representing diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index eba7a9b0e5..f19f6c91e1 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -657,14 +657,18 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - data, _, err := f.Handle(ctx).Read(ctx) if err != nil { t.Error(err) continue } + m := protocol.NewColumnMapper(f.URI(), f.URI().Filename(), r.data.Exported.ExpectFileSet, nil, data) filename := filepath.Base(editSpn.Filename()) - contents := applyEdits(string(data), edits) + diffEdits, err := source.FromProtocolEdits(m, edits) + if err != nil { + t.Fatal(err) + } + contents := applyEdits(string(data), diffEdits) res = append(res, fmt.Sprintf("%s:\n%s", filename, contents)) } diff --git a/internal/lsp/source/suggested_fix.go b/internal/lsp/source/suggested_fix.go index 0e4d84786f..a9706103a8 100644 --- a/internal/lsp/source/suggested_fix.go +++ b/internal/lsp/source/suggested_fix.go @@ -1,26 +1,34 @@ package source import ( - "go/token" + "context" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/internal/lsp/diff" - "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/lsp/protocol" ) -func getCodeActions(fset *token.FileSet, diag analysis.Diagnostic) ([]SuggestedFixes, error) { - var cas []SuggestedFixes +func getCodeActions(ctx context.Context, view View, diag analysis.Diagnostic) ([]SuggestedFix, error) { + var fixes []SuggestedFix for _, fix := range diag.SuggestedFixes { - var ca SuggestedFixes - ca.Title = fix.Message - for _, te := range fix.TextEdits { - span, err := span.NewRange(fset, te.Pos, te.End).Span() + var edits []protocol.TextEdit + for _, e := range fix.TextEdits { + mrng, err := posToRange(ctx, view, e.Pos, e.End) if err != nil { return nil, err } - ca.Edits = append(ca.Edits, diff.TextEdit{Span: span, NewText: string(te.NewText)}) + rng, err := mrng.Range() + if err != nil { + return nil, err + } + edits = append(edits, protocol.TextEdit{ + Range: rng, + NewText: string(e.NewText), + }) } - cas = append(cas, ca) + fixes = append(fixes, SuggestedFix{ + Title: fix.Message, + Edits: edits, + }) } - return cas, nil + return fixes, nil } diff --git a/internal/lsp/util.go b/internal/lsp/util.go index 33890b7ab2..60323b1ebd 100644 --- a/internal/lsp/util.go +++ b/internal/lsp/util.go @@ -7,7 +7,6 @@ package lsp import ( "context" - "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" errors "golang.org/x/xerrors" @@ -24,15 +23,3 @@ func getGoFile(ctx context.Context, view source.View, uri span.URI) (source.GoFi } return gof, nil } - -func getMapper(ctx context.Context, f source.File) (*protocol.ColumnMapper, error) { - data, _, err := f.Handle(ctx).Read(ctx) - if err != nil { - return nil, err - } - tok, err := f.GetToken(ctx) - if err != nil { - return nil, err - } - return protocol.NewColumnMapper(f.URI(), f.URI().Filename(), f.FileSet(), tok, data), nil -}