diff --git a/knowledge/arg.go b/knowledge/arg.go index 4fab2eba..8a154860 100644 --- a/knowledge/arg.go +++ b/knowledge/arg.go @@ -22,6 +22,8 @@ var Args = map[string]int{ "(*encoding/json.Encoder).Encode.v": 0, "(*encoding/xml.Decoder).Decode.v": 0, "(*encoding/xml.Encoder).Encode.v": 0, + "errors.As.err": 0, + "errors.Is.err": 0, "errors.New.text": 0, "fmt.Fprintf.format": 1, "fmt.Printf.format": 0, diff --git a/staticcheck/analysis.go b/staticcheck/analysis.go index cce61975..b1bd8f77 100644 --- a/staticcheck/analysis.go +++ b/staticcheck/analysis.go @@ -34,6 +34,7 @@ import ( "honnef.co/go/tools/staticcheck/sa1029" "honnef.co/go/tools/staticcheck/sa1030" "honnef.co/go/tools/staticcheck/sa1031" + "honnef.co/go/tools/staticcheck/sa1032" "honnef.co/go/tools/staticcheck/sa2000" "honnef.co/go/tools/staticcheck/sa2001" "honnef.co/go/tools/staticcheck/sa2002" @@ -130,6 +131,7 @@ var Analyzers = []*lint.Analyzer{ sa1029.SCAnalyzer, sa1030.SCAnalyzer, sa1031.SCAnalyzer, + sa1032.SCAnalyzer, sa2000.SCAnalyzer, sa2001.SCAnalyzer, sa2002.SCAnalyzer, diff --git a/staticcheck/sa1032/sa1032.go b/staticcheck/sa1032/sa1032.go new file mode 100644 index 00000000..736b26d7 --- /dev/null +++ b/staticcheck/sa1032/sa1032.go @@ -0,0 +1,55 @@ +package sa1032 + +import ( + "honnef.co/go/tools/analysis/callcheck" + "honnef.co/go/tools/analysis/lint" + "honnef.co/go/tools/go/ir" + "honnef.co/go/tools/internal/passes/buildir" + "honnef.co/go/tools/knowledge" + + "golang.org/x/tools/go/analysis" +) + +var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{ + Analyzer: &analysis.Analyzer{ + Name: "SA1032", + Requires: []*analysis.Analyzer{buildir.Analyzer}, + Run: callcheck.Analyzer(rules), + }, + Doc: &lint.Documentation{ + Title: `Swapped arguments in errors.Is() or errors.As()`, + Text: ` +If the first argument to \'errors.As()\' or \'errors.Is()\' contains a package +selector it assumes the error and target were swapped, since the target should +practically always be a local variable and not a reference to another package. +`, + Since: "Unreleased", + Severity: lint.SeverityError, + MergeIf: lint.MergeIfAny, + }, +}) + +var Analyzer = SCAnalyzer.Analyzer + +var rules = map[string]callcheck.Check{ + "errors.As": func(call *callcheck.Call) { + validateIs(call.Pass.Pkg.Path(), call.Args[knowledge.Arg("errors.As.err")]) + }, + "errors.Is": func(call *callcheck.Call) { + validateIs(call.Pass.Pkg.Path(), call.Args[knowledge.Arg("errors.Is.err")]) + }, +} + +func validateIs(curPkg string, arg *callcheck.Argument) { + v, ok := arg.Value.Value.(*ir.Load) + if !ok { + return + } + g, ok := v.X.(*ir.Global) + if !ok { + return + } + if pkg := g.Package().Pkg; pkg != nil && pkg.Path() != curPkg { + arg.Invalid("arguments have the wrong order") + } +} diff --git a/staticcheck/sa1032/sa1032_test.go b/staticcheck/sa1032/sa1032_test.go new file mode 100644 index 00000000..5b2c7994 --- /dev/null +++ b/staticcheck/sa1032/sa1032_test.go @@ -0,0 +1,13 @@ +// Code generated by generate.go. DO NOT EDIT. + +package sa1032 + +import ( + "testing" + + "honnef.co/go/tools/analysis/lint/testutil" +) + +func TestTestdata(t *testing.T) { + testutil.Run(t, SCAnalyzer) +} diff --git a/staticcheck/sa1032/testdata/go1.0/example.com/ErrorsOrder/ErrorsOrder.go b/staticcheck/sa1032/testdata/go1.0/example.com/ErrorsOrder/ErrorsOrder.go new file mode 100644 index 00000000..f659faa2 --- /dev/null +++ b/staticcheck/sa1032/testdata/go1.0/example.com/ErrorsOrder/ErrorsOrder.go @@ -0,0 +1,76 @@ +package errors + +import ( + "errors" + "io/fs" +) + +var gErr = errors.New("global") + +type myErr struct{} + +func (myErr) Error() string { return "" } + +func is() { + err := errors.New("oh noes") + + _ = errors.Is(err, fs.ErrNotExist) + _ = errors.Is(fs.ErrNotExist, err) //@ diag(`wrong order`) + if errors.Is(err, fs.ErrNotExist) { + } + if errors.Is(fs.ErrNotExist, err) { //@ diag(`wrong order`) + } + + _ = errors.Is(gErr, fs.ErrNotExist) + _ = errors.Is(fs.ErrNotExist, gErr) //@ diag(`wrong order`) + if errors.Is(gErr, fs.ErrNotExist) { + } + if errors.Is(fs.ErrNotExist, gErr) { //@ diag(`wrong order`) + } + + _ = errors.Is(myErr{}, fs.ErrNotExist) + _ = errors.Is(fs.ErrNotExist, myErr{}) //@ diag(`wrong order`) + if errors.Is(myErr{}, fs.ErrNotExist) { + } + if errors.Is(fs.ErrNotExist, myErr{}) { //@ diag(`wrong order`) + } + + _ = errors.Is(&myErr{}, fs.ErrNotExist) + _ = errors.Is(fs.ErrNotExist, &myErr{}) //@ diag(`wrong order`) + if errors.Is(&myErr{}, fs.ErrNotExist) { + } + if errors.Is(fs.ErrNotExist, &myErr{}) { //@ diag(`wrong order`) + } +} + +func as() { + err := errors.New("oh noes") + + _ = errors.As(err, fs.ErrNotExist) + _ = errors.As(fs.ErrNotExist, err) //@ diag(`wrong order`) + if errors.As(err, fs.ErrNotExist) { + } + if errors.As(fs.ErrNotExist, err) { //@ diag(`wrong order`) + } + + _ = errors.As(gErr, fs.ErrNotExist) + _ = errors.As(fs.ErrNotExist, gErr) //@ diag(`wrong order`) + if errors.As(gErr, fs.ErrNotExist) { + } + if errors.As(fs.ErrNotExist, gErr) { //@ diag(`wrong order`) + } + + _ = errors.As(myErr{}, fs.ErrNotExist) + _ = errors.As(fs.ErrNotExist, myErr{}) //@ diag(`wrong order`) + if errors.As(myErr{}, fs.ErrNotExist) { + } + if errors.As(fs.ErrNotExist, myErr{}) { //@ diag(`wrong order`) + } + + _ = errors.As(&myErr{}, fs.ErrNotExist) + _ = errors.As(fs.ErrNotExist, &myErr{}) //@ diag(`wrong order`) + if errors.As(&myErr{}, fs.ErrNotExist) { + } + if errors.As(fs.ErrNotExist, &myErr{}) { //@ diag(`wrong order`) + } +}