diff options
| author | dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> | 2024-04-02 14:37:40 +0000 |
|---|---|---|
| committer | Taras Madan <tarasmadan@google.com> | 2024-04-03 09:59:40 +0000 |
| commit | 9d2a90af8850a414d2da20b806d7aa8fd9a297ae (patch) | |
| tree | b6ce5a1bc2ecaed9f94b06b36eca20b98929970c /vendor/github.com/nunnatsa | |
| parent | b90978ba49e3321a2d1cd77c07c196b088c97386 (diff) | |
mod: bump github.com/golangci/golangci-lint from 1.56.2 to 1.57.2
Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.56.2 to 1.57.2.
- [Release notes](https://github.com/golangci/golangci-lint/releases)
- [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
- [Commits](https://github.com/golangci/golangci-lint/compare/v1.56.2...v1.57.2)
---
updated-dependencies:
- dependency-name: github.com/golangci/golangci-lint
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
Diffstat (limited to 'vendor/github.com/nunnatsa')
11 files changed, 1167 insertions, 291 deletions
diff --git a/vendor/github.com/nunnatsa/ginkgolinter/README.md b/vendor/github.com/nunnatsa/ginkgolinter/README.md index 3832f610d..977cec903 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/README.md +++ b/vendor/github.com/nunnatsa/ginkgolinter/README.md @@ -227,6 +227,54 @@ ginkgolinter checks the following: * If the first parameter is a function with the format of `func(error)bool`, ginkgolinter makes sure that the second parameter exists and its type is string. +### Async timing interval: timeout is shorter than polling interval [BUG] +***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals` +flag **is** set. + +***Note***: This rule work with best-effort approach. It can't find many cases, like const defined not in the same +package, or when using variables. + +The timeout and polling intervals may be passed as optional arguments to the `Eventually` or `Constanly` functions, or +using the `WithTimeout` or , `Within` methods (timeout), and `WithPolling` or `ProbeEvery` methods (polling). + +This rule checks if the async (`Eventually` or `Consistently`) timeout duration, is not shorter than the polling interval. + +For example: + ```go + Eventually(aFunc).WithTimeout(500 * time.Millisecond).WithPolling(10 * time.Second).Should(Succeed()) + ``` + +This will probably happen when using the old format: + ```go + Eventually(aFunc, 500 * time.Millisecond /*timeout*/, 10 * time.Second /*polling*/).Should(Succeed()) + ``` + +### Avoid Spec Pollution: Don't Initialize Variables in Container Nodes [BUG/STYLE]: +***Note***: Only applied when the `--forbid-spec-pollution=true` flag is set (disabled by default). + +According to [ginkgo documentation](https://onsi.github.io/ginkgo/#avoid-spec-pollution-dont-initialize-variables-in-container-nodes), +no variable should be assigned within a container node (`Describe`, `Context`, `When` or their `F`, `P` or `X` forms) + +For example: +```go +var _ = Describe("description", func(){ + var x = 10 + ... +}) +``` + +Instead, use `BeforeEach()`; e.g. +```go +var _ = Describe("description", func (){ + var x int + + BeforeEach(func (){ + x = 10 + }) + ... +}) +``` + ### Wrong Length Assertion [STYLE] The linter finds assertion of the golang built-in `len` function, with all kind of matchers, while there are already gomega matchers for these usecases; We want to assert the item, rather than its length. @@ -259,6 +307,20 @@ The output of the linter,when finding issues, looks like this: ./testdata/src/a/a.go:18:5: ginkgo-linter: wrong length assertion; consider using `Expect("").Should(BeEmpty())` instead ./testdata/src/a/a.go:22:5: ginkgo-linter: wrong length assertion; consider using `Expect("").Should(BeEmpty())` instead ``` + +### Wrong Cap Assertion [STYLE] +The linter finds assertion of the golang built-in `cap` function, with all kind of matchers, while there are already +gomega matchers for these usecases; We want to assert the item, rather than its cap. + +There are several wrong patterns: +```go +Expect(cap(x)).To(Equal(0)) // should be: Expect(x).To(HaveCap(0)) +Expect(cap(x)).To(BeZero()) // should be: Expect(x).To(HaveCap(0)) +Expect(cap(x)).To(BeNumeric(">", 0)) // should be: Expect(x).ToNot(HaveCap(0)) +Expect(cap(x)).To(BeNumeric("==", 2)) // should be: Expect(x).To(HaveCap(2)) +Expect(cap(x)).To(BeNumeric("!=", 3)) // should be: Expect(x).ToNot(HaveCap(3)) +``` + #### use the `HaveLen(0)` matcher. [STYLE] The linter will also warn about the `HaveLen(0)` matcher, and will suggest to replace it with `BeEmpty()` @@ -356,9 +418,67 @@ Expect(x1 == c1).Should(BeTrue()) // ==> Expect(x1).Should(Equal(c1)) Expect(c1 == x1).Should(BeTrue()) // ==> Expect(x1).Should(Equal(c1)) ``` +### Don't Allow Using `Expect` with `Should` or `ShouldNot` [STYLE] +This optional rule forces the usage of the `Expect` method only with the `To`, `ToNot` or `NotTo` +assertion methods; e.g. +```go +Expect("abc").Should(HaveLen(3)) // => Expect("abc").To(HaveLen(3)) +Expect("abc").ShouldNot(BeEmpty()) // => Expect("abc").ToNot(BeEmpty()) +``` +This rule support auto fixing. + +***This rule is disabled by default***. Use the `--force-expect-to=true` command line flag to enable it. + +### Async timing interval: multiple timeout or polling intervals [STYLE] +***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals` +flag **is** set. + +The timeout and polling intervals may be passed as optional arguments to the `Eventually` or `Constanly` functions, or +using the `WithTimeout` or , `Within` methods (timeout), and `WithPolling` or `ProbeEvery` methods (polling). + +The linter checks that there is up to one polling argument and up to one timeout argument. + +For example: + +```go +// both WithTimeout() and Within() +Eventually(aFunc).WithTimeout(time.Second * 10).Within(time.Second * 10).WithPolling(time.Millisecond * 500).Should(BeTrue()) +// both polling argument, and WithPolling() method +Eventually(aFunc, time.Second*10, time.Millisecond * 500).WithPolling(time.Millisecond * 500).Should(BeTrue()) +``` + +### Async timing interval: non-time.Duration intervals [STYLE] +***Note***: Only applied when the `suppress-async-assertion` flag is **not set** *and* the `validate-async-intervals` +flag **is** set. + +gomega supports a few formats for timeout and polling intervals, when using the old format (the last two parameters of Eventually and Constantly): +* a `time.Duration` value +* any kind of numeric value (int(8/16/32/64), uint(8/16/32/64) or float(32/64), as the number of seconds. +* duration string like `"12s"` + +The linter triggers a warning for any duration value that is not of the `time.Duration` type, assuming that this is +the desired type, given the type of the argument of the newer "WithTimeout", "WithPolling", "Within" and "ProbeEvery" +methods. + +For example: + ```go + Eventually(func() bool { return true }, "1s").Should(BeTrue()) + Eventually(context.Background(), func() bool { return true }, time.Second*60, float64(2)).Should(BeTrue()) + ``` + +This rule offers a limited auto fix: for integer values, or integer consts, the linter will suggest multiply the +value with `time.Second`; e.g. +```go +const polling = 1 +Eventually(aFunc, 5, polling) +``` +will be changed to: +```go +Eventually(aFunc, time.Second*5, time.Second*polling) +``` ## Suppress the linter ### Suppress warning from command line -* Use the `--suppress-len-assertion=true` flag to suppress the wrong length assertion warning +* Use the `--suppress-len-assertion=true` flag to suppress the wrong length and cap assertions warning * Use the `--suppress-nil-assertion=true` flag to suppress the wrong nil assertion warning * Use the `--suppress-err-assertion=true` flag to suppress the wrong error assertion warning * Use the `--suppress-compare-assertion=true` flag to suppress the wrong comparison assertion warning @@ -369,7 +489,7 @@ Expect(c1 == x1).Should(BeTrue()) // ==> Expect(x1).Should(Equal(c1)) command line, and not from a comment. ### Suppress warning from the code -To suppress the wrong length assertion warning, add a comment with (only) +To suppress the wrong length and cap assertions warning, add a comment with (only) `ginkgo-linter:ignore-len-assert-warning`. diff --git a/vendor/github.com/nunnatsa/ginkgolinter/analyzer.go b/vendor/github.com/nunnatsa/ginkgolinter/analyzer.go new file mode 100644 index 000000000..edff57acd --- /dev/null +++ b/vendor/github.com/nunnatsa/ginkgolinter/analyzer.go @@ -0,0 +1,58 @@ +package ginkgolinter + +import ( + "flag" + "fmt" + + "golang.org/x/tools/go/analysis" + + "github.com/nunnatsa/ginkgolinter/linter" + "github.com/nunnatsa/ginkgolinter/types" + "github.com/nunnatsa/ginkgolinter/version" +) + +// NewAnalyzerWithConfig returns an Analyzer. +func NewAnalyzerWithConfig(config *types.Config) *analysis.Analyzer { + theLinter := linter.NewGinkgoLinter(config) + + return &analysis.Analyzer{ + Name: "ginkgolinter", + Doc: fmt.Sprintf(doc, version.Version()), + Run: theLinter.Run, + } +} + +// NewAnalyzer returns an Analyzer - the package interface with nogo +func NewAnalyzer() *analysis.Analyzer { + config := &types.Config{ + SuppressLen: false, + SuppressNil: false, + SuppressErr: false, + SuppressCompare: false, + ForbidFocus: false, + AllowHaveLen0: false, + ForceExpectTo: false, + } + + a := NewAnalyzerWithConfig(config) + + var ignored bool + a.Flags.Init("ginkgolinter", flag.ExitOnError) + a.Flags.Var(&config.SuppressLen, "suppress-len-assertion", "Suppress warning for wrong length assertions") + a.Flags.Var(&config.SuppressNil, "suppress-nil-assertion", "Suppress warning for wrong nil assertions") + a.Flags.Var(&config.SuppressErr, "suppress-err-assertion", "Suppress warning for wrong error assertions") + a.Flags.Var(&config.SuppressCompare, "suppress-compare-assertion", "Suppress warning for wrong comparison assertions") + a.Flags.Var(&config.SuppressAsync, "suppress-async-assertion", "Suppress warning for function call in async assertion, like Eventually") + a.Flags.Var(&config.ValidateAsyncIntervals, "validate-async-intervals", "best effort validation of async intervals (timeout and polling); ignored the suppress-async-assertion flag is true") + a.Flags.Var(&config.SuppressTypeCompare, "suppress-type-compare-assertion", "Suppress warning for comparing values from different types, like int32 and uint32") + a.Flags.Var(&config.AllowHaveLen0, "allow-havelen-0", "Do not warn for HaveLen(0); default = false") + a.Flags.Var(&config.ForceExpectTo, "force-expect-to", "force using `Expect` with `To`, `ToNot` or `NotTo`. reject using `Expect` with `Should` or `ShouldNot`; default = false (not forced)") + a.Flags.BoolVar(&ignored, "suppress-focus-container", true, "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt. Deprecated and ignored: use --forbid-focus-container instead") + a.Flags.Var(&config.ForbidFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.") + a.Flags.Var(&config.ForbidSpecPollution, "forbid-spec-pollution", "trigger a warning for variable assignments in ginkgo containers like Describe, Context and When, instead of in BeforeEach(); default = false.") + + return a +} + +// Analyzer is the interface to go_vet +var Analyzer = NewAnalyzer() diff --git a/vendor/github.com/nunnatsa/ginkgolinter/doc.go b/vendor/github.com/nunnatsa/ginkgolinter/doc.go new file mode 100644 index 000000000..dd9ecf58a --- /dev/null +++ b/vendor/github.com/nunnatsa/ginkgolinter/doc.go @@ -0,0 +1,99 @@ +package ginkgolinter + +const doc = `enforces standards of using ginkgo and gomega + +or + ginkgolinter version + +version: %s + +currently, the linter searches for following: +* trigger a warning when using Eventually or Consistently with a function call. This is in order to prevent the case when + using a function call instead of a function. Function call returns a value only once, and so the original value + is tested again and again and is never changed. [Bug] + +* trigger a warning when comparing a pointer to a value. [Bug] + +* trigger a warning for missing assertion method: [Bug] + Eventually(checkSomething) + +* trigger a warning when a ginkgo focus container (FDescribe, FContext, FWhen or FIt) is found. [Bug] + +* validate the MatchError gomega matcher [Bug] + +* trigger a warning when using the Equal or the BeIdentical matcher with two different types, as these matchers will + fail in runtime. + +* async timing interval: timeout is shorter than polling interval [Bug] +For example: + Eventually(aFunc).WithTimeout(500 * time.Millisecond).WithPolling(10 * time.Second).Should(Succeed()) +This will probably happen when using the old format: + Eventually(aFunc, 500 * time.Millisecond, 10 * time.Second).Should(Succeed()) + +* reject variable assignments in ginkgo containers [Bug/Style]: +For example: + var _ = Describe("description", func(){ + var x = 10 + }) + +Should use BeforeEach instead; e.g. + var _ = Describe("description", func(){ + var x int + BeforeEach(func(){ + x = 10 + }) + }) + +* wrong length assertions. We want to assert the item rather than its length. [Style] +For example: + Expect(len(x)).Should(Equal(1)) +This should be replaced with: + Expect(x)).Should(HavelLen(1)) + +* wrong cap assertions. We want to assert the item rather than its cap. [Style] +For example: + Expect(cap(x)).Should(Equal(1)) +This should be replaced with: + Expect(x)).Should(HavelCap(1)) + +* wrong nil assertions. We want to assert the item rather than a comparison result. [Style] +For example: + Expect(x == nil).Should(BeTrue()) +This should be replaced with: + Expect(x).Should(BeNil()) + +* wrong error assertions. For example: [Style] + Expect(err == nil).Should(BeTrue()) +This should be replaced with: + Expect(err).ShouldNot(HaveOccurred()) + +* wrong boolean comparison, for example: [Style] + Expect(x == 8).Should(BeTrue()) +This should be replaced with: + Expect(x).Should(BeEqual(8)) + +* replaces Equal(true/false) with BeTrue()/BeFalse() [Style] + +* replaces HaveLen(0) with BeEmpty() [Style] + +* replaces Expect(...).Should(...) with Expect(...).To() [Style] + +* async timing interval: multiple timeout or polling interval [Style] +For example: + Eventually(context.Background(), func() bool { return true }, time.Second*10).WithTimeout(time.Second * 10).WithPolling(time.Millisecond * 500).Should(BeTrue()) + Eventually(context.Background(), func() bool { return true }, time.Second*10).Within(time.Second * 10).WithPolling(time.Millisecond * 500).Should(BeTrue()) + Eventually(func() bool { return true }, time.Second*10, 500*time.Millisecond).WithPolling(time.Millisecond * 500).Should(BeTrue()) + Eventually(func() bool { return true }, time.Second*10, 500*time.Millisecond).ProbeEvery(time.Millisecond * 500).Should(BeTrue()) + +* async timing interval: non-time.Duration intervals [Style] +gomega supports a few formats for timeout and polling intervals, when using the old format (the last two parameters of Eventually and Constantly): + * time.Duration + * any kind of numeric value, as number of seconds + * duration string like "12s" +The linter triggers a warning for any duration value that is not of the time.Duration type, assuming that this is +the desired type, given the type of the argument of the newer "WithTimeout", "WithPolling", "Within" and "ProbeEvery" +methods. +For example: + Eventually(context.Background(), func() bool { return true }, "1s").Should(BeTrue()) + Eventually(context.Background(), func() bool { return true }, time.Second*60, 15).Should(BeTrue()) +` diff --git a/vendor/github.com/nunnatsa/ginkgolinter/ginkgohandler/handler.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/ginkgohandler/handler.go index c0829c469..f10d83184 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/ginkgohandler/handler.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/ginkgohandler/handler.go @@ -15,6 +15,7 @@ const ( // in imported with "." name, custom name or without any name. type Handler interface { GetFocusContainerName(*ast.CallExpr) (bool, *ast.Ident) + IsWrapContainer(*ast.CallExpr) bool IsFocusSpec(ident ast.Expr) bool } @@ -49,6 +50,13 @@ func (h dotHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) return false, nil } +func (h dotHandler) IsWrapContainer(exp *ast.CallExpr) bool { + if fun, ok := exp.Fun.(*ast.Ident); ok { + return IsWrapContainer(fun.Name) + } + return false +} + func (h dotHandler) IsFocusSpec(exp ast.Expr) bool { id, ok := exp.(*ast.Ident) return ok && id.Name == focusSpec @@ -70,6 +78,16 @@ func (h nameHandler) GetFocusContainerName(exp *ast.CallExpr) (bool, *ast.Ident) return false, nil } +func (h nameHandler) IsWrapContainer(exp *ast.CallExpr) bool { + if sel, ok := exp.Fun.(*ast.SelectorExpr); ok { + if id, ok := sel.X.(*ast.Ident); ok && id.Name == string(h) { + return IsWrapContainer(sel.Sel.Name) + } + } + return false + +} + func (h nameHandler) IsFocusSpec(exp ast.Expr) bool { if selExp, ok := exp.(*ast.SelectorExpr); ok { if x, ok := selExp.X.(*ast.Ident); ok && x.Name == string(h) { @@ -88,10 +106,24 @@ func isFocusContainer(name string) bool { return false } -func IsContainer(id *ast.Ident) bool { - switch id.Name { - case "It", "When", "Context", "Describe", "DescribeTable", "Entry": +func IsContainer(name string) bool { + switch name { + case "It", "When", "Context", "Describe", "DescribeTable", "Entry", + "PIt", "PWhen", "PContext", "PDescribe", "PDescribeTable", "PEntry", + "XIt", "XWhen", "XContext", "XDescribe", "XDescribeTable", "XEntry": + return true + } + return isFocusContainer(name) +} + +func IsWrapContainer(name string) bool { + switch name { + case "When", "Context", "Describe", + "FWhen", "FContext", "FDescribe", + "PWhen", "PContext", "PDescribe", + "XWhen", "XContext", "XDescribe": return true } - return isFocusContainer(id.Name) + + return false } diff --git a/vendor/github.com/nunnatsa/ginkgolinter/gomegahandler/handler.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/gomegahandler/handler.go index 419145b75..4290e7373 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/gomegahandler/handler.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/gomegahandler/handler.go @@ -70,7 +70,12 @@ func (h dotHandler) GetActualFuncName(expr *ast.CallExpr) (string, bool) { // ReplaceFunction replaces the function with another one, for fix suggestions func (dotHandler) ReplaceFunction(caller *ast.CallExpr, newExpr *ast.Ident) { - caller.Fun = newExpr + switch f := caller.Fun.(type) { + case *ast.Ident: + caller.Fun = newExpr + case *ast.SelectorExpr: + f.Sel = newExpr + } } func (dotHandler) getDefFuncName(expr *ast.CallExpr) string { diff --git a/vendor/github.com/nunnatsa/ginkgolinter/interfaces/interfaces.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/interfaces/interfaces.go index dafeacd4f..dafeacd4f 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/interfaces/interfaces.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/interfaces/interfaces.go diff --git a/vendor/github.com/nunnatsa/ginkgolinter/internal/intervals/intervals.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/intervals/intervals.go new file mode 100644 index 000000000..b8166bdb2 --- /dev/null +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/intervals/intervals.go @@ -0,0 +1,285 @@ +package intervals + +import ( + "errors" + "go/ast" + "go/constant" + "go/token" + gotypes "go/types" + "strconv" + "time" + + "golang.org/x/tools/go/analysis" + + "github.com/nunnatsa/ginkgolinter/internal/gomegahandler" + "github.com/nunnatsa/ginkgolinter/internal/reports" +) + +type noDurationIntervalErr struct { + value string +} + +func (err noDurationIntervalErr) Error() string { + return "only use time.Duration for timeout and polling in Eventually() or Consistently()" +} + +func CheckIntervals(pass *analysis.Pass, expr *ast.CallExpr, actualExpr *ast.CallExpr, reportBuilder *reports.Builder, handler gomegahandler.Handler, timePkg string, funcIndex int) { + var ( + timeout time.Duration + polling time.Duration + err error + ) + + timeoutOffset := funcIndex + 1 + if len(actualExpr.Args) > timeoutOffset { + timeout, err = getDuration(pass, actualExpr.Args[timeoutOffset], timePkg) + if err != nil { + suggestFix := false + if tryFixIntDuration(expr, err, handler, timePkg, timeoutOffset) { + suggestFix = true + } + reportBuilder.AddIssue(suggestFix, err.Error()) + } + pollingOffset := funcIndex + 2 + if len(actualExpr.Args) > pollingOffset { + polling, err = getDuration(pass, actualExpr.Args[pollingOffset], timePkg) + if err != nil { + suggestFix := false + if tryFixIntDuration(expr, err, handler, timePkg, pollingOffset) { + suggestFix = true + } + reportBuilder.AddIssue(suggestFix, err.Error()) + } + } + } + + selExp := expr.Fun.(*ast.SelectorExpr) + for { + call, ok := selExp.X.(*ast.CallExpr) + if !ok { + break + } + + fun, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + break + } + + switch fun.Sel.Name { + case "WithTimeout", "Within": + if timeout != 0 { + reportBuilder.AddIssue(false, "timeout defined more than once") + } else if len(call.Args) == 1 { + timeout, err = getDurationFromValue(pass, call.Args[0], timePkg) + if err != nil { + reportBuilder.AddIssue(false, err.Error()) + } + } + + case "WithPolling", "ProbeEvery": + if polling != 0 { + reportBuilder.AddIssue(false, "polling defined more than once") + } else if len(call.Args) == 1 { + polling, err = getDurationFromValue(pass, call.Args[0], timePkg) + if err != nil { + reportBuilder.AddIssue(false, err.Error()) + } + } + } + + selExp = fun + } + + if timeout != 0 && polling != 0 && timeout < polling { + reportBuilder.AddIssue(false, "timeout must not be shorter than the polling interval") + } +} + +func tryFixIntDuration(expr *ast.CallExpr, err error, handler gomegahandler.Handler, timePkg string, offset int) bool { + suggestFix := false + var durErr noDurationIntervalErr + if errors.As(err, &durErr) { + if len(durErr.value) > 0 { + actualExpr := handler.GetActualExpr(expr.Fun.(*ast.SelectorExpr)) + var newArg ast.Expr + second := &ast.SelectorExpr{ + Sel: ast.NewIdent("Second"), + X: ast.NewIdent(timePkg), + } + if durErr.value == "1" { + newArg = second + } else { + newArg = &ast.BinaryExpr{ + X: second, + Op: token.MUL, + Y: actualExpr.Args[offset], + } + } + actualExpr.Args[offset] = newArg + suggestFix = true + } + } + + return suggestFix +} + +func getDuration(pass *analysis.Pass, interval ast.Expr, timePkg string) (time.Duration, error) { + argType := pass.TypesInfo.TypeOf(interval) + if durType, ok := argType.(*gotypes.Named); ok { + if durType.Obj().Name() == "Duration" && durType.Obj().Pkg().Name() == "time" { + return getDurationFromValue(pass, interval, timePkg) + } + } + + value := "" + switch val := interval.(type) { + case *ast.BasicLit: + if val.Kind == token.INT { + value = val.Value + } + case *ast.Ident: + i, err := getConstDuration(pass, val, timePkg) + if err != nil || i == 0 { + return 0, nil + } + value = val.Name + } + + return 0, noDurationIntervalErr{value: value} +} + +func getDurationFromValue(pass *analysis.Pass, interval ast.Expr, timePkg string) (time.Duration, error) { + switch dur := interval.(type) { + case *ast.SelectorExpr: + ident, ok := dur.X.(*ast.Ident) + if ok { + if ident.Name == timePkg { + return getTimeDurationValue(dur) + } + return getDurationFromValue(pass, dur.Sel, timePkg) + } + case *ast.BinaryExpr: + return getBinaryExprDuration(pass, dur, timePkg) + + case *ast.Ident: + return getConstDuration(pass, dur, timePkg) + } + + return 0, nil +} + +func getConstDuration(pass *analysis.Pass, ident *ast.Ident, timePkg string) (time.Duration, error) { + o := pass.TypesInfo.ObjectOf(ident) + if o != nil { + if c, ok := o.(*gotypes.Const); ok { + if c.Val().Kind() == constant.Int { + i, err := strconv.Atoi(c.Val().String()) + if err != nil { + return 0, nil + } + return time.Duration(i), nil + } + } + } + + if ident.Obj != nil && ident.Obj.Kind == ast.Con && ident.Obj.Decl != nil { + if vals, ok := ident.Obj.Decl.(*ast.ValueSpec); ok { + if len(vals.Values) == 1 { + switch val := vals.Values[0].(type) { + case *ast.BasicLit: + if val.Kind == token.INT { + i, err := strconv.Atoi(val.Value) + if err != nil { + return 0, nil + } + return time.Duration(i), nil + } + return 0, nil + case *ast.BinaryExpr: + return getBinaryExprDuration(pass, val, timePkg) + } + } + } + } + + return 0, nil +} + +func getTimeDurationValue(dur *ast.SelectorExpr) (time.Duration, error) { + switch dur.Sel.Name { + case "Nanosecond": + return time.Nanosecond, nil + case "Microsecond": + return time.Microsecond, nil + case "Millisecond": + return time.Millisecond, nil + case "Second": + return time.Second, nil + case "Minute": + return time.Minute, nil + case "Hour": + return time.Hour, nil + default: + return 0, errors.New("unknown duration value") // should never happen + } +} + +func getBinaryExprDuration(pass *analysis.Pass, expr *ast.BinaryExpr, timePkg string) (time.Duration, error) { + x, err := getBinaryDurValue(pass, expr.X, timePkg) + if err != nil || x == 0 { + return 0, nil + } + y, err := getBinaryDurValue(pass, expr.Y, timePkg) + if err != nil || y == 0 { + return 0, nil + } + + switch expr.Op { + case token.ADD: + return x + y, nil + case token.SUB: + val := x - y + if val > 0 { + return val, nil + } + return 0, nil + case token.MUL: + return x * y, nil + case token.QUO: + if y == 0 { + return 0, nil + } + return x / y, nil + case token.REM: + if y == 0 { + return 0, nil + } + return x % y, nil + default: + return 0, nil + } +} + +func getBinaryDurValue(pass *analysis.Pass, expr ast.Expr, timePkg string) (time.Duration, error) { + switch x := expr.(type) { + case *ast.SelectorExpr: + return getDurationFromValue(pass, x, timePkg) + case *ast.BinaryExpr: + return getBinaryExprDuration(pass, x, timePkg) + case *ast.BasicLit: + if x.Kind == token.INT { + val, err := strconv.Atoi(x.Value) + if err != nil { + return 0, err + } + return time.Duration(val), nil + } + case *ast.ParenExpr: + return getBinaryDurValue(pass, x.X, timePkg) + + case *ast.Ident: + return getConstDuration(pass, x, timePkg) + } + + return 0, nil +} diff --git a/vendor/github.com/nunnatsa/ginkgolinter/internal/reports/report-builder.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/reports/report-builder.go new file mode 100644 index 000000000..c7f931ca7 --- /dev/null +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/reports/report-builder.go @@ -0,0 +1,98 @@ +package reports + +import ( + "bytes" + "fmt" + "go/ast" + "go/printer" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" +) + +type Builder struct { + pos token.Pos + end token.Pos + oldExpr string + issues []string + fixOffer string + suggestFix bool +} + +func NewBuilder(fset *token.FileSet, oldExpr ast.Expr) *Builder { + b := &Builder{ + pos: oldExpr.Pos(), + end: oldExpr.End(), + oldExpr: goFmt(fset, oldExpr), + suggestFix: false, + } + + return b +} + +func (b *Builder) AddIssue(suggestFix bool, issue string, args ...any) { + if len(args) > 0 { + issue = fmt.Sprintf(issue, args...) + } + b.issues = append(b.issues, issue) + + if suggestFix { + b.suggestFix = true + } +} + +func (b *Builder) SetFixOffer(fset *token.FileSet, fixOffer ast.Expr) { + if offer := goFmt(fset, fixOffer); offer != b.oldExpr { + b.fixOffer = offer + } +} + +func (b *Builder) HasReport() bool { + return len(b.issues) > 0 +} + +func (b *Builder) Build() analysis.Diagnostic { + diagnostic := analysis.Diagnostic{ + Pos: b.pos, + Message: b.getMessage(), + } + + if b.suggestFix && len(b.fixOffer) > 0 { + diagnostic.SuggestedFixes = []analysis.SuggestedFix{ + { + Message: fmt.Sprintf("should replace %s with %s", b.oldExpr, b.fixOffer), + TextEdits: []analysis.TextEdit{ + { + Pos: b.pos, + End: b.end, + NewText: []byte(b.fixOffer), + }, + }, + }, + } + } + + return diagnostic +} + +func goFmt(fset *token.FileSet, x ast.Expr) string { + var b bytes.Buffer + _ = printer.Fprint(&b, fset, x) + return b.String() +} + +func (b *Builder) getMessage() string { + sb := strings.Builder{} + sb.WriteString("ginkgo-linter: ") + if len(b.issues) > 1 { + sb.WriteString("multiple issues: ") + } + sb.WriteString(strings.Join(b.issues, "; ")) + + if b.suggestFix && len(b.fixOffer) != 0 { + sb.WriteString(fmt.Sprintf(". Consider using `%s` instead", b.fixOffer)) + } + + return sb.String() +} diff --git a/vendor/github.com/nunnatsa/ginkgolinter/reverseassertion/reverse_assertion.go b/vendor/github.com/nunnatsa/ginkgolinter/internal/reverseassertion/reverse_assertion.go index 1dbd89810..1dbd89810 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/reverseassertion/reverse_assertion.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/internal/reverseassertion/reverse_assertion.go diff --git a/vendor/github.com/nunnatsa/ginkgolinter/ginkgo_linter.go b/vendor/github.com/nunnatsa/ginkgolinter/linter/ginkgo_linter.go index a0aff1612..574fdfadf 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/ginkgo_linter.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/linter/ginkgo_linter.go @@ -1,8 +1,7 @@ -package ginkgolinter +package linter import ( "bytes" - "flag" "fmt" "go/ast" "go/constant" @@ -14,12 +13,13 @@ import ( "github.com/go-toolsmith/astcopy" "golang.org/x/tools/go/analysis" - "github.com/nunnatsa/ginkgolinter/ginkgohandler" - "github.com/nunnatsa/ginkgolinter/gomegahandler" - "github.com/nunnatsa/ginkgolinter/interfaces" - "github.com/nunnatsa/ginkgolinter/reverseassertion" + "github.com/nunnatsa/ginkgolinter/internal/ginkgohandler" + "github.com/nunnatsa/ginkgolinter/internal/gomegahandler" + "github.com/nunnatsa/ginkgolinter/internal/interfaces" + "github.com/nunnatsa/ginkgolinter/internal/intervals" + "github.com/nunnatsa/ginkgolinter/internal/reports" + "github.com/nunnatsa/ginkgolinter/internal/reverseassertion" "github.com/nunnatsa/ginkgolinter/types" - "github.com/nunnatsa/ginkgolinter/version" ) // The ginkgolinter enforces standards of using ginkgo and gomega. @@ -28,24 +28,26 @@ import ( const ( linterName = "ginkgo-linter" - wrongLengthWarningTemplate = linterName + ": wrong length assertion; consider using `%s` instead" - wrongNilWarningTemplate = linterName + ": wrong nil assertion; consider using `%s` instead" - wrongBoolWarningTemplate = linterName + ": wrong boolean assertion; consider using `%s` instead" - wrongErrWarningTemplate = linterName + ": wrong error assertion; consider using `%s` instead" - wrongCompareWarningTemplate = linterName + ": wrong comparison assertion; consider using `%s` instead" - doubleNegativeWarningTemplate = linterName + ": avoid double negative assertion; consider using `%s` instead" - valueInEventually = linterName + ": use a function call in %s. This actually checks nothing, because %s receives the function returned value, instead of function itself, and this value is never changed" - comparePointerToValue = linterName + ": comparing a pointer to a value will always fail. consider using `%s` instead" - missingAssertionMessage = linterName + `: %q: missing assertion method. Expected "Should()", "To()", "ShouldNot()", "ToNot()" or "NotTo()"` - missingAsyncAssertionMessage = linterName + `: %q: missing assertion method. Expected "Should()" or "ShouldNot()"` - focusContainerFound = linterName + ": Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with %q" - focusSpecFound = linterName + ": Focus spec found. This is used only for local debug and should not be part of the actual source code, consider to remove it" - compareDifferentTypes = linterName + ": use %[1]s with different types: Comparing %[2]s with %[3]s; either change the expected value type if possible, or use the BeEquivalentTo() matcher, instead of %[1]s()" - matchErrorArgWrongType = linterName + ": the MatchError matcher used to assert a non error type (%s)" - matchErrorWrongTypeAssertion = linterName + ": MatchError first parameter (%s) must be error, string, GomegaMatcher or func(error)bool are allowed" - matchErrorMissingDescription = linterName + ": missing function description as second parameter of MatchError" - matchErrorRedundantArg = linterName + ": redundant MatchError arguments; consider removing them; consider using `%s` instead" - matchErrorNoFuncDescription = linterName + ": The second parameter of MatchError must be the function description (string)" + wrongLengthWarningTemplate = "wrong length assertion" + wrongCapWarningTemplate = "wrong cap assertion" + wrongNilWarningTemplate = "wrong nil assertion" + wrongBoolWarningTemplate = "wrong boolean assertion" + wrongErrWarningTemplate = "wrong error assertion" + wrongCompareWarningTemplate = "wrong comparison assertion" + doubleNegativeWarningTemplate = "avoid double negative assertion" + valueInEventually = "use a function call in %s. This actually checks nothing, because %s receives the function returned value, instead of function itself, and this value is never changed" + comparePointerToValue = "comparing a pointer to a value will always fail" + missingAssertionMessage = linterName + `: %q: missing assertion method. Expected %s` + focusContainerFound = linterName + ": Focus container found. This is used only for local debug and should not be part of the actual source code. Consider to replace with %q" + focusSpecFound = linterName + ": Focus spec found. This is used only for local debug and should not be part of the actual source code. Consider to remove it" + compareDifferentTypes = "use %[1]s with different types: Comparing %[2]s with %[3]s; either change the expected value type if possible, or use the BeEquivalentTo() matcher, instead of %[1]s()" + matchErrorArgWrongType = "the MatchError matcher used to assert a non error type (%s)" + matchErrorWrongTypeAssertion = "MatchError first parameter (%s) must be error, string, GomegaMatcher or func(error)bool are allowed" + matchErrorMissingDescription = "missing function description as second parameter of MatchError" + matchErrorRedundantArg = "redundant MatchError arguments; consider removing them" + matchErrorNoFuncDescription = "The second parameter of MatchError must be the function description (string)" + forceExpectToTemplate = "must not use Expect with %s" + useBeforeEachTemplate = "use BeforeEach() to assign variable %s" ) const ( // gomega matchers @@ -59,6 +61,7 @@ const ( // gomega matchers beZero = "BeZero" equal = "Equal" haveLen = "HaveLen" + haveCap = "HaveCap" haveOccurred = "HaveOccurred" haveValue = "HaveValue" not = "Not" @@ -79,101 +82,19 @@ const ( // gomega actuals consistentlyWithOffset = "ConsistentlyWithOffset" ) -// Analyzer is the interface to go_vet -var Analyzer = NewAnalyzer() - -type ginkgoLinter struct { +type GinkgoLinter struct { config *types.Config } -// NewAnalyzer returns an Analyzer - the package interface with nogo -func NewAnalyzer() *analysis.Analyzer { - linter := ginkgoLinter{ - config: &types.Config{ - SuppressLen: false, - SuppressNil: false, - SuppressErr: false, - SuppressCompare: false, - ForbidFocus: false, - AllowHaveLen0: false, - }, - } - - a := &analysis.Analyzer{ - Name: "ginkgolinter", - Doc: fmt.Sprintf(doc, version.Version()), - Run: linter.run, +// NewGinkgoLinter return new ginkgolinter object +func NewGinkgoLinter(config *types.Config) *GinkgoLinter { + return &GinkgoLinter{ + config: config, } - - var ignored bool - a.Flags.Init("ginkgolinter", flag.ExitOnError) - a.Flags.Var(&linter.config.SuppressLen, "suppress-len-assertion", "Suppress warning for wrong length assertions") - a.Flags.Var(&linter.config.SuppressNil, "suppress-nil-assertion", "Suppress warning for wrong nil assertions") - a.Flags.Var(&linter.config.SuppressErr, "suppress-err-assertion", "Suppress warning for wrong error assertions") - a.Flags.Var(&linter.config.SuppressCompare, "suppress-compare-assertion", "Suppress warning for wrong comparison assertions") - a.Flags.Var(&linter.config.SuppressAsync, "suppress-async-assertion", "Suppress warning for function call in async assertion, like Eventually") - a.Flags.Var(&linter.config.SuppressTypeCompare, "suppress-type-compare-assertion", "Suppress warning for comparing values from different types, like int32 and uint32") - a.Flags.Var(&linter.config.AllowHaveLen0, "allow-havelen-0", "Do not warn for HaveLen(0); default = false") - - a.Flags.BoolVar(&ignored, "suppress-focus-container", true, "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt. Deprecated and ignored: use --forbid-focus-container instead") - a.Flags.Var(&linter.config.ForbidFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.") - - return a } -const doc = `enforces standards of using ginkgo and gomega - -or - ginkgolinter version - -version: %s - -currently, the linter searches for following: -* trigger a warning when using Eventually or Constantly with a function call. This is in order to prevent the case when - using a function call instead of a function. Function call returns a value only once, and so the original value - is tested again and again and is never changed. [Bug] - -* trigger a warning when comparing a pointer to a value. [Bug] - -* trigger a warning for missing assertion method: [Bug] - Eventually(checkSomething) - -* trigger a warning when a ginkgo focus container (FDescribe, FContext, FWhen or FIt) is found. [Bug] - -* validate the MatchError gomega matcher [Bug] - -* trigger a warning when using the Equal or the BeIdentical matcher with two different types, as these matchers will - fail in runtime. - -* wrong length assertions. We want to assert the item rather than its length. [Style] -For example: - Expect(len(x)).Should(Equal(1)) -This should be replaced with: - Expect(x)).Should(HavelLen(1)) - -* wrong nil assertions. We want to assert the item rather than a comparison result. [Style] -For example: - Expect(x == nil).Should(BeTrue()) -This should be replaced with: - Expect(x).Should(BeNil()) - -* wrong error assertions. For example: [Style] - Expect(err == nil).Should(BeTrue()) -This should be replaced with: - Expect(err).ShouldNot(HaveOccurred()) - -* wrong boolean comparison, for example: [Style] - Expect(x == 8).Should(BeTrue()) -This should be replaced with: - Expect(x).Should(BeEqual(8)) - -* replaces Equal(true/false) with BeTrue()/BeFalse() [Style] - -* replaces HaveLen(0) with BeEmpty() [Style] -` - -// main assertion function -func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) { +// Run is the main assertion function +func (l *GinkgoLinter) Run(pass *analysis.Pass) (interface{}, error) { for _, file := range pass.Files { fileConfig := l.config.Clone() @@ -188,20 +109,37 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) { continue } - //gomegaMatcher = getMatcherInterface(pass, file) + timePks := "" + for _, imp := range file.Imports { + if imp.Path.Value == `"time"` { + if imp.Name == nil { + timePks = "time" + } else { + timePks = imp.Name.Name + } + } + } ast.Inspect(file, func(n ast.Node) bool { - if ginkgoHndlr != nil && fileConfig.ForbidFocus { + if ginkgoHndlr != nil { + goDeeper := false spec, ok := n.(*ast.ValueSpec) if ok { for _, val := range spec.Values { if exp, ok := val.(*ast.CallExpr); ok { - if checkFocusContainer(pass, ginkgoHndlr, exp) { - return true + if bool(fileConfig.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, exp) { + goDeeper = true + } + + if bool(fileConfig.ForbidSpecPollution) && checkAssignmentsInContainer(pass, ginkgoHndlr, exp) { + goDeeper = true } } } } + if goDeeper { + return true + } } stmt, ok := n.(*ast.ExprStmt) @@ -221,8 +159,17 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) { return true } - if ginkgoHndlr != nil && bool(config.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, assertionExp) { - return true + if ginkgoHndlr != nil { + goDeeper := false + if bool(config.ForbidFocus) && checkFocusContainer(pass, ginkgoHndlr, assertionExp) { + goDeeper = true + } + if bool(config.ForbidSpecPollution) && checkAssignmentsInContainer(pass, ginkgoHndlr, assertionExp) { + goDeeper = true + } + if goDeeper { + return true + } } // no more ginkgo checks. From here it's only gomega. So if there is no gomega handler, exit here. This is @@ -247,12 +194,89 @@ func (l *ginkgoLinter) run(pass *analysis.Pass) (interface{}, error) { return true } - return checkExpression(pass, config, assertionExp, actualExpr, gomegaHndlr) + return checkExpression(pass, config, assertionExp, actualExpr, gomegaHndlr, timePks) }) } return nil, nil } +func checkAssignmentsInContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, exp *ast.CallExpr) bool { + foundSomething := false + if ginkgoHndlr.IsWrapContainer(exp) { + for _, arg := range exp.Args { + if fn, ok := arg.(*ast.FuncLit); ok { + if fn.Body != nil { + if checkAssignments(pass, fn.Body.List) { + foundSomething = true + } + break + } + } + } + } + + return foundSomething +} + +func checkAssignments(pass *analysis.Pass, list []ast.Stmt) bool { + foundSomething := false + for _, stmt := range list { + switch st := stmt.(type) { + case *ast.DeclStmt: + if gen, ok := st.Decl.(*ast.GenDecl); ok { + if gen.Tok != token.VAR { + continue + } + for _, spec := range gen.Specs { + if valSpec, ok := spec.(*ast.ValueSpec); ok { + if checkAssignmentsValues(pass, valSpec.Names, valSpec.Values) { + foundSomething = true + } + } + } + } + + case *ast.AssignStmt: + for i, val := range st.Rhs { + if !is[*ast.FuncLit](val) { + if id, isIdent := st.Lhs[i].(*ast.Ident); isIdent && id.Name != "_" { + reportNoFix(pass, id.Pos(), useBeforeEachTemplate, id.Name) + foundSomething = true + } + } + } + + case *ast.IfStmt: + if st.Body != nil { + if checkAssignments(pass, st.Body.List) { + foundSomething = true + } + } + if st.Else != nil { + if block, isBlock := st.Else.(*ast.BlockStmt); isBlock { + if checkAssignments(pass, block.List) { + foundSomething = true + } + } + } + } + } + + return foundSomething +} + +func checkAssignmentsValues(pass *analysis.Pass, names []*ast.Ident, values []ast.Expr) bool { + foundSomething := false + for i, val := range values { + if !is[*ast.FuncLit](val) { + reportNoFix(pass, names[i].Pos(), useBeforeEachTemplate, names[i].Name) + foundSomething = true + } + } + + return foundSomething +} + func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, exp *ast.CallExpr) bool { foundFocus := false isFocus, id := ginkgoHndlr.GetFocusContainerName(exp) @@ -261,7 +285,7 @@ func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, foundFocus = true } - if id != nil && ginkgohandler.IsContainer(id) { + if id != nil && ginkgohandler.IsContainer(id.Name) { for _, arg := range exp.Args { if ginkgoHndlr.IsFocusSpec(arg) { reportNoFix(pass, arg.Pos(), focusSpecFound) @@ -277,21 +301,69 @@ func checkFocusContainer(pass *analysis.Pass, ginkgoHndlr ginkgohandler.Handler, return foundFocus } -func checkExpression(pass *analysis.Pass, config types.Config, assertionExp *ast.CallExpr, actualExpr *ast.CallExpr, handler gomegahandler.Handler) bool { +func checkExpression(pass *analysis.Pass, config types.Config, assertionExp *ast.CallExpr, actualExpr *ast.CallExpr, handler gomegahandler.Handler, timePkg string) bool { expr := astcopy.CallExpr(assertionExp) - oldExpr := goFmt(pass.Fset, expr) - if checkAsyncAssertion(pass, config, expr, actualExpr, handler, oldExpr) { - return true + reportBuilder := reports.NewBuilder(pass.Fset, expr) + + goNested := false + if checkAsyncAssertion(pass, config, expr, actualExpr, handler, reportBuilder, timePkg) { + goNested = true + } else { + + actualArg := getActualArg(actualExpr, handler) + if actualArg == nil { + return true + } + + if config.ForceExpectTo { + goNested = forceExpectTo(expr, handler, reportBuilder) || goNested + } + + goNested = doCheckExpression(pass, config, assertionExp, actualArg, expr, handler, reportBuilder) || goNested } - actualArg := getActualArg(actualExpr, handler) - if actualArg == nil { - return true + if reportBuilder.HasReport() { + reportBuilder.SetFixOffer(pass.Fset, expr) + pass.Report(reportBuilder.Build()) + } + + return goNested +} + +func forceExpectTo(expr *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { + if asrtFun, ok := expr.Fun.(*ast.SelectorExpr); ok { + if actualFuncName, ok := handler.GetActualFuncName(expr); ok && actualFuncName == expect { + var ( + name string + newIdent *ast.Ident + ) + + switch name = asrtFun.Sel.Name; name { + case "Should": + newIdent = ast.NewIdent("To") + case "ShouldNot": + newIdent = ast.NewIdent("ToNot") + default: + return false + } + + handler.ReplaceFunction(expr, newIdent) + reportBuilder.AddIssue(true, fmt.Sprintf(forceExpectToTemplate, name)) + return true + } } + return false +} + +func doCheckExpression(pass *analysis.Pass, config types.Config, assertionExp *ast.CallExpr, actualArg ast.Expr, expr *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { if !bool(config.SuppressLen) && isActualIsLenFunc(actualArg) { - return checkLengthMatcher(expr, pass, handler, oldExpr) + return checkLengthMatcher(expr, pass, handler, reportBuilder) + + } else if !bool(config.SuppressLen) && isActualIsCapFunc(actualArg) { + return checkCapMatcher(expr, handler, reportBuilder) + } else if nilable, compOp := getNilableFromComparison(actualArg); nilable != nil { if isExprError(pass, nilable) { if config.SuppressErr { @@ -301,46 +373,53 @@ func checkExpression(pass *analysis.Pass, config types.Config, assertionExp *ast return true } - return checkNilMatcher(expr, pass, nilable, handler, compOp == token.NEQ, oldExpr) + return checkNilMatcher(expr, pass, nilable, handler, compOp == token.NEQ, reportBuilder) } else if first, second, op, ok := isComparison(pass, actualArg); ok { matcher, shouldContinue := startCheckComparison(expr, handler) if !shouldContinue { return false } - if !bool(config.SuppressLen) && isActualIsLenFunc(first) { - if handleLenComparison(pass, expr, matcher, first, second, op, handler, oldExpr) { - return false + if !config.SuppressLen { + if isActualIsLenFunc(first) { + if handleLenComparison(pass, expr, matcher, first, second, op, handler, reportBuilder) { + return false + } + } + if isActualIsCapFunc(first) { + if handleCapComparison(expr, matcher, first, second, op, handler, reportBuilder) { + return false + } } } - return bool(config.SuppressCompare) || checkComparison(expr, pass, matcher, handler, first, second, op, oldExpr) + return bool(config.SuppressCompare) || checkComparison(expr, pass, matcher, handler, first, second, op, reportBuilder) - } else if checkMatchError(pass, assertionExp, actualArg, handler, oldExpr) { + } else if checkMatchError(pass, assertionExp, actualArg, handler, reportBuilder) { return false } else if isExprError(pass, actualArg) { - return bool(config.SuppressErr) || checkNilError(pass, expr, handler, actualArg, oldExpr) + return bool(config.SuppressErr) || checkNilError(pass, expr, handler, actualArg, reportBuilder) - } else if checkPointerComparison(pass, config, assertionExp, expr, actualArg, handler, oldExpr) { + } else if checkPointerComparison(pass, config, assertionExp, expr, actualArg, handler, reportBuilder) { return false - } else if !handleAssertionOnly(pass, config, expr, handler, actualArg, oldExpr, true) { + } else if !handleAssertionOnly(pass, config, expr, handler, actualArg, reportBuilder) { return false } else if !config.SuppressTypeCompare { - return !checkEqualWrongType(pass, assertionExp, actualArg, handler, oldExpr) + return !checkEqualWrongType(pass, assertionExp, actualArg, handler, reportBuilder) } return true } -func checkMatchError(pass *analysis.Pass, origExp *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, oldExpr string) bool { +func checkMatchError(pass *analysis.Pass, origExp *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { matcher, ok := origExp.Args[0].(*ast.CallExpr) if !ok { return false } - return doCheckMatchError(pass, origExp, matcher, actualArg, handler, oldExpr) + return doCheckMatchError(pass, origExp, matcher, actualArg, handler, reportBuilder) } -func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, oldExpr string) bool { +func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { name, ok := handler.GetActualFuncName(matcher) if !ok { return false @@ -353,13 +432,13 @@ func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast. return false } - return doCheckMatchError(pass, origExp, nested, actualArg, handler, oldExpr) + return doCheckMatchError(pass, origExp, nested, actualArg, handler, reportBuilder) case and, or: - res := true + res := false for _, arg := range matcher.Args { if nested, ok := arg.(*ast.CallExpr); ok { - if !doCheckMatchError(pass, origExp, nested, actualArg, handler, oldExpr) { - res = false + if valid := doCheckMatchError(pass, origExp, nested, actualArg, handler, reportBuilder); valid { + res = true } } } @@ -369,15 +448,14 @@ func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast. } if !isExprError(pass, actualArg) { - reportNoFix(pass, origExp.Pos(), matchErrorArgWrongType, goFmt(pass.Fset, actualArg)) + reportBuilder.AddIssue(false, matchErrorArgWrongType, goFmt(pass.Fset, actualArg)) } expr := astcopy.CallExpr(matcher) validAssertion, requiredParams := checkMatchErrorAssertion(pass, matcher) if !validAssertion { - reportNoFix(pass, expr.Pos(), matchErrorWrongTypeAssertion, goFmt(pass.Fset, matcher.Args[0])) - return false + reportBuilder.AddIssue(false, matchErrorWrongTypeAssertion, goFmt(pass.Fset, matcher.Args[0])) } numParams := len(matcher.Args) @@ -385,16 +463,16 @@ func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast. if numParams == 2 { t := pass.TypesInfo.TypeOf(matcher.Args[1]) if !gotypes.Identical(t, gotypes.Typ[gotypes.String]) { - pass.Reportf(expr.Pos(), matchErrorNoFuncDescription) - return false + reportBuilder.AddIssue(false, matchErrorNoFuncDescription) + return true } } return true } if requiredParams == 2 && numParams == 1 { - reportNoFix(pass, expr.Pos(), matchErrorMissingDescription) - return false + reportBuilder.AddIssue(false, matchErrorMissingDescription) + return true } var newArgsSuggestion = []ast.Expr{expr.Args[0]} @@ -402,8 +480,9 @@ func doCheckMatchError(pass *analysis.Pass, origExp *ast.CallExpr, matcher *ast. newArgsSuggestion = append(newArgsSuggestion, expr.Args[1]) } expr.Args = newArgsSuggestion - report(pass, expr, matchErrorRedundantArg, oldExpr) - return false + + reportBuilder.AddIssue(true, matchErrorRedundantArg) + return true } func checkMatchErrorAssertion(pass *analysis.Pass, matcher *ast.CallExpr) (bool, int) { @@ -455,16 +534,16 @@ func isErrorMatcherValidArg(pass *analysis.Pass, arg ast.Expr) bool { return interfaces.ImplementsGomegaMatcher(t) } -func checkEqualWrongType(pass *analysis.Pass, origExp *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, old string) bool { +func checkEqualWrongType(pass *analysis.Pass, origExp *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { matcher, ok := origExp.Args[0].(*ast.CallExpr) if !ok { return false } - return checkEqualDifferentTypes(pass, matcher, actualArg, handler, old, false) + return checkEqualDifferentTypes(pass, matcher, actualArg, handler, false, reportBuilder) } -func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, old string, parentPointer bool) bool { +func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, parentPointer bool, reportBuilder *reports.Builder) bool { matcherFuncName, ok := handler.GetActualFuncName(matcher) if !ok { return false @@ -481,7 +560,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual if !ok { continue } - if checkEqualDifferentTypes(pass, nested, actualArg, handler, old, parentPointer) { + if checkEqualDifferentTypes(pass, nested, actualArg, handler, parentPointer, reportBuilder) { foundIssue = true } } @@ -497,7 +576,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual switch matcherFuncName { case equal, beIdenticalTo: case not: - return checkEqualDifferentTypes(pass, nested, actualArg, handler, old, parentPointer) + return checkEqualDifferentTypes(pass, nested, actualArg, handler, parentPointer, reportBuilder) default: return false } @@ -510,7 +589,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual return false } } else { - return checkEqualDifferentTypes(pass, nested, actualArg, handler, old, parentPointer) + return checkEqualDifferentTypes(pass, nested, actualArg, handler, parentPointer, reportBuilder) } case not: @@ -519,7 +598,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual return false } - return checkEqualDifferentTypes(pass, nested, actualArg, handler, old, parentPointer) + return checkEqualDifferentTypes(pass, nested, actualArg, handler, parentPointer, reportBuilder) case haveValue: nested, ok := matcher.Args[0].(*ast.CallExpr) @@ -527,7 +606,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual return false } - return checkEqualDifferentTypes(pass, nested, actualArg, handler, old, true) + return checkEqualDifferentTypes(pass, nested, actualArg, handler, true, reportBuilder) default: return false } @@ -551,7 +630,7 @@ func checkEqualDifferentTypes(pass *analysis.Pass, matcher *ast.CallExpr, actual return false } - reportNoFix(pass, matcher.Pos(), compareDifferentTypes, matcherFuncName, actualType, matcherType) + reportBuilder.AddIssue(false, compareDifferentTypes, matcherFuncName, actualType, matcherType) return true } @@ -596,7 +675,7 @@ func isImplementing(ifs, impl gotypes.Type) bool { } // be careful - never change origExp!!! only modify its clone, expr!!! -func checkPointerComparison(pass *analysis.Pass, config types.Config, origExp *ast.CallExpr, expr *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, oldExpr string) bool { +func checkPointerComparison(pass *analysis.Pass, config types.Config, origExp *ast.CallExpr, expr *ast.CallExpr, actualArg ast.Expr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { if !isPointer(pass, actualArg) { return false } @@ -643,20 +722,19 @@ func checkPointerComparison(pass *analysis.Pass, config types.Config, origExp *a return false } - handleAssertionOnly(pass, config, expr, handler, actualArg, oldExpr, false) + handleAssertionOnly(pass, config, expr, handler, actualArg, reportBuilder) args := []ast.Expr{astcopy.CallExpr(expr.Args[0].(*ast.CallExpr))} handler.ReplaceFunction(expr.Args[0].(*ast.CallExpr), ast.NewIdent(haveValue)) expr.Args[0].(*ast.CallExpr).Args = args - report(pass, expr, comparePointerToValue, oldExpr) + reportBuilder.AddIssue(true, comparePointerToValue) return true - } // check async assertion does not assert function call. This is a real bug in the test. In this case, the assertion is // done on the returned value, instead of polling the result of a function, for instance. -func checkAsyncAssertion(pass *analysis.Pass, config types.Config, expr *ast.CallExpr, actualExpr *ast.CallExpr, handler gomegahandler.Handler, oldExpr string) bool { +func checkAsyncAssertion(pass *analysis.Pass, config types.Config, expr *ast.CallExpr, actualExpr *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder, timePkg string) bool { funcName, ok := handler.GetActualFuncName(actualExpr) if !ok { return false @@ -702,19 +780,22 @@ func checkAsyncAssertion(pass *analysis.Pass, config types.Config, expr *ast.Cal actualExpr.Fun = call actualExpr.Args = fun.Args + actualExpr = actualExpr.Fun.(*ast.SelectorExpr).X.(*ast.CallExpr) } else { actualExpr.Args[funcIndex] = fun.Fun } - handleAssertionOnly(pass, config, expr, handler, actualExpr, oldExpr, false) - report(pass, expr, fmt.Sprintf(valueInEventually, funcName, funcName)+"; consider using `%s` instead", oldExpr) - return true + reportBuilder.AddIssue(true, valueInEventually, funcName, funcName) } } } + + if config.ValidateAsyncIntervals { + intervals.CheckIntervals(pass, expr, actualExpr, reportBuilder, handler, timePkg, funcIndex) + } } - handleAssertionOnly(pass, config, expr, handler, actualExpr, oldExpr, true) + handleAssertionOnly(pass, config, expr, handler, actualExpr, reportBuilder) return true } @@ -769,7 +850,7 @@ func startCheckComparison(exp *ast.CallExpr, handler gomegahandler.Handler) (*as return matcher, true } -func checkComparison(exp *ast.CallExpr, pass *analysis.Pass, matcher *ast.CallExpr, handler gomegahandler.Handler, first ast.Expr, second ast.Expr, op token.Token, oldExp string) bool { +func checkComparison(exp *ast.CallExpr, pass *analysis.Pass, matcher *ast.CallExpr, handler gomegahandler.Handler, first ast.Expr, second ast.Expr, op token.Token, reportBuilder *reports.Builder) bool { fun, ok := exp.Fun.(*ast.SelectorExpr) if !ok { return true @@ -801,7 +882,7 @@ func checkComparison(exp *ast.CallExpr, pass *analysis.Pass, matcher *ast.CallEx } call.Args = []ast.Expr{first} - report(pass, exp, wrongCompareWarningTemplate, oldExp) + reportBuilder.AddIssue(true, wrongCompareWarningTemplate) return false } @@ -813,7 +894,7 @@ func handleEqualComparison(pass *analysis.Pass, matcher *ast.CallExpr, first ast t := pass.TypesInfo.TypeOf(first) if gotypes.IsInterface(t) { handler.ReplaceFunction(matcher, ast.NewIdent(beIdenticalTo)) - } else if _, ok := t.(*gotypes.Pointer); ok { + } else if is[*gotypes.Pointer](t) { handler.ReplaceFunction(matcher, ast.NewIdent(beIdenticalTo)) } else { handler.ReplaceFunction(matcher, ast.NewIdent(equal)) @@ -823,7 +904,7 @@ func handleEqualComparison(pass *analysis.Pass, matcher *ast.CallExpr, first ast } } -func handleLenComparison(pass *analysis.Pass, exp *ast.CallExpr, matcher *ast.CallExpr, first ast.Expr, second ast.Expr, op token.Token, handler gomegahandler.Handler, oldExpr string) bool { +func handleLenComparison(pass *analysis.Pass, exp *ast.CallExpr, matcher *ast.CallExpr, first ast.Expr, second ast.Expr, op token.Token, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { switch op { case token.EQL: case token.NEQ: @@ -850,23 +931,58 @@ func handleLenComparison(pass *analysis.Pass, exp *ast.CallExpr, matcher *ast.Ca fun := handler.GetActualExpr(exp.Fun.(*ast.SelectorExpr)) fun.Args = []ast.Expr{val} - report(pass, exp, wrongLengthWarningTemplate, oldExpr) + reportBuilder.AddIssue(true, wrongLengthWarningTemplate) + return true +} + +func handleCapComparison(exp *ast.CallExpr, matcher *ast.CallExpr, first ast.Expr, second ast.Expr, op token.Token, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { + switch op { + case token.EQL: + case token.NEQ: + reverseAssertionFuncLogic(exp) + default: + return false + } + + eql := ast.NewIdent(haveCap) + matcher.Args = []ast.Expr{second} + + handler.ReplaceFunction(matcher, eql) + firstLen, ok := first.(*ast.CallExpr) // assuming it's len() + if !ok { + return false // should never happen + } + + val := firstLen.Args[0] + fun := handler.GetActualExpr(exp.Fun.(*ast.SelectorExpr)) + fun.Args = []ast.Expr{val} + + reportBuilder.AddIssue(true, wrongCapWarningTemplate) return true } // Check if the "actual" argument is a call to the golang built-in len() function func isActualIsLenFunc(actualArg ast.Expr) bool { + return checkActualFuncName(actualArg, "len") +} + +// Check if the "actual" argument is a call to the golang built-in len() function +func isActualIsCapFunc(actualArg ast.Expr) bool { + return checkActualFuncName(actualArg, "cap") +} + +func checkActualFuncName(actualArg ast.Expr, name string) bool { lenArgExp, ok := actualArg.(*ast.CallExpr) if !ok { return false } lenFunc, ok := lenArgExp.Fun.(*ast.Ident) - return ok && lenFunc.Name == "len" + return ok && lenFunc.Name == name } // Check if matcher function is in one of the patterns we want to avoid -func checkLengthMatcher(exp *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, oldExp string) bool { +func checkLengthMatcher(exp *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { matcher, ok := exp.Args[0].(*ast.CallExpr) if !ok { return true @@ -879,20 +995,20 @@ func checkLengthMatcher(exp *ast.CallExpr, pass *analysis.Pass, handler gomegaha switch matcherFuncName { case equal: - handleEqualMatcher(matcher, pass, exp, handler, oldExp) + handleEqualLenMatcher(matcher, pass, exp, handler, reportBuilder) return false case beZero: - handleBeZero(pass, exp, handler, oldExp) + handleBeZero(exp, handler, reportBuilder) return false case beNumerically: - return handleBeNumerically(matcher, pass, exp, handler, oldExp) + return handleBeNumerically(matcher, pass, exp, handler, reportBuilder) case not: reverseAssertionFuncLogic(exp) exp.Args[0] = exp.Args[0].(*ast.CallExpr).Args[0] - return checkLengthMatcher(exp, pass, handler, oldExp) + return checkLengthMatcher(exp, pass, handler, reportBuilder) default: return true @@ -900,7 +1016,7 @@ func checkLengthMatcher(exp *ast.CallExpr, pass *analysis.Pass, handler gomegaha } // Check if matcher function is in one of the patterns we want to avoid -func checkNilMatcher(exp *ast.CallExpr, pass *analysis.Pass, nilable ast.Expr, handler gomegahandler.Handler, notEqual bool, oldExp string) bool { +func checkCapMatcher(exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { matcher, ok := exp.Args[0].(*ast.CallExpr) if !ok { return true @@ -913,19 +1029,53 @@ func checkNilMatcher(exp *ast.CallExpr, pass *analysis.Pass, nilable ast.Expr, h switch matcherFuncName { case equal: - handleEqualNilMatcher(matcher, pass, exp, handler, nilable, notEqual, oldExp) + handleEqualCapMatcher(matcher, exp, handler, reportBuilder) + return false + + case beZero: + handleCapBeZero(exp, handler, reportBuilder) + return false + + case beNumerically: + return handleCapBeNumerically(matcher, exp, handler, reportBuilder) + + case not: + reverseAssertionFuncLogic(exp) + exp.Args[0] = exp.Args[0].(*ast.CallExpr).Args[0] + return checkCapMatcher(exp, handler, reportBuilder) + + default: + return true + } +} + +// Check if matcher function is in one of the patterns we want to avoid +func checkNilMatcher(exp *ast.CallExpr, pass *analysis.Pass, nilable ast.Expr, handler gomegahandler.Handler, notEqual bool, reportBuilder *reports.Builder) bool { + matcher, ok := exp.Args[0].(*ast.CallExpr) + if !ok { + return true + } + + matcherFuncName, ok := handler.GetActualFuncName(matcher) + if !ok { + return true + } + + switch matcherFuncName { + case equal: + handleEqualNilMatcher(matcher, pass, exp, handler, nilable, notEqual, reportBuilder) case beTrue: - handleNilBeBoolMatcher(pass, exp, handler, nilable, notEqual, oldExp) + handleNilBeBoolMatcher(pass, exp, handler, nilable, notEqual, reportBuilder) case beFalse: reverseAssertionFuncLogic(exp) - handleNilBeBoolMatcher(pass, exp, handler, nilable, notEqual, oldExp) + handleNilBeBoolMatcher(pass, exp, handler, nilable, notEqual, reportBuilder) case not: reverseAssertionFuncLogic(exp) exp.Args[0] = exp.Args[0].(*ast.CallExpr).Args[0] - return checkNilMatcher(exp, pass, nilable, handler, notEqual, oldExp) + return checkNilMatcher(exp, pass, nilable, handler, notEqual, reportBuilder) default: return true @@ -933,7 +1083,7 @@ func checkNilMatcher(exp *ast.CallExpr, pass *analysis.Pass, nilable ast.Expr, h return false } -func checkNilError(pass *analysis.Pass, assertionExp *ast.CallExpr, handler gomegahandler.Handler, actualArg ast.Expr, oldExpr string) bool { +func checkNilError(pass *analysis.Pass, assertionExp *ast.CallExpr, handler gomegahandler.Handler, actualArg ast.Expr, reportBuilder *reports.Builder) bool { if len(assertionExp.Args) == 0 { return true } @@ -964,13 +1114,13 @@ func checkNilError(pass *analysis.Pass, assertionExp *ast.CallExpr, handler gome case not: reverseAssertionFuncLogic(assertionExp) assertionExp.Args[0] = assertionExp.Args[0].(*ast.CallExpr).Args[0] - return checkNilError(pass, assertionExp, handler, actualArg, oldExpr) + return checkNilError(pass, assertionExp, handler, actualArg, reportBuilder) default: return true } var newFuncName string - if _, ok := actualArg.(*ast.CallExpr); ok { + if is[*ast.CallExpr](actualArg) { newFuncName = succeed } else { reverseAssertionFuncLogic(assertionExp) @@ -980,7 +1130,7 @@ func checkNilError(pass *analysis.Pass, assertionExp *ast.CallExpr, handler gome handler.ReplaceFunction(equalFuncExpr, ast.NewIdent(newFuncName)) equalFuncExpr.Args = nil - report(pass, assertionExp, wrongErrWarningTemplate, oldExpr) + reportBuilder.AddIssue(true, wrongErrWarningTemplate) return false } @@ -991,7 +1141,7 @@ func checkNilError(pass *analysis.Pass, assertionExp *ast.CallExpr, handler gome // Equal(true) => BeTrue() // Equal(false) => BeFalse() // HaveLen(0) => BeEmpty() -func handleAssertionOnly(pass *analysis.Pass, config types.Config, expr *ast.CallExpr, handler gomegahandler.Handler, actualArg ast.Expr, oldExpr string, shouldReport bool) bool { +func handleAssertionOnly(pass *analysis.Pass, config types.Config, expr *ast.CallExpr, handler gomegahandler.Handler, actualArg ast.Expr, reportBuilder *reports.Builder) bool { if len(expr.Args) == 0 { return true } @@ -1044,19 +1194,15 @@ func handleAssertionOnly(pass *analysis.Pass, config types.Config, expr *ast.Cal handler.ReplaceFunction(equalFuncExpr, ast.NewIdent(replacement)) equalFuncExpr.Args = nil - if shouldReport { - report(pass, expr, template, oldExpr) - } - + reportBuilder.AddIssue(true, template) return false case beFalse: if isNegativeAssertion(expr) { reverseAssertionFuncLogic(expr) handler.ReplaceFunction(equalFuncExpr, ast.NewIdent(beTrue)) - if shouldReport { - report(pass, expr, doubleNegativeWarningTemplate, oldExpr) - } + reportBuilder.AddIssue(true, doubleNegativeWarningTemplate) + return false } return false @@ -1069,9 +1215,7 @@ func handleAssertionOnly(pass *analysis.Pass, config types.Config, expr *ast.Cal if isZero(pass, equalFuncExpr.Args[0]) { handler.ReplaceFunction(equalFuncExpr, ast.NewIdent(beEmpty)) equalFuncExpr.Args = nil - if shouldReport { - report(pass, expr, wrongLengthWarningTemplate, oldExpr) - } + reportBuilder.AddIssue(true, wrongLengthWarningTemplate) return false } } @@ -1081,7 +1225,7 @@ func handleAssertionOnly(pass *analysis.Pass, config types.Config, expr *ast.Cal case not: reverseAssertionFuncLogic(expr) expr.Args[0] = expr.Args[0].(*ast.CallExpr).Args[0] - return handleAssertionOnly(pass, config, expr, handler, actualArg, oldExpr, shouldReport) + return handleAssertionOnly(pass, config, expr, handler, actualArg, reportBuilder) default: return true } @@ -1139,13 +1283,13 @@ func replaceLenActualArg(actualExpr *ast.CallExpr, handler gomegahandler.Handler switch name { case expect, omega: arg := actualExpr.Args[0] - if isActualIsLenFunc(arg) { + if isActualIsLenFunc(arg) || isActualIsCapFunc(arg) { // replace the len function call by its parameter, to create a fix suggestion actualExpr.Args[0] = arg.(*ast.CallExpr).Args[0] } case expectWithOffset: arg := actualExpr.Args[1] - if isActualIsLenFunc(arg) { + if isActualIsLenFunc(arg) || isActualIsCapFunc(arg) { // replace the len function call by its parameter, to create a fix suggestion actualExpr.Args[1] = arg.(*ast.CallExpr).Args[0] } @@ -1174,7 +1318,7 @@ func replaceNilActualArg(actualExpr *ast.CallExpr, handler gomegahandler.Handler } // For the BeNumerically matcher, we want to avoid the assertion of length to be > 0 or >= 1, or just == number -func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, oldExp string) bool { +func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { opExp, ok1 := matcher.Args[0].(*ast.BasicLit) valExp, ok2 := matcher.Args[1].(*ast.BasicLit) @@ -1186,20 +1330,45 @@ func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Ca reverseAssertionFuncLogic(exp) handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(beEmpty)) exp.Args[0].(*ast.CallExpr).Args = nil - reportLengthAssertion(pass, exp, handler, oldExp) - return false } else if op == `"=="` { chooseNumericMatcher(pass, exp, handler, valExp) - reportLengthAssertion(pass, exp, handler, oldExp) - - return false } else if op == `"!="` { reverseAssertionFuncLogic(exp) chooseNumericMatcher(pass, exp, handler, valExp) - reportLengthAssertion(pass, exp, handler, oldExp) + } else { + return true + } - return false + reportLengthAssertion(exp, handler, reportBuilder) + return false + } + return true +} + +// For the BeNumerically matcher, we want to avoid the assertion of length to be > 0 or >= 1, or just == number +func handleCapBeNumerically(matcher *ast.CallExpr, exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) bool { + opExp, ok1 := matcher.Args[0].(*ast.BasicLit) + valExp, ok2 := matcher.Args[1].(*ast.BasicLit) + + if ok1 && ok2 { + op := opExp.Value + val := valExp.Value + + if (op == `">"` && val == "0") || (op == `">="` && val == "1") { + reverseAssertionFuncLogic(exp) + handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(haveCap)) + exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{&ast.BasicLit{Kind: token.INT, Value: "0"}} + } else if op == `"=="` { + replaceNumericCapMatcher(exp, handler, valExp) + } else if op == `"!="` { + reverseAssertionFuncLogic(exp) + replaceNumericCapMatcher(exp, handler, valExp) + } else { + return true } + + reportCapAssertion(exp, handler, reportBuilder) + return false } return true } @@ -1215,6 +1384,12 @@ func chooseNumericMatcher(pass *analysis.Pass, exp *ast.CallExpr, handler gomega } } +func replaceNumericCapMatcher(exp *ast.CallExpr, handler gomegahandler.Handler, valExp ast.Expr) { + caller := exp.Args[0].(*ast.CallExpr) + handler.ReplaceFunction(caller, ast.NewIdent(haveCap)) + exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{valExp} +} + func reverseAssertionFuncLogic(exp *ast.CallExpr) { assertionFunc := exp.Fun.(*ast.SelectorExpr).Sel assertionFunc.Name = reverseassertion.ChangeAssertionLogic(assertionFunc.Name) @@ -1225,7 +1400,7 @@ func isNegativeAssertion(exp *ast.CallExpr) bool { return reverseassertion.IsNegativeLogic(assertionFunc.Name) } -func handleEqualMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, oldExp string) { +func handleEqualLenMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { equalTo, ok := matcher.Args[0].(*ast.BasicLit) if ok { chooseNumericMatcher(pass, exp, handler, equalTo) @@ -1233,16 +1408,29 @@ func handleEqualMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Cal handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(haveLen)) exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{matcher.Args[0]} } - reportLengthAssertion(pass, exp, handler, oldExp) + reportLengthAssertion(exp, handler, reportBuilder) } -func handleBeZero(pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, oldExp string) { +func handleEqualCapMatcher(matcher *ast.CallExpr, exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { + handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(haveCap)) + exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{matcher.Args[0]} + reportCapAssertion(exp, handler, reportBuilder) +} + +func handleBeZero(exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { exp.Args[0].(*ast.CallExpr).Args = nil handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(beEmpty)) - reportLengthAssertion(pass, exp, handler, oldExp) + reportLengthAssertion(exp, handler, reportBuilder) +} + +func handleCapBeZero(exp *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { + exp.Args[0].(*ast.CallExpr).Args = nil + handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(haveCap)) + exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{&ast.BasicLit{Kind: token.INT, Value: "0"}} + reportCapAssertion(exp, handler, reportBuilder) } -func handleEqualNilMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, oldExp string) { +func handleEqualNilMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, reportBuilder *reports.Builder) { equalTo, ok := matcher.Args[0].(*ast.Ident) if !ok { return @@ -1259,22 +1447,22 @@ func handleEqualNilMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast. handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(newFuncName)) exp.Args[0].(*ast.CallExpr).Args = nil - reportNilAssertion(pass, exp, handler, nilable, notEqual, oldExp, isItError) + reportNilAssertion(exp, handler, nilable, notEqual, isItError, reportBuilder) } -func handleNilBeBoolMatcher(pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, oldExp string) { +func handleNilBeBoolMatcher(pass *analysis.Pass, exp *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, reportBuilder *reports.Builder) { newFuncName, isItError := handleNilComparisonErr(pass, exp, nilable) handler.ReplaceFunction(exp.Args[0].(*ast.CallExpr), ast.NewIdent(newFuncName)) exp.Args[0].(*ast.CallExpr).Args = nil - reportNilAssertion(pass, exp, handler, nilable, notEqual, oldExp, isItError) + reportNilAssertion(exp, handler, nilable, notEqual, isItError, reportBuilder) } func handleNilComparisonErr(pass *analysis.Pass, exp *ast.CallExpr, nilable ast.Expr) (string, bool) { newFuncName := beNil isItError := isExprError(pass, nilable) if isItError { - if _, ok := nilable.(*ast.CallExpr); ok { + if is[*ast.CallExpr](nilable) { newFuncName = succeed } else { reverseAssertionFuncLogic(exp) @@ -1293,14 +1481,21 @@ func isAssertionFunc(name string) bool { return false } -func reportLengthAssertion(pass *analysis.Pass, expr *ast.CallExpr, handler gomegahandler.Handler, oldExpr string) { +func reportLengthAssertion(expr *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { + actualExpr := handler.GetActualExpr(expr.Fun.(*ast.SelectorExpr)) + replaceLenActualArg(actualExpr, handler) + + reportBuilder.AddIssue(true, wrongLengthWarningTemplate) +} + +func reportCapAssertion(expr *ast.CallExpr, handler gomegahandler.Handler, reportBuilder *reports.Builder) { actualExpr := handler.GetActualExpr(expr.Fun.(*ast.SelectorExpr)) replaceLenActualArg(actualExpr, handler) - report(pass, expr, wrongLengthWarningTemplate, oldExpr) + reportBuilder.AddIssue(true, wrongCapWarningTemplate) } -func reportNilAssertion(pass *analysis.Pass, expr *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, oldExpr string, isItError bool) { +func reportNilAssertion(expr *ast.CallExpr, handler gomegahandler.Handler, nilable ast.Expr, notEqual bool, isItError bool, reportBuilder *reports.Builder) { actualExpr := handler.GetActualExpr(expr.Fun.(*ast.SelectorExpr)) changed := replaceNilActualArg(actualExpr, handler, nilable) if !changed { @@ -1315,27 +1510,7 @@ func reportNilAssertion(pass *analysis.Pass, expr *ast.CallExpr, handler gomegah template = wrongErrWarningTemplate } - report(pass, expr, template, oldExpr) -} - -func report(pass *analysis.Pass, expr ast.Expr, messageTemplate, oldExpr string) { - newExp := goFmt(pass.Fset, expr) - pass.Report(analysis.Diagnostic{ - Pos: expr.Pos(), - Message: fmt.Sprintf(messageTemplate, newExp), - SuggestedFixes: []analysis.SuggestedFix{ - { - Message: fmt.Sprintf("should replace %s with %s", oldExpr, newExp), - TextEdits: []analysis.TextEdit{ - { - Pos: expr.Pos(), - End: expr.End(), - NewText: []byte(newExp), - }, - }, - }, - }, - }) + reportBuilder.AddIssue(true, template) } func reportNewName(pass *analysis.Pass, id *ast.Ident, newName string, messageTemplate, oldExpr string) { @@ -1402,7 +1577,7 @@ func isComparison(pass *analysis.Pass, actualArg ast.Expr) (ast.Expr, ast.Expr, case *ast.Ident: // check if const info, ok := pass.TypesInfo.Types[realFirst] if ok { - if _, ok := info.Type.(*gotypes.Basic); ok && info.Value != nil { + if is[*gotypes.Basic](info.Type) && info.Value != nil { replace = true } } @@ -1456,8 +1631,7 @@ func isExprError(pass *analysis.Pass, expr ast.Expr) bool { func isPointer(pass *analysis.Pass, expr ast.Expr) bool { t := pass.TypesInfo.TypeOf(expr) - _, ok := t.(*gotypes.Pointer) - return ok + return is[*gotypes.Pointer](t) } func isInterface(pass *analysis.Pass, expr ast.Expr) bool { @@ -1478,22 +1652,22 @@ func isNumeric(pass *analysis.Pass, node ast.Expr) bool { func checkNoAssertion(pass *analysis.Pass, expr *ast.CallExpr, handler gomegahandler.Handler) { funcName, ok := handler.GetActualFuncName(expr) if ok { - if isActualFunc(funcName) { - reportNoFix(pass, expr.Pos(), missingAssertionMessage, funcName) - } else if isActualAsyncFunc(funcName) { - reportNoFix(pass, expr.Pos(), missingAsyncAssertionMessage, funcName) + var allowedFunction string + switch funcName { + case expect, expectWithOffset: + allowedFunction = `"To()", "ToNot()" or "NotTo()"` + case eventually, eventuallyWithOffset, consistently, consistentlyWithOffset: + allowedFunction = `"Should()" or "ShouldNot()"` + case omega: + allowedFunction = `"Should()", "To()", "ShouldNot()", "ToNot()" or "NotTo()"` + default: + return } + reportNoFix(pass, expr.Pos(), missingAssertionMessage, funcName, allowedFunction) } } -func isActualFunc(name string) bool { - return name == expect || name == expectWithOffset -} - -func isActualAsyncFunc(name string) bool { - switch name { - case eventually, eventuallyWithOffset, consistently, consistentlyWithOffset: - return true - } - return false +func is[T any](x any) bool { + _, matchType := x.(T) + return matchType } diff --git a/vendor/github.com/nunnatsa/ginkgolinter/types/config.go b/vendor/github.com/nunnatsa/ginkgolinter/types/config.go index 6de22738d..b6838e524 100644 --- a/vendor/github.com/nunnatsa/ginkgolinter/types/config.go +++ b/vendor/github.com/nunnatsa/ginkgolinter/types/config.go @@ -1,9 +1,8 @@ package types import ( - "strings" - "go/ast" + "strings" ) const ( @@ -18,14 +17,17 @@ const ( ) type Config struct { - SuppressLen Boolean - SuppressNil Boolean - SuppressErr Boolean - SuppressCompare Boolean - SuppressAsync Boolean - ForbidFocus Boolean - SuppressTypeCompare Boolean - AllowHaveLen0 Boolean + SuppressLen Boolean + SuppressNil Boolean + SuppressErr Boolean + SuppressCompare Boolean + SuppressAsync Boolean + ForbidFocus Boolean + SuppressTypeCompare Boolean + AllowHaveLen0 Boolean + ForceExpectTo Boolean + ValidateAsyncIntervals Boolean + ForbidSpecPollution Boolean } func (s *Config) AllTrue() bool { @@ -34,14 +36,17 @@ func (s *Config) AllTrue() bool { func (s *Config) Clone() Config { return Config{ - SuppressLen: s.SuppressLen, - SuppressNil: s.SuppressNil, - SuppressErr: s.SuppressErr, - SuppressCompare: s.SuppressCompare, - SuppressAsync: s.SuppressAsync, - ForbidFocus: s.ForbidFocus, - SuppressTypeCompare: s.SuppressTypeCompare, - AllowHaveLen0: s.AllowHaveLen0, + SuppressLen: s.SuppressLen, + SuppressNil: s.SuppressNil, + SuppressErr: s.SuppressErr, + SuppressCompare: s.SuppressCompare, + SuppressAsync: s.SuppressAsync, + ForbidFocus: s.ForbidFocus, + SuppressTypeCompare: s.SuppressTypeCompare, + AllowHaveLen0: s.AllowHaveLen0, + ForceExpectTo: s.ForceExpectTo, + ValidateAsyncIntervals: s.ValidateAsyncIntervals, + ForbidSpecPollution: s.ForbidSpecPollution, } } |
