search

nunnatsa / ginkgolinter

golang linter for ginkgo and gomega
MIT License
25 stars 6 forks source link

New check suggestion: `HaveLen(0)` should be `BeEmpty()` #46

Closed oscr closed 1 year ago

oscr commented 1 year ago

Hi,

When adding ginkgolinter len assertaion check to Cluster API I got a really good comment about changing HaveLen(0) to BeEmpty(). I would therefore like to suggest it as a new check for the linter.

Incorrect example. Original code: g.Expect(len(handlers)).To(Equal(0))

Suggested code by gingkolinter: g.Expect(handlers).To(HaveLen(0))

Suggested improvement: g.Expect(handlers).To(BeEmpty())

Thanks for this great new linter @nunnatsa!

nunnatsa commented 1 year ago

Hi @oscr

It's very cool you adopted this linter! Thanks for that. And thanks for this comment.

I wander: did you run the linter executable with the -fix parameter? or did you change it manually? What was the original warning from the linter?

I'm asking to understand if this is a bug, since the linter should already change/suggest Expect(len(x)).Should(Equal(0)) ==> Expect(x).Should(BeEmpty()).

If you manually changed to HaveLen(0), then I guess I can add this check as well (HaveLen(0) ==> BeEmpty())

oscr commented 1 year ago

Hi @nunnatsa!

Thanks for the quick reply! Looking at it again I made a mistake in the issue description. ginkgolinter does suggest BeEmpty() so my example wasn't correct. Sorry for that. It seems like ginkgolinter doesn't pick up e g: g.Expect(nsList.Items).Should(HaveLen(0)).

nunnatsa commented 1 year ago

@oscr hi. Will you have some time to review #47 with the implementation of this issue?