Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions gopls/internal/golang/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,13 @@ type completionContext struct {
// commentCompletion is true if we are completing a comment.
commentCompletion bool

// commentNeedsLeadingSpace is true when the completion edit range starts
// immediately after "//" with no intervening space. Completion items should
// prepend a space so that accepting a suggestion produces "// Name" rather
// than "//Name". This applies both when the cursor is right after "//" and
// when the user has partially typed an identifier (e.g. "//Fo").
commentNeedsLeadingSpace bool

// packageCompletion is true if we are completing a package name.
packageCompletion bool
}
Expand Down Expand Up @@ -1049,6 +1056,20 @@ func (c *completer) populateCommentCompletions(comment *ast.CommentGroup) {
// comment is valid, set surrounding as word boundaries around cursor
c.setSurroundingForComment(comment)

// If the edit range (what's being replaced) starts immediately after "//"
// with no space, completions must prepend a space to produce proper Go doc
// comment style ("// Name" rather than "//Name"). This covers both the case
// where the cursor is right after "//" and where the user has already typed
// some characters (e.g. "//Fo" completing to "// FooBar").
if c.surrounding != nil {
for _, cmt := range comment.List {
if c.surrounding.start == cmt.Slash+2 {
c.completionContext.commentNeedsLeadingSpace = true
break
}
}
}

// Using the next line pos, grab and parse the exported symbol on that line
for _, n := range c.pgf.File.Decls {
declLine := safetoken.Line(file, n.Pos())
Expand Down
8 changes: 8 additions & 0 deletions gopls/internal/golang/completion/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,14 @@ Suffixes:
if cand.detail != "" {
detail = cand.detail
}

// When completing right after "//" (cursor at the slashes with no space),
// prepend a space so the result is "// Name" rather than "//Name".
if c.completionContext.commentNeedsLeadingSpace {
insert = " " + insert
snip.PrependText(" ")
}

item := CompletionItem{
Label: label,
InsertText: insert,
Expand Down
95 changes: 95 additions & 0 deletions gopls/internal/golang/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"golang.org/x/tools/gopls/internal/util/cursorutil"
"golang.org/x/tools/gopls/internal/util/safetoken"
"golang.org/x/tools/gopls/internal/util/tokeninternal"
"golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/astutil"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/stdlib"
Expand Down Expand Up @@ -321,6 +322,35 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng pr
return hoverReturnStatement(pgf, cur)
case *ast.Ident:
// fall through to rest of function
case *ast.FuncType:
// When hovering over the "func" keyword of a func literal that is
// implicitly converted to a named function type, show the named
// type's documentation (e.g. hovering over "func" in a call like
// filepath.WalkDir(dir, func(path string, ...) error { ... }) shows
// the docs for fs.WalkDirFunc).
//
// Note: FindByPos at a FuncLit's "func" token returns the inner
// *ast.FuncType node (not *ast.FuncLit), so we check the parent.
if inToken(node.Func, "func", posRange.Pos(), posRange.End()) {
if lit, ok := cur.Parent().Node().(*ast.FuncLit); ok {
if rng, res, err := hoverFuncLit(ctx, snapshot, pkg, pgf, cur.Parent(), lit); res != nil || err != nil {
return rng, res, err
}
}
}
// No named type found; fall through to the generic ast.Expr behavior.
if _, ok := pkg.TypesInfo().Types[node]; !ok {
return protocol.Range{}, nil, nil
}
exprResult := &hoverResult{
Synopsis: goastutil.NodeDescription(node),
FullDocumentation: types.TypeString(pkg.TypesInfo().TypeOf(node), qual),
}
exprHighlight, err := pgf.NodeRange(node)
if err != nil {
return protocol.Range{}, nil, err
}
return exprHighlight, exprResult, nil
case ast.Expr:
tv, ok := pkg.TypesInfo().Types[node]
if !ok {
Expand Down Expand Up @@ -1142,6 +1172,71 @@ func hoverConstantExpr(pgf *parsego.File, expr ast.Expr, tv types.TypeAndValue,
}, nil
}

// hoverFuncLit handles hover over the "func" keyword of a func literal that
// is implicitly converted to a named function type (e.g. fs.WalkDirFunc).
// It returns a non-nil result if a named type is found; otherwise it returns
// nil, nil, nil to signal that the caller should fall through to default hover.
func hoverFuncLit(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, cur inspector.Cursor, lit *ast.FuncLit) (protocol.Range, *hoverResult, error) {
// Use TypesFromContext to find the type the func literal must satisfy.
var named *types.Named
for _, t := range typesutil.TypesFromContext(pkg.TypesInfo(), cur) {
n, ok := types.Unalias(t).(*types.Named)
if !ok {
continue
}
if _, ok := n.Underlying().(*types.Signature); !ok {
continue // not a named function type
}
named = n
break
}
if named == nil {
return protocol.Range{}, nil, nil
}

obj := named.Obj()

// Load the declaring package to get documentation and signature.
declPkg, declPGF, declPos, err := NarrowestDeclaringPackage(ctx, snapshot, pkg, obj)
if err != nil {
return protocol.Range{}, nil, err
}

decl, spec, _ := findDeclInfo([]*ast.File{declPGF.File}, declPos)
var docText string
if docComment := chooseDocComment(decl, spec, nil); docComment != nil {
docText = docComment.Text()
}

qual := typesinternal.FileQualifier(pgf.File, pkg.Types())
signature := objectString(obj, qual, declPos, declPGF.Tok, spec)

var linkPath, anchor string
if obj.Exported() && typesinternal.IsPackageLevel(obj) && !snapshot.IsGoPrivatePath(obj.Pkg().Path()) {
linkPath = obj.Pkg().Path()
anchor = obj.Name()
if declPkg.Metadata().Module != nil && declPkg.Metadata().Module.Version != "" {
mod := declPkg.Metadata().Module
linkPath = strings.Replace(linkPath, mod.Path, cache.ResolvedString(mod), 1)
}
}

rng, err := pgf.NodeRange(lit.Type)
if err != nil {
return protocol.Range{}, nil, err
}

return rng, &hoverResult{
Synopsis: doc.Synopsis(docText),
FullDocumentation: docText,
Signature: signature,
SingleLine: signature,
SymbolName: fmt.Sprintf("%s.%s", obj.Pkg().Name(), obj.Name()),
LinkPath: linkPath,
LinkAnchor: anchor,
}, nil
}

func hoverReturnStatement(pgf *parsego.File, curReturn inspector.Cursor) (protocol.Range, *hoverResult, error) {
var funcType *ast.FuncType
// Find innermost enclosing function.
Expand Down
5 changes: 4 additions & 1 deletion gopls/internal/golang/implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,10 @@ func implFuncs(pkg *cache.Package, curSel inspector.Cursor, start, end token.Pos
if inToken(n.Lparen, "(", start, end) {
t := dynamicFuncCallType(info, n)
if t == nil {
return nil, fmt.Errorf("not a dynamic function call")
// Not a dynamic function call (e.g. a regular method call
// like err.Error()). Fall back to the method-sets algorithm
// so the caller can find interface implementations normally.
return nil, errNotHandled
}
// Case 2b: dynamic call of function value.
// Report declarations of corresponding concrete functions.
Expand Down
8 changes: 8 additions & 0 deletions gopls/internal/test/marker/testdata/completion/comment.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ func Function() int { //@item(function, "Function", "func() int", "func") //@com
return 0
}

// This tests that completing right after "//" (no space) above a declaration
// suggests the identifier name, prepending a space for proper doc comment style.
//@complete(re"//()", plainFunc)
// This tests completing with a partial prefix after "//" (no space) also
// prepends a space, covering the case where the user has typed "//Plain".
//Plain//@complete(re"//Plain()", plainFunc)
func PlainFunc() {} //@item(plainFunc, "PlainFunc", "func()", "func")

// This tests multiline block comments and completion with prefix
// Lorem Ipsum Multili//@complete(re"()Multi", multiline)
// Lorem ipsum dolor sit ametom
Expand Down
50 changes: 50 additions & 0 deletions gopls/internal/test/marker/testdata/hover/funclittype.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
This test checks that hovering over the "func" keyword of a func literal
that is implicitly converted to a named function type shows the named
type's documentation.

-- go.mod --
module example.com

go 1.18

-- a/a.go --
package a

// WalkFunc is the type of the function called by Walk to visit each
// file or directory.
type WalkFunc func(path string, err error) error

// Walk calls fn for each file.
func Walk(root string, fn WalkFunc) {}

-- b/b.go --
package b

import "example.com/a"

func _() {
// Hovering over "func" shows WalkFunc's documentation.
a.Walk(".", func(path string, err error) error { //@hover("func", "func(path string, err error) error", funclittype)
return nil
})

// Hovering over "func" when no named type — falls back to showing type.
f := func(x int) int { return x } //@hover("func", "func(x int) int", funclitdefault)
_ = f
}

-- @funclittype --
```go
type a.WalkFunc func(path string, err error) error
```

---

WalkFunc is the type of the function called by Walk to visit each file or directory.


---

[`a.WalkFunc` on pkg.go.dev](https://pkg.go.dev/example.com/a#WalkFunc)
-- @funclitdefault --
func(x int) int
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ go 1.18
-- a/a.go --
package a

var _ = undefined() //@ diag("undefined", re"undefined"), implementation("(", err="not a dynamic function call")
var _ = undefined() //@ diag("undefined", re"undefined"), implementation("(", err="no identifier found")
15 changes: 9 additions & 6 deletions gopls/internal/test/marker/testdata/implementation/signature.txt
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,15 @@ func _(

struct{x Nullary}{}.x() //@ loc(fieldCall, "("), implementation("(", nullaryFunc)

// Calls that are not dynamic function calls:
_ = len("") //@ implementation("(", err="not a dynamic function call")
_ = int(0) //@ implementation("(", err="not a dynamic function call")
_ = error.Error(nil) //@ implementation("(", err="not a dynamic function call")
_ = err.Error() //@ implementation("(", err="not a dynamic function call")
_ = f4(0) //@ implementation("(", err="not a dynamic function call"), loc(f4Call, "(")
// Calls that are not dynamic function calls: implFuncs returns
// errNotHandled so the method-sets fallback runs, but finds no
// identifier at "(" and returns "no identifier found".
// Users should query on the method name instead (e.g. "Error").
_ = len("") //@ implementation("(", err="no identifier found")
_ = int(0) //@ implementation("(", err="no identifier found")
_ = error.Error(nil) //@ implementation("(", err="no identifier found")
_ = err.Error() //@ implementation("(", err="no identifier found")
_ = f4(0) //@ implementation("(", err="no identifier found"), loc(f4Call, "(")
}


Expand Down