From 7d7faa4812bd548dafb3ef3238a968e0aa9b70d4 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 14 May 2019 03:09:27 +0000 Subject: [PATCH] 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 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/source/completion_snippet.go | 21 ++++++++++++++------- internal/lsp/testdata/snippets/snippets.go | 2 ++ internal/lsp/tests/tests.go | 2 +- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go index dfbecd25..2e8b0e93 100644 --- a/internal/lsp/source/completion_snippet.go +++ b/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++ { - call, ok := c.path[i].(*ast.CallExpr) - - // If we are the left side (i.e. "Fun") part of a call expression, - // we don't want a snippet since there are already parens present. - if ok && call.Fun == c.path[i-1] { - return nil, nil + // If we are the left side (i.e. "Fun") part of a call expression, + // we don't want a snippet since there are already parens present. + if len(c.path) > 1 { + switch n := c.path[1].(type) { + case *ast.CallExpr: + if n.Fun == c.path[0] { + 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 + } + } } } diff --git a/internal/lsp/testdata/snippets/snippets.go b/internal/lsp/testdata/snippets/snippets.go index 9871e154..d9c9580b 100644 --- a/internal/lsp/testdata/snippets/snippets.go +++ b/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},") diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index fc5a606c..5ffe3acf 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -36,7 +36,7 @@ const ( ExpectedHighlightsCount = 2 ExpectedSymbolsCount = 1 ExpectedSignaturesCount = 19 - ExpectedCompletionSnippetCount = 9 + ExpectedCompletionSnippetCount = 11 ExpectedLinksCount = 2 )