gopls/internal/regtest/source/completion: reuse functionCallSnippet in

unimported completion

The unimported completion logic was using ad-hoc snippet logic that
didn't correctly handle "usePlaceholders", and wasn't updated for the
recently added "completeFunctionCalls" setting.

Refactor to re-use the functionCallSnippet helper, and add a test.

Fixes golang/go#62676

Change-Id: I08277cfc556455fbe17900ad56821e269990ead6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531458
Commit-Queue: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

Author: findleyr

gopls/internal/lsp/source/completion/completion.go

@@ -1320,32 +1320,18 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error {
 
 			// For functions, add a parameter snippet.
 			if fn != nil {
-				var sn snippet.Builder
-				sn.WriteText(id.Name)
-
-				paramList := func(open, close string, list *ast.FieldList) {
+				paramList := func(list *ast.FieldList) []string {
+					var params []string
 					if list != nil {
 						var cfg printer.Config // slight overkill
-						var nparams int
 						param := func(name string, typ ast.Expr) {
-							if nparams > 0 {
-								sn.WriteText(", ")
-							}
-							nparams++
-							if c.opts.placeholders {
-								sn.WritePlaceholder(func(b *snippet.Builder) {
-									var buf strings.Builder
-									buf.WriteString(name)
-									buf.WriteByte(' ')
-									cfg.Fprint(&buf, token.NewFileSet(), typ)
-									b.WriteText(buf.String())
-								})
-							} else {
-								sn.WriteText(name)
-							}
+							var buf strings.Builder
+							buf.WriteString(name)
+							buf.WriteByte(' ')
+							cfg.Fprint(&buf, token.NewFileSet(), typ)
+							params = append(params, buf.String())
 						}
 
-						sn.WriteText(open)
 						for _, field := range list.List {
 							if field.Names != nil {
 								for _, name := range field.Names {
@@ -1355,13 +1341,14 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error {
 								param("_", field.Type)
 							}
 						}
-						sn.WriteText(close)
 					}
+					return params
 				}
 
-				paramList("[", "]", typeparams.ForFuncType(fn.Type))
-				paramList("(", ")", fn.Type.Params)
-
+				tparams := paramList(fn.Type.TypeParams)
+				params := paramList(fn.Type.Params)
+				var sn snippet.Builder
+				c.functionCallSnippet(id.Name, tparams, params, &sn)
 				item.snippet = &sn
 			}
 

gopls/internal/regtest/marker/testdata/completion/issue62676.txt

@@ -0,0 +1,63 @@
+This test verifies that unimported completion respects the usePlaceholders setting.
+
+-- flags --
+-ignore_extra_diags
+
+-- settings.json --
+{
+	"usePlaceholders": false
+}
+
+-- go.mod --
+module mod.test
+
+go 1.21
+
+-- foo/foo.go --
+package foo
+
+func _() {
+	// This uses goimports-based completion; TODO: this should insert snippets.
+	os.Open //@acceptcompletion(re"Open()", "Open", open)
+}
+
+func _() {
+	// This uses metadata-based completion.
+	errors.New //@acceptcompletion(re"New()", "New", new)
+}
+
+-- bar/bar.go --
+package bar
+
+import _ "errors" // important: doesn't transitively import os.
+
+-- @new/foo/foo.go --
+package foo
+
+import "errors"
+
+func _() {
+	// This uses goimports-based completion; TODO: this should insert snippets.
+	os.Open //@acceptcompletion(re"Open()", "Open", open)
+}
+
+func _() {
+	// This uses metadata-based completion.
+	errors.New(${1:}) //@acceptcompletion(re"New()", "New", new)
+}
+
+-- @open/foo/foo.go --
+package foo
+
+import "os"
+
+func _() {
+	// This uses goimports-based completion; TODO: this should insert snippets.
+	os.Open //@acceptcompletion(re"Open()", "Open", open)
+}
+
+func _() {
+	// This uses metadata-based completion.
+	errors.New //@acceptcompletion(re"New()", "New", new)
+}
+