Skip to content

Commit 608f078

Browse files
committed
Fix bug: ginkgolinter ignores the Error() method
as in: ```go Expect(func() (int, error) {return 42, nil}()).Error().ToNot(HaveOccurred()) ```
1 parent be3cbdb commit 608f078

File tree

8 files changed

+58
-11
lines changed

8 files changed

+58
-11
lines changed

analyzer_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ func TestAllUseCases(t *testing.T) {
101101
testName: "nil error-func variable",
102102
testData: "a/issue-171",
103103
},
104+
{
105+
testName: "respect the Error() method",
106+
testData: "a/issue-173",
107+
},
104108
{
105109
testName: "matchError with func return error-func",
106110
testData: "a/issue-174",

internal/expression/actual/actual.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ type Actual struct {
2121
actualOffset int
2222
}
2323

24-
func New(origExpr, cloneExpr *ast.CallExpr, orig *ast.CallExpr, clone *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, timePkg string) (*Actual, bool) {
24+
func New(origExpr, cloneExpr *ast.CallExpr, orig *ast.CallExpr, clone *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Handler, timePkg string, errMethodExists bool) (*Actual, bool) {
2525
funcName, ok := handler.GetActualFuncName(orig)
2626
if !ok {
2727
return nil, false
2828
}
2929

30-
arg, actualOffset := getActualArgPayload(orig, clone, pass, funcName)
30+
arg, actualOffset := getActualArgPayload(orig, clone, pass, funcName, errMethodExists)
3131
if arg == nil {
3232
return nil, false
3333
}

internal/expression/actual/actualarg.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const (
2828
ErrFuncActualArgType
2929
GomegaParamArgType
3030
MultiRetsArgType
31+
ErrorMethodArgType
3132

3233
ErrorTypeArgType
3334

@@ -39,15 +40,17 @@ func (a ArgType) Is(val ArgType) bool {
3940
return a&val != 0
4041
}
4142

42-
func getActualArgPayload(origActualExpr, actualExprClone *ast.CallExpr, pass *analysis.Pass, actualMethodName string) (ArgPayload, int) {
43+
func getActualArgPayload(origActualExpr, actualExprClone *ast.CallExpr, pass *analysis.Pass, actualMethodName string, errMethodExists bool) (ArgPayload, int) {
4344
origArgExpr, argExprClone, actualOffset, isGomegaExpr := getActualArg(origActualExpr, actualExprClone, actualMethodName, pass)
4445
if !isGomegaExpr {
4546
return nil, 0
4647
}
4748

4849
var arg ArgPayload
4950

50-
if value.IsExprError(pass, origArgExpr) {
51+
if errMethodExists {
52+
arg = &ErrorMethodPayload{}
53+
} else if value.IsExprError(pass, origArgExpr) {
5154
arg = newErrPayload(origArgExpr, argExprClone, pass)
5255
} else {
5356
switch expr := origArgExpr.(type) {
@@ -183,6 +186,12 @@ func (*ErrPayload) ArgType() ArgType {
183186
return ErrActualArgType | ErrorTypeArgType
184187
}
185188

189+
type ErrorMethodPayload struct{}
190+
191+
func (ErrorMethodPayload) ArgType() ArgType {
192+
return ErrorMethodArgType | ErrorTypeArgType
193+
}
194+
186195
func parseBinaryExpr(origActualExpr, argExprClone *ast.BinaryExpr, pass *analysis.Pass) ArgPayload {
187196
left, right, op := origActualExpr.X, origActualExpr.Y, origActualExpr.Op
188197
replace := false

internal/expression/expression.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ func New(origExpr *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Hand
5252
exprClone := astcopy.CallExpr(origExpr)
5353
selClone := exprClone.Fun.(*ast.SelectorExpr)
5454

55-
origActual := handler.GetActualExpr(origSel)
55+
errMethodExists := false
56+
57+
origActual := handler.GetActualExpr(origSel, &errMethodExists)
5658
if origActual == nil {
5759
return nil, false
5860
}
@@ -62,7 +64,7 @@ func New(origExpr *ast.CallExpr, pass *analysis.Pass, handler gomegahandler.Hand
6264
return nil, false
6365
}
6466

65-
actl, ok := actual.New(origExpr, exprClone, origActual, actualClone, pass, handler, timePkg)
67+
actl, ok := actual.New(origExpr, exprClone, origActual, actualClone, pass, handler, timePkg, errMethodExists)
6668
if !ok {
6769
return nil, false
6870
}

internal/gomegahandler/dothandler.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (dotHandler) GetNewWrapperMatcher(name string, existing *ast.CallExpr) *ast
5151
}
5252
}
5353

54-
func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr {
54+
func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExists *bool) *ast.CallExpr {
5555
actualExpr, ok := assertionFunc.X.(*ast.CallExpr)
5656
if !ok {
5757
return nil
@@ -66,7 +66,11 @@ func (h dotHandler) GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr
6666
return actualExpr
6767
}
6868
} else {
69-
return h.GetActualExpr(fun)
69+
if fun.Sel.Name == "Error" {
70+
*errMethodExists = true
71+
}
72+
73+
return h.GetActualExpr(fun, errMethodExists)
7074
}
7175
}
7276
return nil

internal/gomegahandler/handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type Handler interface {
1818
// ReplaceFunction replaces the function with another one, for fix suggestions
1919
ReplaceFunction(*ast.CallExpr, *ast.Ident)
2020

21-
GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr
21+
GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExists *bool) *ast.CallExpr
2222

2323
GetActualExprClone(origFunc, funcClone *ast.SelectorExpr) *ast.CallExpr
2424

internal/gomegahandler/namedhandler.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (g nameHandler) isGomegaVar(x ast.Expr) bool {
5151
return gomegainfo.IsGomegaVar(x, g.pass)
5252
}
5353

54-
func (g nameHandler) GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExpr {
54+
func (g nameHandler) GetActualExpr(assertionFunc *ast.SelectorExpr, errMethodExists *bool) *ast.CallExpr {
5555
actualExpr, ok := assertionFunc.X.(*ast.CallExpr)
5656
if !ok {
5757
return nil
@@ -69,7 +69,10 @@ func (g nameHandler) GetActualExpr(assertionFunc *ast.SelectorExpr) *ast.CallExp
6969
return actualExpr
7070
}
7171
} else {
72-
return g.GetActualExpr(fun)
72+
if fun.Sel.Name == "Error" {
73+
*errMethodExists = true
74+
}
75+
return g.GetActualExpr(fun, errMethodExists)
7376
}
7477
}
7578
return nil

testdata/src/a/issue-173/issue173.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package issue_173
2+
3+
import (
4+
"errors"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
func intErrFunc() (int, error) {
11+
return 42, nil
12+
}
13+
14+
func strErr() (string, error) {
15+
return "", errors.New("error")
16+
}
17+
18+
var _ = Describe("test if issue 173 was solved", func() {
19+
It("should respect the Error() method", func() {
20+
Expect(intErrFunc()).Error().To(BeNil()) // want `ginkgo-linter: wrong error assertion. Consider using .Expect\(intErrFunc\(\)\)\.Error\(\)\.ToNot\(HaveOccurred\(\)\). instead`
21+
Expect(intErrFunc()).Error().ToNot(HaveOccurred())
22+
Expect(intErrFunc()).Error().ToNot(Succeed())
23+
Expect(strErr()).Error().To(MatchError(ContainSubstring("error")))
24+
})
25+
})

0 commit comments

Comments
 (0)