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

x/tools/gopls: crash in renaming inside packages that use type param composite lits #61614

Closed
willdhorn opened this issue Jul 27, 2023 · 4 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@willdhorn
Copy link

willdhorn commented Jul 27, 2023

gopls version: v0.12.4 (go1.20.3)
gopls flags:
update flags: proxy
extension version: 0.39.1
go version: 1.20.3
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Thu, 27 Jul 2023 03:27:43 GMT
restart history:
Tue, 25 Jul 2023 20:01:41 GMT: activation (enabled: true)
Tue, 25 Jul 2023 20:08:52 GMT: config change (enabled: true)
Tue, 25 Jul 2023 20:09:30 GMT: config change (enabled: true)
Tue, 25 Jul 2023 20:09:41 GMT: config change (enabled: true)
Tue, 25 Jul 2023 20:11:07 GMT: config change (enabled: true)

gopls crashed after trying to refactor/rename a struct field with a generic type.

I don't want to post the exact code that caused this as it's part of a private codebase, but I believe the following is a close enough approximation as far as this issue is concerned:

type builder[S ~[]F, F ~string] struct {
	name string
	elements S
	elemData map[F][]ElemData[F]
	// other fields...
}
type ElemData struct {
  Name F
  // other fields...
}
type BuilderImpl[S ~[]F, F ~string] struct{ builder[S, F] }
func NewBuilderImpl[S ~[]F, F ~string](name string)  *BuilderImpl[S, F] {
  return &BuilderImpl[S,F]{
	builder[S, F]{
	  name: name,
	  elements: S{},  // <-- This seems to be causing the panic
	  elemData: map[F][]ElemData[F]{},
	}
  }
}

I tried renaming all of the fields of builder, but only the non generic-typed fields (i.e. name and the other fields) succeeded. Before it crashed, whenever I attempted to rename 'elements' or 'elemData', I just got a notification popup that said "No Result". After a couple times of trying this, gopls eventually just crashed (actually, the logs are showing other identical panics, so it may have crashed the other times too, but it was being restarted until it hit a limit of number of restarts in a certain time period)

edit: I just restarted the language server and tried it again, and while it doesn't crash or panic when trying to rename a generic field, the rename only succeeds in the local scope. So if I try renaming it from the struct's type definition, that's the only place it changes (as if I had just selected it and typed a new name). If I rename it from where it's being used, every use of it in the local scope will change, but everything outside of that scope remains the same.

panic: unexpected composite literal type: S

goroutine 4547 [running]:
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003d90000, {0x100d5e4c0, 0xc001bfd1c0})
	  find.go:390  0xdda
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003d90000, {0x100d5e4c0, 0xc001bfd300})
	  find.go:363  0xfe5
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003d90000, {0x100d5e4c0, 0xc001bfd340})
	  find.go:365  0x103a
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003d90000, {0x100d5eb50, 0xc00203a680})
	  find.go:470  0x7dd
golang.org/x/tools/refactor/satisfy.(*Finder).stmt(0xc003d90000, {0x100d5e940%3F, 0xc00203a6a0%3F})
	  find.go:574  0xfa5
golang.org/x/tools/refactor/satisfy.(*Finder).stmt(0xc003d90000, {0x100d5e3a0%3F, 0xc002038660%3F})
	  find.go:589  0xe45
golang.org/x/tools/refactor/satisfy.(*Finder).Find(0xc003d90000, 0xc002f051d0%3F, {0xc0018d1ad0, 0x1, 0x0%3F})
	  find.go:106  0x1e5
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).satisfy(0xc0020c6600)
	  rename_check.go:836  0xcc
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).checkMethod(0xc0020c6600, 0xc002d1bc80)
	  rename_check.go:753  0x3d3
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).check(0xc0020c6600, {0x100d688b8, 0xc002d1bc80})
	  rename_check.go:105  0x167
golang.org/x/tools/gopls/internal/lsp/source.renameObjects({0xc00719a690%3F, 0x100a86cc0%3F}, {0x660b%3F, 0x1%3F}, {0xc0006b6500, 0x9}, {0x100d65ea8, 0xc00225b9c0}, {0xc00719a710, 0x1, ...})
	  rename.go:1010  0x26e
golang.org/x/tools/gopls/internal/lsp/source.renameOrdinary({0x100d5fb58, 0xc0020b3d10}, {0x100d6ced0, 0xc000002780}, {0x100d609e0, 0xc00007f0e0}, {0x9d0e20%3F, 0x1%3F}, {0xc0006b6500, 0x9})
	  rename.go:372  0xb05
golang.org/x/tools/gopls/internal/lsp/source.Rename({0x100d5fb58%3F, 0xc0020b3b90%3F}, {0x100d6ced0, 0xc000002780}, {0x100d609e0, 0xc00007f0e0}, {0xa0a7e0%3F, 0xc0%3F}, {0xc0006b6500, 0x9})
	  rename.go:234  0x1d7
golang.org/x/tools/gopls/internal/lsp.(*Server).rename(0x1009635a0%3F, {0x100d5fab0, 0xc004d081e0}, 0xc001a0f4c0)
	  rename.go:29  0x24b
golang.org/x/tools/gopls/internal/lsp.(*Server).Rename(0xc007306640%3F, {0x100d5fab0%3F, 0xc004d081e0%3F}, 0x1009635a0%3F)
	  server_gen.go:208  0x25
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch({0x100d5fab0, 0xc004d081e0}, {0x100d70810, 0xc00048e410}, 0xc0020b39e0, {0x100d5fd88, 0xc0011cdc40})
	  tsserver.go:515  0xe05
golang.org/x/tools/gopls/internal/lsp/protocol.ServerHandler.func1({0x100d5fab0, 0xc004d081e0}, 0xc0020b39e0, {0x100d5fd88, 0xc0011cdc40})
	  protocol.go:157  0x90
golang.org/x/tools/gopls/internal/lsp/lsprpc.handshaker.func1({0x100d5fab0, 0xc004d081e0}, 0xc0020b39e0, {0x100d5fd88%3F, 0xc0011cdc40%3F})
	  lsprpc.go:519  0x9f9
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0x100d5fab0, 0xc004d081e0}, 0xc00347e8b8, {0x100d5fd88%3F, 0xc0011cdc40%3F})
	  handler.go:35  0xf6
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
	  handler.go:103  0xa3
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
	  handler.go:100  0x20a
[Error - 11:26:52 PM] 
@findleyr
Copy link
Contributor

Thanks, this is very clearly an oversight in the rename check; I have reproduced based on your minimal example: by reverse engineering, all that is additionally required is to add a method to some time in the package, and rename that method.

@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: crash in renaming inside packages that use type param composite lits Jul 27, 2023
@findleyr
Copy link
Contributor

Transferring to the Go issue tracker, since this is a gopls bug.

@findleyr findleyr transferred this issue from golang/vscode-go Jul 27, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 27, 2023
@findleyr findleyr self-assigned this Jul 27, 2023
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jul 27, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 27, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.13.0 Jul 27, 2023
@findleyr findleyr added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 27, 2023
@gopherbot
Copy link

Change https://go.dev/cl/513775 mentions this issue: refactor/satisfy/find: composite lits may have type parameter type

@willdhorn
Copy link
Author

I created another issue (#61618) for a related problem that occurs when trying to use the "Go to references" action in VSCode (i.e. cmd/ctrl+clicking on something in most editors) on the generic fields.

Also thanks for being responsive and fixing this so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants