Skip to content

Commit 049b11c

Browse files
authored
Merge pull request #64 from Antonboom/fixes/require-error-else-if
require-error: support '} else if'
2 parents 151f0e4 + b59161a commit 049b11c

File tree

3 files changed

+129
-8
lines changed

3 files changed

+129
-8
lines changed

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ For example, `--require-error.fn-pattern="^(Errorf?|NoErrorf?)$"` will only chec
427427

428428
Also, to minimize the number of false positives, `require-error` ignores:
429429
- assertion in the `if` condition;
430-
- the entire `if-else` block, if there is an assertion in the `if` condition;
430+
- the entire `if-else[-if]` block, if there is an assertion in any `if` condition;
431431
- the last assertion in the block, if there are no methods/functions calls after it;
432432
- assertions in an explicit goroutine;
433433
- assertions in an explicit testing cleanup function or suite teardown methods;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package requireerrorskiplogic
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestComplexCondition(t *testing.T) {
11+
testCases := []struct {
12+
testName string
13+
someValue any
14+
someOtherValue any
15+
expectedError error
16+
expectedValue any
17+
}{
18+
{},
19+
}
20+
21+
for _, tc := range testCases {
22+
t.Run(tc.testName, func(t *testing.T) {
23+
result, err := operationWithResult()
24+
if tc.someValue == nil && tc.someOtherValue == nil {
25+
assert.Nil(t, result)
26+
assert.NoError(t, err)
27+
} else if tc.someOtherValue != nil {
28+
assert.EqualError(t, err, tc.expectedError.Error())
29+
} else {
30+
assert.Equal(t, tc.expectedError, err)
31+
assert.Equal(t, tc.expectedValue, result)
32+
}
33+
})
34+
}
35+
}
36+
37+
func TestCrazyCondition(t *testing.T) {
38+
testCases := []struct {
39+
testName string
40+
someValue any
41+
someOtherValue any
42+
expectedError error
43+
expectedValue any
44+
}{
45+
{},
46+
}
47+
48+
for _, tc := range testCases {
49+
t.Run(tc.testName, func(t *testing.T) {
50+
result, err := operationWithResult()
51+
if tc.someValue == nil && tc.someOtherValue == nil {
52+
assert.Nil(t, result)
53+
assert.NoError(t, err)
54+
} else if tc.someOtherValue != nil {
55+
assert.EqualError(t, err, tc.expectedError.Error())
56+
} else if tc.someOtherValue == nil {
57+
require.NoError(t, err)
58+
assert.Equal(t, tc.expectedError, err)
59+
assert.Equal(t, tc.expectedValue, result)
60+
} else if tc.someValue != nil {
61+
assert.NoError(t, err)
62+
}
63+
})
64+
}
65+
}
66+
67+
func TestAssertInElseIf(t *testing.T) {
68+
for _, tc := range []struct {
69+
filenames []string
70+
restartCount int
71+
}{
72+
{},
73+
} {
74+
t.Run("", func(t *testing.T) {
75+
count, err := calcRestartCountByLogDir(tc.filenames)
76+
if assert.NoError(t, err) {
77+
assert.Equal(t, count, tc.restartCount)
78+
assert.NoError(t, err)
79+
} else if true {
80+
assert.Error(t, err)
81+
assert.Equal(t, count, tc.restartCount)
82+
} else {
83+
assert.Error(t, err)
84+
assert.Equal(t, count, tc.restartCount)
85+
}
86+
87+
if true {
88+
assert.Equal(t, count, tc.restartCount)
89+
assert.NoError(t, err)
90+
} else if assert.NoError(t, err) {
91+
assert.Error(t, err)
92+
assert.Equal(t, count, tc.restartCount)
93+
} else {
94+
assert.Error(t, err)
95+
assert.Equal(t, count, tc.restartCount)
96+
}
97+
98+
assert.NoError(t, operation())
99+
})
100+
}
101+
}

internal/checkers/require_error.go

+27-7
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const requireErrorReport = "for error assertions use require"
3131
//
3232
// RequireError ignores:
3333
// - assertion in the `if` condition;
34-
// - the entire `if-else` block, if there is an assertion in the `if` condition;
34+
// - the entire `if-else[-if]` block, if there is an assertion in any `if` condition;
3535
// - the last assertion in the block, if there are no methods/functions calls after it;
3636
// - assertions in an explicit goroutine;
3737
// - assertions in an explicit testing cleanup function or suite teardown methods;
@@ -80,6 +80,7 @@ func (checker RequireError) Check(pass *analysis.Pass, inspector *inspector.Insp
8080
call := &callMeta{
8181
call: callExpr,
8282
testifyCall: testifyCall,
83+
rootIf: findRootIf(stack),
8384
parentIf: findNearestNode[*ast.IfStmt](stack),
8485
parentBlock: findNearestNode[*ast.BlockStmt](stack),
8586
inIfCond: inIfCond,
@@ -157,10 +158,10 @@ func needToSkipBasedOnContext(
157158
return true
158159
}
159160

160-
if currCall.parentIf != nil {
161+
if currCall.rootIf != nil {
161162
for _, rootCall := range otherCalls {
162-
if (rootCall.parentIf == currCall.parentIf) && rootCall.inIfCond {
163-
// Skip assertions in the entire if-else parentBlock, if the "if condition" contains assertion.
163+
if (rootCall.rootIf == currCall.rootIf) && rootCall.inIfCond {
164+
// Skip assertions in the entire if-else[-if] block, if some of "if condition" contains assertion.
164165
return true
165166
}
166167
}
@@ -175,7 +176,7 @@ func needToSkipBasedOnContext(
175176
_, blockEndWithReturn := block.List[len(block.List)-1].(*ast.ReturnStmt)
176177
if !blockEndWithReturn {
177178
for i := currCallIndex + 1; i < len(otherCalls); i++ {
178-
if (otherCalls[i].parentIf == nil) || (otherCalls[i].parentIf != currCall.parentIf) {
179+
if (otherCalls[i].rootIf == nil) || (otherCalls[i].rootIf != currCall.rootIf) {
179180
noCallsAfter = false
180181
break
181182
}
@@ -233,10 +234,28 @@ func findSurroundingFunc(pass *analysis.Pass, stack []ast.Node) *funcID {
233234
return nil
234235
}
235236

237+
func findRootIf(stack []ast.Node) *ast.IfStmt {
238+
nearestIf, i := findNearestNodeWithIdx[*ast.IfStmt](stack)
239+
for ; i > 0; i-- {
240+
parent, ok := stack[i-1].(*ast.IfStmt)
241+
if ok {
242+
nearestIf = parent
243+
} else {
244+
break
245+
}
246+
}
247+
return nearestIf
248+
}
249+
236250
func findNearestNode[T ast.Node](stack []ast.Node) (v T) {
251+
v, _ = findNearestNodeWithIdx[T](stack)
252+
return
253+
}
254+
255+
func findNearestNodeWithIdx[T ast.Node](stack []ast.Node) (v T, index int) {
237256
for i := len(stack) - 2; i >= 0; i-- {
238257
if n, ok := stack[i].(T); ok {
239-
return n
258+
return n, i
240259
}
241260
}
242261
return
@@ -273,7 +292,8 @@ func markCallsInNoErrorSequence(callsByBlock map[*ast.BlockStmt][]*callMeta) {
273292
type callMeta struct {
274293
call *ast.CallExpr
275294
testifyCall *CallMeta
276-
parentIf *ast.IfStmt
295+
rootIf *ast.IfStmt // The root `if` in if-else[-if] chain.
296+
parentIf *ast.IfStmt // The nearest `if`, can be equal with rootIf.
277297
parentBlock *ast.BlockStmt
278298
inIfCond bool // True for code like `if assert.ErrorAs(t, err, &target) {`.
279299
inNoErrorSeq bool // True for sequence of `assert.NoError` assertions.

0 commit comments

Comments
 (0)