len: simplify []bytes to string conversion to detected use cases

[ccoVeille]

The following pattern could be reported

assert.Len(t, string(headContents), 40)

to use

assert.Len(t, headContents, 40)

I might be wrong, but I don't think there is a case where the length differs between the bytes and string.

This pattern is currently not detected, while it's pretty commonly used

https://github.com/search?q=language%3Ago+%22assert.Len%28t%2C+string%28%22&type=code&p=2

https://github.com/sean-ahn/user/blob/c662a9ecf73d2c467ee6d2ba4b6c8241c6ebbcaf/backend/crypto/scrypt_test.go#L41 https://github.com/fredrikekre/gitea/blob/c88f2e2acce8240ddd2161644ac37db5b6ec1337/modules/git/repo_compare_test.go#L120

⚠️ I don't think we should do it for a struct with a stringer

[Antonboom]

Looks like you are right

https://go.dev/ref/spec#Length_and_capacity

len(s)   string type   string length in bytes
         []T           slice length

I accept this proposal 👍

P.S. https://chatgpt.com/share/6b7bbc7a-498f-4ef5-ba0f-ff9ba1f773b0 P.P.S. It's strange that I didn't find a linter about this

[ccoVeille]

It's strange that I didn't find a linter about this

The only one I can think about is mirror

https://github.com/butuzov/mirror

I might open an issue on gosimple then anyway.

As you are right, it's pretty common https://github.com/search?q=+%22len%28string%28%22+language%3AGo+NOT+is%3Afork&type=code

[Antonboom]

I might open an issue on gosimple then anyway.

JFYI, already exists: https://github.com/dominikh/go-tools/issues/760

[ccoVeille]

Thanks.

Yes, it's the issue about what I was looking for, so no need to create it.

Do you plan to detect len(string(v)) and len([]bytes(v)) to assert.Len(t, v) ?

[Antonboom]

Do you plan to detect ...

Yep, in progress in the framework of current milestone.

[Antonboom]

Hi, @ccoVeille

Unfortunately, having trusted, I implemented it in empty and len, but reverted the code after testing.

In the last version of testify (including https://github.com/stretchr/testify/issues/1482), *Len and *Empty assertions use %v to print object, not %s. So, such type conversions help with debug a lot.

Example: https://go.dev/play/p/599fGh9Cdrq / https://github.com/Antonboom/testifylint/commit/1bf6ee1a6901ba3db216f2528c012d25c2881eba

In opposite we can implement the checker that propose add string( and remove []byte/json.RawMessage( to improve failure message readability. But I am not sure about it 🤔 Moved in https://github.com/Antonboom/testifylint/discussions/203

[ccoVeille]

That's indeed not fun. But I understand

[ccoVeille]

But wait, the use case that is reported badly is bytes used with Len, Empty, and their negations.

But the linter would have still an interest for string converted to []bytes then passed to testify.

But look at the stats, I shared here:

https://github.com/dominikh/go-tools/issues/760#issuecomment-2480989858

len([]byte(...: 6.5k len(string(: 5.4k (but they are struct with stringer among them) len(json.RawMessage(...: 4

len([]byte(a)) is pretty badly used.

Example: https://github.com/walles/moar/blob/95308688a6ac0f27d028753c5dfc17962bc522af/m/reader_test.go#L548

You couldn't suggest something that convert in assert.Len(t, 1, b)

But you can convert something that converts into assert.Len(t, 1, s)

[Antonboom]

But you can convert something that converts into assert.Len(t, 1, s)

Exactly, and I consider this as part of other checker, how I have wrote above

.. and remove []byte/json.RawMessage( to improve failure message readability

Other checker, because:

1. don't overload the already complex len and empty
2. type conversions do not affect len and empty logics now
3. []byte removing will be covered by gosimple (I hope, but not sure – issue was created 4 years ago 😢 )
4. string adding will conflict with gosimple
5. I feel the other cases of failure message readability here too, not Len only.
6. [not critical] Cases like p3 & p4 depend closely on the testify version.

[ccoVeille]

Thanks for the detailed status, it's clearer