Skip to content

Commit

Permalink
Permit comparisons in (*t).Is(error) bool functions (fixes #11)
Browse files Browse the repository at this point in the history
  • Loading branch information
polyfloyd committed Sep 9, 2022
1 parent e319edb commit d64847f
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 2 deletions.
15 changes: 14 additions & 1 deletion errorlint/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
33 changes: 32 additions & 1 deletion errorlint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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",
Expand All @@ -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{}

Expand Down
35 changes: 35 additions & 0 deletions errorlint/testdata/src/issues/github-11.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit d64847f

Please sign in to comment.