Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

len: typed len check #131

Open
ccoVeille opened this issue Jun 19, 2024 · 4 comments
Open

len: typed len check #131

ccoVeille opened this issue Jun 19, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@ccoVeille
Copy link
Contributor

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
Copy link
Contributor Author

What are your thoughts about these ?

@Antonboom
Copy link
Owner

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.

@Antonboom Antonboom added the enhancement New feature or request label Jun 20, 2024
@ccoVeille
Copy link
Contributor Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants