internal/lsp/source: show an original declaration and a value for var…

…iables and constants

This change improves the hover information for variables and constants by showing
both an original declaration and a value. The value is shown as an inline comment.

Two kinds of declarations are currently supported:

1. Basic literals of type int:

	* var x int64 = 0xff // 255
	* const x untyped int = 0b11 // 3
	* const x uint = 1_000_000 // 1000000

2. Binary expressions with bitwise operators:

	* var x uint = 0b10 | 0b1 // 3
	* const x untyped int = 10 << 20 // 10485760

This change also removes TestHoverIntLiteral in favor of marker tests

Fixes golang/go#47453

gopls/internal/regtest/misc/hover_test.go

@@ -70,37 +70,6 @@ func main() {
 	})
 }
 
-func TestHoverIntLiteral(t *testing.T) {
-	testenv.NeedsGo1Point(t, 13)
-	const source = `
--- main.go --
-package main
-
-var (
-	bigBin = 0b1001001
-)
-
-var hex = 0xe34e
-
-func main() {
-}
-`
-	Run(t, source, func(t *testing.T, env *Env) {
-		env.OpenFile("main.go")
-		hexExpected := "58190"
-		got, _ := env.Hover("main.go", env.RegexpSearch("main.go", "hex"))
-		if got != nil && !strings.Contains(got.Value, hexExpected) {
-			t.Errorf("Hover: missing expected field '%s'. Got:\n%q", hexExpected, got.Value)
-		}
-
-		binExpected := "73"
-		got, _ = env.Hover("main.go", env.RegexpSearch("main.go", "bigBin"))
-		if got != nil && !strings.Contains(got.Value, binExpected) {
-			t.Errorf("Hover: missing expected field '%s'. Got:\n%q", binExpected, got.Value)
-		}
-	})
-}
-
 // Tests that hovering does not trigger the panic in golang/go#48249.
 func TestPanicInHoverBrokenCode(t *testing.T) {
 	testenv.NeedsGo1Point(t, 13)

internal/lsp/source/hover.go

@@ -64,6 +64,9 @@ type HoverInformation struct {
 	// isTypeAlias indicates whether the identifier is a type alias declaration.
 	// If it is true, the hover will have the prefix "type <typeName> = ".
 	isTypeAlias bool
+	// originalValue is used to show how a constant or variable was originally
+	// declared. For example, "var x int = 0xff" instead of "var x int = 255"
+	originalValue ast.Expr
 }
 
 func Hover(ctx context.Context, snapshot Snapshot, fh FileHandle, position protocol.Position) (*protocol.Hover, error) {
@@ -273,15 +276,6 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverInformation,
 			h.Signature = prefix + h.Signature
 		}
 
-		// Check if the variable is an integer whose value we can present in a more
-		// user-friendly way, i.e. `var hex = 0xe34e` becomes `var hex = 58190`
-		if spec, ok := x.(*ast.ValueSpec); ok && len(spec.Values) > 0 {
-			if lit, ok := spec.Values[0].(*ast.BasicLit); ok && len(spec.Names) > 0 {
-				val := constant.MakeFromLiteral(types.ExprString(lit), lit.Kind, 0)
-				h.Signature = fmt.Sprintf("var %s = %s", spec.Names[0], val)
-			}
-		}
-
 	case types.Object:
 		// If the variable is implicitly declared in a type switch, we need to
 		// manually generate its object string.
@@ -291,10 +285,10 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverInformation,
 				break
 			}
 		}
-		h.Signature = objectString(x, i.qf, i.Inferred)
+		h.Signature = objectString(x, i.qf, i.Inferred, h.originalValue)
 	}
 	if obj := i.Declaration.obj; obj != nil {
-		h.SingleLine = objectString(obj, i.qf, nil)
+		h.SingleLine = objectString(obj, i.qf, nil, nil)
 	}
 	obj := i.Declaration.obj
 	if obj == nil {
@@ -403,7 +397,7 @@ func moduleAtVersion(path string, i *IdentifierInfo) (string, string, bool) {
 
 // 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, inferred *types.Signature) string {
+func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signature, originalValue ast.Expr) string {
 	// If the signature type was inferred, prefer the preferred signature with a
 	// comment showing the generic signature.
 	if sig, _ := obj.Type().(*types.Signature); sig != nil && typeparams.ForSignature(sig).Len() > 0 && inferred != nil {
@@ -418,26 +412,87 @@ func objectString(obj types.Object, qf types.Qualifier, inferred *types.Signatur
 		str += "// " + types.TypeString(sig, qf)
 		return str
 	}
+
 	str := types.ObjectString(obj, qf)
+
+	var assignment, comment string
+	if originalValue != nil {
+		switch v := originalValue.(type) {
+		case *ast.BasicLit:
+			// It's useful to show the original and formatted values only for integers because only integers
+			// have multiple ways of initializing: decimal, binary, octal, hex and with underscores.
+			if v.Kind == token.INT {
+				assignment = v.Value
+				comment = constant.MakeFromLiteral(types.ExprString(v), v.Kind, 0).ExactString()
+			}
+
+		case *ast.BinaryExpr:
+			// Binary expressions with bitwise operators have a greater chance of carrying
+			// important information than expressions with other operators. So, show the original
+			// assignment only for bitwise operators
+			if isBitwiseOperator(v.Op) {
+				x, xOk := v.X.(*ast.BasicLit)
+				y, yOk := v.Y.(*ast.BasicLit)
+				if xOk && yOk && x.Kind == token.INT && y.Kind == token.INT {
+					assignment = x.Value + " " + v.Op.String() + " " + y.Value
+
+					xValue := constant.MakeFromLiteral(types.ExprString(x), x.Kind, 0)
+					yValue := constant.MakeFromLiteral(types.ExprString(y), y.Kind, 0)
+					switch v.Op {
+					case token.SHL, token.SHR:
+						s, ok := constant.Uint64Val(yValue)
+						if ok {
+							comment = constant.Shift(xValue, v.Op, uint(s)).ExactString()
+						}
+					default:
+						comment = constant.BinaryOp(xValue, v.Op, yValue).ExactString()
+					}
+				}
+			}
+		}
+	}
 	switch obj := obj.(type) {
 	case *types.Const:
-		str = fmt.Sprintf("%s = %s", str, obj.Val())
-
-		// Try to add a formatted duration as an inline comment
-		typ, ok := obj.Type().(*types.Named)
-		if !ok {
+		if obj.Val().Kind() == constant.Unknown {
 			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()
+
+		if assignment == "" {
+			assignment = obj.Val().String()
+		}
+		if comment == "" {
+			comment = obj.Val().String()
+		}
+
+		// If a constant has type 'time.Duration', show formatted duration as a comment
+		switch typ := obj.Type().(type) {
+		case *types.Named:
+			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 assignment != "" {
+		str += " = " + assignment
+		if comment != "" && comment != "unknown" && comment != assignment {
+			str += " // " + comment
+		}
+	}
 	return str
 }
 
+func isBitwiseOperator(op token.Token) bool {
+	switch op {
+	case token.AND, token.OR, token.XOR, token.SHL, token.SHR, token.AND_NOT:
+		return true
+	}
+	return false
+}
+
 // HoverInfo returns a HoverInformation struct for an ast node and its type
 // object. node should be the actual node used in type checking, while fullNode
 // could be a separate node with more complete syntactic information.
@@ -653,15 +708,19 @@ func formatVar(node ast.Spec, obj types.Object, decl *ast.GenDecl) *HoverInforma
 			comment = spec.Comment
 		}
 
-		// We need the AST nodes for variable declarations of basic literals with
-		// associated values so that we can augment their hover with more information.
-		if _, ok := obj.(*types.Var); ok && spec.Type == nil && len(spec.Values) > 0 {
-			if _, ok := spec.Values[0].(*ast.BasicLit); ok {
-				return &HoverInformation{source: spec, comment: comment}
+		var originalValue ast.Expr
+		for i, name := range spec.Names {
+			if obj.Pos() == name.Pos() {
+				if i < len(spec.Values) {
+					switch v := spec.Values[i].(type) {
+					case *ast.BasicLit, *ast.BinaryExpr:
+						originalValue = v
+					}
+				}
+				break
 			}
 		}
-
-		return &HoverInformation{source: obj, comment: comment}
+		return &HoverInformation{source: obj, comment: comment, originalValue: originalValue}
 	}
 
 	if fieldList != nil {

internal/lsp/testdata/godef/a/i.go

@@ -0,0 +1,50 @@
+package a
+
+import "time"
+
+var globalVar = 0xfffff3
+
+func _() {
+	var hex, bin, test = 0xe34e, 0b1001001, time.Hour
+
+	var (
+		// Number with underscore
+		numberWithUnderscore       = 10_000_000_000
+		octal                int64 = 0o666
+		bitwiseShift               = 10 << 20
+		bitwiseXor                 = 0b111 ^ 0b101
+		// No original value
+		str = "string"
+	)
+
+	_ = globalVar            //@mark(globalVar, "globalVar"),hoverdef("globalVar", globalVar)
+	_ = hex                  //@mark(hexVar, "hex"),hoverdef("hex", hexVar)
+	_ = bin                  //@mark(binVar, "bin"),hoverdef("bin", binVar)
+	_ = numberWithUnderscore //@mark(numberWithUnderscoreVar, "numberWithUnderscore"),hoverdef("numberWithUnderscore", numberWithUnderscoreVar)
+	_ = octal                //@mark(octalVar, "octal"),hoverdef("octal", octalVar)
+	_ = bitwiseShift         //@mark(bitwiseShiftVar, "bitwiseShift"),hoverdef("bitwiseShift", bitwiseShiftVar)
+	_ = bitwiseXor           //@mark(bitwiseXorVar, "bitwiseXor"),hoverdef("bitwiseXor", bitwiseXorVar)
+	_ = str                  //@mark(strVar, "str"),hoverdef("str", strVar)
+}
+
+const globalConst = 0xfffff3
+
+func _() {
+	const hex, bin = 0xe34e, 0b1001001
+
+	const (
+		numberWithUnderscore int64 = 10_000_000_000
+		// Comment for test
+		octal   = 0o777
+		bitwise = 8 >> 1
+		str     = "string"
+	)
+
+	_ = hex                  //@mark(hexConst, "hex"),hoverdef("hex", hexConst)
+	_ = bin                  //@mark(binConst, "bin"),hoverdef("bin", binConst)
+	_ = numberWithUnderscore //@mark(numberWithUnderscoreConst, "numberWithUnderscore"),hoverdef("numberWithUnderscore", numberWithUnderscoreConst)
+	_ = globalConst          //@mark(globalConst, "globalConst"),hoverdef("globalConst", globalConst)
+	_ = octal                //@mark(octalConst, "octal"),hoverdef("octal", octalConst)
+	_ = bitwise              //@mark(bitwiseConst, "bitwise"),hoverdef("bitwise", bitwiseConst)
+	_ = str                  //@mark(strConst, "str"),hoverdef("str", strConst)
+}

internal/lsp/testdata/godef/a/i.go.golden

@@ -0,0 +1,66 @@
+-- globalVar-hoverdef --
+```go
+var globalVar int = 0xfffff3 // 16777203
+```
+-- hexVar-hoverdef --
+```go
+var hex int = 0xe34e // 58190
+```
+-- binVar-hoverdef --
+```go
+var bin int = 0b1001001 // 73
+```
+-- numberWithUnderscoreVar-hoverdef --
+```go
+var numberWithUnderscore int = 10_000_000_000 // 10000000000
+```
+
+Number with underscore
+-- octalVar-hoverdef --
+```go
+var octal int64 = 0o666 // 438
+```
+-- bitwiseShiftVar-hoverdef --
+```go
+var bitwiseShift int = 10 << 20 // 10485760
+```
+-- bitwiseXorVar-hoverdef --
+```go
+var bitwiseXor int = 0b111 ^ 0b101 // 2
+```
+-- strVar-hoverdef --
+```go
+var str string
+```
+
+No original value
+-- globalConst-hoverdef --
+```go
+const globalConst untyped int = 0xfffff3 // 16777203
+```
+-- 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
+```
+
+Comment for test
+-- bitwiseConst-hoverdef --
+```go
+const bitwise untyped int = 8 >> 1 // 4
+```
+-- strConst-hoverdef --
+```go
+const str untyped string = "string"
+```

internal/lsp/testdata/summary.txt.golden

@@ -16,7 +16,7 @@ SemanticTokenCount = 3
 SuggestedFixCount = 49
 FunctionExtractionCount = 25
 MethodExtractionCount = 6
-DefinitionsCount = 95
+DefinitionsCount = 110
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 ReferencesCount = 27

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

@@ -16,7 +16,7 @@ SemanticTokenCount = 3
 SuggestedFixCount = 49
 FunctionExtractionCount = 25
 MethodExtractionCount = 6
-DefinitionsCount = 99
+DefinitionsCount = 114
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 ReferencesCount = 27