Skip to content

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

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
Closed
myitcv opened this issue Nov 18, 2020 · 6 comments
Closed

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

myitcv opened this issue Nov 18, 2020 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.6.7

Comments

@myitcv
Copy link
Member

myitcv commented Nov 18, 2020

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
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Nov 18, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 18, 2020
@gopherbot gopherbot added this to the Unreleased milestone Nov 18, 2020
@muirdm
Copy link

muirdm commented Nov 19, 2020

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
Copy link
Member Author

myitcv commented Nov 19, 2020

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
Copy link

muirdm commented Nov 19, 2020

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

Yes, that is definitely an option too.

@mattheusv
Copy link

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
Copy link

muirdm commented Feb 24, 2021

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 "...".

mattheusv added a commit to mattheusv/tools that referenced this issue Feb 24, 2021
@mattheusv
internal/lsp: fix bad completion for variadic functions
35402d2 
Add variadic completion in functions only if placeHolders is enabled.

Fixes golang/go#42691
@mattheusv mattheusv mentioned this issue Feb 24, 2021
Closed
@gopherbot
Copy link
Contributor

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

mattheusv added a commit to mattheusv/tools that referenced this issue Feb 24, 2021
@mattheusv
internal/lsp: fix bad completion for variadic functions
b596dac 
Add variadic completion in functions only if placeHolders is enabled.

Fixes golang/go#42691
mattheusv added a commit to mattheusv/tools that referenced this issue Feb 24, 2021
@mattheusv
internal/lsp: fix bad completion for variadic functions
3bdc9fa 
Add variadic completion in functions only if snippets is enabled.

Fixes golang/go#42691
mattheusv added a commit to mattheusv/tools that referenced this issue Mar 1, 2021
@mattheusv
internal/lsp: fix bad completion for variadic functions
04b2ad0 
Add variadic completion in functions only if snippets is enabled.

Fixes golang/go#42691
@golang golang locked and limited conversation to collaborators Mar 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.6.7
Development

Successfully merging a pull request may close this issue.

internal/lsp: fix bad completion for variadic functions mattheusv/tools
5 participants
@myitcv @stamblerre @gopherbot @muirdm @mattheusv