x/tools/gopls: rename crashes in objectpath

[adonovan]

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]

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

[adonovan]

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]

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

[findleyr]

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?