[BUG] Panic during checkMatchErrorAssertion with `-fix`

sam-heilbron commented 6 months ago

Describe the bug When the -fix flag is enabled, I experience a panic when running the linter.

To Reproduce

An example PR where I encountered this, and outlines the steps taken is: https://github.com/solo-io/gloo/pull/9005

panic: runtime error: index out of range [0] with length 0

goroutine 23146 [running]:
github.com/nunnatsa/ginkgolinter.checkMatchErrorAssertion(0xc008a26a40?, 0xbe405e9?)
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:408 +0xaf
github.com/nunnatsa/ginkgolinter.doCheckMatchError(0xc001fae0d0, 0xc008a26a80, 0xc008a26a40, {0x13d30c0, 0xc008a269c0}, {0x13d41d0?, 0x15bfc00?}, {0xc00502c380, 0x62})
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:375 +0x279
github.com/nunnatsa/ginkgolinter.checkMatchError(0x13d30c0?, 0x13d30c0?, {0x13d30c0?, 0xc008a269c0?}, {0x13d41d0?, 0x15bfc00?}, {0xc00502c380?, 0x0?})
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:340 +0x65
github.com/nunnatsa/ginkgolinter.checkExpression(0xc001fae0d0, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0xc008a26a80, ...)
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:318 +0x53f
github.com/nunnatsa/ginkgolinter.(*ginkgoLinter).run.func1({0x13d20b0?, 0xc008a22490})
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:250 +0x308
go/ast.inspector.Visit(0xc008be6080, {0x13d20b0?, 0xc008a22490?})
        /Users/samheilbron/.go/src/go/ast/walk.go:386 +0x2b
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d20b0?, 0xc008a22490?})
        /Users/samheilbron/.go/src/go/ast/walk.go:51 +0x5c
go/ast.walkStmtList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:32
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2498?, 0xc008a24840?})
        /Users/samheilbron/.go/src/go/ast/walk.go:234 +0x1f13
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2448?, 0xc008a22580?})
        /Users/samheilbron/.go/src/go/ast/walk.go:99 +0x245
go/ast.walkExprList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:26
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d25d8?, 0xc008a26dc0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:144 +0x2125
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2510?, 0xc008a1e4b0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:111 +0x385
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d25d8?, 0xc008a26e40?})
        /Users/samheilbron/.go/src/go/ast/walk.go:143 +0x6f9
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d20b0?, 0xc008a225c0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:206 +0xd16
go/ast.walkStmtList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:32
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2498?, 0xc008a248d0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:234 +0x1f13
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2448?, 0xc008a225d0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:99 +0x245
go/ast.walkExprList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:26
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d25d8?, 0xc008a26e80?})
        /Users/samheilbron/.go/src/go/ast/walk.go:144 +0x2125
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d20b0?, 0xc008a225f0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:206 +0xd16
go/ast.walkStmtList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:32
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2498?, 0xc008a24900?})
        /Users/samheilbron/.go/src/go/ast/walk.go:234 +0x1f13
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2448?, 0xc008a22600?})
        /Users/samheilbron/.go/src/go/ast/walk.go:99 +0x245
go/ast.walkExprList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:26
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d25d8?, 0xc008a26f00?})
        /Users/samheilbron/.go/src/go/ast/walk.go:144 +0x2125
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d20b0?, 0xc008a22620?})
        /Users/samheilbron/.go/src/go/ast/walk.go:206 +0xd16
go/ast.walkStmtList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:32
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2498?, 0xc008a25770?})
        /Users/samheilbron/.go/src/go/ast/walk.go:234 +0x1f13
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2448?, 0xc008a23440?})
        /Users/samheilbron/.go/src/go/ast/walk.go:99 +0x245
go/ast.walkExprList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:26
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d25d8?, 0xc008a61a00?})
        /Users/samheilbron/.go/src/go/ast/walk.go:144 +0x2125
go/ast.walkExprList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:26
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2088?, 0xc008a380f0?})
        /Users/samheilbron/.go/src/go/ast/walk.go:318 +0x1ccd
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2ad8?, 0xc008a61a40?})
        /Users/samheilbron/.go/src/go/ast/walk.go:344 +0x1b85
go/ast.walkDeclList(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:38
go/ast.Walk({0x13d0e40?, 0xc008be6080?}, {0x13d2060?, 0xc008a68000?})
        /Users/samheilbron/.go/src/go/ast/walk.go:366 +0x1b05
go/ast.Inspect(...)
        /Users/samheilbron/.go/src/go/ast/walk.go:397
github.com/nunnatsa/ginkgolinter.(*ginkgoLinter).run(0xc000116440, 0xc001fae0d0)
        /Users/samheilbron/go/pkg/mod/github.com/nunnatsa/ginkgolinter@v0.15.2-0.20231218063552-f0f42939d765/ginkgo_linter.go:193 +0x3a5
golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce(0xc008983220)
        /Users/samheilbron/go/pkg/mod/golang.org/x/tools@v0.16.1/go/analysis/internal/checker/checker.go:775 +0x9f7
sync.(*Once).doSlow(0xc0002286e0?, 0xc008cc4ca8?)
        /Users/samheilbron/.go/src/sync/once.go:74 +0xbf
sync.(*Once).Do(...)
        /Users/samheilbron/.go/src/sync/once.go:65
golang.org/x/tools/go/analysis/internal/checker.(*action).exec(...)
        /Users/samheilbron/go/pkg/mod/golang.org/x/tools@v0.16.1/go/analysis/internal/checker/checker.go:691
golang.org/x/tools/go/analysis/internal/checker.execAll.func1(0x75622d6f672f7365?)
        /Users/samheilbron/go/pkg/mod/golang.org/x/tools@v0.16.1/go/analysis/internal/checker/checker.go:679 +0x45
created by golang.org/x/tools/go/analysis/internal/checker.execAll in goroutine 1
        /Users/samheilbron/go/pkg/mod/golang.org/x/tools@v0.16.1/go/analysis/internal/checker/checker.go:685 +0x15a

Expected behavior I would expect that an unknown state does not cause the tool to panic, but instead reports that error back to the user to address

Additional context

I attempted to open a PR but was unable to, due to permission restictions
I was able to side-step the issue by modifying the method locally (https://github.com/nunnatsa/ginkgolinter/blob/main/ginkgo_linter.go#L407) and adding guards to prevent the panic. However, I don't know the codebase well enough to understand if that is the appropriate place for this guard

nunnatsa commented 6 months ago

Thanks @sam-heilbron, for this report.

In order to open a PR, you'll need to fork this project and push your fix there, then open a PR.

nunnatsa commented 6 months ago

OK. Found it. Great catch @sam-heilbron!

nunnatsa commented 6 months ago

Version v0.15.2 with a fix to this issue was just released.

nunnatsa / ginkgolinter

[BUG] Panic during checkMatchErrorAssertion with `-fix` #124