Fix issue with comparing different types and WithTransform

The bug is that when checking the comparison of two different types, if the matcher is `WithTransform`, ginkgo linter does not check the nested matcher, causing false positive for any matcher other than `Equal` or `BeIdentical`. This PR fixes this issue by checking the nested matcher name, and only trigger error if the nested matcher is `Equal` or `BeIdentical`.
nunnatsa · Oct 28, 2023 · 75638ff · 75638ff
1 parent 17b6557
commit 75638ff
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 5 deletions.
diff --git a/ginkgo_linter.go b/ginkgo_linter.go
@@ -356,10 +356,19 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual
 			return false
 		}
 
+		matcherFuncName, ok = handler.GetActualFuncName(nested)
+		switch matcherFuncName {
+		case equal, beIdenticalTo:
+		case not:
+			return checkEqualDifferentTypes(pass, nested, actualArg, handler, old, parentPointer)
+		default:
+			return false
+		}
+
 		if t := getFuncType(pass, matcher.Args[0]); t != nil {
 			actualType = t
 			matcher = nested
-			matcherFuncName, ok = handler.GetActualFuncName(nested)
+
 			if !ok {
 				return false
 			}

diff --git a/testdata/src/a/comparetypes/comparetypes.gomega_test.go b/testdata/src/a/comparetypes/comparetypes.gomega_test.go
@@ -76,4 +76,25 @@ var _ = Describe("compare different types", func() {
 		gomega.Expect(a).Should(gomega.Or(gomega.Equal(uint64(5)), gomega.Not(gomega.Equal(int32(6))), gomega.Not(gomega.Equal(int8(4))))) // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)` `ginkgo-linter: use Equal with different types: Comparing int with int32; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)` `ginkgo-linter: use Equal with different types: Comparing int with int8; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		gomega.Expect(a).Should(gomega.WithTransform(func(i int) int { return i + 1 }, gomega.Equal(uint(6))))                             // want `ginkgo-linter: use Equal with different types: Comparing int with uint; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 	})
+
+	It("test WithTransform", func() {
+		a := uint(5)
+		gomega.Expect(uint(5)).Should(gomega.WithTransform(func(i uint) int { return int(i) }, gomega.Equal(5)))
+		gomega.Expect(uint(5)).Should(gomega.WithTransform(func(i uint) uint64 { return uint64(i) }, gomega.Equal(5))) // want `ginkgo-linter: use Equal with different types: Comparing uint64 with int; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).Should(gomega.WithTransform(func(i uint) int { return int(i) }, gomega.Equal(5)))
+		gomega.Expect(a).Should(gomega.WithTransform(uint2int, gomega.Equal(5)))
+		gomega.Expect(a).Should(gomega.WithTransform(uint2int, gomega.Equal(uint64(5))))             // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(a).Should(gomega.WithTransform(uint2int, gomega.Not(gomega.Equal(uint64(5))))) // want `ginkgo-linter: use Equal with different types: Comparing uint with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		gomega.Expect(5).Should(gomega.WithTransform(func(i int) myinf { return imp1(i) }, gomega.Equal(imp1(5))))
+	})
+
+	It("issue 115", func() {
+		gomega.Expect([]int{42, 23}).Should(gomega.WithTransform(func(v []int) []string {
+			ret := make([]string, 0, len(v))
+			for _, i := range v {
+				ret = append(ret, fmt.Sprintf("%v", i))
+			}
+			return ret
+		}, gomega.ContainElement("42")))
+	})
 })
diff --git a/testdata/src/a/comparetypes/comparetypes_test.go b/testdata/src/a/comparetypes/comparetypes_test.go
@@ -1,6 +1,7 @@
 package comparetypes_test
 
 import (
+	"fmt"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -10,6 +11,7 @@ func uint2int(u uint) int {
 }
 
 var _ = Describe("compare different types", func() {
+
 	It("find false positive check", func() {
 		a := 5
 		Expect(multi()).ShouldNot(Equal(4)) // want `ginkgo-linter: use Equal with different types: Comparing int64 with int; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
@@ -65,7 +67,18 @@ var _ = Describe("compare different types", func() {
 		Expect(uint(5)).Should(WithTransform(func(i uint) uint64 { return uint64(i) }, Equal(5))) // want `ginkgo-linter: use Equal with different types: Comparing uint64 with int; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		Expect(a).Should(WithTransform(func(i uint) int { return int(i) }, Equal(5)))
 		Expect(a).Should(WithTransform(uint2int, Equal(5)))
-		Expect(a).Should(WithTransform(uint2int, Equal(uint64(5)))) // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		Expect(a).Should(WithTransform(uint2int, Equal(uint64(5))))      // want `ginkgo-linter: use Equal with different types: Comparing int with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
+		Expect(a).Should(WithTransform(uint2int, Not(Equal(uint64(5))))) // want `ginkgo-linter: use Equal with different types: Comparing uint with uint64; either change the expected value type if possible, or use the BeEquivalentTo\(\) matcher, instead of Equal\(\)`
 		Expect(5).Should(WithTransform(func(i int) myinf { return imp1(i) }, Equal(imp1(5))))
 	})
+
+	It("issue 115", func() {
+		Expect([]int{42, 23}).Should(WithTransform(func(v []int) []string {
+			ret := make([]string, 0, len(v))
+			for _, i := range v {
+				ret = append(ret, fmt.Sprintf("%v", i))
+			}
+			return ret
+		}, ContainElement("42")))
+	})
 })
diff --git a/tests/e2e.sh b/tests/e2e.sh
@@ -6,7 +6,7 @@ cp ginkgolinter testdata/src/a
 cd testdata/src/a
 
 # no suppress
-[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2464 ]]
+[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2468 ]]
 # suppress all but nil
 [[ $(./ginkgolinter --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-type-compare-assertion=true a/... 2>&1 | wc -l) == 1452 ]]
 # suppress all but len
@@ -20,8 +20,8 @@ cd testdata/src/a
 # suppress all but focus
 [[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=true --suppress-type-compare-assertion=true a/... 2>&1 | wc -l) == 143 ]]
 # suppress all but compare different types
-[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-compare-assertion=true a/... 2>&1 | wc -l) == 211 ]]
+[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-compare-assertion=true a/... 2>&1 | wc -l) == 215 ]]
 # allow HaveLen(0)
-[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2451 ]]
+[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2455 ]]
 # suppress all - should only return the few non-suppressble
 [[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=false --suppress-type-compare-assertion=true a/... 2>&1 | wc -l) == 88 ]]