From fb7d328aeccfb2657ee83e19ca0b1d73ba488f0b Mon Sep 17 00:00:00 2001 From: Nahshon Unna Tsameret <60659093+nunnatsa@users.noreply.github.com> Date: Sun, 26 Mar 2023 18:55:13 +0300 Subject: [PATCH] ginkgolinter: add suppress-async-assertion option (#3735) --- .golangci.reference.yml | 4 + pkg/config/linters_settings.go | 1 + pkg/golinters/ginkgolinter.go | 1 + .../configs/ginkgolinter_suppress_async.yml | 3 + test/testdata/ginkgolinter/ginkgolinter.go | 12 +++ .../ginkgolinter/ginkgolinter_havelen0.go | 12 +++ .../ginkgolinter_suppress_async.go | 80 +++++++++++++++++++ .../ginkgolinter_suppress_compare.go | 12 +++ .../ginkgolinter/ginkgolinter_suppress_err.go | 12 +++ .../ginkgolinter/ginkgolinter_suppress_len.go | 12 +++ .../ginkgolinter/ginkgolinter_suppress_nil.go | 12 +++ test/testdata/ginkgolinter/go.mod | 6 +- test/testdata/ginkgolinter/go.sum | 19 +++-- 13 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 test/testdata/ginkgolinter/configs/ginkgolinter_suppress_async.yml create mode 100644 test/testdata/ginkgolinter/ginkgolinter_suppress_async.go diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 575852c5..f7e67fbb 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -423,6 +423,10 @@ linters-settings: # Default: false suppress-compare-assertion: true + # Suppress the function all in async assertion warning. + # Default: false + suppress-async-assertion: true + # Don't trigger warnings for HaveLen(0) # Default: false allow-havelen-zero: true diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 4ab94af0..819e1e2d 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -341,6 +341,7 @@ type GinkgoLinterSettings struct { SuppressNilAssertion bool `mapstructure:"suppress-nil-assertion"` SuppressErrAssertion bool `mapstructure:"suppress-err-assertion"` SuppressCompareAssertion bool `mapstructure:"suppress-compare-assertion"` + SuppressAsyncAssertion bool `mapstructure:"suppress-async-assertion"` AllowHaveLenZero bool `mapstructure:"allow-havelen-zero"` } diff --git a/pkg/golinters/ginkgolinter.go b/pkg/golinters/ginkgolinter.go index ea2f4752..a7a089dc 100644 --- a/pkg/golinters/ginkgolinter.go +++ b/pkg/golinters/ginkgolinter.go @@ -18,6 +18,7 @@ func NewGinkgoLinter(cfg *config.GinkgoLinterSettings) *goanalysis.Linter { "suppress-nil-assertion": cfg.SuppressNilAssertion, "suppress-err-assertion": cfg.SuppressErrAssertion, "suppress-compare-assertion": cfg.SuppressCompareAssertion, + "suppress-async-assertion": cfg.SuppressAsyncAssertion, "allow-havelen-0": cfg.AllowHaveLenZero, } } diff --git a/test/testdata/ginkgolinter/configs/ginkgolinter_suppress_async.yml b/test/testdata/ginkgolinter/configs/ginkgolinter_suppress_async.yml new file mode 100644 index 00000000..cb074cdd --- /dev/null +++ b/test/testdata/ginkgolinter/configs/ginkgolinter_suppress_async.yml @@ -0,0 +1,3 @@ +linters-settings: + ginkgolinter: + suppress-async-assertion: true diff --git a/test/testdata/ginkgolinter/ginkgolinter.go b/test/testdata/ginkgolinter/ginkgolinter.go index 364011a7..b9a0bae3 100644 --- a/test/testdata/ginkgolinter/ginkgolinter.go +++ b/test/testdata/ginkgolinter/ginkgolinter.go @@ -3,6 +3,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -64,3 +66,13 @@ func WrongComparisonUsecase() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" } + +func slowInt() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction() { + Eventually(slowInt).Should(Equal(42)) // valid + Eventually(slowInt()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_havelen0.go b/test/testdata/ginkgolinter/ginkgolinter_havelen0.go index 2e22a488..7278145a 100644 --- a/test/testdata/ginkgolinter/ginkgolinter_havelen0.go +++ b/test/testdata/ginkgolinter/ginkgolinter_havelen0.go @@ -4,6 +4,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -65,3 +67,13 @@ func WrongComparisonUsecase_havelen0() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" } + +func slowInt_havelen0() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction_havelen0() { + Eventually(slowInt_havelen0).Should(Equal(42)) // valid + Eventually(slowInt_havelen0()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt_havelen0\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_suppress_async.go b/test/testdata/ginkgolinter/ginkgolinter_suppress_async.go new file mode 100644 index 00000000..265c7cff --- /dev/null +++ b/test/testdata/ginkgolinter/ginkgolinter_suppress_async.go @@ -0,0 +1,80 @@ +//golangcitest:config_path configs/ginkgolinter_suppress_async.yml +//golangcitest:args --disable-all -Eginkgolinter +package ginkgolinter + +import ( + "errors" + "time" + + . "github.com/onsi/gomega" +) + +func LenUsecase_async() { + var fakeVarUnderTest []int + Expect(fakeVarUnderTest).Should(BeEmpty()) // valid + Expect(fakeVarUnderTest).ShouldNot(HaveLen(5)) // valid + + Expect(len(fakeVarUnderTest)).Should(Equal(0)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.Should\\(BeEmpty\\(\\)\\). instead" + Expect(len(fakeVarUnderTest)).ShouldNot(Equal(2)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.ShouldNot\\(HaveLen\\(2\\)\\). instead" + Expect(len(fakeVarUnderTest)).To(BeNumerically("==", 0)) // // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.To\\(BeEmpty\\(\\)\\). instead" + + fakeVarUnderTest = append(fakeVarUnderTest, 3) + Expect(len(fakeVarUnderTest)).ShouldNot(Equal(0)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.ShouldNot\\(BeEmpty\\(\\)\\). instead" + Expect(len(fakeVarUnderTest)).Should(Equal(1)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.Should\\(HaveLen\\(1\\)\\). instead" + Expect(len(fakeVarUnderTest)).To(BeNumerically(">", 0)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.ToNot\\(BeEmpty\\(\\)\\). instead" + Expect(len(fakeVarUnderTest)).To(BeNumerically(">=", 1)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.ToNot\\(BeEmpty\\(\\)\\). instead" + Expect(len(fakeVarUnderTest)).To(BeNumerically("!=", 0)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(fakeVarUnderTest\\)\\.ToNot\\(BeEmpty\\(\\)\\). instead" +} + +func NilUsecase_async() { + y := 5 + x := &y + Expect(x == nil).To(Equal(true)) // want "ginkgo-linter: wrong nil assertion; consider using .Expect\\(x\\)\\.To\\(BeNil\\(\\)\\). instead" + Expect(nil == x).To(Equal(true)) // want "ginkgo-linter: wrong nil assertion; consider using .Expect\\(x\\)\\.To\\(BeNil\\(\\)\\). instead" + Expect(x != nil).To(Equal(true)) // want "ginkgo-linter: wrong nil assertion; consider using .Expect\\(x\\)\\.ToNot\\(BeNil\\(\\)\\). instead" + Expect(x == nil).To(BeTrue()) // want "ginkgo-linter: wrong nil assertion; consider using .Expect\\(x\\)\\.To\\(BeNil\\(\\)\\). instead" + Expect(x == nil).To(BeFalse()) // want "ginkgo-linter: wrong nil assertion; consider using .Expect\\(x\\)\\.ToNot\\(BeNil\\(\\)\\). instead" +} +func BooleanUsecase_async() { + x := true + Expect(x).To(Equal(true)) // want "ginkgo-linter: wrong boolean assertion; consider using .Expect\\(x\\)\\.To\\(BeTrue\\(\\)\\). instead" + x = false + Expect(x).To(Equal(false)) // want "ginkgo-linter: wrong boolean assertion; consider using .Expect\\(x\\)\\.To\\(BeFalse\\(\\)\\). instead" +} + +func ErrorUsecase_async() { + err := errors.New("fake error") + funcReturnsErr := func() error { return err } + + Expect(err).To(BeNil()) // want "ginkgo-linter: wrong error assertion; consider using .Expect\\(err\\)\\.ToNot\\(HaveOccurred\\(\\)\\). instead" + Expect(err == nil).To(Equal(true)) // want "ginkgo-linter: wrong error assertion; consider using .Expect\\(err\\)\\.ToNot\\(HaveOccurred\\(\\)\\). instead" + Expect(err == nil).To(BeFalse()) // want "ginkgo-linter: wrong error assertion; consider using .Expect\\(err\\)\\.To\\(HaveOccurred\\(\\)\\). instead" + Expect(err != nil).To(BeTrue()) // want "ginkgo-linter: wrong error assertion; consider using .Expect\\(err\\)\\.To\\(HaveOccurred\\(\\)\\). instead" + Expect(funcReturnsErr()).To(BeNil()) // want "ginkgo-linter: wrong error assertion; consider using .Expect\\(funcReturnsErr\\(\\)\\)\\.To\\(Succeed\\(\\)\\). instead" +} + +func HaveLen0Usecase_async() { + x := make([]string, 0) + Expect(x).To(HaveLen(0)) // want "ginkgo-linter: wrong length assertion; consider using .Expect\\(x\\)\\.To\\(BeEmpty\\(\\)\\). instead" +} + +func WrongComparisonUsecase_async() { + x := 8 + Expect(x == 8).To(BeTrue()) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(x\\)\\.To\\(Equal\\(8\\)\\). instead" + Expect(x < 9).To(BeTrue()) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(x\\)\\.To\\(BeNumerically\\(\"<\", 9\\)\\). instead" + Expect(x < 7).To(Equal(false)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(x\\)\\.ToNot\\(BeNumerically\\(\"<\", 7\\)\\). instead" + + p1, p2 := &x, &x + Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" +} + +func slowInt_async() int { + time.Sleep(time.Second) + return 42 +} + +// WrongEventuallyWithFunction_async Should trigger no warning +func WrongEventuallyWithFunction_async() { + Eventually(slowInt_async).Should(Equal(42)) // valid + Eventually(slowInt_async()).Should(Equal(42)) // suppressed +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_suppress_compare.go b/test/testdata/ginkgolinter/ginkgolinter_suppress_compare.go index a0b88647..f61c6cec 100644 --- a/test/testdata/ginkgolinter/ginkgolinter_suppress_compare.go +++ b/test/testdata/ginkgolinter/ginkgolinter_suppress_compare.go @@ -4,6 +4,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -66,3 +68,13 @@ func WrongComparisonUsecase_compare() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) } + +func slowInt_compare() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction_compare() { + Eventually(slowInt_compare).Should(Equal(42)) // valid + Eventually(slowInt_compare()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt_compare\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_suppress_err.go b/test/testdata/ginkgolinter/ginkgolinter_suppress_err.go index 98e4146c..b7e949a9 100644 --- a/test/testdata/ginkgolinter/ginkgolinter_suppress_err.go +++ b/test/testdata/ginkgolinter/ginkgolinter_suppress_err.go @@ -4,6 +4,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -66,3 +68,13 @@ func WrongComparisonUsecase_err() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" } + +func slowInt_err() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction_err() { + Eventually(slowInt_err).Should(Equal(42)) // valid + Eventually(slowInt_err()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt_err\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_suppress_len.go b/test/testdata/ginkgolinter/ginkgolinter_suppress_len.go index 00b76677..babad04e 100644 --- a/test/testdata/ginkgolinter/ginkgolinter_suppress_len.go +++ b/test/testdata/ginkgolinter/ginkgolinter_suppress_len.go @@ -4,6 +4,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -66,3 +68,13 @@ func WrongComparisonUsecase_len() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" } + +func slowInt_len() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction_len() { + Eventually(slowInt_len).Should(Equal(42)) // valid + Eventually(slowInt_len()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt_len\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/ginkgolinter_suppress_nil.go b/test/testdata/ginkgolinter/ginkgolinter_suppress_nil.go index 6525afce..85d08906 100644 --- a/test/testdata/ginkgolinter/ginkgolinter_suppress_nil.go +++ b/test/testdata/ginkgolinter/ginkgolinter_suppress_nil.go @@ -4,6 +4,8 @@ package ginkgolinter import ( "errors" + "time" + . "github.com/onsi/gomega" ) @@ -66,3 +68,13 @@ func WrongComparisonUsecase_nil() { p1, p2 := &x, &x Expect(p1 == p2).To(Equal(true)) // want "ginkgo-linter: wrong comparison assertion; consider using .Expect\\(p1\\).To\\(BeIdenticalTo\\(p2\\)\\). instead" } + +func slowInt_nil() int { + time.Sleep(time.Second) + return 42 +} + +func WrongEventuallyWithFunction_nil() { + Eventually(slowInt_nil).Should(Equal(42)) // valid + Eventually(slowInt_nil()).Should(Equal(42)) // want "ginkgo-linter: use a function call in Eventually. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed; consider using .Eventually\\(slowInt_nil\\)\\.Should\\(Equal\\(42\\)\\). instead" +} diff --git a/test/testdata/ginkgolinter/go.mod b/test/testdata/ginkgolinter/go.mod index 48dca07b..99b913d5 100644 --- a/test/testdata/ginkgolinter/go.mod +++ b/test/testdata/ginkgolinter/go.mod @@ -2,11 +2,11 @@ module ginkgolinter go 1.19 -require github.com/onsi/gomega v1.24.1 +require github.com/onsi/gomega v1.27.4 require ( github.com/google/go-cmp v0.5.9 // indirect - golang.org/x/net v0.2.0 // indirect - golang.org/x/text v0.4.0 // indirect + golang.org/x/net v0.8.0 // indirect + golang.org/x/text v0.8.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/test/testdata/ginkgolinter/go.sum b/test/testdata/ginkgolinter/go.sum index a3d1dba1..61dda065 100644 --- a/test/testdata/ginkgolinter/go.sum +++ b/test/testdata/ginkgolinter/go.sum @@ -1,14 +1,17 @@ github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= +github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 h1:p104kn46Q8WdvHunIJ9dAyjPVtrBPhSr3KT2yUst43I= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/onsi/ginkgo/v2 v2.5.0 h1:TRtrvv2vdQqzkwrQ1ke6vtXf7IK34RBUJafIy1wMwls= -github.com/onsi/gomega v1.24.1 h1:KORJXNNTzJXzu4ScJWssJfJMnJ+2QJqhoQSRwNlze9E= -github.com/onsi/gomega v1.24.1/go.mod h1:3AOiACssS3/MajrniINInwbfOOtfZvplPzuRSmvt1jM= -golang.org/x/net v0.2.0 h1:sZfSu1wtKLGlWI4ZZayP0ck9Y73K1ynO6gqzTdBVdPU= -golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= -golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= -golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= -golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE8dj7HMvPfh66eeA2JYW7eFpSE= +github.com/onsi/ginkgo/v2 v2.9.1 h1:zie5Ly042PD3bsCvsSOPvRnFwyo3rKe64TJlD6nu0mk= +github.com/onsi/gomega v1.27.4 h1:Z2AnStgsdSayCMDiCU42qIz+HLqEPcgiOCXjAU/w+8E= +github.com/onsi/gomega v1.27.4/go.mod h1:riYq/GJKh8hhoM01HN6Vmuy93AarCXCBGpvFDK3q3fQ= +golang.org/x/net v0.8.0 h1:Zrh2ngAOFYneWTAIAPethzeaQLuHwhuBkuV6ZiRnUaQ= +golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= +golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= +golang.org/x/text v0.8.0 h1:57P1ETyNKtuIjB4SRd15iJxuhj8Gc416Y78H3qgMh68= +golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=