Skip to content

Commit

Permalink
rename "check" to "assertion"
Browse files Browse the repository at this point in the history
  • Loading branch information
nunnatsa committed Jul 10, 2022
1 parent 057d0e8 commit f1842d4
Show file tree
Hide file tree
Showing 6 changed files with 328 additions and 328 deletions.
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Use the `-fix` flag to apply the fix suggestions to the source code.

## Linter Checks
### Wrong Length checks
The linter finds usage of the golang built-in `len` function, and then all kind of matchers, while there are already gomega matchers for these usecases.
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.

There are several wrong patterns:
```go
Expand All @@ -28,9 +28,9 @@ Expect(len(x)).To(BeNumeric("==", 2)) // should be Expect(x).To(HaveLen(2))
Expect(len(x)).To(BeNumeric("!=", 3)) // should be Expect(x).ToNot(HaveLen(3))
```

The linter supports the `Expect`, `ExpectWithOffset` and the `Ω` functions, and the `Should`, `ShouldNot`, `To`, `ToNot` and `NotTo` assertion functions.
The linter supports the `Expect`, `ExpectWithOffset` and the `Ω` "actual" functions, and the `Should`, `ShouldNot`, `To`, `ToNot` and `NotTo` assertion functions.

It also supports the embedded `Not()` function; e.g.
It also supports the embedded `Not()` matcher; e.g.

`Ω(len(x)).Should(Not(Equal(4)))` => `Ω(x).ShouldNot(HaveLen(4))`

Expand All @@ -40,19 +40,19 @@ Or even (double negative):

The output of the linter,when finding issues, looks like this:
```
./testdata/src/a/a.go:14:5: ginkgo-linter: wrong length check; consider using `Expect("abcd").Should(HaveLen(4))` instead
./testdata/src/a/a.go:18:5: ginkgo-linter: wrong length check; consider using `Expect("").Should(BeEmpty())` instead
./testdata/src/a/a.go:22:5: ginkgo-linter: wrong length check; consider using `Expect("").Should(BeEmpty())` instead
./testdata/src/a/a.go:14:5: ginkgo-linter: wrong length assertion; consider using `Expect("abcd").Should(HaveLen(4))` instead
./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
```

## Suppress the linter
To suppress the wrong length check warning, add a comment with (only)
`ginkgo-linter:supressLengthCheckWarning`. There are two options to use this comment:
To suppress the wrong length assertion warning, add a comment with (only)
`ginkgo-linter:ignore-len-assert-warning`. There are two options to use this comment:
1. If the comment is at the top of the file, supress the warning for the whole file; e.g.:
```go
package mypackage

// ginkgo-linter:ignore-length-warning
// ginkgo-linter:ignore-len-assert-warning

import (
. "github.com/onsi/ginkgo/v2"
Expand All @@ -68,7 +68,7 @@ To suppress the wrong length check warning, add a comment with (only)
2. If the comment is before a wrong length check expression, the warning is suppressed for this expression only; for example:
```go
It("should test something", func() {
// ginkgo-linter:ignore-length-warning
// ginkgo-linter:ignore-len-assert-warning
Expect(len("abc")).Should(Equal(3)) // this line will not trigger the warning
Expect(len("abc")).Should(Equal(3)) // this line will trigger the warning
}
Expand Down
22 changes: 11 additions & 11 deletions ginkgo_linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
// The ginkgolinter enforces standards of using ginkgo and gomega.
//
// The current checks are:
// * enforce right length check - warn for assertion of len(something):
// * enforce right length assertion - warn for assertion of len(something):
//
// This check finds the following patterns and suggests an alternative
// * Expect(len(something)).To(Equal(number)) ===> Expect(x).To(HaveLen(number))
Expand All @@ -39,15 +39,15 @@ This should be replaced with:

const (
linterName = "ginkgo-linter"
wrongLengthWarningTemplate = linterName + ": wrong length check; consider using `%s` instead"
wrongLengthWarningTemplate = linterName + ": wrong length assertion; consider using `%s` instead"
beEmpty = "BeEmpty"
haveLen = "HaveLen"
expect = "Expect"
omega = "Ω"
expectWithOffset = "ExpectWithOffset"

supressPrefix = "ginkgo-linter"
supressLengthCheckWarning = supressPrefix + ":ignore-length-warning"
supressPrefix = "ginkgo-linter"
supressLengthAssertionWarning = supressPrefix + ":ignore-len-assert-warning"
)

// main assertion function
Expand Down Expand Up @@ -200,7 +200,7 @@ func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Ca
reverseAssertionFuncLogic(exp)
exp.Args[0].(*ast.CallExpr).Fun = ast.NewIdent(beEmpty)
exp.Args[0].(*ast.CallExpr).Args = nil
reportLengthCheck(pass, exp, oldExp)
reportLengthAssertion(pass, exp, oldExp)
return false
} else if op == `"=="` {
if val == "0" {
Expand All @@ -211,7 +211,7 @@ func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Ca
exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{valExp}
}

reportLengthCheck(pass, exp, oldExp)
reportLengthAssertion(pass, exp, oldExp)
return false
} else if op == `"!="` {
reverseAssertionFuncLogic(exp)
Expand All @@ -224,7 +224,7 @@ func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Ca
exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{valExp}
}

reportLengthCheck(pass, exp, oldExp)
reportLengthAssertion(pass, exp, oldExp)
return false
}
}
Expand All @@ -250,14 +250,14 @@ func handleEqualMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.Cal
exp.Args[0].(*ast.CallExpr).Fun = ast.NewIdent(haveLen)
exp.Args[0].(*ast.CallExpr).Args = []ast.Expr{matcher.Args[0]}
}
reportLengthCheck(pass, exp, oldExp)
reportLengthAssertion(pass, exp, oldExp)
}

func handleBeZero(pass *analysis.Pass, exp *ast.CallExpr, oldExp string) {
exp.Args[0].(*ast.CallExpr).Args = nil
exp.Args[0].(*ast.CallExpr).Fun.(*ast.Ident).Name = beEmpty

reportLengthCheck(pass, exp, oldExp)
reportLengthAssertion(pass, exp, oldExp)
}

func isAssertionFunc(name string) bool {
Expand All @@ -268,7 +268,7 @@ func isAssertionFunc(name string) bool {
return false
}

func reportLengthCheck(pass *analysis.Pass, expr *ast.CallExpr, oldExpr string) {
func reportLengthAssertion(pass *analysis.Pass, expr *ast.CallExpr, oldExpr string) {
replaceLenActualArg(expr.Fun.(*ast.SelectorExpr).X.(*ast.CallExpr))

newExp := goFmt(pass.Fset, expr)
Expand Down Expand Up @@ -305,7 +305,7 @@ func isSuppressComment(commentGroup []*ast.CommentGroup) bool {
comment = strings.TrimPrefix(comment, "/*")
comment = strings.TrimSuffix(comment, "*/")
comment = strings.TrimSpace(comment)
if comment == supressLengthCheckWarning {
if comment == supressLengthAssertionWarning {
return true
}
}
Expand Down
Loading

0 comments on commit f1842d4

Please sign in to comment.