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.

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

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

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

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jun 27, 2019

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

@gopherbot gopherbot added this to the Unreleased milestone Jun 27, 2019
@muirdm
Copy link

@muirdm 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

@stamblerre stamblerre commented Jun 27, 2019

@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 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 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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.