internal/lsp: fix and re-enable godef tests

None of the godef tests were running due to a mistake in the test
harness code. Fix them and re-enable.

We decided that the range for an import statement should be the whole
import path, not just the first character, so make that change and
adjust the PrepareRename tests accordingly.

Change-Id: I45756a78f2a1beb3c5180b5f288ce078075624bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207900
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Heschi Kreinick 2019-11-19 14:51:46 -05:00
parent 17847a8424
commit 4bf2f4069d
12 changed files with 112 additions and 61 deletions

View File

@ -408,7 +408,9 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if len(locs) != 1 {
t.Errorf("got %d locations for definition, expected 1", len(locs))
}
didSomething := false
if hover != nil {
didSomething = true
tag := fmt.Sprintf("%s-hover", d.Name)
expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover.Contents.Value), nil
@ -416,7 +418,9 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if hover.Contents.Value != expectHover {
t.Errorf("for %v got %q want %q", d.Src, hover.Contents.Value, expectHover)
}
} else if !d.OnlyHover {
}
if !d.OnlyHover {
didSomething = true
locURI := span.NewURI(locs[0].URI)
lm, err := r.data.Mapper(locURI)
if err != nil {
@ -427,7 +431,8 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
} else if def != d.Def {
t.Errorf("for %v got %v want %v", d.Src, def, d.Def)
}
} else {
}
if !didSomething {
t.Errorf("no tests ran for %s", d.Src.URI())
}
}
@ -638,8 +643,16 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare
if !ok {
t.Fatalf("got %T, wanted Range", got)
}
if protocol.CompareRange(*xx, want.Range) != 0 {
t.Errorf("prepare rename failed: incorrect range got %v want %v", *xx, want.Range)
if xx.Start == xx.End {
// Special case for 0-length ranges. Marks can't specify a 0-length range,
// so just compare the start.
if xx.Start != want.Range.Start {
t.Errorf("prepare rename failed: incorrect point, got %v want %v", xx.Start, want.Range.Start)
}
} else {
if protocol.CompareRange(*xx, want.Range) != 0 {
t.Errorf("prepare rename failed: incorrect range got %v want %v", *xx, want.Range)
}
}
}

View File

@ -32,10 +32,9 @@ type IdentifierInfo struct {
Declaration Declaration
pkg Package
ident *ast.Ident
wasEmbeddedField bool
qf types.Qualifier
pkg Package
ident *ast.Ident
qf types.Qualifier
}
type Declaration struct {
@ -127,9 +126,10 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F
if result.ident == nil {
return nil, nil
}
var wasEmbeddedField bool
for _, n := range path[1:] {
if field, ok := n.(*ast.Field); ok {
result.wasEmbeddedField = len(field.Names) == 0
wasEmbeddedField = len(field.Names) == 0
break
}
}
@ -171,7 +171,7 @@ func identifier(ctx context.Context, snapshot Snapshot, pkg Package, file *ast.F
return result, nil
}
if result.wasEmbeddedField {
if wasEmbeddedField {
// The original position was on the embedded field declaration, so we
// try to dig out the type and jump to that instead.
if v, ok := result.Declaration.obj.(*types.Var); ok {

View File

@ -54,7 +54,16 @@ func (i *IdentifierInfo) PrepareRename(ctx context.Context) (*PrepareItem, error
if err != nil {
return nil, err
}
i = ident
rng, err := ident.mappedRange.Range()
if err != nil {
return nil, err
}
// We're not really renaming the import path.
rng.End = rng.Start
return &PrepareItem{
Range: rng,
Text: ident.Name,
}, nil
}
// Do not rename builtin identifiers.
@ -220,14 +229,13 @@ func getPkgNameIdentifier(ctx context.Context, ident *IdentifierInfo, pkgName *t
return nil, err
}
return &IdentifierInfo{
Snapshot: ident.Snapshot,
Name: pkgName.Name(),
mappedRange: decl.mappedRange,
File: ident.File,
Declaration: decl,
pkg: ident.pkg,
wasEmbeddedField: false,
qf: ident.qf,
Snapshot: ident.Snapshot,
Name: pkgName.Name(),
mappedRange: decl.mappedRange,
File: ident.File,
Declaration: decl,
pkg: ident.pkg,
qf: ident.qf,
}, nil
}

View File

@ -510,7 +510,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
hover += h.Synopsis + "\n"
}
hover += h.Signature
rng, err := ident.Range()
rng, err := ident.Declaration.Range()
if err != nil {
t.Fatal(err)
}
@ -521,7 +521,9 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
}
hover = ""
}
didSomething := false
if hover != "" {
didSomething = true
tag := fmt.Sprintf("%s-hover", d.Name)
expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover), nil
@ -529,13 +531,16 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if hover != expectHover {
t.Errorf("for %v got %q want %q", d.Src, hover, expectHover)
}
} else if !d.OnlyHover {
}
if !d.OnlyHover {
didSomething = true
if _, defRng, err := spanToRange(r.data, d.Def); err != nil {
t.Fatal(err)
} else if rng != defRng {
t.Errorf("for %v got %v want %v", d.Src, rng, d.Def)
t.Errorf("for %v got %v want %v", d.Src, rng, defRng)
}
} else {
}
if !didSomething {
t.Errorf("no tests ran for %s", d.Src.URI())
}
}
@ -774,8 +779,16 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare
t.Errorf("prepare rename failed for %v: expected nil, got %v", src, item)
return
}
if protocol.CompareRange(want.Range, item.Range) != 0 {
t.Errorf("prepare rename failed: incorrect range got %v want %v", item.Range, want.Range)
if item.Range.Start == item.Range.End {
// Special case for 0-length ranges. Marks can't specify a 0-length range,
// so just compare the start.
if item.Range.Start != want.Range.Start {
t.Errorf("prepare rename failed: incorrect point, got %v want %v", item.Range.Start, want.Range.Start)
}
} else {
if protocol.CompareRange(item.Range, want.Range) != 0 {
t.Errorf("prepare rename failed: incorrect range got %v want %v", item.Range, want.Range)
}
}
}

View File

@ -138,9 +138,9 @@ func objToMappedRange(ctx context.Context, v View, pkg Package, obj types.Object
// import a "go/ast" // name "a" does not match package name
//
// When the identifier does not appear in the source, have the range
// of the object be the point at the beginning of the declaration.
// of the object be the import path, including quotes.
if pkgName.Imported().Name() == pkgName.Name() {
return nameToMappedRange(ctx, v, pkg, obj.Pos(), "")
return posToMappedRange(ctx, v, pkg, obj.Pos(), obj.Pos()+token.Pos(len(pkgName.Imported().Path())+2))
}
}
return nameToMappedRange(ctx, v, pkg, obj.Pos(), obj.Name())

View File

@ -6,7 +6,7 @@ import "fmt"
type A string //@A
func Stuff() { //@Stuff
func AStuff() { //@AStuff
x := 5
Random2(x) //@godef("dom2", Random2)
Random() //@godef("()", Random)

View File

@ -51,12 +51,12 @@ godef/a/a.go:14:6-9: defined here as var err error
"start": {
"line": 14,
"column": 6,
"offset": 201
"offset": 203
},
"end": {
"line": 14,
"column": 9,
"offset": 204
"offset": 206
}
},
"description": "var err error"

View File

@ -1,8 +1,8 @@
package b
import (
myFoo "golang.org/x/tools/internal/lsp/foo" //@mark(myFoo, "myFoo"),godef("foo", PackageFoo),godef("myFoo", myFoo)
"golang.org/x/tools/internal/lsp/godef/a" //@mark(AImport, "\"")
myFoo "golang.org/x/tools/internal/lsp/foo" //@mark(myFoo, "myFoo"),godef("myFoo", myFoo)
"golang.org/x/tools/internal/lsp/godef/a" //@mark(AImport, re"\".*\"")
)
type S1 struct { //@S1
@ -24,7 +24,7 @@ type S3 struct {
}
func Bar() {
a.Stuff() //@godef("Stuff", Stuff)
a.AStuff() //@godef("AStuff", AStuff)
var x S1 //@godef("S1", S1)
_ = x.S2 //@godef("S2", S1S2)
_ = x.F1 //@godef("F1", S1F1)

View File

@ -22,7 +22,7 @@ godef/a/a.go:7:6-7: defined here as A string //@A
A string //@A
-- AImport-definition --
godef/b/b.go:5:2: defined here as package a ("golang.org/x/tools/internal/lsp/godef/a")
godef/b/b.go:5:2-43: defined here as package a ("golang.org/x/tools/internal/lsp/godef/a")
-- AImport-definition-json --
{
"span": {
@ -30,12 +30,12 @@ godef/b/b.go:5:2: defined here as package a ("golang.org/x/tools/internal/lsp/go
"start": {
"line": 5,
"column": 2,
"offset": 137
"offset": 112
},
"end": {
"line": 5,
"column": 2,
"offset": 137
"column": 43,
"offset": 153
}
},
"description": "package a (\"golang.org/x/tools/internal/lsp/godef/a\")"
@ -43,6 +43,28 @@ godef/b/b.go:5:2: defined here as package a ("golang.org/x/tools/internal/lsp/go
-- AImport-hover --
package a ("golang.org/x/tools/internal/lsp/godef/a")
-- AStuff-definition --
godef/a/a.go:9:6-12: defined here as func a.AStuff()
-- AStuff-definition-json --
{
"span": {
"uri": "file://godef/a/a.go",
"start": {
"line": 9,
"column": 6,
"offset": 95
},
"end": {
"line": 9,
"column": 12,
"offset": 101
}
},
"description": "func a.AStuff()"
}
-- AStuff-hover --
func a.AStuff()
-- PackageFoo-definition --
foo/foo.go:1:1-30:16: defined here as myFoo "golang.org/x/tools/internal/lsp/foo" //@mark(myFoo, "myFoo"),godef("foo", PackageFoo),godef("myFoo", myFoo)
-- PackageFoo-definition-json --
@ -79,12 +101,12 @@ godef/b/b.go:8:6-8: defined here as S1 struct {
"start": {
"line": 8,
"column": 6,
"offset": 212
"offset": 193
},
"end": {
"line": 8,
"column": 8,
"offset": 214
"offset": 195
}
},
"description": "S1 struct {\n\tF1 int //@mark(S1F1, \"F1\")\n\tS2 //@godef(\"S2\", S2), mark(S1S2, \"S2\")\n\ta.A //@godef(\"A\", A)\n}"
@ -106,12 +128,12 @@ field F1 int
"start": {
"line": 9,
"column": 2,
"offset": 231
"offset": 212
},
"end": {
"line": 9,
"column": 4,
"offset": 233
"offset": 214
}
},
"description": "@mark(S1F1, \"F1\")\nfield F1 int"
@ -130,12 +152,12 @@ field S2 S2
"start": {
"line": 10,
"column": 2,
"offset": 260
"offset": 241
},
"end": {
"line": 10,
"column": 4,
"offset": 262
"offset": 243
}
},
"description": "@godef(\"S2\", S2), mark(S1S2, \"S2\")\nfield S2 S2"
@ -157,12 +179,12 @@ godef/b/b.go:14:6-8: defined here as S2 struct {
"start": {
"line": 14,
"column": 6,
"offset": 339
"offset": 320
},
"end": {
"line": 14,
"column": 8,
"offset": 341
"offset": 322
}
},
"description": "S2 struct {\n\tF1 string //@mark(S2F1, \"F1\")\n\tF2 int //@mark(S2F2, \"F2\")\n\t*a.A //@godef(\"A\", A),godef(\"a\",AImport)\n}"
@ -184,12 +206,12 @@ field F1 string
"start": {
"line": 15,
"column": 2,
"offset": 358
"offset": 339
},
"end": {
"line": 15,
"column": 4,
"offset": 360
"offset": 341
}
},
"description": "@mark(S2F1, \"F1\")\nfield F1 string"
@ -208,12 +230,12 @@ field F2 int
"start": {
"line": 16,
"column": 2,
"offset": 391
"offset": 372
},
"end": {
"line": 16,
"column": 4,
"offset": 393
"offset": 374
}
},
"description": "@mark(S2F2, \"F2\")\nfield F2 int"
@ -253,12 +275,12 @@ godef/b/b.go:37:7-8: defined here as const X untyped int = 0
"start": {
"line": 37,
"column": 7,
"offset": 812
"offset": 795
},
"end": {
"line": 37,
"column": 8,
"offset": 813
"offset": 796
}
},
"description": "const X untyped int = 0"

View File

@ -11,12 +11,12 @@ godef/b/b.go:8:6-8: defined here as S1 struct {
"start": {
"line": 8,
"column": 6,
"offset": 212
"offset": 193
},
"end": {
"line": 8,
"column": 8,
"offset": 214
"offset": 195
}
},
"description": "S1 struct {\n\tF1 int //@mark(S1F1, \"F1\")\n\tS2 //@godef(\"S2\", S2), mark(S1S2, \"S2\")\n\ta.A //@godef(\"A\", A)\n}"
@ -38,12 +38,12 @@ field F1 int
"start": {
"line": 9,
"column": 2,
"offset": 231
"offset": 212
},
"end": {
"line": 9,
"column": 4,
"offset": 233
"offset": 214
}
},
"description": "@mark(S1F1, \"F1\")\nfield F1 int"

View File

@ -11,7 +11,7 @@ FoldingRangesCount = 2
FormatCount = 6
ImportCount = 7
SuggestedFixCount = 1
DefinitionsCount = 38
DefinitionsCount = 37
TypeDefinitionsCount = 2
HighlightsCount = 12
ReferencesCount = 7

View File

@ -840,11 +840,6 @@ func (data *Data) collectRenames(src span.Span, newText string) {
}
func (data *Data) collectPrepareRenames(src span.Span, rng span.Range, placeholder string) {
if int(rng.End-rng.Start) != len(placeholder) {
// If the length of the placeholder and the length of the range do not match,
// make the range just be the start.
rng = span.NewRange(rng.FileSet, rng.Start, rng.Start)
}
m, err := data.Mapper(src.URI())
if err != nil {
data.t.Fatal(err)