gopls/internal/lsp/source: show both the original declaration and the value of constants in hover
This change improves the hover information for constants by showing
both the original declaration and the value. The value is displayed
as an inline comment. If the original declaration and the value are
the same, there will be no inline comment.
Examples:
```go
const duration time.Duration = 15*time.Minute + 10*time.Second // 15m10s
const octal untyped int = 0o777 // 511
const expr untyped int = 2 << (0b111&0b101 - 2) // 16
const boolean untyped bool = (55 - 3) == (26 * 2) // true
const dec untyped int = 500
```
Other changes:
- Calls to `objectString` that format methods or variables have been
replaced with `types.ObjectString`.
- The logic of inferred signature formatting has been extracted from
`objectString` to a new function `inferredSignatureString`.
- Remove unused function `extractFieldList`.
- Port `gopls/internal/lsp/testdata/godef/a/g.go` to the new marker
tests.
- Fix `-min_go` constraint for marker tests by replacing hardcoded
`1.18` with an actual flag value.
Fixes golang/go#47453
Change-Id: Ib9012dae8554a3b646c3059d2f8967e425974fbf
GitHub-Last-Rev: 8cc75a79d5651b3f5e1e3a0a7bfc4e3795d77763
GitHub-Pull-Request: golang/tools#432
Reviewed-on: https://go-review.googlesource.com/c/tools/+/480695
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index 9fe63e2..2d18496 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -308,7 +308,7 @@
if _, err := fmt.Sscanf(test.minGoVersion, "go1.%d", &go1point); err != nil {
t.Fatalf("parsing -min_go version: %v", err)
}
- testenv.NeedsGo1Point(t, 18)
+ testenv.NeedsGo1Point(t, go1point)
}
config := fake.EditorConfig{
Settings: test.settings,
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index 3e197b4..0d1aa6b 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -143,7 +143,7 @@
// There's not much useful information to provide.
if selectedType != nil {
fakeObj := types.NewVar(obj.Pos(), obj.Pkg(), obj.Name(), selectedType)
- signature := objectString(fakeObj, qf, nil)
+ signature := types.ObjectString(fakeObj, qf)
return rng, &HoverJSON{
Signature: signature,
SingleLine: signature,
@@ -168,10 +168,14 @@
docText := comment.Text()
// By default, types.ObjectString provides a reasonable signature.
- signature := objectString(obj, qf, nil)
+ signature := objectString(obj, qf, declPos, declPGF.Tok, spec)
+ singleLineSignature := signature
+
// TODO(rfindley): we could do much better for inferred signatures.
if inferred := inferredSignature(pkg.GetTypesInfo(), ident); inferred != nil {
- signature = objectString(obj, qf, inferred)
+ if s := inferredSignatureString(obj, qf, inferred); s != "" {
+ signature = s
+ }
}
// For "objects defined by a type spec", the signature produced by
@@ -214,7 +218,7 @@
if (m.Obj().Exported() || m.Obj().Pkg() == pkg.GetTypes()) && len(m.Index()) == 1 {
b.WriteString(sep)
sep = "\n"
- b.WriteString(objectString(m.Obj(), qf, nil))
+ b.WriteString(types.ObjectString(m.Obj(), qf))
}
}
}
@@ -321,7 +325,7 @@
return rng, &HoverJSON{
Synopsis: doc.Synopsis(docText),
FullDocumentation: docText,
- SingleLine: objectString(obj, qf, nil),
+ SingleLine: singleLineSignature,
SymbolName: linkName,
Signature: signature,
LinkPath: linkPath,
@@ -577,12 +581,10 @@
}, nil
}
-// objectString is a wrapper around the types.ObjectString function.
-// It handles adding more information to the object string.
-//
-// TODO(rfindley): this function does too much. We should lift the special
-// handling to callsites.
-func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signature) string {
+// inferredSignatureString is a wrapper around the types.ObjectString function
+// that adds more information to inferred signatures. It will return an empty string
+// if the passed types.Object is not a signature.
+func inferredSignatureString(obj types.Object, qf types.Qualifier, inferred *types.Signature) string {
// If the signature type was inferred, prefer the inferred signature with a
// comment showing the generic signature.
if sig, _ := obj.Type().(*types.Signature); sig != nil && typeparams.ForSignature(sig).Len() > 0 && inferred != nil {
@@ -597,22 +599,65 @@
str += "// " + types.TypeString(sig, qf)
return str
}
+ return ""
+}
+
+// objectString is a wrapper around the types.ObjectString function.
+// It handles adding more information to the object string.
+// If spec is non-nil, it may be used to format additional declaration
+// syntax, and file must be the token.File describing its positions.
+func objectString(obj types.Object, qf types.Qualifier, declPos token.Pos, file *token.File, spec ast.Spec) string {
str := types.ObjectString(obj, qf)
+
switch obj := obj.(type) {
case *types.Const:
- str = fmt.Sprintf("%s = %s", str, obj.Val())
+ var (
+ declaration = obj.Val().String() // default formatted declaration
+ comment = "" // if non-empty, a clarifying comment
+ )
- // Try to add a formatted duration as an inline comment
- typ, ok := obj.Type().(*types.Named)
- if !ok {
- break
- }
- pkg := typ.Obj().Pkg()
- if pkg.Path() == "time" && typ.Obj().Name() == "Duration" {
- if d, ok := constant.Int64Val(obj.Val()); ok {
- str += " // " + time.Duration(d).String()
+ // Try to use the original declaration.
+ switch obj.Val().Kind() {
+ case constant.String:
+ // Usually the original declaration of a string doesn't carry much information.
+ // Also strings can be very long. So, just use the constant's value.
+
+ default:
+ if spec, _ := spec.(*ast.ValueSpec); spec != nil {
+ for i, name := range spec.Names {
+ if declPos == name.Pos() {
+ if i < len(spec.Values) {
+ originalDeclaration := FormatNodeFile(file, spec.Values[i])
+ if originalDeclaration != declaration {
+ comment = declaration
+ declaration = originalDeclaration
+ }
+ }
+ break
+ }
+ }
}
}
+
+ // Special formatting cases.
+ switch typ := obj.Type().(type) {
+ case *types.Named:
+ // Try to add a formatted duration as an inline comment.
+ pkg := typ.Obj().Pkg()
+ if pkg.Path() == "time" && typ.Obj().Name() == "Duration" {
+ if d, ok := constant.Int64Val(obj.Val()); ok {
+ comment = time.Duration(d).String()
+ }
+ }
+ }
+ if comment == declaration {
+ comment = ""
+ }
+
+ str += " = " + declaration
+ if comment != "" {
+ str += " // " + comment
+ }
}
return str
}
@@ -708,28 +753,6 @@
return pgf, fullPos, nil
}
-// extractFieldList recursively tries to extract a field list.
-// If it is not found, nil is returned.
-func extractFieldList(specType ast.Expr) *ast.FieldList {
- switch t := specType.(type) {
- case *ast.StructType:
- return t.Fields
- case *ast.InterfaceType:
- return t.Methods
- case *ast.ArrayType:
- return extractFieldList(t.Elt)
- case *ast.MapType:
- // Map value has a greater chance to be a struct
- if fields := extractFieldList(t.Value); fields != nil {
- return fields
- }
- return extractFieldList(t.Key)
- case *ast.ChanType:
- return extractFieldList(t.Value)
- }
- return nil
-}
-
func formatHover(h *HoverJSON, options *Options) (string, error) {
signature := formatSignature(h, options)
diff --git a/gopls/internal/lsp/testdata/godef/a/g.go b/gopls/internal/lsp/testdata/godef/a/g.go
deleted file mode 100644
index dfef2fb..0000000
--- a/gopls/internal/lsp/testdata/godef/a/g.go
+++ /dev/null
@@ -1,6 +0,0 @@
-package a
-
-import "time"
-
-// dur is a constant of type time.Duration.
-const dur = 15*time.Minute + 10*time.Second + 350*time.Millisecond //@dur,hoverdef("dur", dur)
diff --git a/gopls/internal/lsp/testdata/godef/a/g.go.golden b/gopls/internal/lsp/testdata/godef/a/g.go.golden
deleted file mode 100644
index f7a2e1b..0000000
--- a/gopls/internal/lsp/testdata/godef/a/g.go.golden
+++ /dev/null
@@ -1,7 +0,0 @@
--- dur-hoverdef --
-```go
-const dur time.Duration = 910350000000 // 15m10.35s
-```
-
-dur is a constant of type time.Duration.
-
diff --git a/gopls/internal/lsp/testdata/summary.txt.golden b/gopls/internal/lsp/testdata/summary.txt.golden
index e5c84a6..9e13d42 100644
--- a/gopls/internal/lsp/testdata/summary.txt.golden
+++ b/gopls/internal/lsp/testdata/summary.txt.golden
@@ -16,7 +16,7 @@
SuggestedFixCount = 65
FunctionExtractionCount = 27
MethodExtractionCount = 6
-DefinitionsCount = 47
+DefinitionsCount = 46
TypeDefinitionsCount = 18
HighlightsCount = 69
InlayHintsCount = 4
diff --git a/gopls/internal/lsp/testdata/summary_go1.18.txt.golden b/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
index 6b4ee22..184695e 100644
--- a/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
+++ b/gopls/internal/lsp/testdata/summary_go1.18.txt.golden
@@ -16,7 +16,7 @@
SuggestedFixCount = 71
FunctionExtractionCount = 27
MethodExtractionCount = 6
-DefinitionsCount = 47
+DefinitionsCount = 46
TypeDefinitionsCount = 18
HighlightsCount = 69
InlayHintsCount = 5
diff --git a/gopls/internal/regtest/marker/testdata/hover/const.txt b/gopls/internal/regtest/marker/testdata/hover/const.txt
index cdb0e51..6effaf4 100644
--- a/gopls/internal/regtest/marker/testdata/hover/const.txt
+++ b/gopls/internal/regtest/marker/testdata/hover/const.txt
@@ -1,12 +1,86 @@
This test checks hovering over constants.
+
+-- flags --
+-min_go=go1.17
+
-- go.mod --
module mod.com
-go 1.18
+go 1.17
+
-- c.go --
package c
+import (
+ "math"
+ "time"
+)
+
const X = 0 //@hover("X", "X", bX)
+
+// dur is a constant of type time.Duration.
+const dur = 15*time.Minute + 10*time.Second + 350*time.Millisecond //@hover("dur", "dur", dur)
+
+// MaxFloat32 is used in another package.
+const MaxFloat32 = 0x1p127 * (1 + (1 - 0x1p-23))
+
+// Numbers.
+func _() {
+ const hex, bin = 0xe34e, 0b1001001
+
+ const (
+ // no inline comment
+ decimal = 153
+
+ numberWithUnderscore int64 = 10_000_000_000
+ octal = 0o777
+ expr = 2 << (0b111&0b101 - 2)
+ boolean = (55 - 3) == (26 * 2)
+ )
+
+ _ = decimal //@hover("decimal", "decimal", decimalConst)
+ _ = hex //@hover("hex", "hex", hexConst)
+ _ = bin //@hover("bin", "bin", binConst)
+ _ = numberWithUnderscore //@hover("numberWithUnderscore", "numberWithUnderscore", numberWithUnderscoreConst)
+ _ = octal //@hover("octal", "octal", octalConst)
+ _ = expr //@hover("expr", "expr", exprConst)
+ _ = boolean //@hover("boolean", "boolean", boolConst)
+
+ const ln10 = 2.30258509299404568401799145468436420760110148862877297603332790
+
+ _ = ln10 //@hover("ln10", "ln10", ln10Const)
+}
+
+// Iota.
+func _() {
+ const (
+ a = 1 << iota
+ b
+ )
+
+ _ = a //@hover("a", "a", aIota)
+ _ = b //@hover("b", "b", bIota)
+}
+
+// Strings.
+func _() {
+ const (
+ str = "hello" + " " + "world"
+ longStr = `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur eget ipsum non nunc
+molestie mattis id quis augue. Mauris dictum tincidunt ipsum, in auctor arcu congue eu.
+Morbi hendrerit fringilla libero commodo varius. Vestibulum in enim rutrum, rutrum tellus
+aliquet, luctus enim. Nunc sem ex, consectetur id porta nec, placerat vel urna.`
+ )
+
+ _ = str //@hover("str", "str", strConst)
+ _ = longStr //@hover("longStr", "longStr", longStrConst)
+}
+
+// Constants from other packages.
+func _() {
+ _ = math.Log2E //@hover("Log2E", "Log2E", log2eConst)
+}
+
-- @bX/hover.md --
```go
const X untyped int = 0
@@ -16,3 +90,68 @@
[`c.X` on pkg.go.dev](https://pkg.go.dev/mod.com#X)
+-- @dur/hover.md --
+```go
+const dur time.Duration = 15*time.Minute + 10*time.Second + 350*time.Millisecond // 15m10.35s
+```
+
+dur is a constant of type time.Duration.
+-- @decimalConst/hover.md --
+```go
+const decimal untyped int = 153
+```
+
+no inline comment
+-- @hexConst/hover.md --
+```go
+const hex untyped int = 0xe34e // 58190
+```
+-- @binConst/hover.md --
+```go
+const bin untyped int = 0b1001001 // 73
+```
+-- @numberWithUnderscoreConst/hover.md --
+```go
+const numberWithUnderscore int64 = 10_000_000_000 // 10000000000
+```
+-- @octalConst/hover.md --
+```go
+const octal untyped int = 0o777 // 511
+```
+-- @exprConst/hover.md --
+```go
+const expr untyped int = 2 << (0b111&0b101 - 2) // 16
+```
+-- @boolConst/hover.md --
+```go
+const boolean untyped bool = (55 - 3) == (26 * 2) // true
+```
+-- @ln10Const/hover.md --
+```go
+const ln10 untyped float = 2.30258509299404568401799145468436420760110148862877297603332790 // 2.30259
+```
+-- @aIota/hover.md --
+```go
+const a untyped int = 1 << iota // 1
+```
+-- @bIota/hover.md --
+```go
+const b untyped int = 2
+```
+-- @strConst/hover.md --
+```go
+const str untyped string = "hello world"
+```
+-- @longStrConst/hover.md --
+```go
+const longStr untyped string = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur e...
+```
+-- @log2eConst/hover.md --
+```go
+const math.Log2E untyped float = 1 / Ln2 // 1.4427
+```
+
+Mathematical constants.
+
+
+[`math.Log2E` on pkg.go.dev](https://pkg.go.dev/math#Log2E)
diff --git a/gopls/internal/regtest/marker/testdata/hover/hover.txt b/gopls/internal/regtest/marker/testdata/hover/hover.txt
index f9cd331..f6a5432 100644
--- a/gopls/internal/regtest/marker/testdata/hover/hover.txt
+++ b/gopls/internal/regtest/marker/testdata/hover/hover.txt
@@ -15,7 +15,7 @@
}
-- @abc/hover.md --
```go
-const abc untyped int = 42
+const abc untyped int = 0x2a // 42
```
@hover("b", "abc", abc),hover(" =", "abc", abc)
