mvdan / gofumpt

A stricter gofmt
https://pkg.go.dev/mvdan.cc/gofumpt
BSD 3-Clause "New" or "Revised" License
3.15k stars 110 forks source link

panic while handling some //line directives #288

Closed flimzy closed 5 months ago

flimzy commented 8 months ago

Parsing of this file causes gofumpt to panic. It appears to be related to //line directives.

$ gofumpt -w pkg/logql/log/pattern/lexer.rl.go 
panic: invalid line number 4 (should be < 4) [recovered]
        panic: invalid line number 4 (should be < 4)

goroutine 8 [running]:
golang.org/x/tools/go/ast/astutil.Apply.func1()
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:48 +0xab
panic({0x66f1e0?, 0xc00002d600?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
go/token.(*File).MergeLine(0xc0000829c0, 0xc000128140?)
        /usr/local/go/src/go/token/position.go:152 +0x1c8
mvdan.cc/gofumpt/format.(*fumpter).removeLines(...)
        /home/jonhall/src/gofumpt/format/format.go:249
mvdan.cc/gofumpt/format.(*fumpter).removeLinesBetween(0xc00008dd60, 0xc00002d420?, 0x1038?)
        /home/jonhall/src/gofumpt/format/format.go:257 +0x13f
mvdan.cc/gofumpt/format.(*fumpter).applyPre(0xc00008dd60, 0xc000082a30)
        /home/jonhall/src/gofumpt/format/format.go:545 +0x7b2
mvdan.cc/gofumpt/format.File.func1(0xc000082a30)
        /home/jonhall/src/gofumpt/format/format.go:109 +0x34
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x723e98?, 0xc000136300?}, {0x6bf529?, 0x0?}, 0x0?, {0x723998?, 0xc0001362d0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:199 +0x1aa
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x723998?, 0xc000136330?}, {0x6bf69d?, 0x1?}, 0x1?, {0x723e98?, 0xc000136300?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:368 +0x187b
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc000082a20, {0x723998?, 0xc000136330}, {0x6bf69d, 0x4})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:484 +0xa5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x723f38?, 0xc000132c80?}, {0x6bf529?, 0x0?}, 0x0?, {0x723998?, 0xc000136330?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:353 +0x15a5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x723998?, 0xc000136570?}, {0x6bf69d?, 0x4b850e?}, 0x698d20?, {0x723f38?, 0xc000132c80?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:386 +0x1bdb
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc000082a20, {0x723998?, 0xc000136570}, {0x6bf69d, 0x4})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:484 +0xa5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x723998?, 0xc0001365a0?}, {0x6bf69d?, 0x1?}, 0x1?, {0x723998?, 0xc000136570?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:353 +0x15a5
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc000082a20, {0x723998?, 0xc0001365a0}, {0x6bf69d, 0x4})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:484 +0xa5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x724050?, 0xc0001365d0?}, {0x6bf529?, 0x0?}, 0x0?, {0x723998?, 0xc0001365a0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:353 +0x15a5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x724078?, 0xc0000d46e0?}, {0x6bf95e?, 0x40d1da?}, 0x34?, {0x724050?, 0xc0001365d0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:429 +0x225d
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc000082a20, {0x724078?, 0xc0000d46e0}, {0x6bf95e, 0x5})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:484 +0xa5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc000082a20, {0x7242d8?, 0xc00002d5e0?}, {0x6bf699?, 0x697fe0?}, 0xc000129cb0?, {0x724078?, 0xc0000d46e0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:435 +0x230e
golang.org/x/tools/go/ast/astutil.Apply({0x724078?, 0xc0000d46e0}, 0xc000010c78, 0xc000010c90)
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:53 +0x152
mvdan.cc/gofumpt/format.File(0xc00009e640, 0xc0000d46e0, {{0xc000012f58, 0x5}, {0xc00001a360, 0x17}, 0x0})
        /home/jonhall/src/gofumpt/format/format.go:157 +0x2d7
main.processFile({0x7ffe5617be4a, 0x21}, {0x725828, 0xc0000964e0}, {0x0?, 0x0?}, 0x0?, 0x1)
        /home/jonhall/src/gofumpt/gofmt.go:313 +0x308
main.gofmtMain.func3(0x0?)
        /home/jonhall/src/gofumpt/gofmt.go:508 +0x34
main.(*sequencer).Add.func2()
        /home/jonhall/src/gofumpt/gofmt.go:191 +0x42
created by main.(*sequencer).Add in goroutine 1
        /home/jonhall/src/gofumpt/gofmt.go:190 +0x1ae
flimzy commented 8 months ago

Similar panic in the stdlib on this file, although this one appears to be intentionally "broken" (not sure if that means gofumpt should actually panic, though):

$ gofumpt -w test/dwarf/linedirectives.go 
panic: invalid line number 289 (should be < 83) [recovered]
        panic: invalid line number 289 (should be < 83)

goroutine 19 [running]:
golang.org/x/tools/go/ast/astutil.Apply.func1()
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:48 +0xab
panic({0x66f1e0?, 0xc0001143d0?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
go/token.(*File).MergeLine(0xc000134480, 0xc000112a48?)
        /usr/local/go/src/go/token/position.go:152 +0x1c8
mvdan.cc/gofumpt/format.(*fumpter).removeLines(...)
        /home/jonhall/src/gofumpt/format/format.go:249
mvdan.cc/gofumpt/format.(*fumpter).removeLinesBetween(0xc0001320a0, 0xc00015c200?, 0x5b8?)
        /home/jonhall/src/gofumpt/format/format.go:257 +0x13f
mvdan.cc/gofumpt/format.(*fumpter).applyPre(0xc0001320a0, 0xc0001344f0)
        /home/jonhall/src/gofumpt/format/format.go:545 +0x7b2
mvdan.cc/gofumpt/format.File.func1(0xc0001344f0)
        /home/jonhall/src/gofumpt/format/format.go:109 +0x34
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0001344e0, {0x724050?, 0xc00011a420?}, {0x6bf529?, 0x0?}, 0x0?, {0x723998?, 0xc00011a3f0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:199 +0x1aa
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0001344e0, {0x724078?, 0xc0001520a0?}, {0x6bf95e?, 0x40d1da?}, 0x14?, {0x724050?, 0xc00011a420?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:429 +0x225d
golang.org/x/tools/go/ast/astutil.(*application).applyList(0xc0001344e0, {0x724078?, 0xc0001520a0}, {0x6bf95e, 0x5})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:484 +0xa5
golang.org/x/tools/go/ast/astutil.(*application).apply(0xc0001344e0, {0x7242d8?, 0xc0001143a0?}, {0x6bf699?, 0x697fe0?}, 0x6b7040?, {0x724078?, 0xc0001520a0?})
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:435 +0x230e
golang.org/x/tools/go/ast/astutil.Apply({0x724078?, 0xc0001520a0}, 0xc000128b58, 0xc000128b70)
        /home/jonhall/go/pkg/mod/golang.org/x/tools@v0.9.3/go/ast/astutil/rewrite.go:53 +0x152
mvdan.cc/gofumpt/format.File(0xc000116080, 0xc0001520a0, {{0x6bf2e6, 0x2}, {0x0, 0x0}, 0x0})
        /home/jonhall/src/gofumpt/format/format.go:157 +0x2d7
main.processFile({0x7ffd6c1f2e83, 0x1c}, {0x725828, 0xc0000964e0}, {0x0?, 0x0?}, 0x0?, 0x1)
        /home/jonhall/src/gofumpt/gofmt.go:313 +0x308
main.gofmtMain.func3(0x0?)
        /home/jonhall/src/gofumpt/gofmt.go:508 +0x34
main.(*sequencer).Add.func2()
        /home/jonhall/src/gofumpt/gofmt.go:191 +0x42
created by main.(*sequencer).Add in goroutine 1
        /home/jonhall/src/gofumpt/gofmt.go:190 +0x1ae
mvdan commented 5 months ago

Thanks for spotting - I've never liked those line directives to be honest. They are definitely an obstacle for a formatter.