gopls/internal/lsp/source: show both the original declaration and the value of constants in hover

gopls/internal/lsp/regtest/marker.go

@@ -308,7 +308,7 @@ func RunMarkerTests(t *testing.T, dir string) {
 				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,

gopls/internal/lsp/source/hover.go

@@ -143,7 +143,7 @@ func hover(ctx context.Context, snapshot Snapshot, fh FileHandle, pp protocol.Po
 	// 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 @@ func hover(ctx context.Context, snapshot Snapshot, fh FileHandle, pp protocol.Po
 	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 @@ func hover(ctx context.Context, snapshot Snapshot, fh FileHandle, pp protocol.Po
 				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 @@ func hover(ctx context.Context, snapshot Snapshot, fh FileHandle, pp protocol.Po
 	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 @@ func hoverLit(pgf *ParsedGoFile, lit *ast.BasicLit, pos token.Pos) (protocol.Ran
 	}, 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 @@ func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signatur
 		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
+		// 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
+					}
+				}
+			}
 		}
-		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()
+
+		// 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 @@ func parseFull(ctx context.Context, snapshot Snapshot, fset *token.FileSet, pos
 	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)
 

gopls/internal/lsp/testdata/summary.txt.golden

@@ -16,7 +16,7 @@ SemanticTokenCount = 3
 SuggestedFixCount = 65
 FunctionExtractionCount = 27
 MethodExtractionCount = 6
-DefinitionsCount = 47
+DefinitionsCount = 46
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 InlayHintsCount = 4

gopls/internal/lsp/testdata/summary_go1.18.txt.golden

@@ -16,7 +16,7 @@ SemanticTokenCount = 3
 SuggestedFixCount = 71
 FunctionExtractionCount = 27
 MethodExtractionCount = 6
-DefinitionsCount = 47
+DefinitionsCount = 46
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 InlayHintsCount = 5

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 @@ const X untyped int = 0
 
 
 [`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)

gopls/internal/regtest/marker/testdata/hover/hover.txt

@@ -15,7 +15,7 @@ func _() {
 }
 -- @abc/hover.md --
 ```go
-const abc untyped int = 42
+const abc untyped int = 0x2a // 42
 ```
 
 @hover("b", "abc", abc),hover(" =", "abc", abc)