From d64847f1acae190202750856fa522991d86d07ed Mon Sep 17 00:00:00 2001 From: polyfloyd Date: Fri, 9 Sep 2022 14:00:33 +0200 Subject: [PATCH] Permit comparisons in `(*t).Is(error) bool` functions (fixes #11) --- errorlint/analysis.go | 15 +++++++++- errorlint/lint.go | 33 +++++++++++++++++++- errorlint/testdata/src/issues/github-11.go | 35 ++++++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 errorlint/testdata/src/issues/github-11.go diff --git a/errorlint/analysis.go b/errorlint/analysis.go index 58ddb26..c24d1f4 100644 --- a/errorlint/analysis.go +++ b/errorlint/analysis.go @@ -57,7 +57,7 @@ func run(pass *analysis.Pass) (interface{}, error) { type TypesInfoExt struct { types.Info - // Maps AST nodes back to the node they are contain within. + // Maps AST nodes back to the node they are contained within. NodeParent map[ast.Node]ast.Node // Maps an object back to all identifiers to refer to it. @@ -97,3 +97,16 @@ func newTypesInfoExt(info *types.Info) *TypesInfoExt { IdentifiersForObject: identifiersForObject, } } + +func (info *TypesInfoExt) ContainingFuncDecl(node ast.Node) *ast.FuncDecl { + for parent := info.NodeParent[node]; ; parent = info.NodeParent[parent] { + if _, ok := parent.(*ast.File); ok { + break + } + if fun, ok := parent.(*ast.FuncDecl); ok { + return fun + break + } + } + return nil +} diff --git a/errorlint/lint.go b/errorlint/lint.go index 5301a3f..52d7a82 100644 --- a/errorlint/lint.go +++ b/errorlint/lint.go @@ -156,10 +156,14 @@ func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []Lint { if !isErrorComparison(info.Info, binExpr) { continue } - + // Some errors that are returned from some functions are exempt. if isAllowedErrorComparison(info, binExpr) { continue } + // Comparisons that happen in `func (type) Is(error) bool` are okay. + if isNodeInErrorIsFunc(info, binExpr) { + continue + } lints = append(lints, Lint{ Message: fmt.Sprintf("comparing with %s will fail on wrapped errors. Use errors.Is to check for a specific error", binExpr.Op), @@ -181,6 +185,9 @@ func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []Lint { if tagType.Type.String() != "error" { continue } + if isNodeInErrorIsFunc(info, switchStmt) { + continue + } lints = append(lints, Lint{ Message: "switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors", @@ -207,6 +214,30 @@ func isErrorComparison(info types.Info, binExpr *ast.BinaryExpr) bool { return tx.Type.String() == "error" || ty.Type.String() == "error" } +func isNodeInErrorIsFunc(info *TypesInfoExt, node ast.Node) bool { + funcDecl := info.ContainingFuncDecl(node) + if funcDecl == nil { + return false + } + + if funcDecl.Name.Name != "Is" { + return false + } + if funcDecl.Recv == nil { + return false + } + // There should be 1 argument of type error. + if ii := funcDecl.Type.Params.List; len(ii) != 1 || info.Types[ii[0].Type].Type.String() != "error" { + return false + } + // The return type should be bool. + if ii := funcDecl.Type.Results.List; len(ii) != 1 || info.Types[ii[0].Type].Type.String() != "bool" { + return false + } + + return true +} + func LintErrorTypeAssertions(fset *token.FileSet, info types.Info) []Lint { lints := []Lint{} diff --git a/errorlint/testdata/src/issues/github-11.go b/errorlint/testdata/src/issues/github-11.go new file mode 100644 index 0000000..6d19ade --- /dev/null +++ b/errorlint/testdata/src/issues/github-11.go @@ -0,0 +1,35 @@ +package issues + +import ( + "errors" + "os" + "syscall" +) + +// Regression test for: https://github.com/polyfloyd/go-errorlint/issues/11 + +var ErrInvalidConfig = errors.New("invalid configuration") + +// invalidConfig type is needed to make any error returned from Validator +// to be ErrInvalidConfig. +type invalidConfig struct { + err error +} + +func (e *invalidConfig) Is(target error) bool { + return target == ErrInvalidConfig +} + +type Errno uintptr + +func (e Errno) Is(target error) bool { + switch target { + case os.ErrPermission: + return e == Errno(syscall.EACCES) || e == Errno(syscall.EPERM) + case os.ErrExist: + return e == Errno(syscall.EEXIST) || e == Errno(syscall.ENOTEMPTY) + case os.ErrNotExist: + return e == Errno(syscall.ENOENT) + } + return false +}