gopls/completion: avoid duplicating text in test func completions
If a user types 'func Te' in a test file, gopls suggest a
completion using a snippet 'TestXxx(t *testing.T)' which will
fill in the entire function declaration. Until this CL it would
also suggest the snippet if the user had type 'func Te(t *testing.T)',
and went back to fill in the function name. This CL fixes that
by no longer suggesting the snippet if the user has already typed
a parenthesis.
Fixes: golang/go#57480
Change-Id: I5061a6ba5ca22ecba60de537a41fcc04356dc7ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/459559
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/regtest/wrappers.go b/gopls/internal/lsp/regtest/wrappers.go
index 19713f2..fe80d89 100644
--- a/gopls/internal/lsp/regtest/wrappers.go
+++ b/gopls/internal/lsp/regtest/wrappers.go
@@ -138,7 +138,7 @@
pos, err = e.Sandbox.Workdir.RegexpSearch(name, re)
}
if err != nil {
- e.T.Fatalf("RegexpSearch: %v, %v", name, err)
+ e.T.Fatalf("RegexpSearch: %v, %v for %q", name, err, re)
}
return pos
}
diff --git a/gopls/internal/lsp/source/completion/definition.go b/gopls/internal/lsp/source/completion/definition.go
index cc7b42e..bf14ad4 100644
--- a/gopls/internal/lsp/source/completion/definition.go
+++ b/gopls/internal/lsp/source/completion/definition.go
@@ -18,7 +18,7 @@
"golang.org/x/tools/gopls/internal/span"
)
-// some definitions can be completed
+// some function definitions in test files can be completed
// So far, TestFoo(t *testing.T), TestMain(m *testing.M)
// BenchmarkFoo(b *testing.B), FuzzFoo(f *testing.F)
@@ -28,7 +28,7 @@
return nil, nil // not a function at all
}
if !strings.HasSuffix(fh.URI().Filename(), "_test.go") {
- return nil, nil
+ return nil, nil // not a test file
}
name := path[0].(*ast.Ident).Name
@@ -44,21 +44,49 @@
rng: span.NewRange(tokFile, start, end),
}
var ans []CompletionItem
+ var hasParens bool
+ n, ok := path[1].(*ast.FuncDecl)
+ if !ok {
+ return nil, nil // can't happen
+ }
+ if n.Recv != nil {
+ return nil, nil // a method, not a function
+ }
+ t := n.Type.Params
+ if t.Closing != t.Opening {
+ hasParens = true
+ }
// Always suggest TestMain, if possible
if strings.HasPrefix("TestMain", name) {
- ans = []CompletionItem{defItem("TestMain(m *testing.M)", obj)}
+ if hasParens {
+ ans = append(ans, defItem("TestMain", obj))
+ } else {
+ ans = append(ans, defItem("TestMain(m *testing.M)", obj))
+ }
}
// If a snippet is possible, suggest it
if strings.HasPrefix("Test", name) {
- ans = append(ans, defSnippet("Test", "Xxx", "(t *testing.T)", obj))
+ if hasParens {
+ ans = append(ans, defItem("Test", obj))
+ } else {
+ ans = append(ans, defSnippet("Test", "(t *testing.T)", obj))
+ }
return ans, sel
} else if strings.HasPrefix("Benchmark", name) {
- ans = append(ans, defSnippet("Benchmark", "Xxx", "(b *testing.B)", obj))
+ if hasParens {
+ ans = append(ans, defItem("Benchmark", obj))
+ } else {
+ ans = append(ans, defSnippet("Benchmark", "(b *testing.B)", obj))
+ }
return ans, sel
} else if strings.HasPrefix("Fuzz", name) {
- ans = append(ans, defSnippet("Fuzz", "Xxx", "(f *testing.F)", obj))
+ if hasParens {
+ ans = append(ans, defItem("Fuzz", obj))
+ } else {
+ ans = append(ans, defSnippet("Fuzz", "(f *testing.F)", obj))
+ }
return ans, sel
}
@@ -73,9 +101,9 @@
return ans, sel
}
+// defMatches returns text for defItem, never for defSnippet
func defMatches(name, pat string, path []ast.Node, arg string) string {
- idx := strings.Index(name, pat)
- if idx < 0 {
+ if !strings.HasPrefix(name, pat) {
return ""
}
c, _ := utf8.DecodeRuneInString(name[len(pat):])
@@ -88,25 +116,27 @@
return ""
}
fp := fd.Type.Params
- if fp != nil && len(fp.List) > 0 {
- // signature already there, minimal suggestion
- return name
+ if len(fp.List) > 0 {
+ // signature already there, nothing to suggest
+ return ""
+ }
+ if fp.Opening != fp.Closing {
+ // nothing: completion works on words, not easy to insert arg
+ return ""
}
// suggesting signature too
return name + arg
}
-func defSnippet(prefix, placeholder, suffix string, obj types.Object) CompletionItem {
+func defSnippet(prefix, suffix string, obj types.Object) CompletionItem {
var sn snippet.Builder
sn.WriteText(prefix)
- if placeholder != "" {
- sn.WritePlaceholder(func(b *snippet.Builder) { b.WriteText(placeholder) })
- }
- sn.WriteText(suffix + " {\n")
+ sn.WritePlaceholder(func(b *snippet.Builder) { b.WriteText("Xxx") })
+ sn.WriteText(suffix + " {\n\t")
sn.WriteFinalTabstop()
sn.WriteText("\n}")
return CompletionItem{
- Label: prefix + placeholder + suffix,
+ Label: prefix + "Xxx" + suffix,
Detail: "tab, type the rest of the name, then tab",
Kind: protocol.FunctionCompletion,
Depth: 0,
@@ -123,7 +153,7 @@
Kind: protocol.FunctionCompletion,
Depth: 0,
Score: 9, // prefer the snippets when available
- Documentation: "complete the parameter",
+ Documentation: "complete the function name",
obj: obj,
}
}
diff --git a/gopls/internal/regtest/completion/completion18_test.go b/gopls/internal/regtest/completion/completion18_test.go
index 7c53252..936436b 100644
--- a/gopls/internal/regtest/completion/completion18_test.go
+++ b/gopls/internal/regtest/completion/completion18_test.go
@@ -44,7 +44,7 @@
pos := env.RegexpSearch("main.go", tst.pat)
pos.Column += len(tst.pat)
completions := env.Completion("main.go", pos)
- result := compareCompletionResults(tst.want, completions.Items)
+ result := compareCompletionLabels(tst.want, completions.Items)
if result != "" {
t.Errorf("%s: wanted %v", result, tst.want)
for i, g := range completions.Items {
@@ -111,7 +111,7 @@
pos := env.RegexpSearch(test.file, test.pat)
pos.Column += test.offset // character user just typed? will type?
completions := env.Completion(test.file, pos)
- result := compareCompletionResults(test.want, completions.Items)
+ result := compareCompletionLabels(test.want, completions.Items)
if result != "" {
t.Errorf("pat %q %q", test.pat, result)
for i, it := range completions.Items {
diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index 08a1421..5f65593 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -207,7 +207,7 @@
}
}
- diff := compareCompletionResults(tc.want, completions.Items)
+ diff := compareCompletionLabels(tc.want, completions.Items)
if diff != "" {
t.Error(diff)
}
@@ -234,14 +234,14 @@
Column: 10,
})
- diff := compareCompletionResults(want, completions.Items)
+ diff := compareCompletionLabels(want, completions.Items)
if diff != "" {
t.Fatal(diff)
}
})
}
-func compareCompletionResults(want []string, gotItems []protocol.CompletionItem) string {
+func compareCompletionLabels(want []string, gotItems []protocol.CompletionItem) string {
if len(gotItems) != len(want) {
return fmt.Sprintf("got %v completion(s), want %v", len(gotItems), len(want))
}
@@ -391,7 +391,7 @@
Run(t, files, func(t *testing.T, env *Env) {
env.OpenFile("foo.go")
completions := env.Completion("foo.go", env.RegexpSearch("foo.go", `if s\.()`))
- diff := compareCompletionResults([]string{"i"}, completions.Items)
+ diff := compareCompletionLabels([]string{"i"}, completions.Items)
if diff != "" {
t.Fatal(diff)
}
@@ -451,7 +451,7 @@
}
for _, tt := range tests {
completions := env.Completion("main.go", env.RegexpSearch("main.go", tt.re))
- diff := compareCompletionResults(tt.want, completions.Items)
+ diff := compareCompletionLabels(tt.want, completions.Items)
if diff != "" {
t.Errorf("%s: %s", tt.re, diff)
}
@@ -486,7 +486,7 @@
pos := env.RegexpSearch("prog.go", "if fooF")
pos.Column += len("if fooF")
completions := env.Completion("prog.go", pos)
- diff := compareCompletionResults([]string{"fooFunc"}, completions.Items)
+ diff := compareCompletionLabels([]string{"fooFunc"}, completions.Items)
if diff != "" {
t.Error(diff)
}
@@ -496,7 +496,7 @@
pos = env.RegexpSearch("prog.go", "= badP")
pos.Column += len("= badP")
completions = env.Completion("prog.go", pos)
- diff = compareCompletionResults([]string{"badPi"}, completions.Items)
+ diff = compareCompletionLabels([]string{"badPi"}, completions.Items)
if diff != "" {
t.Error(diff)
}
@@ -545,6 +545,7 @@
}
func TestDefinition(t *testing.T) {
+ testenv.NeedsGo1Point(t, 17) // in go1.16, The FieldList in func x is not empty
stuff := `
-- go.mod --
module mod.com
@@ -552,43 +553,42 @@
go 1.18
-- a_test.go --
package foo
-func T()
-func TestG()
-func TestM()
-func TestMi()
-func Ben()
-func Fuz()
-func Testx()
-func TestMe(t *testing.T)
-func BenchmarkFoo()
`
- // All those parentheses are needed for the completion code to see
- // later lines as being definitions
tests := []struct {
- pat string
- want []string
+ line string // the sole line in the buffer after the package statement
+ pat string // the pattern to search for
+ want []string // expected competions
}{
- {"T", []string{"TestXxx(t *testing.T)", "TestMain(m *testing.M)"}},
- {"TestM", []string{"TestMain(m *testing.M)", "TestM(t *testing.T)"}},
- {"TestMi", []string{"TestMi(t *testing.T)"}},
- {"TestG", []string{"TestG(t *testing.T)"}},
- {"B", []string{"BenchmarkXxx(b *testing.B)"}},
- {"BenchmarkFoo", []string{"BenchmarkFoo(b *testing.B)"}},
- {"F", []string{"FuzzXxx(f *testing.F)"}},
- {"Testx", nil},
- {"TestMe", []string{"TestMe"}},
+ {"func T", "T", []string{"TestXxx(t *testing.T)", "TestMain(m *testing.M)"}},
+ {"func T()", "T", []string{"TestMain", "Test"}},
+ {"func TestM", "TestM", []string{"TestMain(m *testing.M)", "TestM(t *testing.T)"}},
+ {"func TestM()", "TestM", []string{"TestMain"}},
+ {"func TestMi", "TestMi", []string{"TestMi(t *testing.T)"}},
+ {"func TestMi()", "TestMi", nil},
+ {"func TestG", "TestG", []string{"TestG(t *testing.T)"}},
+ {"func TestG(", "TestG", nil},
+ {"func Ben", "B", []string{"BenchmarkXxx(b *testing.B)"}},
+ {"func Ben(", "Ben", []string{"Benchmark"}},
+ {"func BenchmarkFoo", "BenchmarkFoo", []string{"BenchmarkFoo(b *testing.B)"}},
+ {"func BenchmarkFoo(", "BenchmarkFoo", nil},
+ {"func Fuz", "F", []string{"FuzzXxx(f *testing.F)"}},
+ {"func Fuz(", "Fuz", []string{"Fuzz"}},
+ {"func Testx", "Testx", nil},
+ {"func TestMe(t *testing.T)", "TestMe", nil},
+ {"func Te(t *testing.T)", "Te", []string{"TestMain", "Test"}},
}
fname := "a_test.go"
Run(t, stuff, func(t *testing.T, env *Env) {
env.OpenFile(fname)
env.Await(env.DoneWithOpen())
for _, tst := range tests {
+ env.SetBufferContent(fname, "package foo\n"+tst.line)
pos := env.RegexpSearch(fname, tst.pat)
pos.Column += len(tst.pat)
completions := env.Completion(fname, pos)
- result := compareCompletionResults(tst.want, completions.Items)
+ result := compareCompletionLabels(tst.want, completions.Items)
if result != "" {
- t.Errorf("%s failed: %s:%q", tst.pat, result, tst.want)
+ t.Errorf("\npat:%q line:%q failed: %s:%q", tst.pat, tst.line, result, tst.want)
for i, it := range completions.Items {
t.Errorf("%d got %q %q", i, it.Label, it.Detail)
}
@@ -651,7 +651,7 @@
package foo_test
func Benchmark${1:Xxx}(b *testing.B) {
-$0
+ $0
\}
`,
},
@@ -675,7 +675,7 @@
env.AcceptCompletion("foo_test.go", pos, completions.Items[0])
env.Await(env.DoneWithChange())
if buf := env.BufferText("foo_test.go"); buf != tst.after {
- t.Errorf("incorrect completion: got %q, want %q", buf, tst.after)
+ t.Errorf("%s:incorrect completion: got %q, want %q", tst.name, buf, tst.after)
}
}
})
@@ -718,7 +718,7 @@
}
for _, tt := range tests {
completions := env.Completion("go.work", env.RegexpSearch("go.work", tt.re))
- diff := compareCompletionResults(tt.want, completions.Items)
+ diff := compareCompletionLabels(tt.want, completions.Items)
if diff != "" {
t.Errorf("%s: %s", tt.re, diff)
}
