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>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index a9a0d71..3369103 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -408,7 +408,9 @@
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 @@
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 @@
} 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 @@
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)
+ }
}
}
diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go
index 8cb11a6..257e309 100644
--- a/internal/lsp/source/identifier.go
+++ b/internal/lsp/source/identifier.go
@@ -32,10 +32,9 @@
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 @@
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 @@
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 {
diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 1b99832..312cf7c 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -54,7 +54,16 @@
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 @@
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
}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index d05ce8a..cdda4cc 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -510,7 +510,7 @@
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 @@
}
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 @@
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 @@
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)
+ }
}
}
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 68df3fb..443de11 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -138,9 +138,9 @@
// 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())
diff --git a/internal/lsp/testdata/godef/a/a.go b/internal/lsp/testdata/godef/a/a.go
index 6f7ecb3..6d81a8b 100644
--- a/internal/lsp/testdata/godef/a/a.go
+++ b/internal/lsp/testdata/godef/a/a.go
@@ -6,7 +6,7 @@
type A string //@A
-func Stuff() { //@Stuff
+func AStuff() { //@AStuff
x := 5
Random2(x) //@godef("dom2", Random2)
Random() //@godef("()", Random)
diff --git a/internal/lsp/testdata/godef/a/a.go.golden b/internal/lsp/testdata/godef/a/a.go.golden
index 87b792e..20853fe 100644
--- a/internal/lsp/testdata/godef/a/a.go.golden
+++ b/internal/lsp/testdata/godef/a/a.go.golden
@@ -51,12 +51,12 @@
"start": {
"line": 14,
"column": 6,
- "offset": 201
+ "offset": 203
},
"end": {
"line": 14,
"column": 9,
- "offset": 204
+ "offset": 206
}
},
"description": "var err error"
diff --git a/internal/lsp/testdata/godef/b/b.go b/internal/lsp/testdata/godef/b/b.go
index 721f9e7..ee3d0f0 100644
--- a/internal/lsp/testdata/godef/b/b.go
+++ b/internal/lsp/testdata/godef/b/b.go
@@ -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 @@
}
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)
diff --git a/internal/lsp/testdata/godef/b/b.go.golden b/internal/lsp/testdata/godef/b/b.go.golden
index e3ba2bc..276f188 100644
--- a/internal/lsp/testdata/godef/b/b.go.golden
+++ b/internal/lsp/testdata/godef/b/b.go.golden
@@ -22,7 +22,7 @@
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 @@
"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 @@
-- 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 @@
"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 @@
"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 @@
"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 @@
"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 @@
"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 @@
"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 @@
"start": {
"line": 37,
"column": 7,
- "offset": 812
+ "offset": 795
},
"end": {
"line": 37,
"column": 8,
- "offset": 813
+ "offset": 796
}
},
"description": "const X untyped int = 0"
diff --git a/internal/lsp/testdata/godef/b/c.go.golden b/internal/lsp/testdata/godef/b/c.go.golden
index e05adcd..c2ab4a2 100644
--- a/internal/lsp/testdata/godef/b/c.go.golden
+++ b/internal/lsp/testdata/godef/b/c.go.golden
@@ -11,12 +11,12 @@
"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 @@
"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"
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index 4410d51..04e67c1 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -11,7 +11,7 @@
FormatCount = 6
ImportCount = 7
SuggestedFixCount = 1
-DefinitionsCount = 38
+DefinitionsCount = 37
TypeDefinitionsCount = 2
HighlightsCount = 12
ReferencesCount = 7
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 2f1ea65..d7c75ee 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -840,11 +840,6 @@
}
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)
