Skip to content

Commit

Permalink
error-nil: support Empty and Zero error (#153)
Browse files Browse the repository at this point in the history 
* error-nil: support empty and notempty error

* review fixes

* review fixes

---------

Co-authored-by: Anton Telyshev <[email protected]>
  • Loading branch information
@mmorel-35 @Antonboom
mmorel-35 and Antonboom authored Oct 3, 2024
1 parent 44419da commit ea890b3
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 14 deletions.
6 changes: 5 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,16 @@ logic, but without autofix.
```go
❌
assert.Nil(t, err)
assert.NotNil(t, err)
assert.Empty(t, err)
assert.Zero(t, err)
assert.Equal(t, nil, err)
assert.EqualValues(t, nil, err)
assert.Exactly(t, nil, err)
assert.ErrorIs(t, err, nil)

assert.NotNil(t, err)
assert.NotEmpty(t, err)
assert.NotZero(t, err)
assert.NotEqual(t, nil, err)
assert.NotEqualValues(t, nil, err)
assert.NotErrorIs(t, err, nil)
Expand Down
37 changes: 33 additions & 4 deletions analyzer/testdata/src/checkers-default/error-nil/error_nil_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 33 additions & 4 deletions analyzer/testdata/src/checkers-default/error-nil/error_nil_test.go.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ import (

func TestErrorNilChecker(t *testing.T) {
var err error
var errs []error

// Invalid.
{
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
Expand All @@ -39,8 +44,12 @@ func TestErrorNilChecker(t *testing.T) {
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.NoError(t, err) // want "error-nil: use assert\\.NoError"
assert.NoErrorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.NoErrorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
assert.Error(t, err) // want "error-nil: use assert\\.Error"
assert.Errorf(t, err, "msg with args %d %s", 42, "42") // want "error-nil: use assert\\.Errorf"
}
Expand All @@ -67,6 +76,26 @@ func TestErrorNilChecker(t *testing.T) {
assert.NotEqualf(t, err, err, "msg with args %d %s", 42, "42")
assert.NotEqual(t, nil, nil)
assert.NotEqualf(t, nil, nil, "msg with args %d %s", 42, "42")
assert.Empty(t, err.Error())
assert.Emptyf(t, err.Error(), "msg with args %d %s", 42, "42")
assert.NotEmpty(t, err.Error())
assert.NotEmptyf(t, err.Error(), "msg with args %d %s", 42, "42")
assert.Zero(t, err.Error())
assert.Zerof(t, err.Error(), "msg with args %d %s", 42, "42")
assert.NotZero(t, err.Error())
assert.NotZerof(t, err.Error(), "msg with args %d %s", 42, "42")
assert.Nil(t, errs)
assert.Nilf(t, errs, "msg with args %d %s", 42, "42")
assert.NotNil(t, errs)
assert.NotNilf(t, errs, "msg with args %d %s", 42, "42")
assert.Empty(t, errs)
assert.Emptyf(t, errs, "msg with args %d %s", 42, "42")
assert.NotEmpty(t, errs)
assert.NotEmptyf(t, errs, "msg with args %d %s", 42, "42")
assert.Zero(t, errs)
assert.Zerof(t, errs, "msg with args %d %s", 42, "42")
assert.NotZero(t, errs)
assert.NotZerof(t, errs, "msg with args %d %s", 42, "42")
}
}

Expand Down
10 changes: 7 additions & 3 deletions internal/checkers/error_nil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ import (
// ErrorNil detects situations like
//
// assert.Nil(t, err)
// assert.NotNil(t, err)
// assert.Empty(t, err)
// assert.Zero(t, err)
// assert.Equal(t, nil, err)
// assert.EqualValues(t, nil, err)
// assert.Exactly(t, nil, err)
// assert.ErrorIs(t, err, nil)
//
// assert.NotNil(t, err)
// assert.NotEmpty(t, err)
// assert.NotZero(t, err)
// assert.NotEqual(t, nil, err)
// assert.NotEqualValues(t, nil, err)
// assert.NotErrorIs(t, err, nil)
Expand All @@ -40,12 +44,12 @@ func (checker ErrorNil) Check(pass *analysis.Pass, call *CallMeta) *analysis.Dia

proposedFn, survivingArg, replacementEndPos := func() (string, ast.Expr, token.Pos) {
switch call.Fn.NameFTrimmed {
case "Nil":
case "Nil", "Empty", "Zero":
if len(call.Args) >= 1 && isError(pass, call.Args[0]) {
return noErrorFn, call.Args[0], call.Args[0].End()
}

case "NotNil":
case "NotNil", "NotEmpty", "NotZero":
if len(call.Args) >= 1 && isError(pass, call.Args[0]) {
return errorFn, call.Args[0], call.Args[0].End()
}
Expand Down
22 changes: 20 additions & 2 deletions internal/testgen/gen_error_nil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,23 @@ func (g ErrorNilTestsGenerator) TemplateData() any {
},
InvalidAssertions: []Assertion{
{Fn: "Nil", Argsf: "err", ReportMsgf: report, ProposedFn: "NoError"},
{Fn: "NotNil", Argsf: "err", ReportMsgf: report, ProposedFn: "Error"},
{Fn: "Empty", Argsf: "err", ReportMsgf: report, ProposedFn: "NoError"},
{Fn: "Zero", Argsf: "err", ReportMsgf: report, ProposedFn: "NoError"},
{Fn: "Equal", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "Equal", Argsf: "nil, err", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "EqualValues", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "EqualValues", Argsf: "nil, err", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "Exactly", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "Exactly", Argsf: "nil, err", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "ErrorIs", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},

{Fn: "NotNil", Argsf: "err", ReportMsgf: report, ProposedFn: "Error"},
{Fn: "NotEmpty", Argsf: "err", ReportMsgf: report, ProposedFn: "Error"},
{Fn: "NotZero", Argsf: "err", ReportMsgf: report, ProposedFn: "Error"},
{Fn: "NotEqual", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "Error", ProposedArgsf: "err"},
{Fn: "NotEqual", Argsf: "nil, err", ReportMsgf: report, ProposedFn: "Error", ProposedArgsf: "err"},
{Fn: "NotEqualValues", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "Error", ProposedArgsf: "err"},
{Fn: "NotEqualValues", Argsf: "nil, err", ReportMsgf: report, ProposedFn: "Error", ProposedArgsf: "err"},
{Fn: "ErrorIs", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "NoError", ProposedArgsf: "err"},
{Fn: "NotErrorIs", Argsf: "err, nil", ReportMsgf: report, ProposedFn: "Error", ProposedArgsf: "err"},
},
ValidAssertions: []Assertion{
Expand All @@ -76,6 +81,18 @@ func (g ErrorNilTestsGenerator) TemplateData() any {
{Fn: "Equal", Argsf: "nil, nil"},
{Fn: "NotEqual", Argsf: "err, err"},
{Fn: "NotEqual", Argsf: "nil, nil"},

{Fn: "Empty", Argsf: "err.Error()"},
{Fn: "NotEmpty", Argsf: "err.Error()"},
{Fn: "Zero", Argsf: "err.Error()"},
{Fn: "NotZero", Argsf: "err.Error()"},

{Fn: "Nil", Argsf: "errs"},
{Fn: "NotNil", Argsf: "errs"},
{Fn: "Empty", Argsf: "errs"},
{Fn: "NotEmpty", Argsf: "errs"},
{Fn: "Zero", Argsf: "errs"},
{Fn: "NotZero", Argsf: "errs"},
},
}
}
Expand Down Expand Up @@ -106,6 +123,7 @@ import (
func {{ .CheckerName.AsTestName }}(t *testing.T) {
var err error
var errs []error
// Invalid.
{
Expand Down

0 comments on commit ea890b3

Please sign in to comment.