Skip to content
This repository has been archived by the owner on Apr 7, 2024. It is now read-only.

Commit

Permalink
gopls/internal/golang: hover: show size/offset info
Browse files Browse the repository at this point in the history
This change causes Hover to report size information for
types and struct fields when hovering over
the declaring identifier, plus offset information for
struct fields.

Some tests needed tweaks to make them CPU-independent,
or to disable them on 32-bit machines.

Also, add the first release notes using the new
git+gerrit workflow (and backfill a missing item for
"View package documentation").

Change-Id: Ibe8ac5937912c3802f3ad79e3d9f92ba24eb51de
Reviewed-on: https://go-review.googlesource.com/c/tools/+/573076
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
  • Loading branch information
adonovan authored and gopherbot committed Mar 29, 2024
1 parent 509ed1c commit 9663999
Show file tree
Hide file tree
Showing 13 changed files with 291 additions and 39 deletions.
10 changes: 10 additions & 0 deletions gopls/doc/release/README
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This directory contains the draft release notes for each upcoming release.

Be sure to update the file for the forthcoming release in the same CL
that you add new features or fix noteworthy bugs.

See https://github.com/golang/tools/releases for all past releases.

Tip: when reviewing edits to markdown files in Gerrit, to see the
rendered form, click the "Open in Code Search" link (magnifying glass
in blue square) then click "View in > gitiles" (shortcut: `v g`).
38 changes: 38 additions & 0 deletions gopls/doc/release/v0.16.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
gopls/v0.16.0

```
go install golang.org/x/tools/[email protected]
```

## New features

### Integrated documentation viewer

Gopls now offers a "View package documentation" code action that opens
a local web page displaying the generated documentation for the
current Go package in a form similar to https://pkg.go.dev. Use it to
preview the marked-up documentation as you prepare API changes, or to
read the documentation for locally edited packages, even ones that
have not yet been saved. Reload the page after an edit to see updated
documentation.

TODO: demo of VS Code `Source action > View package documentation`.

Clicking on the source-code link associated with a declaration will
cause your editor to navigate to the declaration.

TODO: demo of source linking.
TODO: appeal to VS Code users to upvote https://github.com/microsoft/vscode/issues/208093?

### Hover shows size/offset info

Hovering over the identifier that declares a type or struct field now
displays the size information for the type, and the offset information
for the field. This information may be helpful when making space
optimizations to your data structures, or when reading assembly code.

TODO: example hover image.

## Bugs fixed

## Thank you to our contributors!
7 changes: 4 additions & 3 deletions gopls/internal/cache/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -1464,9 +1464,10 @@ func (b *typeCheckBatch) checkPackage(ctx context.Context, ph *packageHandle) (*
defer done()

pkg := &syntaxPackage{
id: inputs.id,
fset: b.fset, // must match parse call below
types: types.NewPackage(string(inputs.pkgPath), string(inputs.name)),
id: inputs.id,
fset: b.fset, // must match parse call below
types: types.NewPackage(string(inputs.pkgPath), string(inputs.name)),
typesSizes: inputs.sizes,
typesInfo: &types.Info{
Types: make(map[ast.Expr]types.TypeAndValue),
Defs: make(map[*ast.Ident]types.Object),
Expand Down
6 changes: 6 additions & 0 deletions gopls/internal/cache/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type syntaxPackage struct {
typeErrors []types.Error
types *types.Package
typesInfo *types.Info
typesSizes types.Sizes
importMap map[PackagePath]*types.Package

xrefsOnce sync.Once
Expand Down Expand Up @@ -155,6 +156,11 @@ func (p *Package) TypesInfo() *types.Info {
return p.pkg.typesInfo
}

// TypesSizes returns the sizing function used for types in this package.
func (p *Package) TypesSizes() types.Sizes {
return p.pkg.typesSizes
}

// DependencyTypes returns the type checker's symbol for the specified
// package. It returns nil if path is not among the transitive
// dependencies of p, or if no symbols from that package were
Expand Down
89 changes: 89 additions & 0 deletions gopls/internal/golang/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,80 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
}
}

// Compute size information for types,
// and (size, offset) for struct fields.
//
// This information is useful when debugging crashes or
// optimizing layout. To reduce distraction, we show it only
// when hovering over the declaring identifier,
// but not referring identifiers.
//
// Size and alignment vary across OS/ARCH.
// Gopls will select the appropriate build configuration when
// viewing a type declaration in a build-tagged file, but will
// use the default build config for all other types, even
// if they embed platform-variant types.
//
var sizeOffset string // optional size/offset description
if def, ok := pkg.TypesInfo().Defs[ident]; ok && ident.Pos() == def.Pos() {
// This is the declaring identifier.
// (We can't simply use ident.Pos() == obj.Pos() because
// referencedObject prefers the TypeName for an embedded field).

// format returns the decimal and hex representation of x.
format := func(x int64) string {
if x < 10 {
return fmt.Sprintf("%d", x)
}
return fmt.Sprintf("%[1]d (%#[1]x)", x)
}

var data []string // {size, offset}, both optional

// If the type has free type parameters, its size cannot be
// computed. For now, we capture panics from go/types.Sizes.
// TODO(adonovan): use newly factored typeparams.Free.
try := func(f func()) bool {
defer func() { recover() }()
f()
return true
}

// size (types and fields)
if v, ok := obj.(*types.Var); ok && v.IsField() || is[*types.TypeName](obj) {
var sz int64
if try(func() { sz = pkg.TypesSizes().Sizeof(obj.Type()) }) {
data = append(data, "size="+format(sz))
}
}

// offset (fields)
if v, ok := obj.(*types.Var); ok && v.IsField() {
for _, n := range pathEnclosingObjNode(pgf.File, pos) {
if n, ok := n.(*ast.StructType); ok {
t := pkg.TypesInfo().TypeOf(n).(*types.Struct)
var fields []*types.Var
for i := 0; i < t.NumFields(); i++ {
f := t.Field(i)
fields = append(fields, f)
if f == v {
var offsets []int64
if try(func() { offsets = pkg.TypesSizes().Offsetsof(fields) }) {
if n := len(offsets); n > 0 {
data = append(data, "offset="+format(offsets[n-1]))
}
}
break
}
}
break
}
}
}

sizeOffset = strings.Join(data, ", ")
}

var typeDecl, methods, fields string

// For "objects defined by a type spec", the signature produced by
Expand Down Expand Up @@ -284,6 +358,16 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
return protocol.Range{}, nil, err
}
typeDecl = b.String()

// Splice in size/offset at end of first line.
// "type T struct { // size=..."
if sizeOffset != "" {
nl := strings.IndexByte(typeDecl, '\n')
if nl < 0 {
nl = len(typeDecl)
}
typeDecl = typeDecl[:nl] + " // " + sizeOffset + typeDecl[nl:]
}
}

// Promoted fields
Expand Down Expand Up @@ -353,6 +437,11 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
methods = b.String()

signature = typeDecl + "\n" + methods
} else {
// Non-types
if sizeOffset != "" {
signature += " // " + sizeOffset
}
}

// Compute link data (on pkg.go.dev or other documentation host).
Expand Down
3 changes: 2 additions & 1 deletion gopls/internal/test/marker/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ treatment by the test runner:
in these directories before running the test.
-skip_goos=a,b,c instructs the test runner to skip the test for the
listed GOOS values.
-skip_goarch=a,b,c does the same for GOARCH.
-ignore_extra_diags suppresses errors for unmatched diagnostics
TODO(rfindley): using build constraint expressions for -skip_goos would
TODO(rfindley): using build constraint expressions for -skip_go{os,arch} would
be clearer.
-filter_builtins=false disables the filtering of builtins from
completion results.
Expand Down
11 changes: 7 additions & 4 deletions gopls/internal/test/marker/marker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@ func Test(t *testing.T) {
if test.skipReason != "" {
t.Skip(test.skipReason)
}
for _, goos := range test.skipGOOS {
if runtime.GOOS == goos {
t.Skipf("skipping on %s due to -skip_goos", runtime.GOOS)
}
if slices.Contains(test.skipGOOS, runtime.GOOS) {
t.Skipf("skipping on %s due to -skip_goos", runtime.GOOS)
}
if slices.Contains(test.skipGOARCH, runtime.GOARCH) {
t.Skipf("skipping on %s due to -skip_goos", runtime.GOOS)
}

// TODO(rfindley): it may be more useful to have full support for build
Expand Down Expand Up @@ -500,6 +501,7 @@ type markerTest struct {
cgo bool
writeGoSum []string // comma separated dirs to write go sum for
skipGOOS []string // comma separated GOOS values to skip
skipGOARCH []string // comma separated GOARCH values to skip
ignoreExtraDiags bool
filterBuiltins bool
filterKeywords bool
Expand All @@ -513,6 +515,7 @@ func (t *markerTest) flagSet() *flag.FlagSet {
flags.BoolVar(&t.cgo, "cgo", false, "if set, requires cgo (both the cgo tool and CGO_ENABLED=1)")
flags.Var((*stringListValue)(&t.writeGoSum), "write_sumfile", "if set, write the sumfile for these directories")
flags.Var((*stringListValue)(&t.skipGOOS), "skip_goos", "if set, skip this test on these GOOS values")
flags.Var((*stringListValue)(&t.skipGOARCH), "skip_goarch", "if set, skip this test on these GOARCH values")
flags.BoolVar(&t.ignoreExtraDiags, "ignore_extra_diags", false, "if set, suppress errors for unmatched diagnostics")
flags.BoolVar(&t.filterBuiltins, "filter_builtins", true, "if set, filter builtins from completion results")
flags.BoolVar(&t.filterKeywords, "filter_keywords", true, "if set, filter keywords from completion results")
Expand Down
13 changes: 10 additions & 3 deletions gopls/internal/test/marker/testdata/definition/embed.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
This test checks definition and hover operations over embedded fields and methods.

Its size expectations assume a 64-bit machine,
and correct sizes information requires go1.21.

-- flags --
-skip_goarch=386
-min_go=go1.21

-- go.mod --
module mod.com

Expand Down Expand Up @@ -203,7 +210,7 @@ field S2 S2
[`(b.S1).S2` on pkg.go.dev](https://pkg.go.dev/mod.com/b#S1.S2)
-- @S2 --
```go
type S2 struct {
type S2 struct { // size=32 (0x20)
F1 string //@loc(S2F1, "F1")
F2 int //@loc(S2F2, "F2")
*a.A //@def("A", AString),def("a",AImport)
Expand Down Expand Up @@ -244,7 +251,7 @@ field Field int
[`(a.S).Field` on pkg.go.dev](https://pkg.go.dev/mod.com/a#S.Field)
-- @aA --
```go
type A string
type A string // size=16 (0x10)
```

@loc(AString, "A")
Expand All @@ -257,7 +264,7 @@ func (a.A) Hi()
[`a.A` on pkg.go.dev](https://pkg.go.dev/mod.com/a#A)
-- @aAlias --
```go
type aAlias = a.A
type aAlias = a.A // size=16 (0x10)
```

@loc(aAlias, "aAlias")
Expand Down
16 changes: 11 additions & 5 deletions gopls/internal/test/marker/testdata/definition/misc.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
This test exercises miscellaneous definition and hover requests.

Its size expectations assume a 64-bit machine.

-- go.mod --
module mod.com

go 1.16

-- flags --
-skip_goarch=386

-- a.go --
package a //@loc(aPackage, re"package (a)"),hover(aPackage, aPackage, aPackage)

Expand Down Expand Up @@ -122,35 +128,35 @@ func _() {
-- @aPackage --
-- @hoverDeclBlocka --
```go
type a struct {
type a struct { // size=16 (0x10)
x string
}
```

1st type declaration block
-- @hoverDeclBlockb --
```go
type b struct{}
type b struct{} // size=0
```

b has a comment
-- @hoverDeclBlockc --
```go
type c struct {
type c struct { // size=16 (0x10)
f string
}
```

c is a struct
-- @hoverDeclBlockd --
```go
type d string
type d string // size=16 (0x10)
```

3rd type declaration block
-- @hoverDeclBlocke --
```go
type e struct {
type e struct { // size=8
f float64
}
```
Expand Down
8 changes: 4 additions & 4 deletions gopls/internal/test/marker/testdata/hover/generics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ package generics

type value[T any] struct { //hover("lue", "value", value),hover("T", "T", valueT)
val T //@hover("T", "T", valuevalT)
Q int //@hover("Q", "Q", valueQ)
Q int64 //@hover("Q", "Q", valueQ)
}

type Value[T any] struct { //@hover("T", "T", ValueT)
val T //@hover("T", "T", ValuevalT)
Q int //@hover("Q", "Q", ValueQ)
Q int64 //@hover("Q", "Q", ValueQ)
}

func F[P interface{ ~int | string }]() { //@hover("P", "P", Ptparam)
Expand All @@ -46,7 +46,7 @@ func _() {

-- @ValueQ --
```go
field Q int
field Q int64 // size=8
```

@hover("Q", "Q", ValueQ)
Expand All @@ -67,7 +67,7 @@ func app(s []int, e int) []int // func[S interface{~[]E}, E interface{}](s S, e
```
-- @valueQ --
```go
field Q int
field Q int64 // size=8
```

@hover("Q", "Q", valueQ)
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/test/marker/testdata/hover/goprivate.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ package lib
type L struct{} //@hover("L", "L", L)
-- @L --
```go
type L struct{}
type L struct{} // size=0
```

GOPRIVATE should also match nested packages.
-- @T --
```go
type T struct{}
type T struct{} // size=0
```

T should not be linked, as it is private.
Loading

0 comments on commit 9663999

Please sign in to comment.