Skip to content

Commit

Permalink
SA1032: add check for errors.Is and errors.As argument order
Browse files Browse the repository at this point in the history
Closes: gh-1530
Closes: gh-1545 [via git-merge-pr]
Co-authored-by: Dominik Honnef <[email protected]>
  • Loading branch information
arp242 and dominikh committed Jun 16, 2024
1 parent 3845421 commit 19ed8fa
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 0 deletions.
2 changes: 2 additions & 0 deletions knowledge/arg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions staticcheck/analysis.go

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

62 changes: 62 additions & 0 deletions staticcheck/sa1032/sa1032.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
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.RawDocumentation{
Title: `Wrong order of arguments to \'errors.Is\' or \'errors.As\'`,
Text: `
The first argument of the functions \'errors.As\' and \'errors.Is\' is the error
that we have and the second argument is the error we're trying to match against.
For example:
if errors.Is(err, io.EOF) { ... }
This check detects some cases where the two arguments have been swapped. It
flags any calls where the first argument is referring to a package-level error
variable, such as
if errors.Is(io.EOF, err) { /* this is wrong */ }`,
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")
}
}
13 changes: 13 additions & 0 deletions staticcheck/sa1032/sa1032_test.go

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

Original file line number Diff line number Diff line change
@@ -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`)
}
}

0 comments on commit 19ed8fa

Please sign in to comment.