accept async functions with testing.TB variant instead of Gomega

In Kubernetes' ktesting package the following assertion is valid:

    tCtx.Eventually(func(tCtx TContext)).Should(Succeed())

That works because tCtx.Eventually wraps the function such that failures
reported via tCtx.Error/Errorf/Fatal/Fatalf are visible to the matcher.

Without this change, the "Success matcher only support a single error value, or
function with Gomega as its first parameter" warning is triggered.

Instead of building in specific support for ktesting, the approach is to check
for a first parameter which has any (not necessarily all!) of the testing.TB
methods for reporting failures. That is considered a valid alternative to
Gomega.

There's now a potential false negative in the linter when passing such a
function directly to gomega.Eventually/Consistently. That'll fail at runtime
because gomega cannot call it, so such a false negative isn't too bad (fails
reliably and obviously).

The warning message doesn't get changed because it would be confusing for most
users to call out some vague alternative (vague because the linter has no
details) that most users won't be able to use.

Author: pohly

internal/expression/actual/actualarg.go

@@ -29,6 +29,7 @@ const (
 	FuncSigArgType
 	ErrFuncActualArgType
 	GomegaParamArgType
+	TBParamArgType
 	MultiRetsArgType
 	ErrorMethodArgType
 

internal/expression/actual/asyncfuncarg.go

@@ -17,8 +17,13 @@ func getAsyncFuncArg(sig *gotypes.Signature) ArgPayload {
 
 	if sig.Params().Len() > 0 {
 		arg := sig.Params().At(0).Type()
-		if gomegainfo.IsGomegaType(arg) && sig.Results().Len() == 0 {
-			argType |= FuncSigArgType | GomegaParamArgType
+		if sig.Results().Len() == 0 {
+			if gomegainfo.IsGomegaType(arg) {
+				argType |= FuncSigArgType | GomegaParamArgType
+			}
+			if implementsTB(arg) {
+				argType |= FuncSigArgType | TBParamArgType
+			}
 		}
 	}
 
@@ -36,3 +41,34 @@ type FuncSigArgPayload struct {
 func (f FuncSigArgPayload) ArgType() ArgType {
 	return f.argType
 }
+
+// implementsTB checks if the argument type implements any of the methods in testing.TB which
+// can be used to report test failures. Such a type is a potential alternative to a Gomega
+// parameter in some Gomega wrappers.
+func implementsTB(t gotypes.Type) bool {
+	for _, interfaceType := range tbInterfaces {
+		if gotypes.Implements(t, interfaceType) {
+			return true
+		}
+	}
+	return false
+}
+
+var tbInterfaces = []*gotypes.Interface{
+	tbInterface("Error", false),
+}
+
+func tbInterface(name string, printf bool) *gotypes.Interface {
+	var params []*gotypes.Var
+	if printf {
+		params = append(params, gotypes.NewVar(0, nil, "", gotypes.Typ[gotypes.String]))
+	}
+	params = append(params, gotypes.NewVar(0, nil, "", gotypes.NewSlice(gotypes.Universe.Lookup("any").Type())))
+	signature := gotypes.NewSignatureType(nil, nil, nil,
+		gotypes.NewTuple(params...),
+		gotypes.NewTuple(),
+		true,
+	)
+	method := gotypes.NewFunc(0, nil, name, signature)
+	return gotypes.NewInterface([]*gotypes.Func{method}, nil)
+}

internal/rules/asyncsucceedrule.go

@@ -17,14 +17,17 @@ func (AsyncSucceedRule) isApply(gexp *expression.GomegaExpression) bool {
 	return gexp.IsAsync() &&
 		gexp.MatcherTypeIs(matcher.SucceedMatcherType) &&
 		gexp.ActualArgTypeIs(actual.FuncSigArgType) &&
-		!gexp.ActualArgTypeIs(actual.ErrorTypeArgType|actual.GomegaParamArgType)
+		!gexp.ActualArgTypeIs(actual.ErrorTypeArgType|actual.GomegaParamArgType|actual.TBParamArgType)
 }
 
 func (r AsyncSucceedRule) Apply(gexp *expression.GomegaExpression, _ config.Config, reportBuilder *reports.Builder) bool {
 	if r.isApply(gexp) {
 		if gexp.ActualArgTypeIs(actual.MultiRetsArgType) {
 			reportBuilder.AddIssue(false, "Success matcher does not support multiple values")
 		} else {
+			// The message intentionally does not call out "function with a TB implementation" as another alternative because
+			// that alternative is not valid for generic Gomega - it would be confusing for many users. Users
+			// of a Gomega wrapper which supports such functions must figure that out themselves.
 			reportBuilder.AddIssue(false, "Success matcher only support a single error value, or function with Gomega as its first parameter")
 		}
 	}

testdata/src/a/eventually/eventuallyFunc.go

@@ -62,11 +62,14 @@ var _ = Describe("Eventually with function", func() {
 
 		Eventually(retFunc(5)).Should(Equal(5)) // valid. retFunc returns a function
 		ch := make(chan int)
-		Eventually(ch).Should(BeClosed())                                                                                        // valid
-		Eventually(slowInt).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))                   // valid
-		Eventually(slowInt).WithPolling(time.Second * 2).WithTimeout(time.Millisecond * 100).Should(Equal(42))                   // not valid, if validate-async-intervals flag is set
-		Eventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))                 // want `ginkgo-linter: use a function call in Eventually\. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .Eventually\(slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`
-		wrappers.MyEventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))      // want `ginkgo-linter: use a function call in MyEventually\. This actually checks nothing, because MyEventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .wrappers.MyEventually\(slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`
+		Eventually(ch).Should(BeClosed())                                                                                   // valid
+		Eventually(slowInt).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))              // valid
+		Eventually(slowInt).WithPolling(time.Second * 2).WithTimeout(time.Millisecond * 100).Should(Equal(42))              // not valid, if validate-async-intervals flag is set
+		Eventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42))            // want `ginkgo-linter: use a function call in Eventually\. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .Eventually\(slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`
+		wrappers.MyEventually(slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42)) // want `ginkgo-linter: use a function call in MyEventually\. This actually checks nothing, because MyEventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .wrappers.MyEventually\(slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`
+		Eventually(func() {}).Should(Succeed())                                                                             // want `ginkgo-linter: Success matcher only support a single error value, or function with Gomega as its first parameter`
+		wrappers.MyEventually(func() {}).Should(Succeed())                                                                  // want `ginkgo-linter: Success matcher only support a single error value, or function with Gomega as its first parameter`
+		wrappers.MyEventually(func(tb wrappers.MyTB) {}).Should(Succeed())
 		Eventually(context.TODO(), slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42)) // want `ginkgo-linter: use a function call in Eventually\. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .Eventually\(context\.TODO\(\), slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`
 		ctx := context.TODO()
 		Eventually(ctx, slowInt()).WithPolling(time.Millisecond * 100).WithTimeout(time.Second * 2).Should(Equal(42)) // want `ginkgo-linter: use a function call in Eventually\. This actually checks nothing, because Eventually receives the function returned value, instead of function itself, and this value is never changed\. Consider using .Eventually\(ctx, slowInt\)\.WithPolling\(time\.Millisecond \* 100\)\.WithTimeout\(time\.Second \* 2\)\.Should\(Equal\(42\)\). instead`

testdata/src/a/wrappers/wrappers.go

@@ -1,17 +1,30 @@
 package wrappers
 
 import (
+	"testing"
+
 	gtypes "github.com/onsi/gomega/types"
 
 	g "github.com/onsi/gomega"
 )
 
+type MyTB interface {
+	Error(args ...any)
+	Errorf(format string, args ...any)
+	Fatal(args ...any)
+	Fatalf(format string, args ...any)
+}
+
+var _ MyTB = &testing.T{}
+var _ MyTB = &testing.B{}
+
 var Expect = g.Expect
 
 func MyExpect(value any) g.Assertion {
 	return g.Expect(value)
 }
 
+// value = func(tb MyTB) without return value is okay, failures can be reported via tb.Error/Fatal/Errorf/Fatalf.
 func MyEventually(value any) g.AsyncAssertion {
 	return g.Eventually(value)
 }