Antonboom / testifylint

The Golang linter that checks usage of github.com/stretchr/testify.
https://github.com/stretchr/testify
MIT License
101 stars 8 forks source link

question: why no autofix for `require-error` and `go-require` ? #132

Closed ccoVeille closed 5 months ago

ccoVeille commented 5 months ago

These are two separate questions:

I mean if a code is using assert.NoError or assert.Error, we could autofix it no ?

I looked at the edge case, like http handler/subroutine, but I didn't find a reason to do not apply it.

Maybe it's simply something I'm missing, and then it would be interesting to document it. Or it is something that was impossible to handle at first, but that could be activated now.

Maybe it's simply something I'm missing, and then it would be interesting to document it.

Antonboom commented 5 months ago

require-error requires a lot of context around. There are many patterns with conditional asserts, group of asserts, etc. But to be honest these patterns are ignored and we can fix warned asserts. The single issue here assert objects – you cannot fix code like

assrtObj := assert.New(t)
assrtObj.NoError(t, err)

because there is no reqObj instead.

For go-require we have not common pattern for fixing. In most cases you need to rewrite test at all and make goroutine free from any assertions.

ccoVeille commented 5 months ago

Thanks for the reply.

OK, for go-require. Could you add a comment about this with the AutoFix: false in the readme?

For require-error, I assumed it was possible, thanks for confirming it.

The assrtObj edge case could be reported without adding the needed information for allowing a fix. It's already the case, right?

Having autofix for require-error would help a lot on large codebase