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/gopls: field completion on the target variable of a type switch is flakey #64709

Open
alecthomas opened this issue Dec 13, 2023 · 5 comments
Open

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

alecthomas opened this issue Dec 13, 2023 · 5 comments
Labels
gopls/completion Issues related to auto-completion in gopls. 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. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@alecthomas
Copy link

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

image
@alecthomas
Copy link
Author

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

image

But if I type P then matching fields show up:

image

@hyangah
Copy link
Contributor

hyangah commented Dec 14, 2023

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.

@hyangah hyangah changed the title Field completion on the target variable of a type switch is flakey x/tools/gopls: field completion on the target variable of a type switch is flakey Dec 14, 2023
@hyangah hyangah transferred this issue from golang/vscode-go Dec 14, 2023
@hyangah hyangah added the gopls/completion Issues related to auto-completion in gopls. label Dec 14, 2023
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 14, 2023
@gopherbot gopherbot added this to the Unreleased milestone Dec 14, 2023
@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 14, 2023
@hyangah hyangah modified the milestones: Unreleased, gopls/backlog Dec 14, 2023
@findleyr
Copy link
Contributor

findleyr commented Dec 14, 2023 •
edited

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

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

muirdm commented Dec 19, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls/completion Issues related to auto-completion in gopls. 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. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
6 participants
@alecthomas @hyangah @adonovan @gopherbot @muirdm @findleyr