internal/lsp: fix missing parens in function call snippet

We were omitting the parens in function completions like "(foo<>)()"
because our check thought "foo" was the Fun in the outer CallExpr so
it already had parens. Fix by tightening up logic to only omit parens
for cases like "foo<>()" and "foo.bar<>()".

Change-Id: Ia602b80275f72baa6cdf6d61c22d3f3a6cfc3019
GitHub-Last-Rev: 41fecf92617e0812ee6552d8c43789eae83889bd
GitHub-Pull-Request: golang/tools#98
Reviewed-on: https://go-review.googlesource.com/c/tools/+/176944
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

internal/lsp/source/completion_snippet.go

@@ -56,13 +56,20 @@ func (c *completer) structFieldSnippets(label, detail string) (*snippet.Builder,
 
 // functionCallSnippets calculates the plain and placeholder snippets for function calls.
 func (c *completer) functionCallSnippets(name string, params []string) (*snippet.Builder, *snippet.Builder) {
-	for i := 1; i <= 2 && i < len(c.path); i++ {
+	// If we are the left side (i.e. "Fun") part of a call expression,
-		call, ok := c.path[i].(*ast.CallExpr)
+	// we don't want a snippet since there are already parens present.
-
+	if len(c.path) > 1 {
-		// If we are the left side (i.e. "Fun") part of a call expression,
+		switch n := c.path[1].(type) {
-		// we don't want a snippet since there are already parens present.
+		case *ast.CallExpr:
-		if ok && call.Fun == c.path[i-1] {
+			if n.Fun == c.path[0] {
-			return nil, nil
+				return nil, nil
+			}
+		case *ast.SelectorExpr:
+			if len(c.path) > 2 {
+				if call, ok := c.path[2].(*ast.CallExpr); ok && call.Fun == c.path[1] {
+					return nil, nil
+				}
+			}
 		}
 	}
 

internal/lsp/testdata/snippets/snippets.go

@@ -18,6 +18,8 @@ func _() {
 	bar(ba) //@snippet(")", snipBar, "bar(${1})", "bar(${1:fn func()})")
 	var f Foo
 	bar(f.Ba) //@snippet(")", snipMethodBaz, "Baz()", "Baz()")
+	(bar)(nil) //@snippet(")", snipBar, "bar(${1})", "bar(${1:fn func()})")
+	(f.Ba)() //@snippet(")", snipMethodBaz, "Baz()", "Baz()")
 
 	Foo{
 		B //@snippet(" //", snipFieldBar, "Bar: ${1},", "Bar: ${1:int},")

internal/lsp/tests/tests.go

@@ -36,7 +36,7 @@ const (
 	ExpectedHighlightsCount        = 2
 	ExpectedSymbolsCount           = 1
 	ExpectedSignaturesCount        = 19
-	ExpectedCompletionSnippetCount = 9
+	ExpectedCompletionSnippetCount = 11
 	ExpectedLinksCount             = 2
 )