x/tools/gopls: bad completion candidate for variadic call where method returns slice

[myitcv]

What version of Go are you using (go version)?

$ go version
go version devel +869e2957b9 Mon Nov 16 22:24:14 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20201111213328-5794f8bd7a57
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20201111213328-5794f8bd7a57

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +869e2957b9 Mon Nov 16 22:24:14 2020 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/.vim/plugged/govim/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build353352024=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Consider:

package main

import (
	"fmt"
)

type S struct {
	vals []string
}

func (s S) Vals() []string {
	return s.vals
}

func X(vs ...string) {
	fmt.Println(vs)
}

func main() {
	var s S
	X(s._)
}

with the cursor at the _ position. Attempt completion and you get the following candidates:

&protocol.CompletionList{
    IsIncomplete: true,
    Items:        {
        {
            Label:            "vals",
            Kind:             5,
            Tags:             nil,
            Detail:           "[]string",
            Documentation:    "",
            Deprecated:       false,
            Preselect:        true,
            SortText:         "00000",
            FilterText:       "vals...",
            InsertText:       "",
            InsertTextFormat: 1,
            TextEdit:         &protocol.TextEdit{
                Range: protocol.Range{
                    Start: protocol.Position{Line:20, Character:5},
                    End:   protocol.Position{Line:20, Character:5},
                },
                NewText: "vals...",
            },
            AdditionalTextEdits: nil,
            CommitCharacters:    nil,
            Command:             (*protocol.Command)(nil),
            Data:                nil,
        },
        {
            Label:            "Vals",
            Kind:             2,
            Tags:             nil,
            Detail:           "func() []string",
            Documentation:    "",
            Deprecated:       false,
            Preselect:        false,
            SortText:         "00001",
            FilterText:       "Vals...",
            InsertText:       "",
            InsertTextFormat: 1,
            TextEdit:         &protocol.TextEdit{
                Range: protocol.Range{
                    Start: protocol.Position{Line:20, Character:5},
                    End:   protocol.Position{Line:20, Character:5},
                },
                NewText: "Vals...",
            },
            AdditionalTextEdits: nil,
            CommitCharacters:    nil,
            Command:             (*protocol.Command)(nil),
            Data:                nil,
        },
    },
}

Note that the new text for the Vals candidate is wrong: it applies the ... without the parentheses required to first call the method.

What did you expect to see?

The candidate Vals()...

What did you see instead?

As above.

---

cc @stamblerre @muirdm

FYI @leitzler

[muirdm]

This is because the non-snippet insert text doesn't get the invocation () parens, but it does tack on the ....

It would be easy to fix the insert text to yield Vals()..., but the below behavior would also change:

func foo() []int {
  return nil
}
var _ []int = f<> // would complete to "foo()" instead of "foo"

Do non-snippet users want the invocation parens when the function takes no arguments?

[myitcv]

Thanks.

Do non-snippet users want the invocation parens when the function takes no arguments?

Would another option be to not add the ... when a call expression is involved?

[muirdm]

Would another option be to not add the ... when a call expression is involved?

Yes, that is definitely an option too.

[mattheusv]

I'm trying to fix this issue, and I would like to know if the behavior of ignoring ... in variadic functions apply to Completion and CompletionSnippets. I'm asking this because I have a wip implementation that ignore ... if the candidate is a function, but the tests of CompletionSnippets has been failed because they expected to have ()... completion. I was in doubt if I should ask here on the issue or open a pr.

[muirdm]

You want to suppress "..." if it is a function call and snippets are disabled. If snippets are enabled then there is no problem with the "...".

[gopherbot]

Change https://golang.org/cl/295950 mentions this issue: internal/lsp: fix bad completion for variadic functions