Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gopls/internal/lsp/source/completion: support more postfix completion for map and slice (len, for, forr) #459

Closed
wants to merge 1 commit into from
Closed

gopls/internal/lsp/source/completion: support more postfix completion for map and slice (len, for, forr) #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 68 additions & 7 deletions gopls/internal/lsp/source/completion/postfix_snippets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type postfixTmplArgs struct {
edits []protocol.TextEdit
qf types.Qualifier
varNames map[string]bool
placeholders bool
}

var postfixTmpls = []postfixTmpl{{
Expand Down Expand Up @@ -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}}`,
Expand All @@ -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}}`,
Expand All @@ -155,21 +188,29 @@ 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}}`,
}, {
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",
Expand Down Expand Up @@ -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}}`,
}}

Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
189 changes: 180 additions & 9 deletions gopls/internal/test/integration/completion/postfix_snippet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ go 1.12
`

cases := []struct {
name string
before, after string
name string
before, after string
allowMultipleItem bool
}{
{
name: "sort",
Expand Down Expand Up @@ -133,7 +134,7 @@ package foo
func _() {
type myThing struct{}
var foo []myThing
for i, mt := range foo {
for ${1:}, ${2:} := range foo {
$0
}
}
Expand Down Expand Up @@ -213,7 +214,7 @@ package foo

func _() {
var foo map[string]int
for k, v := range foo {
for ${1:}, ${2:} := range foo {
$0
}
}
Expand Down Expand Up @@ -279,7 +280,7 @@ package foo

func _() {
foo := make(chan int)
for e := range foo {
for ${1:} := range foo {
$0
}
}
Expand All @@ -302,7 +303,7 @@ package foo
func foo() (int, error) { return 0, nil }

func _() {
i, err := foo()
${1:}, ${2:} := foo()
}
`,
},
Expand All @@ -323,7 +324,7 @@ package foo
func foo() error { return nil }

func _() {
err := foo()
${1:} := foo()
}
`,
},
Expand All @@ -344,7 +345,7 @@ package foo
func foo() (int, int) { return 0, 0 }

func _() {
i, i2 := foo()
${1:}, ${2:} := foo()
}
`,
},
Expand Down Expand Up @@ -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
}
}
`,
},
}
Expand All @@ -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)
}

Expand Down
Loading