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

Jump to bottom

x/tools/gopls: respect go/analysis.Pass.RunDespiteErrors #33689

Closed
muirdm opened this issue Aug 16, 2019 · 6 comments
Closed

x/tools/gopls: respect go/analysis.Pass.RunDespiteErrors #33689

muirdm opened this issue Aug 16, 2019 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@muirdm
Copy link

muirdm commented Aug 16, 2019

I've started to see this panic sometimes as I type:

panic: interface conversion: types.Type is nil, not *types.Struct

goroutine 9909 [running]:
golang.org/x/tools/go/analysis/passes/structtag.run.func1(0x17c1e80, 0xc01c8f8ca0)
	/Users/muir/projects/tools/go/analysis/passes/structtag/structtag.go:43 +0x15c
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc02446f500, 0xc0108facb0, 0x1, 0x1, 0xc0108faca0)
	/Users/muir/projects/tools/go/ast/inspector/inspector.go:77 +0x9f
golang.org/x/tools/go/analysis/passes/structtag.run(0xc02ca6e1e0, 0xc016181600, 0xc00002a3e0, 0xc008f81600, 0x9)
	/Users/muir/projects/tools/go/analysis/passes/structtag/structtag.go:42 +0xca
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc024473c80, 0x17cb480, 0xc0244708c0, 0xc00021c840, 0xc0031d46c8, 0x10)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:174 +0x80b
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:108 +0x4e
sync.(*Once).Do(0xc024473c80, 0xc0031d4708)
	/usr/local/go/src/sync/once.go:44 +0xb3
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc024473c80, 0x17cb480, 0xc0244708c0, 0xc00021c840, 0xc00c59edd0, 0xc0031d4790)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:107 +0x8b
golang.org/x/tools/internal/lsp/source.execAll.func1(0xc000000008, 0x170d780)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:99 +0x48
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc024ec2420, 0xc02446f240)
	/Users/muir/projects/<project>/go/pkg/mod/golang.org/x/sync@v0.0.0-20190423024810-112230192c58/errgroup/errgroup.go:57 +0x57
created by golang.org/x/sync/errgroup.(*Group).Go
	/Users/muir/projects/<project>/go/pkg/mod/golang.org/x/sync@v0.0.0-20190423024810-112230192c58/errgroup/errgroup.go:54 +0x66
@gopherbot gopherbot added this to the Unreleased milestone Aug 16, 2019
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Aug 16, 2019
@stamblerre
Copy link
Contributor

Does the code that this happens in have build tags? I wouldn't be surprised if this were another instance of #32413.

@muirdm
Copy link
Author

muirdm commented Aug 16, 2019

No build tags involved in this case, but you are probably right it is the same issue. I assume type information was incomplete because I was in the middle of typing, but we still run the analyzers (#32413 (comment)). Closing as dupe of #32413.

@muirdm muirdm closed this as completed Aug 16, 2019
@stamblerre
Copy link
Contributor

Well, we shouldn't run analyzers if the type information is incomplete actually - we only run analyses when we didn't return any diagnostics for the file.

@stamblerre stamblerre reopened this Aug 16, 2019
@gopherbot
Copy link

Change https://golang.org/cl/190908 mentions this issue: go/analysis: handle common nil pointers

gopherbot pushed a commit to golang/tools that referenced this issue Aug 24, 2019
@stamblerre
go/analysis: handle common nil pointers
c2567a2 
Updates golang/go#33727, golang/go#33689

Change-Id: Ie32ac4efc9fe0d7b08fcff3feb44b28d83df942d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190908
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@stamblerre
Copy link
Contributor

We've added a recover in the go/analysis driver to handle cases like these (see https://github.com/golang/tools/blob/b1451cf3445bfc43d9982a3e85bec2dacced42ce/internal/lsp/cache/analysis.go#L218), but the correct solution will be to actually respect the go/analysis.Pass.RunDespiteErrors field. This can only be done once we have better support for build tags.

@stamblerre stamblerre changed the title x/tools/gopls: panic in structtag analyzer x/tools/gopls: respect go/analysis.Pass.RunDespiteErrors Dec 4, 2019
@stamblerre stamblerre modified the milestones: Unreleased, gopls unplanned Dec 4, 2019
@stamblerre stamblerre mentioned this issue Dec 4, 2019
Closed
@stamblerre
Copy link
Contributor

The panics have been covered now that our analysis runner has a recover in it. I'll close this issue in favor of the build tags one, since the two are the same.

@golang golang locked and limited conversation to collaborators Jan 28, 2021
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. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stamblerre @gopherbot @muirdm