gopls/internal/lsp/regtest: add @rename marker tests
This change adds rudimentary support for @rename
markers, which make an LSP 'rename' request and
assert that the expected file changes are obtained.
We also introduce a separate marker, @renameerr,
for expectations of failure.
The treatment both of changed files and of expected errors
should be used by other markers, for consistency.
Also:
- Env.Mapper returns an error.
- add diff.ApplyBytes and compare.Bytes convenience wrappers.
- simplify fake.ApplyEdits by using ApplyProtocolEdits.
- change ApplyProtocolEdits to use []byte.
Change-Id: Ib64b3d8e12c025f01dd0783b4faee94f082b4895
Reviewed-on: https://go-review.googlesource.com/c/tools/+/466144
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go
index a3a53ba..be016e7 100644
--- a/go/analysis/analysistest/analysistest.go
+++ b/go/analysis/analysistest/analysistest.go
@@ -184,7 +184,7 @@
for _, vf := range ar.Files {
if vf.Name == sf {
found = true
- out, err := diff.Apply(string(orig), edits)
+ out, err := diff.ApplyBytes(orig, edits)
if err != nil {
t.Errorf("%s: error applying fixes: %v", file.Name(), err)
continue
@@ -194,7 +194,7 @@
// between files in the archive. normalize
// this to a single newline.
want := string(bytes.TrimRight(vf.Data, "\n")) + "\n"
- formatted, err := format.Source([]byte(out))
+ formatted, err := format.Source(out)
if err != nil {
t.Errorf("%s: error formatting edited source: %v\n%s", file.Name(), err, out)
continue
@@ -218,14 +218,14 @@
catchallEdits = append(catchallEdits, edits...)
}
- out, err := diff.Apply(string(orig), catchallEdits)
+ out, err := diff.ApplyBytes(orig, catchallEdits)
if err != nil {
t.Errorf("%s: error applying fixes: %v", file.Name(), err)
continue
}
want := string(ar.Comment)
- formatted, err := format.Source([]byte(out))
+ formatted, err := format.Source(out)
if err != nil {
t.Errorf("%s: error formatting resulting source: %v\n%s", file.Name(), err, out)
continue
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index 8f49e8f..5346acd 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -424,11 +424,10 @@
return err
}
- applied, err := diff.Apply(string(contents), edits)
+ out, err := diff.ApplyBytes(contents, edits)
if err != nil {
return err
}
- out := []byte(applied)
// Try to format the file.
if formatted, err := format.Source(out); err == nil {
diff --git a/gopls/internal/lsp/cmd/format.go b/gopls/internal/lsp/cmd/format.go
index 31dfb17..1602dec 100644
--- a/gopls/internal/lsp/cmd/format.go
+++ b/gopls/internal/lsp/cmd/format.go
@@ -9,6 +9,7 @@
"flag"
"fmt"
"io/ioutil"
+ "os"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
@@ -90,7 +91,7 @@
if c.Write {
printIt = false
if len(edits) > 0 {
- ioutil.WriteFile(filename, []byte(formatted), 0644)
+ ioutil.WriteFile(filename, formatted, 0644)
}
}
if c.Diff {
@@ -102,7 +103,7 @@
fmt.Print(unified)
}
if printIt {
- fmt.Print(formatted)
+ os.Stdout.Write(formatted)
}
}
return nil
diff --git a/gopls/internal/lsp/cmd/imports.go b/gopls/internal/lsp/cmd/imports.go
index fadc846..99141da 100644
--- a/gopls/internal/lsp/cmd/imports.go
+++ b/gopls/internal/lsp/cmd/imports.go
@@ -9,6 +9,7 @@
"flag"
"fmt"
"io/ioutil"
+ "os"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
@@ -89,7 +90,7 @@
switch {
case t.Write:
if len(edits) > 0 {
- ioutil.WriteFile(filename, []byte(newContent), 0644)
+ ioutil.WriteFile(filename, newContent, 0644)
}
case t.Diff:
unified, err := diff.ToUnified(filename+".orig", filename, string(file.mapper.Content), sedits)
@@ -98,7 +99,7 @@
}
fmt.Print(unified)
default:
- fmt.Print(string(newContent))
+ os.Stdout.Write(newContent)
}
return nil
}
diff --git a/gopls/internal/lsp/cmd/rename.go b/gopls/internal/lsp/cmd/rename.go
index 5a6677d..14c789c 100644
--- a/gopls/internal/lsp/cmd/rename.go
+++ b/gopls/internal/lsp/cmd/rename.go
@@ -108,7 +108,7 @@
return fmt.Errorf("%v: %v", edits, err)
}
}
- ioutil.WriteFile(filename, []byte(newContent), 0644)
+ ioutil.WriteFile(filename, newContent, 0644)
case r.Diff:
unified, err := diff.ToUnified(filename+".orig", filename, string(cmdFile.mapper.Content), renameEdits)
if err != nil {
@@ -119,7 +119,7 @@
if len(orderedURIs) > 1 {
fmt.Printf("%s:\n", filepath.Base(filename))
}
- fmt.Print(string(newContent))
+ os.Stdout.Write(newContent)
if changeCount > 1 { // if this wasn't last change, print newline
fmt.Println()
}
diff --git a/gopls/internal/lsp/cmd/suggested_fix.go b/gopls/internal/lsp/cmd/suggested_fix.go
index b96849d..60571e2 100644
--- a/gopls/internal/lsp/cmd/suggested_fix.go
+++ b/gopls/internal/lsp/cmd/suggested_fix.go
@@ -9,6 +9,7 @@
"flag"
"fmt"
"io/ioutil"
+ "os"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
@@ -151,7 +152,7 @@
switch {
case s.Write:
if len(edits) > 0 {
- ioutil.WriteFile(filename, []byte(newContent), 0644)
+ ioutil.WriteFile(filename, newContent, 0644)
}
case s.Diff:
diffs, err := diff.ToUnified(filename+".orig", filename, string(file.mapper.Content), sedits)
@@ -160,7 +161,7 @@
}
fmt.Print(diffs)
default:
- fmt.Print(string(newContent))
+ os.Stdout.Write(newContent)
}
return nil
}
diff --git a/gopls/internal/lsp/fake/edit.go b/gopls/internal/lsp/fake/edit.go
index 5fd65b0..40762f2 100644
--- a/gopls/internal/lsp/fake/edit.go
+++ b/gopls/internal/lsp/fake/edit.go
@@ -6,6 +6,7 @@
import (
"golang.org/x/tools/gopls/internal/lsp/protocol"
+ "golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/internal/diff"
)
@@ -35,25 +36,14 @@
// and returns a new slice containing the lines of the patched file.
// It is a wrapper around diff.Apply; see that function for preconditions.
func applyEdits(mapper *protocol.Mapper, edits []protocol.TextEdit, windowsLineEndings bool) ([]byte, error) {
- // Convert fake.Edits to diff.Edits
- diffEdits := make([]diff.Edit, len(edits))
- for i, edit := range edits {
- start, end, err := mapper.RangeOffsets(edit.Range)
- if err != nil {
- return nil, err
- }
- diffEdits[i] = diff.Edit{
- Start: start,
- End: end,
- New: edit.NewText,
- }
- }
-
- patchedString, err := diff.Apply(string(mapper.Content), diffEdits)
+ diffEdits, err := source.FromProtocolEdits(mapper, edits)
if err != nil {
return nil, err
}
- patched := []byte(patchedString)
+ patched, err := diff.ApplyBytes(mapper.Content, diffEdits)
+ if err != nil {
+ return nil, err
+ }
if windowsLineEndings {
patched = toWindowsLineEndings(patched)
}
diff --git a/gopls/internal/lsp/fake/editor.go b/gopls/internal/lsp/fake/editor.go
index 84d7fb8..6fa9a8b 100644
--- a/gopls/internal/lsp/fake/editor.go
+++ b/gopls/internal/lsp/fake/editor.go
@@ -702,17 +702,15 @@
return buf.text(), true
}
-// Mapper returns the protocol.Mapper for the given buffer name.
-//
-// If there is no open buffer with that name, it returns nil.
-func (e *Editor) Mapper(name string) *protocol.Mapper {
+// Mapper returns the protocol.Mapper for the given buffer name, if it is open.
+func (e *Editor) Mapper(name string) (*protocol.Mapper, error) {
e.mu.Lock()
defer e.mu.Unlock()
buf, ok := e.buffers[name]
if !ok {
- return nil
+ return nil, fmt.Errorf("no mapper for %q", name)
}
- return buf.mapper
+ return buf.mapper, nil
}
// BufferVersion returns the current version of the buffer corresponding to
diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go
index 81e2600..151e7ac 100644
--- a/gopls/internal/lsp/lsp_test.go
+++ b/gopls/internal/lsp/lsp_test.go
@@ -5,6 +5,7 @@
package lsp
import (
+ "bytes"
"context"
"fmt"
"os"
@@ -97,7 +98,7 @@
data: datum,
ctx: ctx,
normalizers: tests.CollectNormalizers(datum.Exported),
- editRecv: make(chan map[span.URI]string, 1),
+ editRecv: make(chan map[span.URI][]byte, 1),
}
r.server = NewServer(session, testClient{runner: r})
@@ -111,7 +112,7 @@
diagnostics map[span.URI][]*source.Diagnostic
ctx context.Context
normalizers []tests.Normalizer
- editRecv chan map[span.URI]string
+ editRecv chan map[span.URI][]byte
}
// testClient stubs any client functions that may be called by LSP functions.
@@ -367,11 +368,11 @@
func (r *runner) Format(t *testing.T, spn span.Span) {
uri := spn.URI()
filename := uri.Filename()
- gofmted := string(r.data.Golden(t, "gofmt", filename, func() ([]byte, error) {
+ gofmted := r.data.Golden(t, "gofmt", filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
return out, nil
- }))
+ })
edits, err := r.server.Formatting(r.ctx, &protocol.DocumentFormattingParams{
TextDocument: protocol.TextDocumentIdentifier{
@@ -379,7 +380,7 @@
},
})
if err != nil {
- if gofmted != "" {
+ if len(gofmted) > 0 {
t.Error(err)
}
return
@@ -392,7 +393,7 @@
if err != nil {
t.Error(err)
}
- if diff := compare.Text(gofmted, got); diff != "" {
+ if diff := compare.Bytes(gofmted, got); diff != "" {
t.Errorf("format failed for %s (-want +got):\n%s", filename, diff)
}
}
@@ -447,7 +448,7 @@
if err != nil {
t.Fatal(err)
}
- got := string(m.Content)
+ got := m.Content
if len(actions) > 0 {
res, err := applyTextDocumentEdits(r, actions[0].Edit.DocumentChanges)
if err != nil {
@@ -455,12 +456,11 @@
}
got = res[uri]
}
- want := string(r.data.Golden(t, "goimports", filename, func() ([]byte, error) {
- return []byte(got), nil
- }))
-
- if d := compare.Text(want, got); d != "" {
- t.Errorf("import failed for %s:\n%s", filename, d)
+ want := r.data.Golden(t, "goimports", filename, func() ([]byte, error) {
+ return got, nil
+ })
+ if diff := compare.Bytes(want, got); diff != "" {
+ t.Errorf("import failed for %s:\n%s", filename, diff)
}
}
@@ -536,7 +536,7 @@
if !match {
t.Fatalf("unexpected kind for code action %s, got %v, want one of %v", action.Title, action.Kind, codeActionKinds)
}
- var res map[span.URI]string
+ var res map[span.URI][]byte
if cmd := action.Command; cmd != nil {
_, err := r.server.ExecuteCommand(r.ctx, &protocol.ExecuteCommandParams{
Command: action.Command.Command,
@@ -553,11 +553,11 @@
}
}
for u, got := range res {
- want := string(r.data.Golden(t, "suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
- return []byte(got), nil
- }))
- if want != got {
- t.Errorf("suggested fixes failed for %s:\n%s", u.Filename(), compare.Text(want, got))
+ want := r.data.Golden(t, "suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
+ return got, nil
+ })
+ if diff := compare.Bytes(want, got); diff != "" {
+ t.Errorf("suggested fixes failed for %s:\n%s", u.Filename(), diff)
}
}
}
@@ -605,11 +605,11 @@
}
res := <-r.editRecv
for u, got := range res {
- want := string(r.data.Golden(t, "functionextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
- return []byte(got), nil
- }))
- if want != got {
- t.Errorf("function extraction failed for %s:\n%s", u.Filename(), compare.Text(want, got))
+ want := r.data.Golden(t, "functionextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
+ return got, nil
+ })
+ if diff := compare.Bytes(want, got); diff != "" {
+ t.Errorf("function extraction failed for %s:\n%s", u.Filename(), diff)
}
}
}
@@ -657,11 +657,11 @@
}
res := <-r.editRecv
for u, got := range res {
- want := string(r.data.Golden(t, "methodextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
- return []byte(got), nil
- }))
- if want != got {
- t.Errorf("method extraction failed for %s:\n%s", u.Filename(), compare.Text(want, got))
+ want := r.data.Golden(t, "methodextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
+ return got, nil
+ })
+ if diff := compare.Bytes(want, got); diff != "" {
+ t.Errorf("method extraction failed for %s:\n%s", u.Filename(), diff)
}
}
}
@@ -941,12 +941,12 @@
t.Error(err)
}
- withinlayHints := string(r.data.Golden(t, "inlayHint", filename, func() ([]byte, error) {
- return []byte(got), nil
- }))
+ withinlayHints := r.data.Golden(t, "inlayHint", filename, func() ([]byte, error) {
+ return got, nil
+ })
- if withinlayHints != got {
- t.Errorf("inlay hints failed for %s, expected:\n%v\ngot:\n%v", filename, withinlayHints, got)
+ if !bytes.Equal(withinlayHints, got) {
+ t.Errorf("inlay hints failed for %s, expected:\n%s\ngot:\n%s", filename, withinlayHints, got)
}
}
@@ -990,23 +990,24 @@
// Print the name and content of each modified file,
// concatenated, and compare against the golden.
- var got string
+ var buf bytes.Buffer
for i := 0; i < len(res); i++ {
if i != 0 {
- got += "\n"
+ buf.WriteByte('\n')
}
uri := span.URIFromURI(orderedURIs[i])
if len(res) > 1 {
- got += filepath.Base(uri.Filename()) + ":\n"
+ buf.WriteString(filepath.Base(uri.Filename()))
+ buf.WriteString(":\n")
}
- val := res[uri]
- got += val
+ buf.Write(res[uri])
}
- want := string(r.data.Golden(t, tag, filename, func() ([]byte, error) {
- return []byte(got), nil
- }))
- if want != got {
- t.Errorf("rename failed for %s:\n%s", newText, compare.Text(want, got))
+ got := buf.Bytes()
+ want := r.data.Golden(t, tag, filename, func() ([]byte, error) {
+ return got, nil
+ })
+ if diff := compare.Bytes(want, got); diff != "" {
+ t.Errorf("rename failed for %s:\n%s", newText, diff)
}
}
@@ -1055,8 +1056,8 @@
}
}
-func applyTextDocumentEdits(r *runner, edits []protocol.DocumentChanges) (map[span.URI]string, error) {
- res := map[span.URI]string{}
+func applyTextDocumentEdits(r *runner, edits []protocol.DocumentChanges) (map[span.URI][]byte, error) {
+ res := make(map[span.URI][]byte)
for _, docEdits := range edits {
if docEdits.TextDocumentEdit != nil {
uri := docEdits.TextDocumentEdit.TextDocument.URI.SpanURI()
@@ -1064,7 +1065,7 @@
// If we have already edited this file, we use the edited version (rather than the
// file in its original state) so that we preserve our initial changes.
if content, ok := res[uri]; ok {
- m = protocol.NewMapper(uri, []byte(content))
+ m = protocol.NewMapper(uri, content)
} else {
var err error
if m, err = r.data.Mapper(uri); err != nil {
@@ -1245,7 +1246,7 @@
if want == nil {
t.Fatalf("golden file %q not found", uri.Filename())
}
- if diff := compare.Text(got, string(want)); diff != "" {
+ if diff := compare.Bytes(want, got); diff != "" {
t.Errorf("%s mismatch\n%s", command.AddImport, diff)
}
}
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index 9f9d485..c10445b 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -13,6 +13,7 @@
"go/token"
"io/fs"
"os"
+ "path"
"path/filepath"
"reflect"
"regexp"
@@ -28,6 +29,7 @@
"golang.org/x/tools/gopls/internal/lsp/lsprpc"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
+ "golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/lsp/tests"
"golang.org/x/tools/gopls/internal/lsp/tests/compare"
"golang.org/x/tools/internal/jsonrpc2"
@@ -119,6 +121,11 @@
// content matching "hover.md" in the golden data g.
// - loc(name, location): specifies the name for a location in the source. These
// locations may be referenced by other markers.
+// - rename(location, new, golden): specifies a renaming of the
+// identifier at the specified location to the new name.
+// The golden directory contains the transformed files.
+// - renameerr(location, new, wantError): specifies a renaming that
+// fails with an error that matches the expectation.
//
// # Argument conversion
//
@@ -143,6 +150,11 @@
// - name->location: the argument is replaced by the named location.
// - name->Golden: the argument is used to look up golden content prefixed by
// @<argument>.
+// - {string,regexp,identifier}->wantError: a wantError type specifies
+// an expected error message, either in the form of a substring that
+// must be present, a regular expression that it must match, or an
+// identifier (e.g. foo) such that the archive entry @foo
+// exists and contains the exact expected error.
//
// # Example
//
@@ -202,6 +214,7 @@
// Remaining TODO:
// - parallelize/optimize test execution
// - reorganize regtest packages (and rename to just 'test'?)
+// - Rename the files .txtar.
//
// Existing marker tests to port:
// - CallHierarchy
@@ -409,10 +422,12 @@
// Marker funcs should not mutate the test environment (e.g. via opening files
// or applying edits in the editor).
var markerFuncs = map[string]markerFunc{
- "def": makeMarkerFunc(defMarker),
- "diag": makeMarkerFunc(diagMarker),
- "hover": makeMarkerFunc(hoverMarker),
- "loc": makeMarkerFunc(locMarker),
+ "def": makeMarkerFunc(defMarker),
+ "diag": makeMarkerFunc(diagMarker),
+ "hover": makeMarkerFunc(hoverMarker),
+ "loc": makeMarkerFunc(locMarker),
+ "rename": makeMarkerFunc(renameMarker),
+ "renameerr": makeMarkerFunc(renameErrMarker),
}
// markerTest holds all the test data extracted from a test txtar archive.
@@ -444,13 +459,30 @@
return flags
}
-// Golden holds extracted golden content for a single @<name> prefix. The
+func (t *markerTest) getGolden(id string) *Golden {
+ golden, ok := t.golden[id]
+ // If there was no golden content for this identifier, we must create one
+ // to handle the case where -update is set: we need a place to store
+ // the updated content.
+ if !ok {
+ golden = &Golden{id: id}
+
+ // TODO(adonovan): the separation of markerTest (the
+ // static aspects) from markerTestRun (the dynamic
+ // ones) is evidently bogus because here we modify
+ // markerTest during execution. Let's merge the two.
+ t.golden[id] = golden
+ }
+ return golden
+}
+
+// Golden holds extracted golden content for a single @<name> prefix.
//
// When -update is set, golden captures the updated golden contents for later
// writing.
type Golden struct {
id string
- data map[string][]byte
+ data map[string][]byte // key "" => @id itself
updated map[string][]byte
}
@@ -462,9 +494,12 @@
// If -update is set, the given update function will be called to get the
// updated golden content that should be written back to testdata.
//
+// Marker functions must use this method instead of accessing data entries
+// directly otherwise the -update operation will delete those entries.
+//
// TODO(rfindley): rethink the logic here. We may want to separate Get and Set,
// and not delete golden content that isn't set.
-func (g *Golden) Get(t testing.TB, name string, updated []byte) []byte {
+func (g *Golden) Get(t testing.TB, name string, updated []byte) ([]byte, bool) {
if existing, ok := g.updated[name]; ok {
// Multiple tests may reference the same golden data, but if they do they
// must agree about its expected content.
@@ -477,9 +512,11 @@
}
g.updated[name] = updated
if *update {
- return updated
+ return updated, true
}
- return g.data[name]
+
+ res, ok := g.data[name]
+ return res, ok
}
// loadMarkerTests walks the given dir looking for .txt files, which it
@@ -546,10 +583,8 @@
}
case strings.HasPrefix(file.Name, "@"): // golden content
- id, name, ok := cut(file.Name[len("@"):], "/")
- if !ok {
- return nil, fmt.Errorf("golden file path %q must contain '/'", file.Name)
- }
+ id, name, _ := cut(file.Name[len("@"):], "/")
+ // Note that a file.Name of just "@id" gives (id, name) = ("id", "").
if _, ok := test.golden[id]; !ok {
test.golden[id] = &Golden{
id: id,
@@ -590,7 +625,7 @@
updatedGolden := make(map[string][]byte)
for id, g := range test.golden {
for name, data := range g.updated {
- filename := fmt.Sprintf("@%s/%s", id, name)
+ filename := "@" + path.Join(id, name) // name may be ""
updatedGolden[filename] = data
}
}
@@ -692,9 +727,9 @@
run.env.T.Errorf("position %d not in test fileset", pos)
return "<invalid location>"
}
- m := run.env.Editor.Mapper(file.Name())
- if m == nil {
- run.env.T.Errorf("%s is not open", file.Name())
+ m, err := run.env.Editor.Mapper(file.Name())
+ if err != nil {
+ run.env.T.Errorf("%s", err)
return "<invalid location>"
}
loc, err := m.PosLocation(file, pos, pos)
@@ -726,7 +761,11 @@
run.env.T.Errorf("unable to find %s in test archive", loc)
return "<invalid location>"
}
- m := run.env.Editor.Mapper(name)
+ m, err := run.env.Editor.Mapper(name)
+ if err != nil {
+ run.env.T.Errorf("internal error: %v", err)
+ return "<invalid location>"
+ }
s, err := m.LocationSpan(loc)
if err != nil {
run.env.T.Errorf("error formatting location %s: %v", loc, err)
@@ -748,10 +787,6 @@
return fmt.Sprintf("%s:%s (%s:%s)", name, innerSpan, run.test.name, outerSpan)
}
-// converter is the signature of argument converters.
-// A converter should return an error rather than calling marker.errorf().
-type converter func(marker, interface{}) (interface{}, error)
-
// makeMarkerFunc uses reflection to create a markerFunc for the given func value.
func makeMarkerFunc(fn interface{}) markerFunc {
mi := markerFunc{
@@ -773,12 +808,19 @@
return mi
}
+// ---- converters ----
+
+// converter is the signature of argument converters.
+// A converter should return an error rather than calling marker.errorf().
+type converter func(marker, interface{}) (interface{}, error)
+
// Types with special conversions.
var (
- goldenType = reflect.TypeOf(&Golden{})
- locationType = reflect.TypeOf(protocol.Location{})
- markerType = reflect.TypeOf(marker{})
- regexpType = reflect.TypeOf(®exp.Regexp{})
+ goldenType = reflect.TypeOf(&Golden{})
+ locationType = reflect.TypeOf(protocol.Location{})
+ markerType = reflect.TypeOf(marker{})
+ regexpType = reflect.TypeOf(®exp.Regexp{})
+ wantErrorType = reflect.TypeOf(wantError{})
)
func makeConverter(paramType reflect.Type) converter {
@@ -787,6 +829,8 @@
return goldenConverter
case locationType:
return locationConverter
+ case wantErrorType:
+ return wantErrorConverter
default:
return func(_ marker, arg interface{}) (interface{}, error) {
if argType := reflect.TypeOf(arg); argType != paramType {
@@ -863,29 +907,126 @@
if err != nil {
return 0, nil, nil, err
}
- m := run.env.Editor.Mapper(file.Name())
+ m, err := run.env.Editor.Mapper(file.Name())
+ if err != nil {
+ return 0, nil, nil, err
+ }
return startOff, m.Content[startOff:endOff], m, nil
}
-// goldenConverter convers an identifier into the Golden directory of content
+// wantErrorConverter converts a string, regexp, or identifier
+// argument into a wantError. The string is a substring of the
+// expected error, the regexp is a pattern than matches the expected
+// error, and the identifier is a golden file containing the expected
+// error.
+func wantErrorConverter(mark marker, arg interface{}) (interface{}, error) {
+ switch arg := arg.(type) {
+ case string:
+ return wantError{substr: arg}, nil
+ case *regexp.Regexp:
+ return wantError{pattern: arg}, nil
+ case expect.Identifier:
+ golden := mark.run.test.getGolden(string(arg))
+ return wantError{golden: golden}, nil
+ default:
+ return nil, fmt.Errorf("cannot convert %T to wantError (want: string, regexp, or identifier)", arg)
+ }
+}
+
+// A wantError represents an expectation of a specific error message.
+//
+// It may be indicated in one of three ways, in 'expect' notation:
+// - an identifier 'foo', to compare with the contents of the golden section @foo;
+// - a pattern expression re"ab.*c", to match against a regular expression;
+// - a string literal "abc", to check for a substring.
+type wantError struct {
+ golden *Golden
+ pattern *regexp.Regexp
+ substr string
+}
+
+func (we wantError) String() string {
+ if we.golden != nil {
+ return fmt.Sprintf("error from @%s entry", we.golden.id)
+ } else if we.pattern != nil {
+ return fmt.Sprintf("error matching %#q", we.pattern)
+ } else {
+ return fmt.Sprintf("error with substring %q", we.substr)
+ }
+}
+
+// check asserts that 'err' matches the wantError's expectations.
+func (we wantError) check(mark marker, err error) {
+ if err == nil {
+ mark.errorf("@%s succeeded unexpectedly, want %v", mark.note.Name, we)
+ return
+ }
+ got := err.Error()
+
+ if we.golden != nil {
+ // Error message must match @id golden file.
+ wantBytes, ok := we.golden.Get(mark.run.env.T, "", []byte(got))
+ if !ok {
+ mark.errorf("@%s: missing @%s entry", mark.note.Name, we.golden.id)
+ return
+ }
+ want := strings.TrimSpace(string(wantBytes))
+ if got != want {
+ // (ignore leading/trailing space)
+ mark.errorf("@%s failed with wrong error: got:\n%s\nwant:\n%s\ndiff:\n%s",
+ mark.note.Name, got, want, compare.Text(want, got))
+ }
+
+ } else if we.pattern != nil {
+ // Error message must match regular expression pattern.
+ if !we.pattern.MatchString(got) {
+ mark.errorf("got error %q, does not match pattern %#q", got, we.pattern)
+ }
+
+ } else if !strings.Contains(got, we.substr) {
+ // Error message must contain expected substring.
+ mark.errorf("got error %q, want substring %q", got, we.substr)
+ }
+}
+
+// goldenConverter converts an identifier into the Golden directory of content
// prefixed by @<ident> in the test archive file.
func goldenConverter(mark marker, arg interface{}) (interface{}, error) {
switch arg := arg.(type) {
case expect.Identifier:
- golden := mark.run.test.golden[string(arg)]
- // If there was no golden content for this identifier, we must create one
- // to handle the case where -update is set: we need a place to store
- // the updated content.
- if golden == nil {
- golden = new(Golden)
- mark.run.test.golden[string(arg)] = golden
- }
- return golden, nil
+ return mark.run.test.getGolden(string(arg)), nil
default:
return nil, fmt.Errorf("invalid input type %T: golden key must be an identifier", arg)
}
}
+// checkChangedFiles compares the files changed by an operation with their expected (golden) state.
+func checkChangedFiles(mark marker, changed map[string][]byte, golden *Golden) {
+ // Check changed files match expectations.
+ for filename, got := range changed {
+ if want, ok := golden.Get(mark.run.env.T, filename, got); !ok {
+ mark.errorf("%s: unexpected change to file %s; got:\n%s",
+ mark.note.Name, filename, got)
+
+ } else if string(got) != string(want) {
+ mark.errorf("%s: wrong file content for %s: got:\n%s\nwant:\n%s\ndiff:\n%s",
+ mark.note.Name, filename, got, want,
+ compare.Bytes(want, got))
+ }
+ }
+
+ // Report unmet expectations.
+ for filename := range golden.data {
+ if _, ok := changed[filename]; !ok {
+ want, _ := golden.Get(mark.run.env.T, filename, nil)
+ mark.errorf("%s: missing change to file %s; want:\n%s",
+ mark.note.Name, filename, want)
+ }
+ }
+}
+
+// ---- marker functions ----
+
// defMarker implements the @godef marker, running textDocument/definition at
// the given src location and asserting that there is exactly one resulting
// location, matching dst.
@@ -914,7 +1055,8 @@
}
wantMD := ""
if golden != nil {
- wantMD = string(golden.Get(mark.run.env.T, "hover.md", []byte(gotMD)))
+ wantBytes, _ := golden.Get(mark.run.env.T, "hover.md", []byte(gotMD))
+ wantMD = string(wantBytes)
}
// Normalize newline termination: archive files can't express non-newline
// terminated files.
@@ -926,14 +1068,14 @@
}
}
-// locMarker implements the @loc hover marker. It is executed before other
+// locMarker implements the @loc marker. It is executed before other
// markers, so that locations are available.
func locMarker(mark marker, name expect.Identifier, loc protocol.Location) {
mark.run.locations[name] = loc
}
// diagMarker implements the @diag hover marker. It eliminates diagnostics from
-// the observed set in the m.file.
+// the observed set in mark.test.
func diagMarker(mark marker, loc protocol.Location, re *regexp.Regexp) {
idx := -1
diags := mark.run.diags[loc]
@@ -949,3 +1091,69 @@
mark.errorf("no diagnostic matches %q", re)
}
}
+
+// renameMarker implements the @rename(location, new, golden) marker.
+func renameMarker(mark marker, loc protocol.Location, newName expect.Identifier, golden *Golden) {
+ changed, err := rename(mark.run.env, loc, string(newName))
+ if err != nil {
+ mark.errorf("rename failed: %v. (Use @renameerr for expected errors.)", err)
+ return
+ }
+ checkChangedFiles(mark, changed, golden)
+}
+
+// renameErrMarker implements the @renamererr(location, new, error) marker.
+func renameErrMarker(mark marker, loc protocol.Location, newName expect.Identifier, wantErr wantError) {
+ _, err := rename(mark.run.env, loc, string(newName))
+ wantErr.check(mark, err)
+}
+
+// rename returns the new contents of the files that would be modified
+// by renaming the identifier at loc to newName.
+func rename(env *Env, loc protocol.Location, newName string) (map[string][]byte, error) {
+ // We call Server.Rename directly, instead of
+ // env.Editor.Rename(env.Ctx, loc, newName)
+ // to isolate Rename from PrepareRename, and because we don't
+ // want to modify the file system in a scenario with multiple
+ // @rename markers.
+
+ editMap, err := env.Editor.Server.Rename(env.Ctx, &protocol.RenameParams{
+ TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},
+ Position: loc.Range.Start,
+ NewName: string(newName),
+ })
+ if err != nil {
+ return nil, err
+ }
+
+ // Apply the edits to the Editor buffers
+ // and return the contents of the changed files.
+ result := make(map[string][]byte)
+ for _, change := range editMap.DocumentChanges {
+ if change.RenameFile != nil {
+ // rename
+ oldFile := env.Sandbox.Workdir.URIToPath(change.RenameFile.OldURI)
+ newFile := env.Sandbox.Workdir.URIToPath(change.RenameFile.NewURI)
+ mapper, err := env.Editor.Mapper(oldFile)
+ if err != nil {
+ return nil, err
+ }
+ result[newFile] = mapper.Content
+
+ } else {
+ // edit
+ filename := env.Sandbox.Workdir.URIToPath(change.TextDocumentEdit.TextDocument.URI)
+ mapper, err := env.Editor.Mapper(filename)
+ if err != nil {
+ return nil, err
+ }
+ patched, _, err := source.ApplyProtocolEdits(mapper, change.TextDocumentEdit.Edits)
+ if err != nil {
+ return nil, err
+ }
+ result[filename] = patched
+ }
+ }
+
+ return result, nil
+}
diff --git a/gopls/internal/lsp/source/format.go b/gopls/internal/lsp/source/format.go
index db843c3..10120c2 100644
--- a/gopls/internal/lsp/source/format.go
+++ b/gopls/internal/lsp/source/format.go
@@ -381,11 +381,11 @@
// ApplyProtocolEdits applies the patch (edits) to m.Content and returns the result.
// It also returns the edits converted to diff-package form.
-func ApplyProtocolEdits(m *protocol.Mapper, edits []protocol.TextEdit) (string, []diff.Edit, error) {
+func ApplyProtocolEdits(m *protocol.Mapper, edits []protocol.TextEdit) ([]byte, []diff.Edit, error) {
diffEdits, err := FromProtocolEdits(m, edits)
if err != nil {
- return "", nil, err
+ return nil, nil, err
}
- out, err := diff.Apply(string(m.Content), diffEdits)
+ out, err := diff.ApplyBytes(m.Content, diffEdits)
return out, diffEdits, err
}
diff --git a/gopls/internal/lsp/tests/compare/text.go b/gopls/internal/lsp/tests/compare/text.go
index f17cc39..4ce2f8c 100644
--- a/gopls/internal/lsp/tests/compare/text.go
+++ b/gopls/internal/lsp/tests/compare/text.go
@@ -5,6 +5,8 @@
package compare
import (
+ "bytes"
+
"golang.org/x/tools/internal/diff"
)
@@ -37,3 +39,11 @@
return unified
}
+
+// Bytes is like Text but using byte slices.
+func Bytes(want, got []byte) string {
+ if bytes.Equal(want, got) {
+ return "" // common case
+ }
+ return Text(string(want), string(got))
+}
diff --git a/gopls/internal/regtest/marker/testdata/rename/basic.txt b/gopls/internal/regtest/marker/testdata/rename/basic.txt
new file mode 100644
index 0000000..fe723cf
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/rename/basic.txt
@@ -0,0 +1,22 @@
+This test performs basic coverage of 'rename' within a single package.
+
+-- basic.go --
+package p
+
+func f(x int) { println(x) } //@rename("x", y, param_x)
+
+-- @param_x/basic.go --
+package p
+
+func f(y int) { println(y) } //@rename("x", y, param_x)
+
+-- errors.go --
+package p
+
+func _(x []int) { //@renameerr("_", blank, `can't rename "_"`)
+ x = append(x, 1) //@renameerr("append", blank, "built in and cannot be renamed")
+ x = nil //@renameerr("nil", blank, "built in and cannot be renamed")
+ x = nil //@renameerr("x", x, "old and new names are the same: x")
+ _ = 1 //@renameerr("1", x, "no identifier found")
+}
+
diff --git a/gopls/internal/regtest/marker/testdata/rename/conflict.txt b/gopls/internal/regtest/marker/testdata/rename/conflict.txt
new file mode 100644
index 0000000..18438c8
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/rename/conflict.txt
@@ -0,0 +1,59 @@
+This test exercises some renaming conflict scenarios
+and ensures that the errors are informative.
+
+-- go.mod --
+module example.com
+go 1.12
+
+-- super/p.go --
+package super
+
+var x int
+
+func f(y int) {
+ println(x)
+ println(y) //@renameerr("y", x, errSuperBlockConflict)
+}
+
+-- @errSuperBlockConflict --
+super/p.go:5:8: renaming this var "y" to "x"
+super/p.go:6:10: would shadow this reference
+super/p.go:3:5: to the var declared here
+-- sub/p.go --
+package sub
+
+var a int
+
+func f2(b int) {
+ println(a) //@renameerr("a", b, errSubBlockConflict)
+ println(b)
+}
+
+-- @errSubBlockConflict --
+sub/p.go:3:5: renaming this var "a" to "b"
+sub/p.go:6:10: would cause this reference to become shadowed
+sub/p.go:5:9: by this intervening var definition
+-- pkgname/p.go --
+package pkgname
+
+import e1 "errors" //@renameerr("e1", errors, errImportConflict)
+import "errors"
+
+var _ = errors.New
+var _ = e1.New
+
+-- @errImportConflict --
+pkgname/p.go:3:8: renaming this imported package name "e1" to "errors"
+pkgname/p.go:4:8: conflicts with imported package name in same block
+-- pkgname2/p1.go --
+package pkgname2
+var x int
+
+-- pkgname2/p2.go --
+package pkgname2
+import "errors" //@renameerr("errors", x, errImportConflict2)
+var _ = errors.New
+
+-- @errImportConflict2 --
+pkgname2/p2.go:2:8: renaming this imported package name "errors" to "x" would conflict
+pkgname2/p1.go:2:5: with this package member var
diff --git a/gopls/internal/regtest/marker/testdata/rename/embed.txt b/gopls/internal/regtest/marker/testdata/rename/embed.txt
new file mode 100644
index 0000000..68cf771
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/rename/embed.txt
@@ -0,0 +1,36 @@
+This test exercises renaming of types used as embedded fields.
+
+-- go.mod --
+module example.com
+go 1.12
+
+-- a/a.go --
+package a
+
+type A int //@rename("A", A2, type)
+
+-- b/b.go --
+package b
+
+import "example.com/a"
+
+type B struct { a.A } //@renameerr("A", A3, errAnonField)
+
+var _ = new(B).A //@renameerr("A", A4, errAnonField)
+
+-- @errAnonField --
+can't rename embedded fields: rename the type directly or name the field
+-- @type/a/a.go --
+package a
+
+type A2 int //@rename("A", A2, type)
+
+-- @type/b/b.go --
+package b
+
+import "example.com/a"
+
+type B struct { a.A2 } //@renameerr("A", A3, errAnonField)
+
+var _ = new(B).A2 //@renameerr("A", A4, errAnonField)
+
diff --git a/gopls/internal/regtest/marker/testdata/rename/methods.txt b/gopls/internal/regtest/marker/testdata/rename/methods.txt
new file mode 100644
index 0000000..1bd985b
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/rename/methods.txt
@@ -0,0 +1,67 @@
+This test exercises renaming of interface methods.
+
+The golden is currently wrong due to https://github.com/golang/go/issues/58506:
+the reference to B.F in package b should be renamed too.
+
+-- go.mod --
+module example.com
+go 1.12
+
+-- a/a.go --
+package a
+
+type A int
+
+func (A) F() {} //@renameerr("F", G, errAfToG)
+
+-- b/b.go --
+package b
+
+import "example.com/a"
+import "example.com/c"
+
+type B interface { F() } //@rename("F", G, BfToG)
+
+var _ B = a.A(0)
+var _ B = c.C(0)
+
+-- c/c.go --
+package c
+
+type C int
+
+func (C) F() {} //@renameerr("F", G, errCfToG)
+
+-- d/d.go --
+package d
+
+import "example.com/b"
+
+var _ = b.B.F
+
+-- @errAfToG --
+a/a.go:5:10: renaming this method "F" to "G"
+b/b.go:6:6: would make example.com/a.A no longer assignable to interface B
+b/b.go:6:20: (rename example.com/b.B.F if you intend to change both types)
+-- @BfToG/b/b.go --
+package b
+
+import "example.com/a"
+import "example.com/c"
+
+type B interface { G() } //@rename("F", G, BfToG)
+
+var _ B = a.A(0)
+var _ B = c.C(0)
+
+-- @BfToG/d/d.go --
+package d
+
+import "example.com/b"
+
+var _ = b.B.G
+
+-- @errCfToG --
+c/c.go:5:10: renaming this method "F" to "G"
+b/b.go:6:6: would make example.com/c.C no longer assignable to interface B
+b/b.go:6:20: (rename example.com/b.B.F if you intend to change both types)
diff --git a/gopls/internal/regtest/marker/testdata/rename/typeswitch.txt b/gopls/internal/regtest/marker/testdata/rename/typeswitch.txt
new file mode 100644
index 0000000..6743b99
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/rename/typeswitch.txt
@@ -0,0 +1,26 @@
+This test covers the special case of renaming a type switch var.
+
+-- p.go --
+package p
+
+func _(x interface{}) {
+ switch y := x.(type) { //@rename("y", z, yToZ)
+ case string:
+ print(y) //@rename("y", z, yToZ)
+ default:
+ print(y) //@rename("y", z, yToZ)
+ }
+}
+
+-- @yToZ/p.go --
+package p
+
+func _(x interface{}) {
+ switch z := x.(type) { //@rename("y", z, yToZ)
+ case string:
+ print(z) //@rename("y", z, yToZ)
+ default:
+ print(z) //@rename("y", z, yToZ)
+ }
+}
+
diff --git a/internal/diff/diff.go b/internal/diff/diff.go
index 3b31505..2bc63c2 100644
--- a/internal/diff/diff.go
+++ b/internal/diff/diff.go
@@ -52,6 +52,13 @@
return string(out), nil
}
+// ApplyBytes is like Apply, but it accepts a byte slice.
+// The result is always a new array.
+func ApplyBytes(src []byte, edits []Edit) ([]byte, error) {
+ res, err := Apply(string(src), edits)
+ return []byte(res), err
+}
+
// validate checks that edits are consistent with src,
// and returns the size of the patched output.
// It may return a different slice.
