error-nil: only fix when the 'expected' arg is nil

[dolmen]

error-nil now fixes only when the 'expected' argument is nil. The cases where 'expected' and 'actual' are inversed must be left for handling by another checker (especially because nil is handled differently by Equal/Exactly/EqualValues).

Also add more comprehensive test cases for Exactly, EqualValues, NotEqualValues.

[Antonboom]

Hi, @dolmen!

Thank you for contribution.

must be left for handling by another checker

Another checkers is not always a reason for this, because they could be disabled by config.
And some linters do extra work in case of obvious mistake, e.g. len and

assert.Equal(t, len(arr), 3)    // Reversed but covered by checker.

Why assert.Equal(t, err, nil) (and similar) is not obvious mistake and should not be covered?

[dolmen]

Testify is full of traps if you reverse the expected and actual arguments. That's why I choose long ago to avoid this package in my own projects because too often the method names are misleading. That's also why I find your work on this linter of a high value for the Testify's users community.

I had a deep look at assert's code, and I found no expected vs actual trap for the nil case (because nil is handled specially).

However assert.Equal (and family) has a different behavior for this kind of error:

type errorFunc func()

func (errorFunc) Error() string {
	return "error"
}

See on the Go Playground:

var err error
assert.Equal(t, err, nil)
assert.Equal(t, nil, err)
assert.NoError(t, err)

err = errors.New("error1")
assert.NotEqual(t, err, nil)
assert.NotEqual(t, nil, err)
assert.Error(t, err)

err = errorFunc(nil)
assert.NotEqual(t, err, nil)
assert.NotEqual(t, nil, err)
assert.Error(t, err)

[Antonboom]

However assert.Equal (and family) has a different behavior for this kind of error:

Anyway linter suggest to replace it with Error/NoError that works correct (based on plyaground).
No issues here?

I had a deep look at assert's code, and I found no expected vs actual trap for the nil case

In this case, could you rollback related changes?

because nil is handled specially

Please, take a look at #36 (comment)

[Antonboom]

@dolmen

However assert.Equal (and family) has a different behavior for this kind of error:

IMO, your snippet is another yet example of misusage of testify.
Equal-based assertions could not be used with functions (according to validateEqualArgs).

Maybe it also related to stretchr/testify#1524

Thanks, but closed 🤝