Proposal: guard or support comparing with untyped nil

[Antonboom]

Hi!

The using of ObjectsAreEqual-based functions with untyped nil doesn't make sense for any non-interface types, because of expected and actual are interfaces:

func ObjectsAreEqual(expected, actual interface{}) bool {
	if expected == nil || actual == nil {
		return expected == actual   // You cannot do this, because 
		                            // expected and actual are interfaces
	}

As a consequence, Equal, EqualValues and Exactly will always fail:

    fmt.Println(assert.Equal(t, nil, nilChan))          // false
    fmt.Println(assert.Equal(t, nil, nilFunc))          // false, cannot take func type as argument
    fmt.Println(assert.Equal(t, nil, nilInterface))     // true   !
    fmt.Println(assert.Equal(t, nil, nilMap))           // false
    fmt.Println(assert.Equal(t, nil, nilPointer))       // false
    fmt.Println(assert.Equal(t, nil, nilSlice))         // false
    fmt.Println(assert.Equal(t, nil, nilUnsafePointer)) // false

    fmt.Println(assert.EqualValues(t, nil, nilChan))          // false
    fmt.Println(assert.EqualValues(t, nil, nilFunc))          // false
    fmt.Println(assert.EqualValues(t, nil, nilInterface))     // true   !
    fmt.Println(assert.EqualValues(t, nil, nilMap))           // false
    fmt.Println(assert.EqualValues(t, nil, nilPointer))       // false
    fmt.Println(assert.EqualValues(t, nil, nilSlice))         // false
    fmt.Println(assert.EqualValues(t, nil, nilUnsafePointer)) // false

    fmt.Println(assert.Exactly(t, nil, nilChan))          // false, Types expected to match exactly
    fmt.Println(assert.Exactly(t, nil, nilFunc))          // false, Types expected to match exactly
    fmt.Println(assert.Exactly(t, nil, nilInterface))     // true   !
    fmt.Println(assert.Exactly(t, nil, nilMap))           // false, Types expected to match exactly
    fmt.Println(assert.Exactly(t, nil, nilPointer))       // false, Types expected to match exactly
    fmt.Println(assert.Exactly(t, nil, nilSlice))         // false, Types expected to match exactly
    fmt.Println(assert.Exactly(t, nil, nilUnsafePointer)) // false, Types expected to match exactly

And NotEqual and NotEqualValues will always pass:

    fmt.Println(assert.NotEqual(t, nil, nilChan))          // true
    fmt.Println(assert.NotEqual(t, nil, nilFunc))          // false, cannot take func type as argument
    fmt.Println(assert.NotEqual(t, nil, nilInterface))     // false  !
    fmt.Println(assert.NotEqual(t, nil, nilMap))           // true
    fmt.Println(assert.NotEqual(t, nil, nilPointer))       // true
    fmt.Println(assert.NotEqual(t, nil, nilSlice))         // true
    fmt.Println(assert.NotEqual(t, nil, nilUnsafePointer)) // true

    fmt.Println(assert.NotEqualValues(t, nil, nilChan))          // true
    fmt.Println(assert.NotEqualValues(t, nil, nilFunc))          // true
    fmt.Println(assert.NotEqualValues(t, nil, nilInterface))     // false  !
    fmt.Println(assert.NotEqualValues(t, nil, nilMap))           // true
    fmt.Println(assert.NotEqualValues(t, nil, nilPointer))       // true
    fmt.Println(assert.NotEqualValues(t, nil, nilSlice))         // true
    fmt.Println(assert.NotEqualValues(t, nil, nilUnsafePointer)) // true

The right way is to use typed nil:

    fmt.Println(assert.Equal(t, (chan struct{})(nil), nilChan))           // true
    fmt.Println(assert.Equal(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
    fmt.Println(assert.Equal(t, (map[int]int)(nil), nilMap))              // true
    fmt.Println(assert.Equal(t, (*int)(nil), nilPointer))                 // true
    fmt.Println(assert.Equal(t, []int(nil), nilSlice))                    // true
    fmt.Println(assert.Equal(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true

    fmt.Println(assert.EqualValues(t, (chan struct{})(nil), nilChan))           // true
    fmt.Println(assert.EqualValues(t, (func())(nil), nilFunc))                  // true
    fmt.Println(assert.EqualValues(t, (map[int]int)(nil), nilMap))              // true
    fmt.Println(assert.EqualValues(t, (*int)(nil), nilPointer))                 // true
    fmt.Println(assert.EqualValues(t, []int(nil), nilSlice))                    // true
    fmt.Println(assert.EqualValues(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true
    
    fmt.Println(assert.Exactly(t, (chan struct{})(nil), nilChan))           // true
    fmt.Println(assert.Exactly(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
    fmt.Println(assert.Exactly(t, (map[int]int)(nil), nilMap))              // true
    fmt.Println(assert.Exactly(t, (*int)(nil), nilPointer))                 // true
    fmt.Println(assert.Exactly(t, []int(nil), nilSlice))                    // true
    fmt.Println(assert.Exactly(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true

    fmt.Println(assert.NotEqual(t, (chan struct{})(nil), nilChan))           // false
    fmt.Println(assert.NotEqual(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
    fmt.Println(assert.NotEqual(t, (map[int]int)(nil), nilMap))              // false
    fmt.Println(assert.NotEqual(t, (*int)(nil), nilPointer))                 // false
    fmt.Println(assert.NotEqual(t, []int(nil), nilSlice))                    // false
    fmt.Println(assert.NotEqual(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // false

    fmt.Println(assert.NotEqualValues(t, (chan struct{})(nil), nilChan))           // false
    fmt.Println(assert.NotEqualValues(t, (func())(nil), nilFunc))                  // false
    fmt.Println(assert.NotEqualValues(t, (map[int]int)(nil), nilMap))              // false
    fmt.Println(assert.NotEqualValues(t, (*int)(nil), nilPointer))                 // false
    fmt.Println(assert.NotEqualValues(t, []int(nil), nilSlice))                    // false
    fmt.Println(assert.NotEqualValues(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // false

But this is verbose and we also see inconsistency of functions when working with nilFunc.

Better to just use Nil/NotNil:

    fmt.Println(nilChan == nil)          // true
    fmt.Println(nilFunc == nil)          // true
    fmt.Println(nilInterface == nil)     // true
    fmt.Println(nilMap == nil)           // true
    fmt.Println(nilPointer == nil)       // true
    fmt.Println(nilSlice == nil)         // true
    fmt.Println(nilUnsafePointer == nil) // true

    fmt.Println(assert.Nil(t, nilChan))          // true
    fmt.Println(assert.Nil(t, nilFunc))          // true
    fmt.Println(assert.Nil(t, nilInterface))     // true
    fmt.Println(assert.Nil(t, nilMap))           // true
    fmt.Println(assert.Nil(t, nilPointer))       // true
    fmt.Println(assert.Nil(t, nilSlice))         // true
    fmt.Println(assert.Nil(t, nilUnsafePointer)) // true

    fmt.Println(assert.NotNil(t, nilChan))          // false
    fmt.Println(assert.NotNil(t, nilFunc))          // false
    fmt.Println(assert.NotNil(t, nilInterface))     // false
    fmt.Println(assert.NotNil(t, nilMap))           // false
    fmt.Println(assert.NotNil(t, nilPointer))       // false
    fmt.Println(assert.NotNil(t, nilSlice))         // false
    fmt.Println(assert.NotNil(t, nilUnsafePointer)) // false

Full snippet:
https://go.dev/play/p/ClGstUJWkYB

package main

import (
	"fmt"
	"testing"
	"unsafe"

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

func Test(t *testing.T) {
	var (
		nilChan          chan struct{}
		nilFunc          func()
		nilInterface     any
		nilMap           map[int]int
		nilPointer       *int
		nilSlice         []int
		nilUnsafePointer unsafe.Pointer
	)

	fmt.Println("\nEqual")
	fmt.Println(assert.Equal(t, nil, nilChan))          // false
	fmt.Println(assert.Equal(t, nil, nilFunc))          // false, cannot take func type as argument
	fmt.Println(assert.Equal(t, nil, nilInterface))     // true   !
	fmt.Println(assert.Equal(t, nil, nilMap))           // false
	fmt.Println(assert.Equal(t, nil, nilPointer))       // false
	fmt.Println(assert.Equal(t, nil, nilSlice))         // false
	fmt.Println(assert.Equal(t, nil, nilUnsafePointer)) // false

	fmt.Println("\nEqualValues")
	fmt.Println(assert.EqualValues(t, nil, nilChan))          // false
	fmt.Println(assert.EqualValues(t, nil, nilFunc))          // false
	fmt.Println(assert.EqualValues(t, nil, nilInterface))     // true   !
	fmt.Println(assert.EqualValues(t, nil, nilMap))           // false
	fmt.Println(assert.EqualValues(t, nil, nilPointer))       // false
	fmt.Println(assert.EqualValues(t, nil, nilSlice))         // false
	fmt.Println(assert.EqualValues(t, nil, nilUnsafePointer)) // false

	fmt.Println("\nExactly")
	fmt.Println(assert.Exactly(t, nil, nilChan))          // false, Types expected to match exactly
	fmt.Println(assert.Exactly(t, nil, nilFunc))          // false, Types expected to match exactly
	fmt.Println(assert.Exactly(t, nil, nilInterface))     // true   !
	fmt.Println(assert.Exactly(t, nil, nilMap))           // false, Types expected to match exactly
	fmt.Println(assert.Exactly(t, nil, nilPointer))       // false, Types expected to match exactly
	fmt.Println(assert.Exactly(t, nil, nilSlice))         // false, Types expected to match exactly
	fmt.Println(assert.Exactly(t, nil, nilUnsafePointer)) // false, Types expected to match exactly

	fmt.Println("\nNotEqual")
	fmt.Println(assert.NotEqual(t, nil, nilChan))          // true
	fmt.Println(assert.NotEqual(t, nil, nilFunc))          // false, cannot take func type as argument
	fmt.Println(assert.NotEqual(t, nil, nilInterface))     // false  !
	fmt.Println(assert.NotEqual(t, nil, nilMap))           // true
	fmt.Println(assert.NotEqual(t, nil, nilPointer))       // true
	fmt.Println(assert.NotEqual(t, nil, nilSlice))         // true
	fmt.Println(assert.NotEqual(t, nil, nilUnsafePointer)) // true

	fmt.Println("\nNotEqualValues")
	fmt.Println(assert.NotEqualValues(t, nil, nilChan))          // true
	fmt.Println(assert.NotEqualValues(t, nil, nilFunc))          // true
	fmt.Println(assert.NotEqualValues(t, nil, nilInterface))     // false  !
	fmt.Println(assert.NotEqualValues(t, nil, nilMap))           // true
	fmt.Println(assert.NotEqualValues(t, nil, nilPointer))       // true
	fmt.Println(assert.NotEqualValues(t, nil, nilSlice))         // true
	fmt.Println(assert.NotEqualValues(t, nil, nilUnsafePointer)) // true

	// ------------------------------------------------------------------------------------------

	fmt.Println("\nEqual with typed nil")
	fmt.Println(assert.Equal(t, (chan struct{})(nil), nilChan))           // true
	fmt.Println(assert.Equal(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
	fmt.Println(assert.Equal(t, (map[int]int)(nil), nilMap))              // true
	fmt.Println(assert.Equal(t, (*int)(nil), nilPointer))                 // true
	fmt.Println(assert.Equal(t, []int(nil), nilSlice))                    // true
	fmt.Println(assert.Equal(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true

	fmt.Println("\nEqualValues with typed nil")
	fmt.Println(assert.EqualValues(t, (chan struct{})(nil), nilChan))           // true
	fmt.Println(assert.EqualValues(t, (func())(nil), nilFunc))                  // true
	fmt.Println(assert.EqualValues(t, (map[int]int)(nil), nilMap))              // true
	fmt.Println(assert.EqualValues(t, (*int)(nil), nilPointer))                 // true
	fmt.Println(assert.EqualValues(t, []int(nil), nilSlice))                    // true
	fmt.Println(assert.EqualValues(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true

	fmt.Println("\nExactly with typed nil")
	fmt.Println(assert.Exactly(t, (chan struct{})(nil), nilChan))           // true
	fmt.Println(assert.Exactly(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
	fmt.Println(assert.Exactly(t, (map[int]int)(nil), nilMap))              // true
	fmt.Println(assert.Exactly(t, (*int)(nil), nilPointer))                 // true
	fmt.Println(assert.Exactly(t, []int(nil), nilSlice))                    // true
	fmt.Println(assert.Exactly(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // true

	fmt.Println("\nNotEqual with typed nil")
	fmt.Println(assert.NotEqual(t, (chan struct{})(nil), nilChan))           // false
	fmt.Println(assert.NotEqual(t, (func())(nil), nilFunc))                  // false, cannot take func type as argument
	fmt.Println(assert.NotEqual(t, (map[int]int)(nil), nilMap))              // false
	fmt.Println(assert.NotEqual(t, (*int)(nil), nilPointer))                 // false
	fmt.Println(assert.NotEqual(t, []int(nil), nilSlice))                    // false
	fmt.Println(assert.NotEqual(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // false

	fmt.Println("\nNotEqualValues with typed nil")
	fmt.Println(assert.NotEqualValues(t, (chan struct{})(nil), nilChan))           // false
	fmt.Println(assert.NotEqualValues(t, (func())(nil), nilFunc))                  // false
	fmt.Println(assert.NotEqualValues(t, (map[int]int)(nil), nilMap))              // false
	fmt.Println(assert.NotEqualValues(t, (*int)(nil), nilPointer))                 // false
	fmt.Println(assert.NotEqualValues(t, []int(nil), nilSlice))                    // false
	fmt.Println(assert.NotEqualValues(t, (unsafe.Pointer)(nil), nilUnsafePointer)) // false

	// ------------------------------------------------------------------------------------------

	fmt.Println("\nGO ==")
	fmt.Println(nilChan == nil)          // true
	fmt.Println(nilFunc == nil)          // true
	fmt.Println(nilInterface == nil)     // true
	fmt.Println(nilMap == nil)           // true
	fmt.Println(nilPointer == nil)       // true
	fmt.Println(nilSlice == nil)         // true
	fmt.Println(nilUnsafePointer == nil) // true

	fmt.Println("\nNil")
	fmt.Println(assert.Nil(t, nilChan))          // true
	fmt.Println(assert.Nil(t, nilFunc))          // true
	fmt.Println(assert.Nil(t, nilInterface))     // true
	fmt.Println(assert.Nil(t, nilMap))           // true
	fmt.Println(assert.Nil(t, nilPointer))       // true
	fmt.Println(assert.Nil(t, nilSlice))         // true
	fmt.Println(assert.Nil(t, nilUnsafePointer)) // true

	fmt.Println("\nNotNil")
	fmt.Println(assert.NotNil(t, nilChan))          // false
	fmt.Println(assert.NotNil(t, nilFunc))          // false
	fmt.Println(assert.NotNil(t, nilInterface))     // false
	fmt.Println(assert.NotNil(t, nilMap))           // false
	fmt.Println(assert.NotNil(t, nilPointer))       // false
	fmt.Println(assert.NotNil(t, nilSlice))         // false
	fmt.Println(assert.NotNil(t, nilUnsafePointer)) // false
}

---

Example of bug/typo in testify:

testify/mock/mock_test.go

Line 765 in db8608e

assert.NotEqual(t, nil, call.WaitFor)

(call.WaitFor is (<-chan time.Time)(nil)).

---

Covered by testifylint#nil-compare.

[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.