From e0d5eebdf8c48791fabd6d4b6ffe1692094da8ce Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Mon, 27 Apr 2020 12:55:45 -0400 Subject: [PATCH] internal/lsp: fix docs on hover for ungrouped package variables Fixes golang/go#38525 Change-Id: I8833c925663b67b2c82ad4cbf580d1c6f3c7a81d Reviewed-on: https://go-review.googlesource.com/c/tools/+/230305 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/lsp_test.go | 2 +- internal/lsp/source/hover.go | 18 ++++++------- .../lsp/testdata/lsp/primarymod/godef/a/a.go | 3 +++ .../lsp/primarymod/godef/a/a.go.golden | 22 +++++++++++----- .../lsp/primarymod/godef/a/d.go.golden | 6 ++++- .../lsp/primarymod/godef/b/b.go.golden | 26 +++++++++++-------- .../lsp/primarymod/godef/b/e.go.golden | 6 ++++- .../godef/broken/unclosedIf.go.golden | 6 ++++- internal/lsp/testdata/lsp/summary.txt.golden | 2 +- internal/lsp/tests/util.go | 11 ++++++++ 10 files changed, 71 insertions(+), 31 deletions(-) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index b7b27aa4aa..5d5c9fe3fd 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -486,7 +486,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { return []byte(hover.Contents.Value), nil })) if hover.Contents.Value != expectHover { - t.Errorf("for %v got %q want %q", d.Src, hover.Contents.Value, expectHover) + t.Errorf("%s:\n%s", d.Src, tests.Diff(expectHover, hover.Contents.Value)) } } if !d.OnlyHover { diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index 6dd0ab9613..0e42e1c8eb 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -281,7 +281,14 @@ func formatVar(node ast.Spec, obj types.Object, decl *ast.GenDecl) *HoverInforma fieldList = t.Methods } case *ast.ValueSpec: - return &HoverInformation{source: obj, comment: spec.Doc} + comment := decl.Doc + if comment == nil { + comment = spec.Doc + } + if comment == nil { + comment = spec.Comment + } + return &HoverInformation{source: obj, comment: comment} } // If we have a struct or interface declaration, // we need to match the object to the corresponding field or method. @@ -296,14 +303,7 @@ func formatVar(node ast.Spec, obj types.Object, decl *ast.GenDecl) *HoverInforma } } } - // If we have a package level variable that does have a - // comment group attached to it but not in the ast.spec. - if decl.Doc.Text() != "" { - return &HoverInformation{source: obj, comment: decl.Doc} - } - - // If we weren't able to find documentation for the object. - return &HoverInformation{source: obj} + return &HoverInformation{source: obj, comment: decl.Doc} } func FormatHover(h *HoverInformation, options Options) (string, error) { diff --git a/internal/lsp/testdata/lsp/primarymod/godef/a/a.go b/internal/lsp/testdata/lsp/primarymod/godef/a/a.go index d7df63d9a1..1f297ce800 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/a/a.go +++ b/internal/lsp/testdata/lsp/primarymod/godef/a/a.go @@ -13,6 +13,9 @@ var ( x string //@x,hover("x", x) ) +// z is a variable too. +var z string //@z,hover("z", z) + type A string //@mark(AString, "A") func AStuff() { //@AStuff diff --git a/internal/lsp/testdata/lsp/primarymod/godef/a/a.go.golden b/internal/lsp/testdata/lsp/primarymod/godef/a/a.go.golden index f8a398a8b3..bf9c2cba11 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/a/a.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/godef/a/a.go.golden @@ -75,28 +75,32 @@ func Random2(y int) int func Random2(y int) int ``` -- err-definition -- -godef/a/a.go:23:6-9: defined here as ```go +godef/a/a.go:26:6-9: defined here as ```go var err error ``` + +\@err -- err-definition-json -- { "span": { "uri": "file://godef/a/a.go", "start": { - "line": 23, + "line": 26, "column": 6, - "offset": 304 + "offset": 361 }, "end": { - "line": 23, + "line": 26, "column": 9, - "offset": 307 + "offset": 364 } }, - "description": "```go\nvar err error\n```" + "description": "```go\nvar err error\n```\n\n\\@err" } -- err-hover -- +\@err + ```go var err error ``` @@ -118,3 +122,9 @@ x is a variable\. ```go var x string ``` +-- z-hover -- +z is a variable too\. + +```go +var z string +``` diff --git a/internal/lsp/testdata/lsp/primarymod/godef/a/d.go.golden b/internal/lsp/testdata/lsp/primarymod/godef/a/d.go.golden index 771f98a595..fd9e21c7d7 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/a/d.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/godef/a/d.go.golden @@ -68,6 +68,8 @@ var Other Thing ``` [`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other) + +\@Other -- Other-definition-json -- { "span": { @@ -83,10 +85,12 @@ var Other Thing "offset": 91 } }, - "description": "```go\nvar Other Thing\n```\n\n[`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other)" + "description": "```go\nvar Other Thing\n```\n\n[`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other)\n\n\\@Other" } -- Other-hover -- +\@Other + [`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other) ```go diff --git a/internal/lsp/testdata/lsp/primarymod/godef/b/b.go.golden b/internal/lsp/testdata/lsp/primarymod/godef/b/b.go.golden index 8d00791211..2baae63c45 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/b/b.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/godef/b/b.go.golden @@ -29,7 +29,7 @@ package a ("golang.org/x/tools/internal/lsp/godef/a") package a ("golang.org/x/tools/internal/lsp/godef/a") ``` -- AString-definition -- -godef/a/a.go:16:6-7: defined here as ```go +godef/a/a.go:19:6-7: defined here as ```go A string //@mark(AString, "A") ``` @@ -40,14 +40,14 @@ A string //@mark(AString, "A") "span": { "uri": "file://godef/a/a.go", "start": { - "line": 16, + "line": 19, "column": 6, - "offset": 159 + "offset": 216 }, "end": { - "line": 16, + "line": 19, "column": 7, - "offset": 160 + "offset": 217 } }, "description": "```go\nA string //@mark(AString, \"A\")\n\n```\n\n[`a.A` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#A)" @@ -61,7 +61,7 @@ A string //@mark(AString, "A") ``` -- AStuff-definition -- -godef/a/a.go:18:6-12: defined here as ```go +godef/a/a.go:21:6-12: defined here as ```go func a.AStuff() ``` @@ -71,14 +71,14 @@ func a.AStuff() "span": { "uri": "file://godef/a/a.go", "start": { - "line": 18, + "line": 21, "column": 6, - "offset": 196 + "offset": 253 }, "end": { - "line": 18, + "line": 21, "column": 12, - "offset": 202 + "offset": 259 } }, "description": "```go\nfunc a.AStuff()\n```\n\n[`a.AStuff` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#AStuff)" @@ -308,6 +308,8 @@ const X untyped int = 0 ``` [`b.X` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/b#X) + +\@mark\(bX, \"X\"\),godef\(\"X\", bX\) -- bX-definition-json -- { "span": { @@ -323,10 +325,12 @@ const X untyped int = 0 "offset": 814 } }, - "description": "```go\nconst X untyped int = 0\n```\n\n[`b.X` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/b#X)" + "description": "```go\nconst X untyped int = 0\n```\n\n[`b.X` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/b#X)\n\n\\@mark\\(bX, \\\"X\\\"\\),godef\\(\\\"X\\\", bX\\)" } -- bX-hover -- +\@mark\(bX, \"X\"\),godef\(\"X\", bX\) + [`b.X` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/b#X) ```go diff --git a/internal/lsp/testdata/lsp/primarymod/godef/b/e.go.golden b/internal/lsp/testdata/lsp/primarymod/godef/b/e.go.golden index 24026cbb75..258eebf1ac 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/b/e.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/godef/b/e.go.golden @@ -38,6 +38,8 @@ var a.Other a.Thing ``` [`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other) + +\@Other -- Other-definition-json -- { "span": { @@ -53,10 +55,12 @@ var a.Other a.Thing "offset": 91 } }, - "description": "```go\nvar a.Other a.Thing\n```\n\n[`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other)" + "description": "```go\nvar a.Other a.Thing\n```\n\n[`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other)\n\n\\@Other" } -- Other-hover -- +\@Other + [`a.Other` on pkg.go.dev](https://pkg.go.dev/golang.org/x/tools/internal/lsp/godef/a#Other) ```go diff --git a/internal/lsp/testdata/lsp/primarymod/godef/broken/unclosedIf.go.golden b/internal/lsp/testdata/lsp/primarymod/godef/broken/unclosedIf.go.golden index 43d55970f6..5809cf7ac3 100644 --- a/internal/lsp/testdata/lsp/primarymod/godef/broken/unclosedIf.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/godef/broken/unclosedIf.go.golden @@ -2,6 +2,8 @@ godef/broken/unclosedIf.go:7:7-19: defined here as ```go var myUnclosedIf string ``` + +\@myUnclosedIf -- myUnclosedIf-definition-json -- { "span": { @@ -17,10 +19,12 @@ var myUnclosedIf string "offset": 80 } }, - "description": "```go\nvar myUnclosedIf string\n```" + "description": "```go\nvar myUnclosedIf string\n```\n\n\\@myUnclosedIf" } -- myUnclosedIf-hover -- +\@myUnclosedIf + ```go var myUnclosedIf string ``` diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index 871fc2fb4d..2d00281847 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -12,7 +12,7 @@ FoldingRangesCount = 2 FormatCount = 6 ImportCount = 8 SuggestedFixCount = 6 -DefinitionsCount = 46 +DefinitionsCount = 47 TypeDefinitionsCount = 2 HighlightsCount = 52 ReferencesCount = 11 diff --git a/internal/lsp/tests/util.go b/internal/lsp/tests/util.go index 58b83280fd..d32170a580 100644 --- a/internal/lsp/tests/util.go +++ b/internal/lsp/tests/util.go @@ -522,3 +522,14 @@ func EnableAllAnalyzers(snapshot source.Snapshot, opts *source.Options) { } } } + +func Diff(want, got string) string { + if want == got { + return "" + } + // Add newlines to avoid newline messages in diff. + want += "\n" + got += "\n" + d := myers.ComputeEdits("", want, got) + return fmt.Sprintf("%q", diff.ToUnified("want", "got", want, d)) +}