internal/lsp: don't overwrite suffix when inserting completion

In cases like "fmt.Pr<>int()" we previously would replace "Print" with
the new completion, yielding for example "fmt.Println()". Now we no
longer overwrite, yielding "fmt.Println()int()". There are some cases
where overwriting the suffix is what the user wants, but it is hard to
tell, so for now stick with the more expected behavior of not
overwriting.

Fixes golang/go#34011.

Change-Id: I8c3ccd8948245c27b52408ad508d8e01dc163ef4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196119
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 0f6b7f4..acb4899 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -46,8 +46,7 @@
 	return &protocol.CompletionList{
 		// When using deep completions/fuzzy matching, report results as incomplete so
 		// client fetches updated completions after every key stroke.
-		IsIncomplete: options.Completion.Deep,
-		Items:        toProtocolCompletionItems(candidates, rng, options),
+		Items: toProtocolCompletionItems(candidates, rng, options),
 	}, nil
 }
 
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 6a4b1c8..4e47a6c 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -226,6 +226,10 @@
 	return p.content[:p.cursor-p.spanRange.Start]
 }
 
+func (p Selection) Suffix() string {
+	return p.content[p.cursor-p.spanRange.Start:]
+}
+
 func (c *completer) setSurrounding(ident *ast.Ident) {
 	if c.surrounding != nil {
 		return
@@ -233,11 +237,13 @@
 	if !(ident.Pos() <= c.pos && c.pos <= ident.End()) {
 		return
 	}
+
 	c.surrounding = &Selection{
 		content: ident.Name,
 		cursor:  c.pos,
 		mappedRange: mappedRange{
-			spanRange: span.NewRange(c.view.Session().Cache().FileSet(), ident.Pos(), ident.End()),
+			// Overwrite the prefix only.
+			spanRange: span.NewRange(c.view.Session().Cache().FileSet(), ident.Pos(), c.pos),
 			m:         c.mapper,
 		},
 	}
diff --git a/internal/lsp/source/completion_snippet.go b/internal/lsp/source/completion_snippet.go
index 02c56f5..74f635a 100644
--- a/internal/lsp/source/completion_snippet.go
+++ b/internal/lsp/source/completion_snippet.go
@@ -41,9 +41,11 @@
 		}
 	})
 
+	fset := c.view.Session().Cache().FileSet()
+
 	// If the cursor position is on a different line from the literal's opening brace,
 	// we are in a multiline literal.
-	if c.view.Session().Cache().FileSet().Position(c.pos).Line != c.view.Session().Cache().FileSet().Position(clInfo.cl.Lbrace).Line {
+	if fset.Position(c.pos).Line != fset.Position(clInfo.cl.Lbrace).Line {
 		snip.WriteText(",")
 	}
 
@@ -52,9 +54,12 @@
 
 // functionCallSnippets calculates the snippet for function calls.
 func (c *completer) functionCallSnippet(name string, params []string) *snippet.Builder {
-	// 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 {
+	// If there is no suffix then we need to reuse existing call parens
+	// "()" if present. If there is an identifer suffix then we always
+	// need to include "()" since we don't overwrite the suffix.
+	if c.surrounding != nil && c.surrounding.Suffix() == "" && len(c.path) > 1 {
+		// 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.
 		switch n := c.path[1].(type) {
 		case *ast.CallExpr:
 			// The Lparen != Rparen check detects fudged CallExprs we
diff --git a/internal/lsp/testdata/snippets/snippets.go.in b/internal/lsp/testdata/snippets/snippets.go.in
index 872753b..bba8c39 100644
--- a/internal/lsp/testdata/snippets/snippets.go.in
+++ b/internal/lsp/testdata/snippets/snippets.go.in
@@ -32,8 +32,8 @@
 	Foo{Foo{}.B} //@snippet("} ", snipFieldBar, "Bar", "Bar")
 
 	var err error
-	err.Error() //@snippet("E", Error, "Error", "Error")
-	f.Baz()     //@snippet("B", snipMethodBaz, "Baz", "Baz")
+	err.Error() //@snippet("E", Error, "Error()", "Error()")
+	f.Baz()     //@snippet("B", snipMethodBaz, "Baz()", "Baz()")
 
 	f.Baz()     //@snippet("(", snipMethodBazBar, "BazBar", "BazBar")
 }