diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index a9a0d71b3b..3369103344 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -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) + } } } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 8cb11a6cc8..257e309dca 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -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 { diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 1b99832be6..312cf7c883 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -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 } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index d05ce8a665..cdda4cc499 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -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) + } } } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 68df3fb7fb..443de115ac 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -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()) diff --git a/internal/lsp/testdata/godef/a/a.go b/internal/lsp/testdata/godef/a/a.go index 6f7ecb3a61..6d81a8b568 100644 --- a/internal/lsp/testdata/godef/a/a.go +++ b/internal/lsp/testdata/godef/a/a.go @@ -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) diff --git a/internal/lsp/testdata/godef/a/a.go.golden b/internal/lsp/testdata/godef/a/a.go.golden index 87b792e5e6..20853fe349 100644 --- a/internal/lsp/testdata/godef/a/a.go.golden +++ b/internal/lsp/testdata/godef/a/a.go.golden @@ -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" diff --git a/internal/lsp/testdata/godef/b/b.go b/internal/lsp/testdata/godef/b/b.go index 721f9e7c92..ee3d0f06b0 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 @@ 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) diff --git a/internal/lsp/testdata/godef/b/b.go.golden b/internal/lsp/testdata/godef/b/b.go.golden index e3ba2bc747..276f18835b 100644 --- a/internal/lsp/testdata/godef/b/b.go.golden +++ b/internal/lsp/testdata/godef/b/b.go.golden @@ -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" diff --git a/internal/lsp/testdata/godef/b/c.go.golden b/internal/lsp/testdata/godef/b/c.go.golden index e05adcdff8..c2ab4a2eb8 100644 --- a/internal/lsp/testdata/godef/b/c.go.golden +++ b/internal/lsp/testdata/godef/b/c.go.golden @@ -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" diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 4410d51a93..04e67c1d5a 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -11,7 +11,7 @@ FoldingRangesCount = 2 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 2f1ea65e98..d7c75eefc3 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -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)