nil-compare: false positives for `assert.Equal(t, nil, value)` (slice, channel)

[dolmen]

assert.Equal(t, nil, []int(nil)) can't be replaced with assert.Nil(t, []int(nil)): the behaviour of testify is different, so nil-compare should not replace it. Same issue for assert.Equal(t, nil, value) if value is of type chan struct{}.

See on the Go Playground: https://go.dev/play/p/jUxLF4j-w89

Note: I found this issue when applying testifylint on package github.com/stretchr/testify/mock: https://github.com/stretchr/testify/blob/db8608ed63f5bd9abdd03f669f5bb80569c114b6/mock/mock_test.go#L765

[dolmen]

I wonder if the testsuite has a check that really executes the original test and the "fixed" test to verify that the behavior of testify is the same. @Antonboom If that check exists, could you point me to it?

[Antonboom]

@dolmen, hi!

If that check exists, could you point me to it?

Before I just checked in playground, but from this issue I introduced some code (show you later).

About issue. It looks like Equal bug:

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

Equal is not consistent with Go ==:

var nilChan chan struct{}
fmt.Println(nilChan == nil)           // true
fmt.Println(any(nilChan) == any(nil)) // false

Also it affects assert.EqualValues(tm, nilChan, nil), because I expect this assertion passed, but this failed.

[dolmen]

Whether there is a bug in Testify or not (that might be fixed in the future), testifylint should aim to work with Testify as it is now and even as it was in the past, because we can expect that users are applying (and will continue to apply) testifylint on codebases that still run older versions of Testify.

[Antonboom]

@dolmen

testifylint should aim to work with Testify as it is now

Totally agree, but not in this case. I dove deeper into your example and it's really a misuse of testify itself. I described details here – https://github.com/stretchr/testify/issues/1524

and even as it was in the past

It could be difficult, because some checkers use only fresh testify functions and features. there is like in Go – support of the two last versions of testify 🙁

Thanks for issue! 🤝