Skip to content

Commit 6ece8fb

Browse files
committed
gopls/internal/golang: improve ergonomics of "Browse documentation"
This change causes the 'Browse documentation...' code action to offer different pages based on the current selection: - in an ImportSpec, or a use of the PkgName, we display the imported package; - in a reference to an imported symbol, we display that symbol (except for fields, or methods of nonexported types). The logic that computed the fragment, now extracted to golang.DocFragment, now also computes an appropriate title. The various cases of this function are exercised by a new integration test. Also, rename s/Render/PkgDoc/ in the integration tests. Updates golang/go#67949 Change-Id: I7f4b014beca8cfde9ca3540dd10b32e1eb8f95e2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/592577 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 693d7fe commit 6ece8fb

File tree

8 files changed

+343
-96
lines changed

8 files changed

+343
-96
lines changed

gopls/internal/golang/codeaction.go

+21-14
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
4848
if wantQuickFixes ||
4949
want[protocol.SourceOrganizeImports] ||
5050
want[protocol.RefactorExtract] ||
51-
want[protocol.GoDoc] ||
5251
want[protocol.GoFreeSymbols] {
5352

5453
pgf, err := snapshot.ParseGo(ctx, fh, parsego.Full)
@@ -103,19 +102,6 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
103102
actions = append(actions, extractions...)
104103
}
105104

106-
if want[protocol.GoDoc] {
107-
loc := protocol.Location{URI: pgf.URI, Range: rng}
108-
cmd, err := command.NewDocCommand("Browse package documentation", loc)
109-
if err != nil {
110-
return nil, err
111-
}
112-
actions = append(actions, protocol.CodeAction{
113-
Title: cmd.Title,
114-
Kind: protocol.GoDoc,
115-
Command: &cmd,
116-
})
117-
}
118-
119105
if want[protocol.GoFreeSymbols] && rng.End != rng.Start {
120106
loc := protocol.Location{URI: pgf.URI, Range: rng}
121107
cmd, err := command.NewFreeSymbolsCommand("Browse free symbols", snapshot.View().ID(), loc)
@@ -135,11 +121,17 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
135121
if want[protocol.RefactorRewrite] ||
136122
want[protocol.RefactorInline] ||
137123
want[protocol.GoAssembly] ||
124+
want[protocol.GoDoc] ||
138125
want[protocol.GoTest] {
139126
pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, fh.URI())
140127
if err != nil {
141128
return nil, err
142129
}
130+
start, end, err := pgf.RangePos(rng)
131+
if err != nil {
132+
return nil, err
133+
}
134+
143135
if want[protocol.RefactorRewrite] {
144136
rewrites, err := getRewriteCodeActions(ctx, pkg, snapshot, pgf, fh, rng, snapshot.Options())
145137
if err != nil {
@@ -166,6 +158,21 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
166158
actions = append(actions, fixes...)
167159
}
168160

161+
if want[protocol.GoDoc] {
162+
// "Browse documentation for ..."
163+
_, _, title := DocFragment(pkg, pgf, start, end)
164+
loc := protocol.Location{URI: pgf.URI, Range: rng}
165+
cmd, err := command.NewDocCommand(title, loc)
166+
if err != nil {
167+
return nil, err
168+
}
169+
actions = append(actions, protocol.CodeAction{
170+
Title: cmd.Title,
171+
Kind: protocol.GoDoc,
172+
Command: &cmd,
173+
})
174+
}
175+
169176
if want[protocol.GoAssembly] {
170177
fixes, err := getGoAssemblyAction(snapshot.View(), pkg, pgf, rng)
171178
if err != nil {

gopls/internal/golang/hover.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, positi
125125
// hover computes hover information at the given position. If we do not support
126126
// hovering at the position, it returns _, nil, nil: an error is only returned
127127
// if the position is valid but we fail to compute hover information.
128+
//
129+
// TODO(adonovan): strength-reduce file.Handle to protocol.DocumentURI.
128130
func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp protocol.Position) (protocol.Range, *hoverJSON, error) {
129131
pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, fh.URI())
130132
if err != nil {
@@ -533,13 +535,13 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
533535
pkg := obj.Pkg()
534536
if recv != nil {
535537
linkName = fmt.Sprintf("(%s.%s).%s", pkg.Name(), recv.Name(), obj.Name())
536-
if obj.Exported() && recv.Exported() && pkg.Scope().Lookup(recv.Name()) == recv {
538+
if obj.Exported() && recv.Exported() && isPackageLevel(recv) {
537539
linkPath = pkg.Path()
538540
anchor = fmt.Sprintf("%s.%s", recv.Name(), obj.Name())
539541
}
540542
} else {
541543
linkName = fmt.Sprintf("%s.%s", pkg.Name(), obj.Name())
542-
if obj.Exported() && pkg.Scope().Lookup(obj.Name()) == obj {
544+
if obj.Exported() && isPackageLevel(obj) {
543545
linkPath = pkg.Path()
544546
anchor = obj.Name()
545547
}

gopls/internal/golang/pkgdoc.go

+207-3
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,219 @@ import (
4242
"path/filepath"
4343
"strings"
4444

45+
"golang.org/x/tools/go/ast/astutil"
4546
"golang.org/x/tools/gopls/internal/cache"
47+
"golang.org/x/tools/gopls/internal/cache/parsego"
4648
"golang.org/x/tools/gopls/internal/protocol"
47-
"golang.org/x/tools/gopls/internal/util/astutil"
49+
goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
4850
"golang.org/x/tools/gopls/internal/util/bug"
4951
"golang.org/x/tools/gopls/internal/util/safetoken"
5052
"golang.org/x/tools/gopls/internal/util/slices"
5153
"golang.org/x/tools/gopls/internal/util/typesutil"
5254
"golang.org/x/tools/internal/typesinternal"
5355
)
5456

57+
// DocFragment finds the package and (optionally) symbol identified by
58+
// the current selection, and returns the package path and the
59+
// optional symbol URL fragment (e.g. "#Buffer.Len") for a symbol,
60+
// along with a title for the code action.
61+
//
62+
// It is called once to offer the code action, and again when the
63+
// command is executed. This is slightly inefficient but ensures that
64+
// the title and package/symbol logic are consistent in all cases.
65+
//
66+
// It returns zeroes if there is nothing to see here (e.g. reference to a builtin).
67+
func DocFragment(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (pkgpath PackagePath, fragment, title string) {
68+
thing := thingAtPoint(pkg, pgf, start, end)
69+
70+
makeTitle := func(kind string, imp *types.Package, name string) string {
71+
title := "Browse documentation for " + kind + " "
72+
if imp != nil && imp != pkg.Types() {
73+
title += imp.Name() + "."
74+
}
75+
return title + name
76+
}
77+
78+
wholePackage := func(pkg *types.Package) (PackagePath, string, string) {
79+
// External test packages don't have /pkg doc pages,
80+
// so instead show the doc for the package under test.
81+
// (This named-based heuristic is imperfect.)
82+
if forTest := strings.TrimSuffix(pkg.Path(), "_test"); forTest != pkg.Path() {
83+
return PackagePath(forTest), "", makeTitle("package", nil, filepath.Base(forTest))
84+
}
85+
86+
return PackagePath(pkg.Path()), "", makeTitle("package", nil, pkg.Name())
87+
}
88+
89+
// Conceptually, we check cases in the order:
90+
// 1. symbol
91+
// 2. package
92+
// 3. enclosing
93+
// but the logic of cases 1 and 3 are identical, hence the odd factoring.
94+
95+
// Imported package?
96+
if thing.pkg != nil && thing.symbol == nil {
97+
return wholePackage(thing.pkg)
98+
}
99+
100+
// Symbol?
101+
var sym types.Object
102+
if thing.symbol != nil {
103+
sym = thing.symbol // reference to a symbol
104+
} else if thing.enclosing != nil {
105+
sym = thing.enclosing // selection is within a declaration of a symbol
106+
}
107+
if sym == nil {
108+
return wholePackage(pkg.Types()) // no symbol
109+
}
110+
111+
// Built-in (error.Error, append or unsafe).
112+
// TODO(adonovan): handle builtins in /pkg viewer.
113+
if sym.Pkg() == nil {
114+
return "", "", "" // nothing to see here
115+
}
116+
pkgpath = PackagePath(sym.Pkg().Path())
117+
118+
// Unexported? Show enclosing type or package.
119+
if !sym.Exported() {
120+
// Unexported method of exported type?
121+
if fn, ok := sym.(*types.Func); ok {
122+
if recv := fn.Type().(*types.Signature).Recv(); recv != nil {
123+
_, named := typesinternal.ReceiverNamed(recv)
124+
if named != nil && named.Obj().Exported() {
125+
sym = named.Obj()
126+
goto below
127+
}
128+
}
129+
}
130+
131+
return wholePackage(sym.Pkg())
132+
below:
133+
}
134+
135+
// Reference to symbol in external test package?
136+
// Short-circuit: see comment in wholePackage.
137+
if strings.HasSuffix(string(pkgpath), "_test") {
138+
return wholePackage(pkg.Types())
139+
}
140+
141+
// package-level symbol?
142+
if isPackageLevel(sym) {
143+
return pkgpath, sym.Name(), makeTitle(objectKind(sym), sym.Pkg(), sym.Name())
144+
}
145+
146+
// Inv: sym is field or method, or local.
147+
switch sym := sym.(type) {
148+
case *types.Func: // => method
149+
sig := sym.Type().(*types.Signature)
150+
isPtr, named := typesinternal.ReceiverNamed(sig.Recv())
151+
if named != nil {
152+
if !named.Obj().Exported() {
153+
return wholePackage(sym.Pkg()) // exported method of unexported type
154+
}
155+
name := fmt.Sprintf("(%s%s).%s",
156+
strings.Repeat("*", btoi(isPtr)), // for *T
157+
named.Obj().Name(),
158+
sym.Name())
159+
fragment := named.Obj().Name() + "." + sym.Name()
160+
return pkgpath, fragment, makeTitle("method", sym.Pkg(), name)
161+
}
162+
163+
case *types.Var:
164+
if sym.IsField() {
165+
// TODO(adonovan): support fields.
166+
// The Var symbol doesn't include the struct
167+
// type, so we need to use the logic from
168+
// Hover. (This isn't important for
169+
// DocFragment as fields don't have fragments,
170+
// but it matters to the grand unification of
171+
// Hover/Definition/DocFragment.
172+
}
173+
}
174+
175+
// Field, non-exported method, or local declaration:
176+
// just show current package.
177+
return wholePackage(pkg.Types())
178+
}
179+
180+
// thing describes the package or symbol denoted by a selection.
181+
//
182+
// TODO(adonovan): Hover, Definition, and References all start by
183+
// identifying the selected object. Let's achieve a better factoring
184+
// of the common parts using this structure, including uniform
185+
// treatment of doc links, linkname, and suchlike.
186+
type thing struct {
187+
// At most one of these fields is set.
188+
// (The 'enclosing' field is a fallback for when neither
189+
// of the first two is set.)
190+
symbol types.Object // referenced symbol
191+
pkg *types.Package // referenced package
192+
enclosing types.Object // package-level symbol or method decl enclosing selection
193+
}
194+
195+
func thingAtPoint(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) thing {
196+
path, _ := astutil.PathEnclosingInterval(pgf.File, start, end)
197+
198+
// In an import spec?
199+
if len(path) >= 3 { // [...ImportSpec GenDecl File]
200+
if spec, ok := path[len(path)-3].(*ast.ImportSpec); ok {
201+
if pkgname, ok := typesutil.ImportedPkgName(pkg.TypesInfo(), spec); ok {
202+
return thing{pkg: pkgname.Imported()}
203+
}
204+
}
205+
}
206+
207+
// Definition or reference to symbol?
208+
var obj types.Object
209+
if id, ok := path[0].(*ast.Ident); ok {
210+
obj = pkg.TypesInfo().ObjectOf(id)
211+
212+
// Treat use to PkgName like ImportSpec.
213+
if pkgname, ok := obj.(*types.PkgName); ok {
214+
return thing{pkg: pkgname.Imported()}
215+
}
216+
217+
} else if sel, ok := path[0].(*ast.SelectorExpr); ok {
218+
// e.g. selection is "fmt.Println" or just a portion ("mt.Prin")
219+
obj = pkg.TypesInfo().Uses[sel.Sel]
220+
}
221+
if obj != nil {
222+
return thing{symbol: obj}
223+
}
224+
225+
// Find enclosing declaration.
226+
if n := len(path); n > 1 {
227+
switch decl := path[n-2].(type) {
228+
case *ast.FuncDecl:
229+
// method?
230+
if fn := pkg.TypesInfo().Defs[decl.Name]; fn != nil {
231+
return thing{enclosing: fn}
232+
}
233+
234+
case *ast.GenDecl:
235+
// path=[... Spec? GenDecl File]
236+
for _, spec := range decl.Specs {
237+
if n > 2 && spec == path[n-3] {
238+
var name *ast.Ident
239+
switch spec := spec.(type) {
240+
case *ast.ValueSpec:
241+
// var, const: use first name
242+
name = spec.Names[0]
243+
case *ast.TypeSpec:
244+
name = spec.Name
245+
}
246+
if name != nil {
247+
return thing{enclosing: pkg.TypesInfo().Defs[name]}
248+
}
249+
break
250+
}
251+
}
252+
}
253+
}
254+
255+
return thing{} // nothing to see here
256+
}
257+
55258
// Web is an abstraction of gopls' web server.
56259
type Web interface {
57260
// PkgURL forms URLs of package or symbol documentation.
@@ -375,7 +578,7 @@ window.addEventListener('load', function() {
375578
// appear as separate decls. We should too.
376579
var buf bytes.Buffer
377580
for _, file := range pkg.CompiledGoFiles() {
378-
if astutil.NodeContains(file.File, n.Pos()) {
581+
if goplsastutil.NodeContains(file.File, n.Pos()) {
379582
pos := n.Pos()
380583

381584
// emit emits source in the interval [pos:to] and updates pos.
@@ -633,7 +836,8 @@ window.addEventListener('load', function() {
633836
method, _, _ := types.LookupFieldOrMethod(tname.Type(), true, tname.Pkg(), docmethod.Name)
634837
fmt.Fprintf(&buf, "<h4 id='%s.%s'>func (%s) %s</h4>\n",
635838
doctype.Name, docmethod.Name,
636-
doctype.Name, objHTML(pkg.FileSet(), web, method))
839+
docmethod.Orig, // T or *T
840+
objHTML(pkg.FileSet(), web, method))
637841

638842
// decl: func (x T) M(params) results
639843
fmt.Fprintf(&buf, "<pre class='code'>%s</pre>\n",

gopls/internal/golang/util.go

+9
Original file line numberDiff line numberDiff line change
@@ -354,3 +354,12 @@ func (f ImporterFunc) Import(path string) (*types.Package, error) { return f(pat
354354
// isBuiltin reports whether obj is a built-in symbol (e.g. append, iota, error.Error, unsafe.Slice).
355355
// All other symbols have a valid position and a valid package.
356356
func isBuiltin(obj types.Object) bool { return !obj.Pos().IsValid() }
357+
358+
// btoi returns int(b) as proposed in #64825.
359+
func btoi(b bool) int {
360+
if b {
361+
return 1
362+
} else {
363+
return 0
364+
}
365+
}

0 commit comments

Comments
 (0)