Antonboom / testifylint

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

len: typed len check #131

Open ccoVeille opened 2 weeks ago

ccoVeille commented 2 weeks ago

This pattern is currently not detected

assert.True(t, uint32(len(arr)) == whatever)
assert.Equal(t, whatever, uint32(len(arr)))

here are examples

https://github.com/hrygo/gosms/blob/a86be4529f49edfd239fcb7a73a0db4c72924bae/codec_test/sgip/submit_test.go#L24

        assert.True(t, uint32(len(dt)) == submit.PacketLength)

https://github.com/foxcpp/go-imap-backend-tests/blob/958ea5829771f889ccc42c89ab0f7a74ad0d5e01/mailbox.go#L233

    assert.Equal(t, uint32(len(testMsg)), msg.Size, "RFC822 size mismatch")

(yes, I know this one inverted expected and actual)

Some other can be found here https://github.com/search?q=language%3Ago+%22assert.True%28t%2C+uint%22&type=code&p=4 https://github.com/search?q=language%3Ago+%22assert.Equal%28t%2C+uint%22+%22len%22&type=code

ccoVeille commented 2 weeks ago

What are your thoughts about these ?

Antonboom commented 2 weeks ago

I thought about it while fix negative-positive PR – we can support it, but we should analyze which conversions are safe.

For example, uint32(len(testMsg)) is a bad code for 64-bit machines. And testifylint should not repeat such errors in his suggestions.

ccoVeille commented 2 weeks ago

I might be missing something, but as the suggestion for these conversions would be:

        assert.True(t, uint32(len(dt)) == submit.PacketLength)
// =>       assert.Len(t, dt, int(submit.PacketLength))
    assert.Equal(t, uint32(len(testMsg)), msg.Size, "RFC822 size mismatch")
// =>       assert.Len(t, testMsg, int(submit.PacketLength))

as we would cast the type to int if not already an int or in64, why would we care about the unsafe code. Their code is unsafe now, they would become valid if we fix them, no ?

Antonboom commented 2 weeks ago

Their code is unsafe now, they would become valid if we fix them, no ?

Yes, in this case we will do picture better. But I meant another cases when we can make worse. Just pay attention to it