Skip to content

Commit

Permalink
Add fix suggestion
Browse files Browse the repository at this point in the history
This PR add support to the `-fix` flag, in order to apply the fixes at
request.
  • Loading branch information
nunnatsa committed Jul 10, 2022
1 parent 9ad7918 commit 057d0e8
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 30 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ This is a golang linter to check usage of the ginkgo and gomega packages.

ginkgo is a testing framework and gomega is its assertion package.

## usage
```shell
ginkgo-linter [-fix] .
```
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.
Expand Down
57 changes: 36 additions & 21 deletions ginkgo_linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ This should be replaced with:

const (
linterName = "ginkgo-linter"
wrongLengthWarningTemplate = "wrong length check; consider using `%s` instead"
wrongLengthWarningTemplate = linterName + ": wrong length check; consider using `%s` instead"
beEmpty = "BeEmpty"
haveLen = "HaveLen"
expect = "Expect"
Expand Down Expand Up @@ -93,7 +93,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
return true
}

return checkMatcher(assertionExp, pass)
oldExpr := goFmt(pass.Fset, assertionExp)

return checkMatcher(assertionExp, pass, oldExpr)
})
}
return nil, nil
Expand All @@ -111,7 +113,7 @@ func isActualIsLenFunc(actualArg ast.Expr) bool {
}

// Check if matcher function is in one of the patterns we want to avoid
func checkMatcher(exp *ast.CallExpr, pass *analysis.Pass) bool {
func checkMatcher(exp *ast.CallExpr, pass *analysis.Pass, oldExp string) bool {
matcher, ok := exp.Args[0].(*ast.CallExpr)
if !ok {
return true
Expand All @@ -124,20 +126,20 @@ func checkMatcher(exp *ast.CallExpr, pass *analysis.Pass) bool {

switch matcherFunc.Name {
case "Equal":
handleEqualMatcher(matcher, pass, exp)
handleEqualMatcher(matcher, pass, exp, oldExp)
return false

case "BeZero":
handleBeZero(pass, exp)
handleBeZero(pass, exp, oldExp)
return false

case "BeNumerically":
return handleBeNumerically(matcher, pass, exp)
return handleBeNumerically(matcher, pass, exp, oldExp)

case "Not":
reverseAssertionFuncLogic(exp)
exp.Args[0] = exp.Args[0].(*ast.CallExpr).Args[0]
return checkMatcher(exp, pass)
return checkMatcher(exp, pass, oldExp)

default:
return true
Expand Down Expand Up @@ -186,7 +188,7 @@ func replaceLenActualArg(actualExpr *ast.CallExpr) {
}

// 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) bool {
func handleBeNumerically(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, oldExp string) bool {
opExp, ok1 := matcher.Args[0].(*ast.BasicLit)
valExp, ok2 := matcher.Args[1].(*ast.BasicLit)

Expand All @@ -198,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)
reportLengthCheck(pass, exp, oldExp)
return false
} else if op == `"=="` {
if val == "0" {
Expand All @@ -209,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)
reportLengthCheck(pass, exp, oldExp)
return false
} else if op == `"!="` {
reverseAssertionFuncLogic(exp)
Expand All @@ -222,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)
reportLengthCheck(pass, exp, oldExp)
return false
}
}
Expand All @@ -234,7 +236,7 @@ func reverseAssertionFuncLogic(exp *ast.CallExpr) {
assertionFunc.Name = reverseassertion.ChangeAssertionLogic(assertionFunc.Name)
}

func handleEqualMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr) {
func handleEqualMatcher(matcher *ast.CallExpr, pass *analysis.Pass, exp *ast.CallExpr, oldExp string) {
equalTo, ok := matcher.Args[0].(*ast.BasicLit)
if ok {
if equalTo.Value == "0" {
Expand All @@ -248,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)
reportLengthCheck(pass, exp, oldExp)
}

func handleBeZero(pass *analysis.Pass, exp *ast.CallExpr) {
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)
reportLengthCheck(pass, exp, oldExp)
}

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

func report(pass *analysis.Pass, pos token.Pos, warning string) {
pass.Reportf(pos, "%s: %s", linterName, warning)
}

func reportLengthCheck(pass *analysis.Pass, expr *ast.CallExpr) {
func reportLengthCheck(pass *analysis.Pass, expr *ast.CallExpr, oldExpr string) {
replaceLenActualArg(expr.Fun.(*ast.SelectorExpr).X.(*ast.CallExpr))
report(pass, expr.Pos(), fmt.Sprintf(wrongLengthWarningTemplate, goFmt(pass.Fset, expr)))

newExp := goFmt(pass.Fset, expr)
pass.Report(analysis.Diagnostic{
Pos: expr.Pos(),
Message: fmt.Sprintf(wrongLengthWarningTemplate, 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),
},
},
},
},
})
}

func goFmt(fset *token.FileSet, x ast.Expr) string {
Expand Down
7 changes: 0 additions & 7 deletions ginkgo_linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@ func TestGinkgoLinter(t *testing.T) {
RunSpecs(t, "Ginkgo Linter Suite")
}

//func TestGinkgoLinter(t *testing.T) {
// testdata := analysistest.TestData()
//
// os.Setenv("GOPATH", "/home/nunnatsa/go")
// analysistest.Run(t, testdata, Analyzer, "a")
//}

var _ = Describe("", func() {
Context("test a", func() {
It("Test all", func() {
Expand Down
2 changes: 0 additions & 2 deletions testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
. "github.com/onsi/gomega"
)

var obj = struct{ Items []int }{Items: []int{1234}}

var _ = Describe("test data for the ginkgo-linter", func() {
Context("test Expect", func() {
Context("test Should", func() {
Expand Down

0 comments on commit 057d0e8

Please sign in to comment.