testifylint icon indicating copy to clipboard operation
testifylint copied to clipboard

len: Len instead of Equal when comparing two len's

Open Limero opened this issue 1 year ago • 1 comments

In a large codebase at work I noticed that people sometimes compare the values of two len() with Equal, something that testifylint v1.4.3 currently doesn't detect. It should suggest using assert.Len() the same as if you compare with a const, i.e. assert.Equal(t, len(expected), 2)

Expected results:

expected := []string{"a", "b"}
actual := map[string]bool{
  "a": true,
  "b": true,
}

❌ assert.Equal(t, len(expected), len(actual))
✅ assert.Len(t, expected, len(actual))

Limero avatar Jul 30 '24 09:07 Limero

Thanks for reporting the issue. It's about a false-negative, so it's a good news.

I'm unsure what testifylint should do here, we might have to discuss about it.

But here are some real life examples

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/pkg/jsonfieldstest/jsonfieldstest.go#L45

https://github.com/uber/tchannel-go/blob/af146f5a54ac5787563a6857ed9b6ff26394f0ca/relay/relaytest/mock_stats.go#L146

https://github.com/bufbuild/buf/blob/3ef45f86e43fea376f93df6ae8c115d27637513e/private/pkg/storage/storagetesting/storagetesting.go#L121

https://github.com/netobserv/netobserv-ebpf-agent/blob/47e9fb8c3ef5f9a651d6e19c73a340ee5dec8520/e2e/basic/common.go#L121

At least the pattern is frequently used

ccoVeille avatar Jul 30 '24 11:07 ccoVeille