testifylint->nil-compare - Should we enable it?

[zak-pawel]

Description

This issue starts a discussion about enabling:

• linter: testifylint - Checks usage of github.com/stretchr/testify.
• checker: nil-compare - Protection from bugs and more appropriate testify API with clearer failure message.

Example

Before:

assert.Equal(t, nil, value)
assert.EqualValues(t, nil, value)
assert.Exactly(t, nil, value)

assert.NotEqual(t, nil, value)
assert.NotEqualValues(t, nil, value)

After:

assert.Nil(t, value)
assert.NotNil(t, value)

Using untyped nil in the functions above along with a non-interface type does not make sense:

assert.Equal(t, nil, eventsChan)    // Always fail.
assert.NotEqual(t, nil, eventsChan) // Always pass.

The right way:

assert.Equal(t, (chan Event)(nil), eventsChan)
assert.NotEqual(t, (chan Event)(nil), eventsChan)

But in the case of Equal, NotEqual and Exactly type casting approach still doesn't work for the function type.
The best option here is to just use Nil / NotNil (see stretchr/testify#1524).

Expected output

Decision about enabling or not enabling this checker.

Findings

For this checker, the following findings were found in the current codebase:

plugins/inputs/intel_dlb/intel_dlb_test.go:424:3       testifylint  nil-compare: use require.Nil
plugins/outputs/postgresql/table_source_test.go:280:2  testifylint  nil-compare: use require.Nil
plugins/outputs/postgresql/table_source_test.go:283:2  testifylint  nil-compare: use require.Nil

Additional configuration

For this checker, no additional configuration can be provided.

[srebhan]

I'm absolutely in favor of this one! This makes tests much more readable I think.

[powersj]

+1