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

x/tools/cmd/gopls: incorrect completion candidates offered where expression expected #32807

Closed
myitcv opened this issue Jun 27, 2019 · 3 comments
Closed

x/tools/cmd/gopls: incorrect completion candidates offered where expression expected #32807

myitcv opened this issue Jun 27, 2019 · 3 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@myitcv
Copy link
Member

myitcv commented Jun 27, 2019 •
edited

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

$ go version
go version devel +998a98984b Thu Jun 27 04:16:38 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190620191750-1fa568393b23
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20190620191750-1fa568393b23

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="on"
GOARCH="amd64"
GOBIN="/home/myitcv/gostuff/src/github.com/myitcv/govim/cmd/govim/.bin"
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
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"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/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-build053720080=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Consider the following example

package main

type special int

const (
	special1 special = iota
)

func main() {
	var c special
	switch c {
	case special<>
	}
}

If a case is added to the switch statement by typing case special and then completion attempted the following candidates are returned:

special
special1

However, special is a type, and therefore not a valid expression.

What did you expect to see?

A single candidate returned: special1

What did you see instead?

Two candidates returned as above.

cc @stamblerre @ianthehat
@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 Jun 27, 2019
@gopherbot gopherbot added this to the Unreleased milestone Jun 27, 2019
@muirdm
Copy link

muirdm commented Jun 27, 2019

This bugs me, too. I think there are two main things preventing us from excluding types in this (and similar) cases:

  1. We aren't perfect yet at detecting when a type name is expected rather than an object. To be on the safe side, we always include type names. This one can be resolved with more thorough logic.
  2. The user may want type names in this situation to construct a composite literal or perform a type conversion. For example, in the case statement the user might want to enter special(someVar) to cast someVar to a special. Or a user may want to enter SomeStruct{}.MethodThatReturnsSpecial() (and want to use the SomeStruct completion). This one is trickier and I'm not sure it is possible to cover all situations.

@stamblerre
Copy link
Contributor

@myitcv: In your repro cases, do you mind marking the place where the completion is being triggered from with <>? It helps when trying to understand the case.
@muirrn: I think your second point makes a pretty compelling case not to exclude special from the completion results. In this example, we very well could have some var x int = 4 in scope, and the user wants to write case special(x):. We have no way to know.

@myitcv
Copy link
Member Author

myitcv commented Jun 29, 2019

@stamblerre

In your repro cases, do you mind marking the place where the completion is being triggered from with <>? It helps when trying to understand the case.

Done. Updated the example.

@muirrn - that's an excellent point, thanks for catching it. For that reason alone (not ignoring your first point, but I think that is covered elsewhere) I'm going to close this issue.

@myitcv myitcv closed this as completed Jun 29, 2019
@golang golang locked and limited conversation to collaborators Jun 28, 2020
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. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@myitcv @stamblerre @gopherbot @muirdm