From 6dfe7efaa95e7279f548deb2e1730469c462b6c8 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Wed, 14 Nov 2018 21:11:16 -0500 Subject: [PATCH] internal/lsp: add definition tests from godef This makes our internal version of go to definition be tested with the same test data that godef now uses Change-Id: I04e488b6b9b2d891181f202ea1125b823a079c50 Reviewed-on: https://go-review.googlesource.com/c/150045 Reviewed-by: Rebecca Stambler --- go/packages/packagestest/expect.go | 5 +++ internal/lsp/lsp_test.go | 42 ++++++++++++++++++- internal/lsp/position.go | 8 ++-- internal/lsp/server.go | 2 +- internal/lsp/testdata/godef/a/a.go | 11 +++++ internal/lsp/testdata/godef/a/random.go | 23 ++++++++++ internal/lsp/testdata/godef/b/b.go | 23 ++++++++++ internal/lsp/testdata/godef/b/c.go | 8 ++++ internal/lsp/testdata/godef/b/c.go.saved | 7 ++++ .../testdata/godef/broken/unclosedIf.go.in | 11 +++++ 10 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 internal/lsp/testdata/godef/a/a.go create mode 100644 internal/lsp/testdata/godef/a/random.go create mode 100644 internal/lsp/testdata/godef/b/b.go create mode 100644 internal/lsp/testdata/godef/b/c.go create mode 100644 internal/lsp/testdata/godef/b/c.go.saved create mode 100644 internal/lsp/testdata/godef/broken/unclosedIf.go.in diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index 960f1151..69d30629 100644 --- a/go/packages/packagestest/expect.go +++ b/go/packages/packagestest/expect.go @@ -164,6 +164,7 @@ var ( posType = reflect.TypeOf(token.Pos(0)) positionType = reflect.TypeOf(token.Position{}) rangeType = reflect.TypeOf(Range{}) + fsetType = reflect.TypeOf((*token.FileSet)(nil)) ) // converter converts from a marker's argument parsed from the comment to @@ -190,6 +191,10 @@ func (e *Exported) buildConverter(pt reflect.Type) (converter, error) { return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { return reflect.ValueOf(n), args, nil }, nil + case pt == fsetType: + return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { + return reflect.ValueOf(e.fset), args, nil + }, nil case pt == posType: return func(n *expect.Note, args []interface{}) (reflect.Value, []interface{}, error) { r, remains, err := e.rangeConverter(n, args) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 422c096b..2b3710bc 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -29,8 +29,9 @@ func TestLSP(t *testing.T) { func testLSP(t *testing.T, exporter packagestest.Exporter) { const dir = "testdata" const expectedCompletionsCount = 4 - const expectedDiagnosticsCount = 7 + const expectedDiagnosticsCount = 9 const expectedFormatCount = 3 + const expectedDefinitionsCount = 16 files := packagestest.MustCopyFileTree(dir) for fragment, operation := range files { @@ -55,6 +56,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { completionItems := make(completionItems) expectedCompletions := make(completions) expectedFormat := make(formats) + expectedDefinitions := make(definitions) s := &server{ view: source.NewView(), @@ -89,6 +91,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { "item": completionItems.collect, "complete": expectedCompletions.collect, "format": expectedFormat.collect, + "godef": expectedDefinitions.collect, }); err != nil { t.Fatal(err) } @@ -116,12 +119,21 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { } expectedFormat.test(t, s) }) + + t.Run("Definitions", func(t *testing.T) { + t.Helper() + if len(expectedDefinitions) != expectedDefinitionsCount { + t.Errorf("got %v definitions expected %v", len(expectedDefinitions), expectedDefinitionsCount) + } + expectedDefinitions.test(t, s) + }) } type diagnostics map[string][]protocol.Diagnostic type completionItems map[token.Pos]*protocol.CompletionItem type completions map[token.Position][]token.Pos type formats map[string]string +type definitions map[protocol.Location]protocol.Location func (c completions) test(t *testing.T, exported *packagestest.Exported, s *server, items completionItems) { for src, itemList := range c { @@ -272,3 +284,31 @@ func (f formats) collect(pos token.Position) { cmd.Run() // ignore error, sometimes we have intentionally ungofmt-able files f[pos.Filename] = stdout.String() } + +func (d definitions) test(t *testing.T, s *server) { + for src, target := range d { + locs, err := s.Definition(context.Background(), &protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: src.URI, + }, + Position: src.Range.Start, + }) + if err != nil { + t.Fatal(err) + } + if len(locs) != 1 { + t.Errorf("got %d locations for definition, expected 1", len(locs)) + } + if locs[0] != target { + t.Errorf("for %v got %v want %v", src, locs[0], target) + } + } +} + +func (d definitions) collect(fset *token.FileSet, src, target packagestest.Range) { + sRange := source.Range{Start: src.Start, End: src.End} + sLoc := toProtocolLocation(fset, sRange) + tRange := source.Range{Start: target.Start, End: target.End} + tLoc := toProtocolLocation(fset, tRange) + d[sLoc] = tLoc +} diff --git a/internal/lsp/position.go b/internal/lsp/position.go index de4ce674..e4b70fb8 100644 --- a/internal/lsp/position.go +++ b/internal/lsp/position.go @@ -24,11 +24,11 @@ func fromProtocolLocation(v *source.View, loc protocol.Location) (source.Range, } // toProtocolLocation converts from a source range back to a protocol location. -func toProtocolLocation(v *source.View, r source.Range) protocol.Location { - tokFile := v.Config.Fset.File(r.Start) - file := v.GetFile(source.ToURI(tokFile.Name())) +func toProtocolLocation(fset *token.FileSet, r source.Range) protocol.Location { + tokFile := fset.File(r.Start) + uri := source.ToURI(tokFile.Name()) return protocol.Location{ - URI: protocol.DocumentURI(file.URI), + URI: protocol.DocumentURI(uri), Range: toProtocolRange(tokFile, r), } } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 00c90756..ad51fa4d 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -204,7 +204,7 @@ func (s *server) Definition(ctx context.Context, params *protocol.TextDocumentPo if err != nil { return nil, err } - return []protocol.Location{toProtocolLocation(s.view, r)}, nil + return []protocol.Location{toProtocolLocation(s.view.Config.Fset, r)}, nil } func (s *server) TypeDefinition(context.Context, *protocol.TextDocumentPositionParams) ([]protocol.Location, error) { diff --git a/internal/lsp/testdata/godef/a/a.go b/internal/lsp/testdata/godef/a/a.go new file mode 100644 index 00000000..e9c96744 --- /dev/null +++ b/internal/lsp/testdata/godef/a/a.go @@ -0,0 +1,11 @@ +// A comment just to push the positions out + +package a + +type A string //@A + +func Stuff() { //@Stuff + x := 5 + Random2(x) //@godef("dom2", Random2) + Random() //@godef("()", Random) +} diff --git a/internal/lsp/testdata/godef/a/random.go b/internal/lsp/testdata/godef/a/random.go new file mode 100644 index 00000000..5dcb6efb --- /dev/null +++ b/internal/lsp/testdata/godef/a/random.go @@ -0,0 +1,23 @@ +package a + +func Random() int { //@Random + y := 6 + 7 + return y +} + +func Random2(y int) int { //@Random2,mark(RandomParamY, "y") + return y //@godef("y", RandomParamY) +} + +type Pos struct { + x, y int //@mark(PosX, "x"),mark(PosY, "y") +} + +func (p *Pos) Sum() int { //@mark(PosSum, "Sum") + return p.x + p.y //@godef("x", PosX) +} + +func _() { + var p Pos + _ = p.Sum() //@godef("()", PosSum) +} diff --git a/internal/lsp/testdata/godef/b/b.go b/internal/lsp/testdata/godef/b/b.go new file mode 100644 index 00000000..ae608156 --- /dev/null +++ b/internal/lsp/testdata/godef/b/b.go @@ -0,0 +1,23 @@ +package b + +import "golang.org/x/tools/internal/lsp/godef/a" + +type S1 struct { //@S1 + F1 int //@mark(S1F1, "F1") + S2 //@godef("S2", S2), mark(S1S2, "S2") + a.A //@godef("A", A) +} + +type S2 struct { //@S2 + F1 string //@mark(S2F1, "F1") + F2 int //@mark(S2F2, "F2") +} + +func Bar() { + a.Stuff() //@godef("Stuff", Stuff) + var x S1 //@godef("S1", S1) + _ = x.S2 //@godef("S2", S1S2) + _ = x.F1 //@godef("F1", S1F1) + _ = x.F2 //@godef("F2", S2F2) + _ = x.S2.F1 //@godef("F1", S2F1) +} diff --git a/internal/lsp/testdata/godef/b/c.go b/internal/lsp/testdata/godef/b/c.go new file mode 100644 index 00000000..c8daf624 --- /dev/null +++ b/internal/lsp/testdata/godef/b/c.go @@ -0,0 +1,8 @@ +package b + +// This is the in-editor version of the file. +// The on-disk version is in c.go.saved. + +var _ = S1{ //@godef("S1", S1) + F1: 99, //@godef("F1", S1F1) +} diff --git a/internal/lsp/testdata/godef/b/c.go.saved b/internal/lsp/testdata/godef/b/c.go.saved new file mode 100644 index 00000000..ff1a8794 --- /dev/null +++ b/internal/lsp/testdata/godef/b/c.go.saved @@ -0,0 +1,7 @@ +package b + +// This is the on-disk version of c.go, which represents +// the in-editor version of the file. + +} + diff --git a/internal/lsp/testdata/godef/broken/unclosedIf.go.in b/internal/lsp/testdata/godef/broken/unclosedIf.go.in new file mode 100644 index 00000000..d29b108a --- /dev/null +++ b/internal/lsp/testdata/godef/broken/unclosedIf.go.in @@ -0,0 +1,11 @@ +package broken + +import "fmt" + +func unclosedIf() { + if false { + var myUnclosedIf string //@myUnclosedIf + fmt.Printf("s = %v\n", myUnclosedIf) //@godef("my", myUnclosedIf) +} +//@diag(EOF, "expected ';', found 'EOF'") +//@diag(EOF, "expected '}', found 'EOF'")