Skip to content

Commit

Permalink
Merge pull request Antonboom#64 from Antonboom/fixes/require-error-el…
Browse files Browse the repository at this point in the history
…se-if

require-error: support '} else if'
  • Loading branch information
Antonboom authored Feb 8, 2024
2 parents 151f0e4 + b59161a commit 049b11c
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 8 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ For example, `--require-error.fn-pattern="^(Errorf?|NoErrorf?)$"` will only chec

Also, to minimize the number of false positives, `require-error` ignores:
- assertion in the `if` condition;
- the entire `if-else` block, if there is an assertion in the `if` condition;
- the entire `if-else[-if]` block, if there is an assertion in any `if` condition;
- the last assertion in the block, if there are no methods/functions calls after it;
- assertions in an explicit goroutine;
- assertions in an explicit testing cleanup function or suite teardown methods;
Expand Down
101 changes: 101 additions & 0 deletions analyzer/testdata/src/require-error-skip-logic/else_if_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package requireerrorskiplogic

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestComplexCondition(t *testing.T) {
testCases := []struct {
testName string
someValue any
someOtherValue any
expectedError error
expectedValue any
}{
{},
}

for _, tc := range testCases {
t.Run(tc.testName, func(t *testing.T) {
result, err := operationWithResult()
if tc.someValue == nil && tc.someOtherValue == nil {
assert.Nil(t, result)
assert.NoError(t, err)
} else if tc.someOtherValue != nil {
assert.EqualError(t, err, tc.expectedError.Error())
} else {
assert.Equal(t, tc.expectedError, err)
assert.Equal(t, tc.expectedValue, result)
}
})
}
}

func TestCrazyCondition(t *testing.T) {
testCases := []struct {
testName string
someValue any
someOtherValue any
expectedError error
expectedValue any
}{
{},
}

for _, tc := range testCases {
t.Run(tc.testName, func(t *testing.T) {
result, err := operationWithResult()
if tc.someValue == nil && tc.someOtherValue == nil {
assert.Nil(t, result)
assert.NoError(t, err)
} else if tc.someOtherValue != nil {
assert.EqualError(t, err, tc.expectedError.Error())
} else if tc.someOtherValue == nil {
require.NoError(t, err)
assert.Equal(t, tc.expectedError, err)
assert.Equal(t, tc.expectedValue, result)
} else if tc.someValue != nil {
assert.NoError(t, err)
}
})
}
}

func TestAssertInElseIf(t *testing.T) {
for _, tc := range []struct {
filenames []string
restartCount int
}{
{},
} {
t.Run("", func(t *testing.T) {
count, err := calcRestartCountByLogDir(tc.filenames)
if assert.NoError(t, err) {
assert.Equal(t, count, tc.restartCount)
assert.NoError(t, err)
} else if true {
assert.Error(t, err)
assert.Equal(t, count, tc.restartCount)
} else {
assert.Error(t, err)
assert.Equal(t, count, tc.restartCount)
}

if true {
assert.Equal(t, count, tc.restartCount)
assert.NoError(t, err)
} else if assert.NoError(t, err) {
assert.Error(t, err)
assert.Equal(t, count, tc.restartCount)
} else {
assert.Error(t, err)
assert.Equal(t, count, tc.restartCount)
}

assert.NoError(t, operation())
})
}
}
34 changes: 27 additions & 7 deletions internal/checkers/require_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const requireErrorReport = "for error assertions use require"
//
// RequireError ignores:
// - assertion in the `if` condition;
// - the entire `if-else` block, if there is an assertion in the `if` condition;
// - the entire `if-else[-if]` block, if there is an assertion in any `if` condition;
// - the last assertion in the block, if there are no methods/functions calls after it;
// - assertions in an explicit goroutine;
// - assertions in an explicit testing cleanup function or suite teardown methods;
Expand Down Expand Up @@ -80,6 +80,7 @@ func (checker RequireError) Check(pass *analysis.Pass, inspector *inspector.Insp
call := &callMeta{
call: callExpr,
testifyCall: testifyCall,
rootIf: findRootIf(stack),
parentIf: findNearestNode[*ast.IfStmt](stack),
parentBlock: findNearestNode[*ast.BlockStmt](stack),
inIfCond: inIfCond,
Expand Down Expand Up @@ -157,10 +158,10 @@ func needToSkipBasedOnContext(
return true
}

if currCall.parentIf != nil {
if currCall.rootIf != nil {
for _, rootCall := range otherCalls {
if (rootCall.parentIf == currCall.parentIf) && rootCall.inIfCond {
// Skip assertions in the entire if-else parentBlock, if the "if condition" contains assertion.
if (rootCall.rootIf == currCall.rootIf) && rootCall.inIfCond {
// Skip assertions in the entire if-else[-if] block, if some of "if condition" contains assertion.
return true
}
}
Expand All @@ -175,7 +176,7 @@ func needToSkipBasedOnContext(
_, blockEndWithReturn := block.List[len(block.List)-1].(*ast.ReturnStmt)
if !blockEndWithReturn {
for i := currCallIndex + 1; i < len(otherCalls); i++ {
if (otherCalls[i].parentIf == nil) || (otherCalls[i].parentIf != currCall.parentIf) {
if (otherCalls[i].rootIf == nil) || (otherCalls[i].rootIf != currCall.rootIf) {
noCallsAfter = false
break
}
Expand Down Expand Up @@ -233,10 +234,28 @@ func findSurroundingFunc(pass *analysis.Pass, stack []ast.Node) *funcID {
return nil
}

func findRootIf(stack []ast.Node) *ast.IfStmt {
nearestIf, i := findNearestNodeWithIdx[*ast.IfStmt](stack)
for ; i > 0; i-- {
parent, ok := stack[i-1].(*ast.IfStmt)
if ok {
nearestIf = parent
} else {
break
}
}
return nearestIf
}

func findNearestNode[T ast.Node](stack []ast.Node) (v T) {
v, _ = findNearestNodeWithIdx[T](stack)
return
}

func findNearestNodeWithIdx[T ast.Node](stack []ast.Node) (v T, index int) {
for i := len(stack) - 2; i >= 0; i-- {
if n, ok := stack[i].(T); ok {
return n
return n, i
}
}
return
Expand Down Expand Up @@ -273,7 +292,8 @@ func markCallsInNoErrorSequence(callsByBlock map[*ast.BlockStmt][]*callMeta) {
type callMeta struct {
call *ast.CallExpr
testifyCall *CallMeta
parentIf *ast.IfStmt
rootIf *ast.IfStmt // The root `if` in if-else[-if] chain.
parentIf *ast.IfStmt // The nearest `if`, can be equal with rootIf.
parentBlock *ast.BlockStmt
inIfCond bool // True for code like `if assert.ErrorAs(t, err, &target) {`.
inNoErrorSeq bool // True for sequence of `assert.NoError` assertions.
Expand Down

0 comments on commit 049b11c

Please sign in to comment.