nunnatsa / ginkgolinter

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

[Enhancement] Comparison of pointer and value #76

Closed BorzdeG closed 1 year ago

BorzdeG commented 1 year ago

Is your feature request related to a problem? Please describe.

var v = "test"
var testPointer = &v
var wantValue = "test"

Expect(testPointer).To(Equal(wantValue))

Describe the solution you'd like

Expect(testPointer).To(HaveValue(Equal(wantValue)))

Describe alternatives you've considered

Expect(testPointer).To(Or(BeNil(), HaveValue(Equal(wantValue))))
nunnatsa commented 1 year ago

~Thanks for this suggestion @BorzdeG. This is a good example and a very common mistake.~

~I started to implement the new linter rule, and I think it's a bit more complex. So for equality assertion, the test will always fail and you will find the problem very fast. The linter can help you find it faster, but the main concern here, I think, is for inequality assertion, when this pattern create a false positive test. For example:~

var p *int
Expect(p).ShouldNot(Equal(5))

~In this case, the assertion passed because p is not equal 5, but for the wrong reasons: they are not even from the same type.~

~Now, what if p is nil? What the user want to check in this case? Some user will want to assert: "p is not nil, but its value is not 5" and some will want to check that "p is either nil, or not equal 5".~

~Assuming most user want the first option, I think the best approach is to combine the two solutions above. For positive assertions (To, Should) we can safely use the first suggestion: To(HaveValue(Equal(5)). But for negative assertions (NotTo, ToNot, ShouldNot), we'll need the second one: ToNot(Or(BeNil(), HaveValue(Equal(5))))~

~WDYT?~ ~onsi - Do you have another idea?~

Sorry - my mistake. ToNot(HaveValue(Equal(5))) works just fine.