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`.

Fixes golang/go#47453

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,15 @@ 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)
+		s := inferredSignatureString(obj, qf, inferred)
+		if s != "" {
+			signature = s
+		}
 	}
 
 	// For "objects defined by a type spec", the signature produced by
@@ -214,7 +219,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 +326,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 +582,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 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 +600,58 @@ 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.
+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())
+		declaration := obj.Val().String()
 
-		// 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 file == nil || spec == nil {
+				break
+			}
+
+			switch spec := spec.(type) {
+			case *ast.ValueSpec:
+				for i, name := range spec.Names {
+					if declPos == name.Pos() {
+						if i < len(spec.Values) {
+							declaration = FormatNodeFile(file, spec.Values[i])
+						}
+						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()
+
+		comment := obj.Val().String()
+		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()
+				}
 			}
 		}
+
+		str += " = " + declaration
+		if declaration != comment {
+			str += " // " + comment
+		}
 	}
 	return str
 }
@@ -708,28 +747,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/godef/a/g.go

@@ -1,6 +1,66 @@
 package a
 
-import "time"
+import (
+	"math"
+	"time"
+)
 
 // dur is a constant of type time.Duration.
 const dur = 15*time.Minute + 10*time.Second + 350*time.Millisecond //@dur,hoverdef("dur", dur)
+
+// 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              //@mark(decimalConst, "decimal"),hoverdef("decimal", decimalConst)
+	_ = hex                  //@mark(hexConst, "hex"),hoverdef("hex", hexConst)
+	_ = bin                  //@mark(binConst, "bin"),hoverdef("bin", binConst)
+	_ = numberWithUnderscore //@mark(numberWithUnderscoreConst, "numberWithUnderscore"),hoverdef("numberWithUnderscore", numberWithUnderscoreConst)
+	_ = octal                //@mark(octalConst, "octal"),hoverdef("octal", octalConst)
+	_ = expr                 //@mark(exprConst, "expr"),hoverdef("expr", exprConst)
+	_ = boolean              //@mark(boolConst, "boolean"),hoverdef("boolean", boolConst)
+
+	const ln10 = 2.30258509299404568401799145468436420760110148862877297603332790
+
+	_ = ln10 //@mark(ln10Const, "ln10"),hoverdef("ln10", ln10Const)
+}
+
+// Iota.
+func _() {
+	const (
+		a = 1 << iota
+		b
+	)
+
+	_ = a //@mark(aIota, "a"),hoverdef("a", aIota)
+	_ = b //@mark(bIota, "b"),hoverdef("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     //@mark(strConst, "str"),hoverdef("str", strConst)
+	_ = longStr //@mark(longStrConst, "longStr"),hoverdef("longStr", longStrConst)
+}
+
+// Constants from other packages.
+func _() {
+	_ = math.MaxFloat32 //@mark(maxFloat32Const, "MaxFloat32"),hoverdef("MaxFloat32", maxFloat32Const)
+}

gopls/internal/lsp/testdata/godef/a/g.go.golden

@@ -1,7 +1,67 @@
 -- dur-hoverdef --
 ```go
-const dur time.Duration = 910350000000 // 15m10.35s
+const dur time.Duration = 15*time.Minute + 10*time.Second + 350*time.Millisecond // 15m10.35s
 ```
 
 dur is a constant of type time.Duration.
 
+-- decimalConst-hoverdef --
+```go
+const decimal untyped int = 153
+```
+
+no inline comment
+
+-- hexConst-hoverdef --
+```go
+const hex untyped int = 0xe34e // 58190
+```
+-- binConst-hoverdef --
+```go
+const bin untyped int = 0b1001001 // 73
+```
+-- numberWithUnderscoreConst-hoverdef --
+```go
+const numberWithUnderscore int64 = 10_000_000_000 // 10000000000
+```
+-- octalConst-hoverdef --
+```go
+const octal untyped int = 0o777 // 511
+```
+-- exprConst-hoverdef --
+```go
+const expr untyped int = 2 << (0b111&0b101 - 2) // 16
+```
+-- boolConst-hoverdef --
+```go
+const boolean untyped bool = (55 - 3) == (26 * 2) // true
+```
+-- ln10Const-hoverdef --
+```go
+const ln10 untyped float = 2.30258509299404568401799145468436420760110148862877297603332790 // 2.30259
+```
+-- aIota-hoverdef --
+```go
+const a untyped int = 1 << iota // 1
+```
+-- bIota-hoverdef --
+```go
+const b untyped int = 2
+```
+-- strConst-hoverdef --
+```go
+const str untyped string = "hello world"
+```
+-- longStrConst-hoverdef --
+```go
+const longStr untyped string = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur e...
+```
+-- maxFloat32Const-hoverdef --
+```go
+const math.MaxFloat32 untyped float = 0x1p127 * (1 + (1 - 0x1p-23)) // 3.40282e+38
+```
+
+Floating-point limit values.
+
+
+[`math.MaxFloat32` on pkg.go.dev](https://pkg.go.dev/math#MaxFloat32)

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

@@ -16,7 +16,7 @@ SemanticTokenCount = 3
 SuggestedFixCount = 65
 FunctionExtractionCount = 27
 MethodExtractionCount = 6
-DefinitionsCount = 47
+DefinitionsCount = 60
 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 = 60
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 InlayHintsCount = 5

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)