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: Len instead of Equal when comparing two len's #166

Open Limero opened 3 months ago

Limero commented 3 months ago

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))
ccoVeille commented 3 months ago

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