From 8cc75a79d5651b3f5e1e3a0a7bfc4e3795d77763 Mon Sep 17 00:00:00 2001 From: Nikita Shoshin Date: Fri, 31 Mar 2023 19:58:23 +0400 Subject: [PATCH] fixes after code review --- gopls/internal/lsp/regtest/marker.go | 2 +- gopls/internal/lsp/source/hover.go | 32 ++-- gopls/internal/lsp/testdata/godef/a/g.go | 66 -------- .../internal/lsp/testdata/godef/a/g.go.golden | 67 --------- .../internal/lsp/testdata/summary.txt.golden | 2 +- .../lsp/testdata/summary_go1.18.txt.golden | 2 +- .../regtest/marker/testdata/hover/const.txt | 141 +++++++++++++++++- 7 files changed, 162 insertions(+), 150 deletions(-) delete mode 100644 gopls/internal/lsp/testdata/godef/a/g.go delete mode 100644 gopls/internal/lsp/testdata/godef/a/g.go.golden diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go index 9fe63e2ccef..2d18496fc60 100644 --- a/gopls/internal/lsp/regtest/marker.go +++ b/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, diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go index 0fb5aab91d8..0d1aa6bbbfe 100644 --- a/gopls/internal/lsp/source/hover.go +++ b/gopls/internal/lsp/source/hover.go @@ -173,8 +173,7 @@ func hover(ctx context.Context, snapshot Snapshot, fh FileHandle, pp protocol.Po // TODO(rfindley): we could do much better for inferred signatures. if inferred := inferredSignature(pkg.GetTypesInfo(), ident); inferred != nil { - s := inferredSignatureString(obj, qf, inferred) - if s != "" { + if s := inferredSignatureString(obj, qf, inferred); s != "" { signature = s } } @@ -584,7 +583,7 @@ func hoverLit(pgf *ParsedGoFile, lit *ast.BasicLit, pos token.Pos) (protocol.Ran // 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. +// 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. @@ -605,12 +604,17 @@ func inferredSignatureString(obj types.Object, qf types.Qualifier, inferred *typ // 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: - declaration := obj.Val().String() + var ( + declaration = obj.Val().String() // default formatted declaration + comment = "" // if non-empty, a clarifying comment + ) // Try to use the original declaration. switch obj.Val().Kind() { @@ -619,16 +623,15 @@ func objectString(obj types.Object, qf types.Qualifier, declPos token.Pos, file // 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: + if spec, _ := spec.(*ast.ValueSpec); spec != nil { for i, name := range spec.Names { if declPos == name.Pos() { if i < len(spec.Values) { - declaration = FormatNodeFile(file, spec.Values[i]) + originalDeclaration := FormatNodeFile(file, spec.Values[i]) + if originalDeclaration != declaration { + comment = declaration + declaration = originalDeclaration + } } break } @@ -636,7 +639,7 @@ func objectString(obj types.Object, qf types.Qualifier, declPos token.Pos, file } } - comment := obj.Val().String() + // Special formatting cases. switch typ := obj.Type().(type) { case *types.Named: // Try to add a formatted duration as an inline comment. @@ -647,9 +650,12 @@ func objectString(obj types.Object, qf types.Qualifier, declPos token.Pos, file } } } + if comment == declaration { + comment = "" + } str += " = " + declaration - if declaration != comment { + if comment != "" { str += " // " + comment } } 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 7d81c19714d..00000000000 --- a/gopls/internal/lsp/testdata/godef/a/g.go +++ /dev/null @@ -1,66 +0,0 @@ -package a - -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) -} 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 abc4a3b0b59..00000000000 --- a/gopls/internal/lsp/testdata/godef/a/g.go.golden +++ /dev/null @@ -1,67 +0,0 @@ --- dur-hoverdef -- -```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-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) diff --git a/gopls/internal/lsp/testdata/summary.txt.golden b/gopls/internal/lsp/testdata/summary.txt.golden index 28cb34ac16a..9e13d423834 100644 --- a/gopls/internal/lsp/testdata/summary.txt.golden +++ b/gopls/internal/lsp/testdata/summary.txt.golden @@ -16,7 +16,7 @@ SemanticTokenCount = 3 SuggestedFixCount = 65 FunctionExtractionCount = 27 MethodExtractionCount = 6 -DefinitionsCount = 60 +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 83ac9e81660..184695e658e 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 @@ SemanticTokenCount = 3 SuggestedFixCount = 71 FunctionExtractionCount = 27 MethodExtractionCount = 6 -DefinitionsCount = 60 +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 cdb0e51e27d..6effaf49de6 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 @@ 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)