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

len: support arr.Len() #161

Closed mmorel-35 closed 1 week ago

mmorel-35 commented 4 months ago

There are many places where an assertion is done on arr.Len() an interface that assert.Len() supports

See. https://github.com/search?q=language%3Ago+testify+%22.Len%28%29%22+path%3A_test.go&type=code

For example

assert.Equal(t, 0, arr.Len())
assert.Equal(t, 1, arr.Len())

Shall be replaced with

assert.Empty(t, arr)
assert.Len(t, arr, 1)
mmorel-35 commented 2 months ago

Special attention here: https://github.com/stretchr/testify/blob/v1.9.0/assert/assertions.go#L723

Len() == 0 => Empty is only for "chan", "map" and "slice'

It probably deserves another issue @ccoVeille , @Antonboom , WDYT ?

ccoVeille commented 2 months ago

You might be right by suggesting the usage of empty when equal(t, 0, whatever.Len()) is used only for these 3.

ccoVeille commented 2 months ago

But then there is something strange/stupid in testify

https://github.com/stretchr/testify/blob/v1.9.0/assert%2Fassertions.go#L776-L803

Because if a struct implements .Len, we can use assert.Len(t, whatever, 0)

But we cannot use assert.Empty(t, whatever)

Maybe it would worth opening an issue on testify.

Could anyone confirm?

mmorel-35 commented 2 months ago

I believe that would worth an issue on testify. Something like every kind(not only chan, map and slice) that implements

interface {
  Len() int
}

shall be able to be converted from

assert.Equal(t, obj.Len(), 0)

To

assert.Len(t, obj, 0)

To

assert.Empty(t, obj)
ccoVeille commented 2 months ago

I would say Empty should support what Len supports.

The fact the could support the Len() int interface might and should be addressed separately, and in another issue

Antonboom commented 1 week ago

Hi, guys

Thank you for request.

But reflect.Value.Len (used in testify) is not about Len interface and this issue doesn't make sense.

So assert.Equal(t, 3, obj.Len()) != assert.Len(t, obj, 3).

Easy test: https://go.dev/play/p/flB3MLf-NUG

Feel free to reopen the issue, if I missed something. Thanks

Antonboom commented 6 days ago

Proof added in https://github.com/Antonboom/testifylint/commit/03ade38d5e5f5cfdb3df10bf986556eb28583c79