format: panic when using type parameters (via gopls)

[leitzler]

$ go version
go version devel go1.18-b1bedc0774 Thu Sep 16 17:44:44 2021 +0000 darwin/amd64
$ go list -m mvdan.cc/gofumpt golang.org/x/tools golang.org/x/tools/gopls
mvdan.cc/gofumpt v0.1.1
golang.org/x/tools v0.1.6-0.20210902182115-3b801c8b8389
golang.org/x/tools/gopls v0.0.0-20210902182115-3b801c8b8389

Using gofumpt via gopls and saving a file that includes a function call with type parameters:

package main

func Foo[A, B any](x A, y B) {}

func main() {
    Foo[int,int](1,2)
    log.Println("") // <- note that "log" isn't imported
}

Results in a panic in x/tools:

Panic: Apply: unexpected node type *ast.IndexListExpr [recovered]
   panic: Apply: unexpected node type *ast.IndexListExpr

goroutine 1045 [running]:
golang.org/x/tools/go/ast/astutil.Apply.func1()
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:49 +0x89
panic({0x180f560, 0xc001101130})
   /Users/leitzler/sdk/gotip/src/runtime/panic.go:814 +0x207
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a90988, 0xc00168d900}, {0x191c701, 0x0}, 0xc00258cb00, {0x1a90e88, 0xc00168d840})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:447 +0x2a74
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a907d0, 0xc00168d940}, {0x191c827, 0x0}, 0x0, {0x1a90988, 0xc00168d900})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:267 +0x233b
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc0022a8de0, {0x1a907d0, 0xc00168d940}, {0x191c827, 0x3})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:481 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a90938, 0xc001f1cdb0}, {0x191d239, 0x9}, 0x8, {0x1a907d0, 0xc00168d940})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:332 +0x81c
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc0022a8de0, {0x1a90938, 0xc001f1cdb0}, {0x191d239, 0x4})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:481 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a90ca8, 0xc001f1ce10}, {0x191cfe5, 0x0}, 0xc00258d170, {0x1a90938, 0xc001f1cdb0})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:347 +0x310
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a90c58, 0xc00196a000}, {0x191e1a4, 0x1fb}, 0x25, {0x1a90ca8, 0xc001f1ce10})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:420 +0x18c9
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc0022a8de0, {0x1a90c58, 0xc00196a000}, {0x191e1a4, 0x5})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:481 +0xae
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0022a8de0, {0x1a92008, 0xc001101120}, {0x191d2b9, 0x29ac0770}, 0x10, {0x1a90c58, 0xc00196a000})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:426 +0xc31
golang.org/x/tools/go/ast/astutil.Apply({0x1a90c58, 0xc00196a000}, 0xc001101100, 0xc001101110)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/go/ast/astutil/rewrite.go:54 +0x166
mvdan.cc/gofumpt/format.File(0xc00168cdc0, 0xc00196a000, {{0x0, 0x18055e0}, 0x20})
   /Users/leitzler/go/pkg/mod/mvdan.cc/gofumpt@v0.1.1/format/format.go:95 +0x245
mvdan.cc/gofumpt/format.Source({0xc0002db440, 0x1fc, 0x214}, {{0x0, 0xc00017a598}, 0xc8})
   /Users/leitzler/go/pkg/mod/mvdan.cc/gofumpt@v0.1.1/format/format.go:55 +0xb9
golang.org/x/tools/gopls/internal/hooks.Options.func1({0x1a7e340, 0xc0002db440}, {0xc0002db440, 0x184df60, 0xc00056fd80})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools/gopls@v0.0.0-20210902182115-3b801c8b8389/internal/hooks/hooks.go:26 +0x2e
golang.org/x/tools/internal/lsp/source.Format({0x1a964d8, 0xc00153ce80}, {0x1abf940, 0xc002da0200}, {0x1aa2698, 0xc0022a8840})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/source/format.go:62 +0x3b8
golang.org/x/tools/internal/lsp.(*Server).formatting(0xc001cc4e38, {0x1a964d8, 0xc00153ce80}, 0xc001cc4e10)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/format.go:25 +0x1ce
golang.org/x/tools/internal/lsp.(*Server).Formatting(0xc002fc4640, {0x1a964d8, 0xc00153ce80}, 0x17f1ba0)
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/server_gen.go:124 +0x25
golang.org/x/tools/internal/lsp/protocol.serverDispatch({0x1a964d8, 0xc00153ce80}, {0x1ac37a0, 0xc0003c4a20}, 0xc001d29e90, {0x1a96740, 0xc00153cd80})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/protocol/tsserver.go:469 +0x1b25
golang.org/x/tools/internal/lsp/protocol.ServerHandler.func1({0x1a964d8, 0xc00153ce80}, 0xc001d29e90, {0x1a96740, 0xc00153cd80})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/protocol/protocol.go:154 +0x90
golang.org/x/tools/internal/lsp/lsprpc.handshaker.func1({0x1a964d8, 0xc00153ce80}, 0xc001d29e90, {0x1a96740, 0xc00153cd80})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/lsp/lsprpc/lsprpc.go:506 +0xa43
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0x1a964d8, 0xc00153ce80}, 0xc001063ea8, {0x1a96740, 0xc00153cd80})
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/jsonrpc2/handler.go:35 +0xf6
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/jsonrpc2/handler.go:103 +0xa3
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
   /Users/leitzler/go/pkg/mod/golang.org/x/tools@v0.1.6-0.20210902182115-3b801c8b8389/internal/jsonrpc2/handler.go:100 +0x20a

[mvdan]

Thanks for filing this! I've been closely following @findleyr's work on golang/go#47781. It landed recently, and just got accepted. So I'll get on updating gofumpt as soon as I can, as I think the APIs are pretty final at this point.

[findleyr]

Thanks @mvdan and @leitzler.

This is definitely an x/tools issue. I believe it is a trivial fix, and will look tomorrow. I'll open an issue in the Go repo if it's anything but trivial.

[findleyr]

Huh, there is handling for this (and has been for a while):
https://cs.opensource.google/go/x/tools/+/master:go/ast/astutil/rewrite_test.go;l=198;drc=322816044c6cd7638d0b0802326ccab576518342

I'm also not reproducing with the repro given here. I think this panic could be possible if we're somehow getting a node of type *ast.IndexListExpr with nil pointer, but I'm not seeing anything in gofumpt that would cause this.

Will need to look more later.

[mvdan]

I could reproduce with x/tools v0.1.7 and go version devel go1.18-7bf7bbc241 Sat Oct 2 13:15:42 2021 +0000 linux/amd64, and the following test:

gofumpt -w foo.go
exec cat foo.go

-- go.mod --
module test

go 1.18
-- foo.go --
package main

func Foo[A, B any](x A, y B) {}

func main() {
    Foo[int,int](1,2)
}

After updating to the latest master as of today, I can no longer reproduce the crash. I think https://go-review.googlesource.com/c/tools/+/352898 might have been the fix. It's that default case in the type switch that we were panicking on, and the IndexListExpr type in question is now handled explicitly.

It's unclear to me why only gofumpt was running into a panic with the older code. I guess I'm not too worried on my end, as long as master works :) I'll lock it in via a test, too.

[mvdan]

I've included the test and the fix in master now. Undoing the update reproduces the panic, too.

[findleyr]

@mvdan this is expected to fail at x/tools@v0.1.7: that version temporarily guarded use of the new APIs behind a type parameter, so that we don't break the build with API changes. We'll tag an x/tools using the APIs once the 1.18 APIs are stable (which might not be for some time, as there are a few late amendments to generics being proposed).

I would actually have expected this to be fixed before https://go-review.googlesource.com/c/tools/+/352898, which was (mostly) an API cleanup (it differs in the case of a typed nil).

Anyway, I currently consider the astutil package to be fully updated with generics support; please let me know if you encounter any further problems.

[mvdan]

Sounds good. I think my needs are met at this point, as gofumpt does not use go/types :)