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: rename crashes in objectpath #60789

Closed
adonovan opened this issue Jun 14, 2023 · 4 comments
Closed

x/tools/gopls: rename crashes in objectpath #60789

adonovan opened this issue Jun 14, 2023 · 4 comments
Assignees
@adonovan
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.13.0

Comments

@adonovan
Copy link
Member

I found this while snooping around in 'gopls stats'. This is v0.12.0.

objectpath.Object(package cache ("golang.org/x/tools/gopls/internal/lsp/cache"), analysisNode.M0) failed: package golang.org/x/tools/gopls/internal/lsp/cache does not contain "analysisNode",
goroutine 7388370 [running]:
runtime/debug.Stack()
	/Users/adonovan/w/homebrew/Cellar/go/1.20.4/libexec/src/runtime/debug/stack.go:24 +0x64
golang.org/x/tools/gopls/internal/bug.report({0x14012efb5c0, 0xbd})
	/Users/adonovan/w/xtools/gopls/internal/bug/bug.go:83 +0xd8
golang.org/x/tools/gopls/internal/bug.Reportf({0x10150a4f8?, 0xf?}, {0x1400d086ff0?, 0x14011a2c580?, 0x1018a58a0?})
	/Users/adonovan/w/xtools/gopls/internal/bug/bug.go:49 +0x28
golang.org/x/tools/gopls/internal/lsp/source.renameExported({0x10191e6c8, 0x140130c40f0}, {0x10192b010, 0x14016d08000}, {0x1400d5a9600, 0x1f, 0x1400d0872b8?}, {0x14028ed6f60, 0x2b}, {0x14024d66950, ...}, ...)
	/Users/adonovan/w/xtools/gopls/internal/lsp/source/rename.go:577 +0x3a0
golang.org/x/tools/gopls/internal/lsp/source.renameOrdinary({0x10191e6c8, 0x140130c40f0}, {0x10192b010, 0x14016d08000}, {0x10191f560, 0x1400a0bc6c0}, {0xd087508?, 0x140?}, {0x1400fd2f5e0, 0x7})
	/Users/adonovan/w/xtools/gopls/internal/lsp/source/rename.go:430 +0x6bc
golang.org/x/tools/gopls/internal/lsp/source.Rename({0x10191e6c8?, 0x1401f9fbf80?}, {0x10192b010, 0x14016d08000}, {0x10191f560, 0x1400a0bc6c0}, {0xd6f0960?, 0x140?}, {0x1400fd2f5e0, 0x7})
	/Users/adonovan/w/xtools/gopls/internal/lsp/source/rename.go:234 +0x120
golang.org/x/tools/gopls/internal/lsp.(*Server).rename(0x140204cbb00?, {0x10191e620, 0x1400998c690}, 0x140204cbb00)
	/Users/adonovan/w/xtools/gopls/internal/lsp/rename.go:29 +0x1a4
golang.org/x/tools/gopls/internal/lsp.(*Server).Rename(0x1401a4b6140?, {0x10191e620?, 0x1400998c690?}, 0x1017a5d80?)
	/Users/adonovan/w/xtools/gopls/internal/lsp/server_gen.go:208 +0x24
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch({0x10191e620, 0x1400998c690}, {0x10192dd28, 0x140001db040}, 0x1401f9fbe90, {0x10191e8f8, 0x140002a3400})
	/Users/adonovan/w/xtools/gopls/internal/lsp/protocol/tsserver.go:515 +0xb6c
golang.org/x/tools/gopls/internal/lsp/protocol.ServerHandler.func1({0x10191e620, 0x1400998c690}, 0x1401f9fbe90, {0x10191e8f8, 0x140002a3400})
	/Users/adonovan/w/xtools/gopls/internal/lsp/protocol/protocol.go:157 +0x6c
golang.org/x/tools/gopls/internal/lsp/lsprpc.handshaker.func1({0x10191e620, 0x1400998c690}, 0x1401f9fbe90, {0x10191e8f8?, 0x140002a3400?})
	/Users/adonovan/w/xtools/gopls/internal/lsp/lsprpc/lsprpc.go:519 +0x768
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0x10191e620, 0x1400998c690}, 0x1400e7b06d8, {0x10191e8f8?, 0x140002a3400?})
	/Users/adonovan/w/xtools/internal/jsonrpc2/handler.go:35 +0xf0
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
	/Users/adonovan/w/xtools/internal/jsonrpc2/handler.go:103 +0x90
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
	/Users/adonovan/w/xtools/internal/jsonrpc2/handler.go:100 +0x1dc
@adonovan adonovan self-assigned this Jun 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 Jun 14, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jun 14, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.12.4 Jun 15, 2023
@findleyr findleyr modified the milestones: gopls/v0.12.4, gopls/v0.12.5 Jun 22, 2023
@adonovan
Copy link
Member Author

I can reproduce this at master by renaming (*cache.analysisNode.String) to String2.

@adonovan
Copy link
Member Author

I think the root cause is the behavior of the objectpath package. Given a syntax package containing an unexported type with an exported method, objectpath.For(obj) will succeed and return a path, even if the unexported type is not reachable from any exported declaration. The resulting path will cause objectpath.Object(path, pkg) to fail if pkg was loaded from export data. I'm not sure whether that's that a bug or a feature.

@gopherbot
Copy link

Change https://go.dev/cl/507495 mentions this issue: gopls/internal/lsp/source: ignore objectpath error in rename

@findleyr
Copy link
Contributor

findleyr commented Jul 5, 2023

I'm not sure whether that's that a bug or a feature.

I think that's a bug. IMO objectpath.Object should work for any non-empty path returned by objectpath.For. Can we "fix" this by failing objectpath.For in these cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
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
gopls/v0.13.0
Development

No branches or pull requests
3 participants
@adonovan @gopherbot @findleyr