Proposal: guard or support comparing with untyped nil

[Antonboom]

Proposal: guard or support comparing with untyped nil

[brackendawson]

Do you mean to guard against users passing an untyped nil or guard against users typing a literal nil? Because the first thing that happens in current testify is the compiler infers that the type of the literal nil is <nil>. At runtime we can't differentiat the source code nil from the source code interface{}(nil). So there's nothing testify can do to stop users typing nil.

If you mean to guard against users comparing untyped nil (interface{}(nil)) with typed nil interfaces (eg: error(nil)), then you can't because any nil interface converted to interface{} becomes untyped nil. Even this example misbehaves:

	assert.Equal(t, error(nil), fmt.GoStringer(nil)) // true !

We could "fix" these cases using type parameters, compilation would fail if you tried to compare interface{}(nil) with error(nil) (or any other interface type). But literal nil would still break it, the type is inferred at compile time.

Making Equal generic may not even be a breaking change (can someone please prove me wrong on that?). But I hesitate to say this issue justifies doing that if the users can break it again by just typing nil.

[Antonboom]

guard against users typing a literal nil

Please, look at my examples above. With typed nil everything is ok (if I am not wrong, because I confused with your snippet 🙂).

At runtime we can't differentiat

Yep, I told about untyped nil literal.
And I fill that we can reuse some code from Nil assertion, is not it?

[brackendawson]

With typed nil everything is ok

If I understand you correctly, that's not true. Typed nil does not work with nil interfaces, as in my example. I don't know why you omitted nilInterface from your "The right way is to use" section?

There's definitely issues here, but I think most are limitations. Can you show me just one example of incorrect behaviour, and state what the behaviour should be?

[Antonboom]

I don't know why you omitted nilInterface

because comparison with both nils works fine for interface (look at the comment near nilInterface asserts).

Can you show me just one example of incorrect behaviour, and state what the behaviour should be?

sure

https://go.dev/play/p/hl1CMwT2921

var nilChan chan struct{}

fmt.Println(nilChan == nil)                  // true, OK
fmt.Println(nilChan == (chan struct{})(nil)) // true, OK

fmt.Println(assert.Equal(t, nil, nilChan))       // false, MUST BE TRUE
fmt.Println(assert.EqualValues(t, nil, nilChan)) // false, MUST BE TRUE
fmt.Println(assert.Exactly(t, nil, nilChan))     // false (Types expected to match exactly), OK

fmt.Println(assert.NotEqual(t, nil, nilChan))       // true, MUST BE FALSE
fmt.Println(assert.NotEqualValues(t, nil, nilChan)) // true, MUST BE FALSE

fmt.Println(assert.Equal(t, (chan struct{})(nil), nilChan))          // true, OK
fmt.Println(assert.EqualValues(t, (chan struct{})(nil), nilChan))    // true, OK
fmt.Println(assert.Exactly(t, (chan struct{})(nil), nilChan))        // true, OK
fmt.Println(assert.NotEqual(t, (chan struct{})(nil), nilChan))       // false, OK
fmt.Println(assert.NotEqualValues(t, (chan struct{})(nil), nilChan)) // false, OK

fmt.Println(assert.Nil(t, nilChan))    // true, OK
fmt.Println(assert.NotNil(t, nilChan)) // false, OK

[brackendawson]

Those assertions are all behaving correctly.

For the reasons I gave above, these three calls to Equal are identical at runtime:

var nilChan chan struct{}
var nilInterface interface{}
assert.Equal(t, nil, nilChan)
assert.Equal(t, interface{}(nil), nilChan)
assert.Equal(t, nilInterface, nilChan)

When a programmer passes the literal nil into a parameter of type interface{}; they are asking the compiler to create a interface{}(nil) for them, this is not considered Equal to a chan struct{}(nil) by testify or by reflect.DeepEqual. Testify is working correctly.

If the programmer intended to assert that it was a nil chan then they should have written one of these:

assert.Equal(t, chan struct{}(nil), nilChan)
assert.Nil(t, nilChan)

Checking that the user is using the compiler properly is the job of a linter, and testifylint does an excellent job in this case.

Can we do anything?

Equal

There is no way that testify can ever know at runtime if the user wrote a literal in the source. Testify could solve this on Equal by using type parameters to ensure both arguments are the same type at compile time, a bit like:

func TestIfy(t *testing.T) {
	var nilChan chan struct{}
	Equal(t, nil, nilChan) // now true because type inference decided the literal `nil` must be type `chan struct{}`
}

func Equal[T any](t assert.TestingT, expected, actual T, msgAndArgs ...interface{}) bool {
	return assert.Equal(t, expected, actual, msgAndArgs...)
}

But we can't control the error message which the compiler spits out if you do it wrong:

func TestIfy(t *testing.T) {
	var nilChan chan struct{}
	var nilInterface interface{}
	Equal(t, nilInterface, nilChan)
}

# github.com/brackendawson/kata_test [github.com/brackendawson/kata.test]
/Users/bracken/src/github.com/brackendawson/kata/kata_test.go:12:25: type chan struct{} of nilChan does not match inferred type interface{} for T
FAIL	github.com/brackendawson/kata [build failed]
FAIL

Which is kind of saying "I expected an interface{} but you gave me a chan struct{}", because type inference picked the first argument.

EqualValues

EqualValues can't use type parameters to enforce the types being the same because they aren't supposed to be. According to the docs:

EqualValues asserts that two objects are equal or convertable to the same types and equal.

interface{} is not convertible to chan struct{} or visa versa because interfaces are not types they are type sets.

EqualValues is already behaving correctly.