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: gopls fails to rename all uses of instantiated struct field #61640

Closed
willdhorn opened this issue Jul 28, 2023 · 6 comments
Closed
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

Comments

@willdhorn
Copy link

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: Fri, 28 Jul 2023 17:48:22 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)
Thu, 27 Jul 2023 04:08:03 GMT: manual (enabled: true)

(This was copied from an autogenerated crash report for the vscode-go repo, but it's related to two other issues that have been with gopls and not the vscode extension)

Related to both #61618 and #61614

gopls crashes when attempting to rename a method of a generic struct if, I think, that struct is instantiated anywhere with a generic composite literal. In the following example, gopls crashes if I try to rename WithName. Note that this happens on any method regardless of arguments, return type, or whether the receiver is a pointer or not.

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]{},
	}
  }
}
func (b  *BuilderImpl[S ~[]F, F ~string]) WithName(name string) *BuilderImpl[S,F]  {
	b.name = name
	return b
}

Failed to auto-collect gopls trace: unrecognized crash pattern.

Logs manually copied with local file information removed

panic: unexpected composite literal type: S

goroutine 4328 [running]:
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003ecdf20, {0x100d5e4c0, 0xc001f56880})
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003ecdf20, {0x100d5e4c0, 0xc001f569c0})
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003ecdf20, {0x100d5e4c0, 0xc001f56a00})
golang.org/x/tools/refactor/satisfy.(*Finder).expr(0xc003ecdf20, {0x100d5eb50, 0xc001f55380})
golang.org/x/tools/refactor/satisfy.(*Finder).stmt(0xc003ecdf20, {0x100d5e940?, 0xc001f553a0?})
golang.org/x/tools/refactor/satisfy.(*Finder).stmt(0xc003ecdf20, {0x100d5e3a0?, 0xc001f47440?})
golang.org/x/tools/refactor/satisfy.(*Finder).Find(0xc003ecdf20, 0xc001c32000?, {0xc004b42388, 0x1, 0x0?})
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).satisfy(0xc00099d200)
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).checkMethod(0xc00099d200, 0xc001c34d20)
golang.org/x/tools/gopls/internal/lsp/source.(*renamer).check(0xc00099d200, {0x100d688b8, 0xc001c34d20})
golang.org/x/tools/gopls/internal/lsp/source.renameObjects({0xc003ecdce0?, 0x100a86cc0?}, {0x1000101c8?, 0xc002218d80?}, {0xc002756a10, 0xa}, {0x100d65ea8, 0xc000526a80}, {0xc002218dc0, 0x1, ...})
golang.org/x/tools/gopls/internal/lsp/source.renameExported({0x100d5fb58, 0xc003de3cb0}, {0x100d6ced0, 0xc00068cf00}, {0xc0021f92e0, 0x2, 0x0?}, {0xc000879320, 0x12}, {0xc0000cb7a0, ...}, ...)
golang.org/x/tools/gopls/internal/lsp/source.renameOrdinary({0x100d5fb58, 0xc003de3cb0}, {0x100d6ced0, 0xc00068cf00}, {0x100d609e0, 0xc00087d680}, {0x9d0e20?, 0x1?}, {0xc002756a10, 0xa})
golang.org/x/tools/gopls/internal/lsp/source.Rename({0x100d5fb58?, 0xc003de3b60?}, {0x100d6ced0, 0xc00068cf00}, {0x100d609e0, 0xc00087d680}, {0x1dd2120?, 0xc0?}, {0xc002756a10, 0xa})
golang.org/x/tools/gopls/internal/lsp.(*Server).rename(0x1009635a0?, {0x100d5fab0, 0xc00272e820}, 0xc001a1bbc0)
golang.org/x/tools/gopls/internal/lsp.(*Server).Rename(0xc000404a00?, {0x100d5fab0?, 0xc00272e820?}, 0x1009635a0?)
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch({0x100d5fab0, 0xc00272e820}, {0x100d70810, 0xc000780a90}, 0xc003de39b0, {0x100d5fd88, 0xc00199f100})
golang.org/x/tools/gopls/internal/lsp/protocol.ServerHandler.func1({0x100d5fab0, 0xc00272e820}, 0xc003de39b0, {0x100d5fd88, 0xc00199f100})
golang.org/x/tools/gopls/internal/lsp/lsprpc.handshaker.func1({0x100d5fab0, 0xc00272e820}, 0xc003de39b0, {0x100d5fd88?, 0xc00199f100?})
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0x100d5fab0, 0xc00272e820}, 0xc0032e9a58, {0x100d5fd88?, 0xc00199f100?})
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
[Error - 1:48:05 PM] Connection to server got closed. Server will not be restarted.
@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 Jul 28, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 28, 2023
@findleyr
Copy link
Contributor

Sorry, perhaps I'm missing something. Is this not a dupe of #61614? Can you try with gopls build from master?

git clone https://go.googlesource.com/tools
cd tools/gopls
go install

@willdhorn
Copy link
Author

Oh, you're probably right. I was thinking struct members and methods would be handled differently, but it'd make sense if they're not in this instance.

Let me try out the updated build

@willdhorn
Copy link
Author

Using the latest build, renaming methods and finding references work, but attempting to rename a struct member with a generic type still only applies in the local scope.

@findleyr findleyr changed the title x/tools/gopls: gopls crashes when renaming method of struct types that use type param composite literals x/tools/gopls: gopls fails to rename all uses of instantiated struct field Jul 31, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.13.1 Jul 31, 2023
@findleyr
Copy link
Contributor

Thanks @willdhorn that sounds like the same type of bug I just fixed for references; we'll fix for v0.13.1.

Thanks for all the reports!

@gopherbot
Copy link

Change https://go.dev/cl/518897 mentions this issue: gopls/internal/lsp/source: fix renaming instantiated fields

@gopherbot
Copy link

Change https://go.dev/cl/528401 mentions this issue: net/http: handle MethodNotAllowed

gopherbot pushed a commit that referenced this issue Sep 15, 2023
If no pattern matches a request, but a pattern would have
matched if the request had a different method, then
serve a 405 (Method Not Allowed), and populate the
"Allow" header with the methods that would have succeeded.

Updates #61640.

Change-Id: I0ae9eb95e62c71ff7766a03043525a97099ac1bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/528401
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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. 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