Merge pull request #89 from Antonboom/fixes/require-error-bool-expr

require-error: ignore assertion in bool expr

CONTRIBUTING.md

@@ -95,6 +95,10 @@ assertion call. For more complex checkers, use the [checkers.AdvancedChecker](./
 If the checker turns out to be too “fat”, then you can omit some obviously rare combinations,
 especially if they are covered by other checkers. Usually these are expressions in `assert.True/False`.
 
+Remember that [assert.TestingT](https://pkg.go.dev/github.com/stretchr/testify/assert#TestingT) and
+[require.TestingT](https://pkg.go.dev/github.com/stretchr/testify/require#TestingT) are different interfaces,
+which may be important in some contexts.
+
 ### 8) Improve tests from p.4 if necessary
 
 Pay attention to `Assertion` and `NewAssertionExpander`, but keep your tests as small as possible.

README.md

@@ -480,8 +480,9 @@ By default `require-error` only checks the `*Error*` assertions, presented above
 You can set `--require-error.fn-pattern` flag to limit the checking to certain calls (but still from the list above).
 For example, `--require-error.fn-pattern="^(Errorf?|NoErrorf?)$"` will only check `Error`, `Errorf`, `NoError`, and `NoErrorf`.
 
-Also, to minimize the number of false positives, `require-error` ignores:
+Also, to minimize false positives, `require-error` ignores:
 - assertion in the `if` condition;
+- assertion in the bool expression;
 - 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;

analyzer/testdata/src/require-error-skip-logic/issue79_test.go

@@ -0,0 +1,56 @@
+package requireerrorskiplogic
+
+import (
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func assertRegexpError1(regexp any) assert.ErrorAssertionFunc {
+	return func(t assert.TestingT, got error, msg ...any) bool {
+		if h, ok := t.(interface{ Helper() }); ok {
+			h.Helper()
+		}
+		return assert.Error(t, got, msg...) && assert.Regexp(t, regexp, got.Error(), msg...)
+	}
+}
+
+func assertRegexpError2(regexp any) assert.ErrorAssertionFunc {
+	return func(t assert.TestingT, got error, msg ...any) bool {
+		if h, ok := t.(interface{ Helper() }); ok {
+			h.Helper()
+		}
+		assert.Error(t, got, msg...) // want "require-error: for error assertions use require"
+		return assert.Regexp(t, regexp, got.Error(), msg...)
+	}
+}
+
+func assertRegexpError3(regexp any) assert.ErrorAssertionFunc {
+	return func(t assert.TestingT, got error, msg ...any) bool {
+		if h, ok := t.(interface{ Helper() }); ok {
+			h.Helper()
+		}
+		return assert.Error(t, got, msg...) &&
+			(assert.Regexp(t, regexp, got.Error(), msg...) ||
+				assert.ErrorContains(t, got, "failed to"))
+	}
+}
+
+func requireRegexpError1(regexp any) require.ErrorAssertionFunc {
+	return func(t require.TestingT, got error, msg ...any) {
+		if h, ok := t.(interface{ Helper() }); ok {
+			h.Helper()
+		}
+		assert.Error(t, got, msg...) // want "require-error: for error assertions use require"
+		assert.Regexp(t, regexp, got.Error(), msg...)
+	}
+}
+
+func requireRegexpError2(regexp any) require.ErrorAssertionFunc {
+	return func(t require.TestingT, got error, msg ...any) {
+		if h, ok := t.(interface{ Helper() }); ok {
+			h.Helper()
+		}
+		require.Error(t, got, msg...)
+		assert.Regexp(t, regexp, got.Error(), msg...)
+	}
+}

internal/checkers/call_meta.go

@@ -121,16 +121,23 @@ func trimTArg(pass *analysis.Pass, args []ast.Expr) []ast.Expr {
 }
 
 func isTestingTPtr(pass *analysis.Pass, arg ast.Expr) bool {
+	return implementsAssertTestingT(pass, arg) || implementsRequireTestingT(pass, arg)
+}
+
+func implementsAssertTestingT(pass *analysis.Pass, e ast.Expr) bool {
 	assertTestingTObj := analysisutil.ObjectOf(pass.Pkg, testify.AssertPkgPath, "TestingT")
+	return (assertTestingTObj != nil) && implements(pass, e, assertTestingTObj)
+}
+
+func implementsRequireTestingT(pass *analysis.Pass, e ast.Expr) bool {
 	requireTestingTObj := analysisutil.ObjectOf(pass.Pkg, testify.RequirePkgPath, "TestingT")
+	return (requireTestingTObj != nil) && implements(pass, e, requireTestingTObj)
+}
 
-	argType := pass.TypesInfo.TypeOf(arg)
-	if argType == nil {
+func implements(pass *analysis.Pass, e ast.Expr, ifaceObj types.Object) bool {
+	t := pass.TypesInfo.TypeOf(e)
+	if t == nil {
 		return false
 	}
-
-	return ((assertTestingTObj != nil) &&
-		types.Implements(argType, assertTestingTObj.Type().Underlying().(*types.Interface))) ||
-		((requireTestingTObj != nil) &&
-			types.Implements(argType, requireTestingTObj.Type().Underlying().(*types.Interface)))
+	return types.Implements(t, ifaceObj.Type().Underlying().(*types.Interface))
 }

internal/checkers/require_error.go

@@ -74,6 +74,8 @@ func (checker RequireError) Check(pass *analysis.Pass, inspector *inspector.Insp
 		_, prevPrevIsIfStmt := stack[len(stack)-3].(*ast.IfStmt)
 		inIfCond := prevIsIfStmt || (prevPrevIsIfStmt && prevIsAssignStmt)
 
+		_, inBoolExpr := stack[len(stack)-2].(*ast.BinaryExpr)
+
 		callExpr := node.(*ast.CallExpr)
 		testifyCall := NewCallMeta(pass, callExpr)
 
@@ -84,6 +86,7 @@ func (checker RequireError) Check(pass *analysis.Pass, inspector *inspector.Insp
 			parentIf:     findNearestNode[*ast.IfStmt](stack),
 			parentBlock:  findNearestNode[*ast.BlockStmt](stack),
 			inIfCond:     inIfCond,
+			inBoolExpr:   inBoolExpr,
 			inNoErrorSeq: false, // Will be filled in below.
 		}
 
@@ -148,13 +151,8 @@ func needToSkipBasedOnContext(
 	otherCalls []*callMeta,
 	callsByBlock map[*ast.BlockStmt][]*callMeta,
 ) bool {
-	if currCall.inNoErrorSeq {
-		// Skip `assert.NoError` sequence.
-		return true
-	}
-
-	if currCall.inIfCond {
-		// Skip assertions in the "if condition".
+	switch {
+	case currCall.inIfCond, currCall.inBoolExpr, currCall.inNoErrorSeq:
 		return true
 	}
 
@@ -309,6 +307,7 @@ type callMeta struct {
 	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) {`.
+	inBoolExpr   bool // True for code like `assert.Error(t, err) && assert.ErrorContains(t, err, "value")`
 	inNoErrorSeq bool // True for sequence of `assert.NoError` assertions.
 }