internal/lsp: improve literal func completion candidates

Previously we were erroneously suggesting a "func() {}" literal in
cases like:

http.Handle("/", <>)

This was happening because saw that the http.HandlerFunc type
satisfied the http.Handler interface, and that http.HandlerFunc is a
function type. However, of course, you can't pass a function literal
to http.Handle().

Make a few tweaks to address the problem:

1. Don't suggest literal "func () {}" candidates if the expected type
   is an interface type.

2. Suggest named function types that implement an interface. This
   causes us to suggest "http.HandlerFunc()" in the above example.

3. Suggest a func literal candidate inside named function type
   conversions. This will suggest "func() {}" when completing
   "http.HandlerFunc(<>)".

This way the false positive func literal is gone, and you still get
literal candidates that help you use an http.HandlerFunc as an
http.Handler. Note that this particular example is not very compelling
in light of http.HandleFunc() which can take a func literal directly,
but such a convenience function may not exist in other analogous
situations.

Change-Id: Ia68097b9a5b8351921349340d18acd8876554691
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205137
Reviewed-by: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: muirdm

internal/lsp/source/completion_literal.go

@@ -21,20 +21,18 @@ import (
 // literal generates composite literal, function literal, and make()
 // completion items.
 func (c *completer) literal(literalType types.Type, imp *importInfo) {
-	if c.expectedType.objType == nil {
-		return
-	}
-
 	// Don't provide literal candidates for variadic function arguments.
 	// For example, don't provide "[]interface{}{}" in "fmt.Print(<>)".
 	if c.expectedType.variadic {
 		return
 	}
 
+	expType := c.expectedType.objType
+
 	// Avoid literal candidates if the expected type is an empty
 	// interface. It isn't very useful to suggest a literal candidate of
 	// every possible type.
-	if isEmptyInterface(c.expectedType.objType) {
+	if expType != nil && isEmptyInterface(expType) {
 		return
 	}
 
@@ -48,8 +46,8 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) {
 	//
 	// don't offer "mySlice{}" since we have already added a candidate
 	// of "[]int{}".
-	if _, named := literalType.(*types.Named); named {
-		if _, named := deref(c.expectedType.objType).(*types.Named); !named {
+	if _, named := literalType.(*types.Named); named && expType != nil {
+		if _, named := deref(expType).(*types.Named); !named {
 			return
 		}
 	}
@@ -121,11 +119,11 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) {
 		switch t := literalType.Underlying().(type) {
 		case *types.Struct, *types.Array, *types.Slice, *types.Map:
 			c.compositeLiteral(t, typeName, float64(score), addlEdits)
-		case *types.Basic:
+		case *types.Basic, *types.Signature:
 			// Add a literal completion for basic types that implement our
 			// expected interface (e.g. named string type http.Dir
 			// implements http.FileSystem).
-			if isInterface(c.expectedType.objType) {
+			if isInterface(expType) {
 				c.basicLiteral(t, typeName, float64(score), addlEdits)
 			}
 		}
@@ -147,7 +145,7 @@ func (c *completer) literal(literalType types.Type, imp *importInfo) {
 	}
 
 	// If prefix matches "func", client may want a function literal.
-	if score := c.matcher.Score("func"); !isPointer && score >= 0 {
+	if score := c.matcher.Score("func"); !isPointer && score >= 0 && !isInterface(expType) {
 		switch t := literalType.Underlying().(type) {
 		case *types.Signature:
 			c.functionLiteral(t, float64(score))

internal/lsp/source/rename_check.go

@@ -930,9 +930,9 @@ func isPackageLevel(obj types.Object) bool {
 	return obj.Pkg().Scope().Lookup(obj.Name()) == obj
 }
 
-// -- Plundered from golang.org/x/tools/go/ssa -----------------
-
-func isInterface(T types.Type) bool { return types.IsInterface(T) }
+func isInterface(T types.Type) bool {
+	return T != nil && types.IsInterface(T)
+}
 
 // -- Plundered from go/scanner: ---------------------------------------
 

internal/lsp/testdata/snippets/literal_snippets.go.in

@@ -119,6 +119,13 @@ func _() {
 
 	http.HandleFunc("", f) //@snippet(")", litFunc, "", "func(${1:rw} http.ResponseWriter, ${2:r} *http.Request) {$0\\}")
 
+	// no literal "func" completions
+	http.Handle("", fun) //@complete(")")
+
+	http.HandlerFunc() //@item(handlerFunc, "http.HandlerFunc()", "", "var")
+	http.Handle("", h) //@snippet(")", handlerFunc, "http.HandlerFunc($0)", "http.HandlerFunc($0)")
+	http.Handle("", http.HandlerFunc()) //@snippet("))", litFunc, "", "func(${1:rw} http.ResponseWriter, ${2:r} *http.Request) {$0\\}")
+
 	var namedReturn func(s string) (b bool)
 	namedReturn = f //@snippet(" //", litFunc, "func(s string) (b bool) {$0\\}", "func(s string) (b bool) {$0\\}")
 

internal/lsp/testdata/summary.txt.golden

@@ -1,6 +1,6 @@
 -- summary --
-CompletionsCount = 214
-CompletionSnippetCount = 43
+CompletionsCount = 215
+CompletionSnippetCount = 45
 UnimportedCompletionsCount = 3
 DeepCompletionsCount = 5
 FuzzyCompletionsCount = 7