x/tools/gopls: field completion on the target variable of a type switch is flakey

[alecthomas]

What version of Go, VS Code & VS Code Go extension are you using?

Version Information

• Run go version to get version of Go from the VS Code integrated terminal.
  • go version go1.21.5 darwin/arm64

• Run gopls -v version to get version of Gopls from the VS Code integrated terminal.

Build info
----------
golang.org/x/tools/gopls v0.14.2
    golang.org/x/tools/gopls@v0.14.2 h1:sIw6vjZiuQ9S7s0auUUkHlWgsCkKZFWDHmrge8LYsnc=
    github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
    golang.org/x/mod@v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
    golang.org/x/sync@v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
    golang.org/x/sys@v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
    golang.org/x/telemetry@v0.0.0-20231114163143-69313e640400 h1:brbkEFfGwNGAEkykUOcryE/JiHUMMJouzE0fWWmz/QU=
    golang.org/x/text@v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
    golang.org/x/tools@v0.14.1-0.20231114185516-c9d3e7de13fd h1:Oku7E+OCrXHyst1dG1z10etCTxewCHXNFLRlyMPbh3w=
    golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
    honnef.co/go/tools@v0.4.5 h1:YGD4H+SuIOOqsyoLOpZDWcieM28W47/zRO7f+9V3nvo=
    mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
    mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.21.3

• Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.

  • 1.85.0
    af28b32d7e553898b2a91af498b1fb666fdebe0c
    arm64

• Check your installed extensions to get the version of the VS Code Go extension

  • v0.40.0

• Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.

Checking configured tools....
GOBIN: /Users/alec/dev/ftl/.hermit/go/bin
toolsGopath: 
gopath: /Users/alec/go
GOROOT: /Users/alec/Library/Caches/hermit/pkg/go-1.21.5
PATH: /Users/alec/Library/Caches/hermit/pkg/go-1.21.5/bin:/Users/alec/dev/ftl/scripts:/Users/alec/dev/ftl/frontend/node_modules/.bin:/Users/alec/dev/ftl/node_modules/.bin:/Users/alec/dev/ftl/.hermit/node/bin:/Users/alec/dev/ftl/.hermit/go/bin:/Users/alec/dev/ftl/bin:/Users/alec/.local/bin:/Users/alec/bin:/opt/homebrew/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/Applications/kitty.app/Contents/MacOS:/Users/alec/.orbstack/bin:/opt/homebrew/opt/fzf/bin
PATH (vscode launched with): /Users/alec/dev/ftl/scripts:/Users/alec/dev/ftl/frontend/node_modules/.bin:/Users/alec/dev/ftl/node_modules/.bin:/Users/alec/dev/ftl/.hermit/node/bin:/Users/alec/dev/ftl/.hermit/go/bin:/Users/alec/dev/ftl/bin:/Users/alec/.local/bin:/Users/alec/bin:/opt/homebrew/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin:/Applications/kitty.app/Contents/MacOS:/Users/alec/.orbstack/bin:/opt/homebrew/opt/fzf/bin

	go:	/Users/alec/Library/Caches/hermit/pkg/go-1.21.5/bin/go: go version go1.21.5 darwin/arm64

	gopls:	/Users/alec/dev/ftl/.hermit/go/bin/gopls	(version: v0.14.2 built with go: go1.21.3)
	gotests:	/Users/alec/dev/ftl/.hermit/go/bin/gotests	(version: v1.6.0 built with go: go1.21.3)
	gomodifytags:	/Users/alec/dev/ftl/.hermit/go/bin/gomodifytags	(version: v1.16.0 built with go: go1.21.3)
	impl:	/Users/alec/dev/ftl/.hermit/go/bin/impl	(version: v1.1.0 built with go: go1.21.3)
	goplay:	/Users/alec/dev/ftl/.hermit/go/bin/goplay	(version: v1.0.0 built with go: go1.21.3)
	dlv:	/Users/alec/dev/ftl/.hermit/go/bin/dlv	(version: v1.21.1 built with go: go1.21.3)
	staticcheck:	/Users/alec/dev/ftl/.hermit/go/bin/staticcheck	(version: v0.4.6 built with go: go1.21.3)

go env
Workspace Folder (ftl): /Users/alec/dev/ftl
	GO111MODULE=''
	GOARCH='arm64'
	GOBIN='/Users/alec/dev/ftl/.hermit/go/bin'
	GOCACHE='/Users/alec/Library/Caches/go-build'
	GOENV='/Users/alec/Library/Application Support/go/env'
	GOEXE=''
	GOEXPERIMENT=''
	GOFLAGS=''
	GOHOSTARCH='arm64'
	GOHOSTOS='darwin'
	GOINSECURE=''
	GOMODCACHE='/Users/alec/go/pkg/mod'
	GONOPROXY='*.sqcorp.co,github.com/squareup'
	GONOSUMDB='*.sqcorp.co,github.com/squareup'
	GOOS='darwin'
	GOPATH='/Users/alec/go'
	GOPRIVATE='*.sqcorp.co,github.com/squareup'
	GOPROXY='https://proxy.golang.org,direct'
	GOROOT='/Users/alec/Library/Caches/hermit/pkg/go-1.21.5'
	GOSUMDB='sum.golang.org'
	GOTMPDIR=''
	GOTOOLCHAIN='local'
	GOTOOLDIR='/Users/alec/Library/Caches/hermit/pkg/go-1.21.5/pkg/tool/darwin_arm64'
	GOVCS=''
	GOVERSION='go1.21.5'
	GCCGO='gccgo'
	AR='ar'
	CC='clang'
	CXX='clang++'
	CGO_ENABLED='1'
	GOMOD='/Users/alec/dev/ftl/go.mod'
	GOWORK='/Users/alec/dev/ftl/go.work'
	CGO_CFLAGS='-O2 -g'
	CGO_CPPFLAGS=''
	CGO_CXXFLAGS='-O2 -g'
	CGO_FFLAGS='-O2 -g'
	CGO_LDFLAGS='-O2 -g'
	PKG_CONFIG='pkg-config'
	GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/h2/_9k30gds1dbdjsv1m3yw834w0000gn/T/go-build2176859146=/tmp/go-build -gno-record-gcc-switches -fno-common'

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

Describe the bug

Steps to reproduce the behavior:

Field completion on the target variable of a type switch seems to be flakey. I've had it work before, but under some circumstances (such as with the code below) it doesn't seem to, instead it repeatedly shows completions for the top-level variable.

The following code seems to reproduce this:

package main

import (
	"fmt"
	"strings"
	"text/template/parse"
)

func main() {
	trees, err := parse.Parse("template", `{{ range .Modules }}{{.Name}}{{ . | branch }}{{ end }}`, `{{`, `}}`, map[string]any{"branch": func(b any) string { return "..." }})
	if err != nil {
		panic(err)
	}
	template := trees["template"]
	visit(template.Root, 0)
}

func visit(node parse.Node, indent int) {
	if node == nil {
		return
	}
	fmt.Printf("%s%T\n", strings.Repeat(" ", indent*2), node)
	switch n := node.(type) {
	case *parse.ActionNode:
		visit(n.Pipe, indent+1)

	case *parse.ListNode:
		for _, child := range n.Nodes {
			visit(child, indent+1)
		}

	case *parse.RangeNode:
		visit(n.Pipe, indent+1)
		visit(n.List, indent+1)
		visit(&n.BranchNode, indent+1)
		n.n.n.n.

	case *parse.IfNode:

	case *parse.WithNode:

	case *parse.TemplateNode:

	case *parse.TextNode:

	case *parse.PipeNode:
		for _, child := range n.Decl {
			visit(child, indent+1)
		}
		for _, child := range n.Cmds {
			visit(child, indent+1)
		}

	case *parse.CommandNode:

	case *parse.ChainNode:

	case *parse.IdentifierNode:

	case *parse.VariableNode:

	case *parse.DotNode:

	case *parse.NilNode:

	case *parse.FieldNode:

	case *parse.StringNode:

	case *parse.BoolNode:

	case *parse.BranchNode:

	case *parse.NumberNode:

	case *parse.BreakNode:

	case *parse.ContinueNode:

	default:
		fmt.Printf("%T\n", n)
	}
}

Screenshots or recordings

[alecthomas]

Actually it seems that they do show up if I happen to type a character that matches one of the field prefixes, but of course I have to know this in advance.

eg. with just .:

But if I type P then matching fields show up:

[hyangah]

Thanks for the report and the repro case.
Looks like gopls has trouble handling completion when struct embedding is involved.

And, including n, indent, ... in the completion response in this case doesn't make sense. That needs investigation too.
Transferring to the gopls issue tracker for further investigation.

[findleyr]

See @adonovan's comment here:
https://cs.opensource.google/go/x/tools/+/master:gopls/internal/lsp/source/completion/util.go;l=32;drc=28b92af2866ab2bc225795ba13f5a1ae765ffec5

There he cites types.NewSelectionSet (an API that does not exist). The lack of said API is exactly why we have this problem. We should propose it.

[adonovan]

Much though I would love to claim prescience, I have no recollection of writing this comment--not even the foggiest. So I checked, and indeed I didn't write it, @stamblerre did, in https://go.dev/173779 (see internal/lsp/source/util.go#32). So she deserves credit, both for the comment and for sneakily assigning it to me. ;-)

[muirdm]

This is the classic trailing "." breaking parsing, I think? i.e. it is parsed as n.case which is invalid due to the keyword.

I know I've tried to hack around similar issues in the past. Maybe we need another special "fix AST" case that looks for dangling selector + newline + keyword and rewrites the selector with a phantom _ to make it parse normally (e.g. a.\ncase becomes a._\ncase. Or maybe the stdlib parser getting more robust soon?