internal/lsp: fix a bug stopped package names from being printed

Also, trigger signature help on completion of a function (the "(" as a
trigger character doesn't work if it's part of a completion).

Change-Id: I952cb875fa72a741d7952178f85e20f9efa3ebff
Reviewed-on: https://go-review.googlesource.com/c/150638
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/completion.go

@@ -23,13 +23,21 @@ func toProtocolCompletionItems(items []source.CompletionItem, snippetsSupported,
 		insertTextFormat = protocol.SnippetTextFormat
 	}
 	for _, item := range items {
-		results = append(results, protocol.CompletionItem{
+		insertText, triggerSignatureHelp := labelToProtocolSnippets(item.Label, item.Kind, insertTextFormat, signatureHelpEnabled)
+		i := protocol.CompletionItem{
 			Label:            item.Label,
-			InsertText:       labelToProtocolSnippets(item.Label, item.Kind, insertTextFormat, signatureHelpEnabled),
+			InsertText:       insertText,
 			Detail:           item.Detail,
 			Kind:             float64(toProtocolCompletionItemKind(item.Kind)),
 			InsertTextFormat: insertTextFormat,
-		})
+		}
+		// If we are completing a function, we should trigger signature help if possible.
+		if triggerSignatureHelp && signatureHelpEnabled {
+			i.Command = &protocol.Command{
+				Command: "editor.action.triggerParameterHints",
+			}
+		}
+		results = append(results, i)
 	}
 	return results
 }
@@ -59,26 +67,26 @@ func toProtocolCompletionItemKind(kind source.CompletionItemKind) protocol.Compl
 	}
 }
 
-func labelToProtocolSnippets(label string, kind source.CompletionItemKind, insertTextFormat protocol.InsertTextFormat, signatureHelpEnabled bool) string {
+func labelToProtocolSnippets(label string, kind source.CompletionItemKind, insertTextFormat protocol.InsertTextFormat, signatureHelpEnabled bool) (string, bool) {
 	switch kind {
 	case source.ConstantCompletionItem:
 		// The label for constants is of the format "<identifier> = <value>".
 		// We should now insert the " = <value>" part of the label.
-		return label[:strings.Index(label, " =")]
+		return label[:strings.Index(label, " =")], false
 	case source.FunctionCompletionItem, source.MethodCompletionItem:
 		trimmed := label[:strings.Index(label, "(")]
 		params := strings.Trim(label[strings.Index(label, "("):], "()")
 		if params == "" {
-			return label
+			return label, true
 		}
 		// Don't add parameters or parens for the plaintext insert format.
 		if insertTextFormat == protocol.PlainTextFormat {
-			return trimmed
+			return trimmed, true
 		}
 		// If we do have signature help enabled, the user can see parameters as
 		// they type in the function, so we just return empty parentheses.
 		if signatureHelpEnabled {
-			return trimmed + "($1)"
+			return trimmed + "($1)", true
 		}
 		// If signature help is not enabled, we should give the user parameters
 		// that they can tab through. The insert text format follows the
@@ -96,8 +104,8 @@ func labelToProtocolSnippets(label string, kind source.CompletionItemKind, inser
 			}
 			trimmed += fmt.Sprintf("${%v:%v}", i+1, r.Replace(strings.Trim(p, " ")))
 		}
-		return trimmed + ")"
+		return trimmed + ")", false
 
 	}
-	return label
+	return label, false
 }

internal/lsp/lsp_test.go

@@ -34,7 +34,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 
 	// We hardcode the expected number of test cases to ensure that all tests
 	// are being executed. If a test is added, this number must be changed.
-	const expectedCompletionsCount = 43
+	const expectedCompletionsCount = 44
 	const expectedDiagnosticsCount = 14
 	const expectedFormatCount = 3
 	const expectedDefinitionsCount = 16

internal/lsp/server.go

@@ -58,7 +58,7 @@ func (s *server) Initialize(ctx context.Context, params *protocol.InitializePara
 			DocumentFormattingProvider:      true,
 			DocumentRangeFormattingProvider: true,
 			SignatureHelpProvider: protocol.SignatureHelpOptions{
-				TriggerCharacters: []string{"("},
+				TriggerCharacters: []string{"(", ","},
 			},
 			TextDocumentSync: protocol.TextDocumentSyncOptions{
 				Change:    float64(protocol.Full), // full contents of file sent on each update

internal/lsp/source/completion.go

@@ -492,14 +492,14 @@ func qualifier(f *ast.File, pkg *types.Package, info *types.Info) types.Qualifie
 		}
 	}
 	// Define qualifier to replace full package paths with names of the imports.
-	return func(pkg *types.Package) string {
-		if pkg == pkg {
+	return func(p *types.Package) string {
+		if p == pkg {
 			return ""
 		}
-		if name, ok := imports[pkg]; ok {
+		if name, ok := imports[p]; ok {
 			return name
 		}
-		return pkg.Name()
+		return p.Name()
 	}
 }
 

internal/lsp/testdata/bar/bar.go.in

@@ -6,7 +6,10 @@ import (
 	"golang.org/x/tools/internal/lsp/foo" //@item(foo, "foo", "\"golang.org/x/tools/internal/lsp/foo\"", "package")
 )
 
+func helper(i foo.IntFoo) {} //@item(helper, "helper(i foo.IntFoo)", "", "func")
+
 func _() {
+	help //@complete("l", helper)
 	_ = foo.StructFoo{} //@complete("S", Foo, IntFoo, StructFoo)
 }
 
@@ -35,9 +38,9 @@ func _() {
 		Value: Valen //@complete("le", Valentine)
 	}
 	_ = foo.StructFoo{
-		Value:       //@complete(re"$", Valentine, foo, Bar)
+		Value:       //@complete(re"$", Valentine, foo, Bar, helper)
 	}
 	_ = foo.StructFoo{
-		Value:       //@complete(" ", Valentine, foo, Bar)
+		Value:       //@complete(" ", Valentine, foo, Bar, helper)
 	}
 }