diff --git a/gopls/internal/lsp/source/completion/postfix_snippets.go b/gopls/internal/lsp/source/completion/postfix_snippets.go index 2575014f5d8..1661709e5dc 100644 --- a/gopls/internal/lsp/source/completion/postfix_snippets.go +++ b/gopls/internal/lsp/source/completion/postfix_snippets.go @@ -74,6 +74,7 @@ type postfixTmplArgs struct { edits []protocol.TextEdit qf types.Qualifier varNames map[string]bool + placeholders bool } var postfixTmpls = []postfixTmpl{{ @@ -103,7 +104,23 @@ for {{$i}}, {{$j}} := 0, len({{.X}})-1; {{$i}} < {{$j}}; {{$i}}, {{$j}} = {{$i}} label: "range", details: "range over slice", body: `{{if and (eq .Kind "slice") .StmtOK -}} -for {{.VarName nil "i"}}, {{.VarName .ElemType "v"}} := range {{.X}} { +for {{.VarName nil "i" | .Placeholder }}, {{.VarName .ElemType "v" | .Placeholder}} := range {{.X}} { + {{.Cursor}} +} +{{- end}}`, +}, { + label: "for", + details: "range over slice by index", + body: `{{if and (eq .Kind "slice") .StmtOK -}} +for {{ .VarName nil "i" | .Placeholder }} := range {{.X}} { + {{.Cursor}} +} +{{- end}}`, +}, { + label: "forr", + details: "range over slice by index and value", + body: `{{if and (eq .Kind "slice") .StmtOK -}} +for {{.VarName nil "i" | .Placeholder }}, {{.VarName .ElemType "v" | .Placeholder }} := range {{.X}} { {{.Cursor}} } {{- end}}`, @@ -130,7 +147,23 @@ copy({{$v}}, {{.X}}) label: "range", details: "range over map", body: `{{if and (eq .Kind "map") .StmtOK -}} -for {{.VarName .KeyType "k"}}, {{.VarName .ElemType "v"}} := range {{.X}} { +for {{.VarName .KeyType "k" | .Placeholder}}, {{.VarName .ElemType "v" | .Placeholder}} := range {{.X}} { + {{.Cursor}} +} +{{- end}}`, +}, { + label: "for", + details: "range over map by key", + body: `{{if and (eq .Kind "map") .StmtOK -}} +for {{.VarName .KeyType "k" | .Placeholder}} := range {{.X}} { + {{.Cursor}} +} +{{- end}}`, +}, { + label: "forr", + details: "range over map by key and value", + body: `{{if and (eq .Kind "map") .StmtOK -}} +for {{.VarName .KeyType "k" | .Placeholder}}, {{.VarName .ElemType "v" | .Placeholder}} := range {{.X}} { {{.Cursor}} } {{- end}}`, @@ -155,7 +188,15 @@ for {{.VarName .KeyType "k"}}, {{.VarName .ElemType "v"}} := range {{.X}} { label: "range", details: "range over channel", body: `{{if and (eq .Kind "chan") .StmtOK -}} -for {{.VarName .ElemType "e"}} := range {{.X}} { +for {{.VarName .ElemType "e" | .Placeholder}} := range {{.X}} { + {{.Cursor}} +} +{{- end}}`, +}, { + label: "for", + details: "range over channel", + body: `{{if and (eq .Kind "chan") .StmtOK -}} +for {{.VarName .ElemType "e" | .Placeholder}} := range {{.X}} { {{.Cursor}} } {{- end}}`, @@ -163,13 +204,13 @@ for {{.VarName .ElemType "e"}} := range {{.X}} { label: "var", details: "assign to variables", body: `{{if and (eq .Kind "tuple") .StmtOK -}} -{{$a := .}}{{range $i, $v := .Tuple}}{{if $i}}, {{end}}{{$a.VarName $v.Type $v.Name}}{{end}} := {{.X}} +{{$a := .}}{{range $i, $v := .Tuple}}{{if $i}}, {{end}}{{$a.VarName $v.Type $v.Name | $a.Placeholder }}{{end}} := {{.X}} {{- end}}`, }, { label: "var", details: "assign to variable", body: `{{if and (ne .Kind "tuple") .StmtOK -}} -{{.VarName .Type ""}} := {{.X}} +{{.VarName .Type "" | .Placeholder }} := {{.X}} {{- end}}`, }, { label: "print", @@ -199,9 +240,15 @@ for {{.VarName .ElemType "e"}} := range {{.X}} { label: "ifnotnil", details: "if expr != nil", body: `{{if and (or (eq .Kind "pointer") (eq .Kind "chan") (eq .Kind "signature") (eq .Kind "interface") (eq .Kind "map") (eq .Kind "slice")) .StmtOK -}} -if {{.X}} != nil {{"{"}} +if {{.X}} != nil { {{.Cursor}} -{{"}"}} +} +{{- end}}`, +}, { + label: "len", + details: "len(s)", + body: `{{if (eq .Kind "slice" "map" "array" "chan") -}} +len({{.X}}) {{- end}}`, }} @@ -212,6 +259,19 @@ func (a *postfixTmplArgs) Cursor() string { return "" } +// Placeholder indicate a tab stops with the placeholder string, the order +// of tab stops is the same as the order of invocation +func (a *postfixTmplArgs) Placeholder(s string) string { + if a.placeholders { + a.snip.WritePlaceholder(func(b *snippet.Builder) { + b.WriteText(s) + }) + } else { + a.snip.WritePlaceholder(nil) + } + return "" +} + // Import makes sure the package corresponding to path is imported, // returning the identifier to use to refer to the package. func (a *postfixTmplArgs) Import(path string) (string, error) { @@ -399,6 +459,7 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se importIfNeeded: c.importIfNeeded, scope: scope, varNames: make(map[string]bool), + placeholders: c.opts.placeholders, } // Feed the template straight into the snippet builder. This diff --git a/gopls/internal/test/integration/completion/postfix_snippet_test.go b/gopls/internal/test/integration/completion/postfix_snippet_test.go index 99071059310..0677280c5ec 100644 --- a/gopls/internal/test/integration/completion/postfix_snippet_test.go +++ b/gopls/internal/test/integration/completion/postfix_snippet_test.go @@ -20,8 +20,9 @@ go 1.12 ` cases := []struct { - name string - before, after string + name string + before, after string + allowMultipleItem bool }{ { name: "sort", @@ -133,7 +134,7 @@ package foo func _() { type myThing struct{} var foo []myThing - for i, mt := range foo { + for ${1:}, ${2:} := range foo { $0 } } @@ -213,7 +214,7 @@ package foo func _() { var foo map[string]int - for k, v := range foo { + for ${1:}, ${2:} := range foo { $0 } } @@ -279,7 +280,7 @@ package foo func _() { foo := make(chan int) - for e := range foo { + for ${1:} := range foo { $0 } } @@ -302,7 +303,7 @@ package foo func foo() (int, error) { return 0, nil } func _() { - i, err := foo() + ${1:}, ${2:} := foo() } `, }, @@ -323,7 +324,7 @@ package foo func foo() error { return nil } func _() { - err := foo() + ${1:} := foo() } `, }, @@ -344,7 +345,7 @@ package foo func foo() (int, int) { return 0, 0 } func _() { - i, i2 := foo() + ${1:}, ${2:} := foo() } `, }, @@ -554,6 +555,173 @@ func _() { $0 } } +`, + }, + { + name: "slice_len", + before: ` +package foo + +func _() { + var foo []int + foo.len +} +`, + after: ` +package foo + +func _() { + var foo []int + len(foo) +} +`, + }, + { + name: "map_len", + before: ` +package foo + +func _() { + var foo map[string]int + foo.len +} +`, + after: ` +package foo + +func _() { + var foo map[string]int + len(foo) +} +`, + }, + { + name: "slice_for", + allowMultipleItem: true, + before: ` +package foo + +func _() { + var foo []int + foo.for +} +`, + after: ` +package foo + +func _() { + var foo []int + for ${1:} := range foo { + $0 +} +} +`, + }, + { + name: "map_for", + allowMultipleItem: true, + before: ` +package foo + +func _() { + var foo map[string]int + foo.for +} +`, + after: ` +package foo + +func _() { + var foo map[string]int + for ${1:} := range foo { + $0 +} +} +`, + }, + { + name: "chan_for", + allowMultipleItem: true, + before: ` +package foo + +func _() { + var foo chan int + foo.for +} +`, + after: ` +package foo + +func _() { + var foo chan int + for ${1:} := range foo { + $0 +} +} +`, + }, + { + name: "slice_forr", + before: ` +package foo + +func _() { + var foo []int + foo.forr +} +`, + after: ` +package foo + +func _() { + var foo []int + for ${1:}, ${2:} := range foo { + $0 +} +} +`, + }, + { + name: "slice_forr", + before: ` +package foo + +func _() { + var foo []int + foo.forr +} +`, + after: ` +package foo + +func _() { + var foo []int + for ${1:}, ${2:} := range foo { + $0 +} +} +`, + }, + { + name: "map_forr", + before: ` +package foo + +func _() { + var foo map[string]int + foo.forr +} +`, + after: ` +package foo + +func _() { + var foo map[string]int + for ${1:}, ${2:} := range foo { + $0 +} +} `, }, } @@ -575,7 +743,10 @@ func _() { loc := env.RegexpSearch("foo.go", "\n}") completions := env.Completion(loc) - if len(completions.Items) != 1 { + if len(completions.Items) < 1 { + t.Fatalf("expected at least one completion, got %v", completions.Items) + } + if !c.allowMultipleItem && len(completions.Items) > 1 { t.Fatalf("expected one completion, got %v", completions.Items) } diff --git a/gopls/internal/test/marker/testdata/completion/postfix.txt b/gopls/internal/test/marker/testdata/completion/postfix.txt index f284137d3bb..63661ee9228 100644 --- a/gopls/internal/test/marker/testdata/completion/postfix.txt +++ b/gopls/internal/test/marker/testdata/completion/postfix.txt @@ -21,7 +21,6 @@ func _() { []int{}.last //@complete(" //") - /* copy! */ //@item(postfixCopy, "copy!", "duplicate slice", "snippet") foo.copy //@rank(" //", postfixCopy) @@ -36,7 +35,11 @@ func _() { func _() { /* append! */ //@item(postfixAppend, "append!", "append and re-assign slice", "snippet") + /* copy! */ //@item(postfixCopy, "copy!", "duplicate slice", "snippet") + /* for! */ //@item(postfixFor, "for!", "range over slice by index", "snippet") + /* forr! */ //@item(postfixForr, "forr!", "range over slice by index and value", "snippet") /* last! */ //@item(postfixLast, "last!", "s[len(s)-1]", "snippet") + /* len! */ //@item(postfixLen, "len!", "len(s)", "snippet") /* print! */ //@item(postfixPrint, "print!", "print to stdout", "snippet") /* range! */ //@item(postfixRange, "range!", "range over slice", "snippet") /* reverse! */ //@item(postfixReverse, "reverse!", "reverse slice", "snippet") @@ -45,7 +48,51 @@ func _() { /* ifnotnil! */ //@item(postfixIfNotNil, "ifnotnil!", "if expr != nil", "snippet") var foo []int - foo. //@complete(" //", postfixAppend, postfixCopy, postfixIfNotNil, postfixLast, postfixPrint, postfixRange, postfixReverse, postfixSort, postfixVar) + foo. //@complete(" //", postfixAppend, postfixCopy, postfixFor, postfixForr, postfixIfNotNil, postfixLast, postfixLen, postfixPrint, postfixRange, postfixReverse, postfixSort, postfixVar) + foo = nil + + foo.append //@snippet(" //", postfixAppend, "foo = append(foo, $0)") + foo.copy //snippet(" //", postfixCopy, "fooCopy := make([]int, len(foo))\ncopy($fooCopy, foo)\n") + foo.fo //@snippet(" //", postfixFor, "for ${1:} := range foo {\n\t$0\n}") + foo.forr //@snippet(" //", postfixForr, "for ${1:}, ${2:} := range foo {\n\t$0\n}") + foo.last //@snippet(" //", postfixLast, "foo[len(foo)-1]") + foo.len //@snippet(" //", postfixLen, "len(foo)") + foo.print //@snippet(" //", postfixPrint, `fmt.Printf("foo: %v\n", foo)`) + foo.rang //@snippet(" //", postfixRange, "for ${1:}, ${2:} := range foo {\n\t$0\n}") + foo.reverse //@snippet(" //", postfixReverse, "for i, j := 0, len(foo)-1; i < j; i, j = i+1, j-1 {\n\tfoo[i], foo[j] = foo[j], foo[i]\n}\n") + foo.sort //@snippet(" //", postfixSort, "sort.Slice(foo, func(i, j int) bool {\n\t$0\n})") + foo.va //@snippet(" //", postfixVar, "${1:} := foo") + foo.ifnotnil //@snippet(" //", postfixIfNotNil, "if foo != nil {\n\t$0\n}") +} + +func _() { + /* for! */ //@item(postfixForMap, "for!", "range over map by key", "snippet") + /* forr! */ //@item(postfixForrMap, "forr!", "range over map by key and value", "snippet") + /* range! */ //@item(postfixRangeMap, "range!", "range over map", "snippet") + /* clear! */ //@item(postfixClear, "clear!", "clear map contents", "snippet") + /* keys! */ //@item(postfixKeys, "keys!", "create slice of keys", "snippet") + + var foo map[int]int + foo. //@complete(" //", postfixClear, postfixForMap, postfixForrMap, postfixIfNotNil, postfixKeys, postfixLen, postfixPrint, postfixRangeMap, postfixVar) + + foo = nil + + foo.fo //@snippet(" //", postfixFor, "for ${1:} := range foo {\n\t$0\n}") + foo.forr //@snippet(" //", postfixForr, "for ${1:}, ${2:} := range foo {\n\t$0\n}") + foo.rang //@snippet(" //", postfixRange, "for ${1:}, ${2:} := range foo {\n\t$0\n}") + foo.clear //@snippet(" //", postfixClear, "for k := range foo {\n\tdelete(foo, k)\n}\n") + foo.keys //@snippet(" //", postfixKeys, "keys := make([]int, 0, len(foo))\nfor k := range foo {\n\tkeys = append(keys, k)\n}\n") +} + +func _() { + /* for! */ //@item(postfixForChannel, "for!", "range over channel", "snippet") + /* range! */ //@item(postfixRangeChannel, "range!", "range over channel", "snippet") + + var foo chan int + foo. //@complete(" //", postfixForChannel, postfixIfNotNil, postfixLen, postfixPrint, postfixRangeChannel, postfixVar) + + foo = nil - foo = nil + foo.fo //@snippet(" //", postfixForChannel, "for ${1:} := range foo {\n\t$0\n}") + foo.rang //@snippet(" //", postfixRangeChannel, "for ${1:} := range foo {\n\t$0\n}") } diff --git a/gopls/internal/test/marker/testdata/completion/postfix_placeholder.txt b/gopls/internal/test/marker/testdata/completion/postfix_placeholder.txt new file mode 100644 index 00000000000..44dfbc96df1 --- /dev/null +++ b/gopls/internal/test/marker/testdata/completion/postfix_placeholder.txt @@ -0,0 +1,53 @@ +These tests check that postfix completions when enable usePlaceholders + +-- flags -- +-ignore_extra_diags + +-- settings.json -- +{ + "usePlaceholders": true +} + +-- go.mod -- +module golang.org/lsptests/snippets + +go 1.18 + +-- postfix.go -- +package snippets + +func _() { + /* for! */ //@item(postfixFor, "for!", "range over slice by index", "snippet") + /* forr! */ //@item(postfixForr, "forr!", "range over slice by index and value", "snippet") + /* range! */ //@item(postfixRange, "range!", "range over slice", "snippet") + /* var! */ //@item(postfixVar, "var!", "assign to variable", "snippet") + + var foo []int + + foo.fo //@snippet(" //", postfixFor, "for ${1:i} := range foo {\n\t$0\n}") + foo.forr //@snippet(" //", postfixForr, "for ${1:i}, ${2:v} := range foo {\n\t$0\n}") + foo.rang //@snippet(" //", postfixRange, "for ${1:i}, ${2:v} := range foo {\n\t$0\n}") + foo.va //@snippet(" //", postfixVar, "${1:i} := foo") +} + +func _() { + /* for! */ //@item(postfixForMap, "for!", "range over map by key", "snippet") + /* forr! */ //@item(postfixForrMap, "forr!", "range over map by key and value", "snippet") + /* range! */ //@item(postfixRangeMap, "range!", "range over map", "snippet") + + var foo map[int]int + + foo.fo //@snippet(" //", postfixFor, "for ${1:k} := range foo {\n\t$0\n}") + foo.forr //@snippet(" //", postfixForr, "for ${1:k}, ${2:v} := range foo {\n\t$0\n}") + foo.rang //@snippet(" //", postfixRange, "for ${1:k}, ${2:v} := range foo {\n\t$0\n}") +} + +func _() { + /* for! */ //@item(postfixForChannel, "for!", "range over channel", "snippet") + /* range! */ //@item(postfixRangeChannel, "range!", "range over channel", "snippet") + + var foo chan int + + foo.fo //@snippet(" //", postfixForChannel, "for ${1:e} := range foo {\n\t$0\n}") + foo.rang //@snippet(" //", postfixRangeChannel, "for ${1:e} := range foo {\n\t$0\n}") +}