diff --git a/README.md b/README.md index 6301a34..2db1957 100644 --- a/README.md +++ b/README.md @@ -300,6 +300,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`import-shadowing`](./RULES_DESCRIPTIONS.md#import-shadowing) | n/a | Spots identifiers that shadow an import | no | no | | [`bare-return`](./RULES_DESCRIPTIONS#bare-return) | n/a | Warns on bare returns | no | no | | [`unused-receiver`](./RULES_DESCRIPTIONS.md#unused-receiver) | n/a | Suggests to rename or remove unused method receivers | no | no | +| [`unhandled-error`](./RULES_DESCRIPTIONS.md#unhandled-error) | n/a | Warns on unhandled errors returned by funcion calls | no | yes | ## Configurable rules diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index e4d6225..3c40712 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -51,6 +51,7 @@ List of all available rules. - [var-naming](#var-naming) - [var-declaration](#var-declaration) - [unexported-return](#unexported-return) + - [unhandled-error](#unhandled-error) - [unnecessary-stmt](#unnecessary-stmt) - [unreachable-code](#unreachable-code) - [unused-parameter](#unused-parameter) @@ -434,6 +435,12 @@ _Description_: This rule warns when an exported function or method returns a val _Configuration_: N/A +## unhandled-error + +_Description_: This rule warns when errors returned by a function are not explicitly handled on the caller side. + +_Configuration_: N/A + ## unnecessary-stmt _Description_: This rule suggests to remove redundant statements like a `break` at the end of a case block, for improving the code's readability. diff --git a/config.go b/config.go index f874326..a577641 100644 --- a/config.go +++ b/config.go @@ -77,6 +77,7 @@ var allRules = append([]lint.Rule{ &rule.ImportShadowingRule{}, &rule.BareReturnRule{}, &rule.UnusedReceiverRule{}, + &rule.UnhandledErrorRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/unhandled-error.go b/fixtures/unhandled-error.go new file mode 100644 index 0000000..d3e9a45 --- /dev/null +++ b/fixtures/unhandled-error.go @@ -0,0 +1,19 @@ +package fixtures + +import ( + "fmt" + "os" +) + +func unhandledError1(a int) (int, error) { + return a, nil +} + +func unhandledError2() error { + _, err := unhandledError1(1) + unhandledError1(1) // MATCH /Unhandled error in call to function unhandledError1/ + fmt.Fprintf(nil, "") // MATCH /Unhandled error in call to function fmt.Fprintf/ + os.Chdir("..") // MATCH /Unhandled error in call to function os.Chdir/ + _ = os.Chdir("..") + return err +} diff --git a/rule/unhandled-error.go b/rule/unhandled-error.go new file mode 100644 index 0000000..8fc925d --- /dev/null +++ b/rule/unhandled-error.go @@ -0,0 +1,100 @@ +package rule + +import ( + "fmt" + "go/ast" + "go/types" + + "github.com/mgechev/revive/lint" +) + +// UnhandledErrorRule lints given else constructs. +type UnhandledErrorRule struct{} + +// Apply applies the rule to given file. +func (r *UnhandledErrorRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + walker := &lintUnhandledErrors{ + pkg: file.Pkg, + onFailure: func(failure lint.Failure) { + failures = append(failures, failure) + }, + } + + file.Pkg.TypeCheck() + ast.Walk(walker, file.AST) + + return failures +} + +// Name returns the rule name. +func (r *UnhandledErrorRule) Name() string { + return "unhandled-error" +} + +type lintUnhandledErrors struct { + pkg *lint.Package + onFailure func(lint.Failure) +} + +// Visit looks for statements that are function calls. +// If the called function returns a value of type error a failure will be created. +func (w *lintUnhandledErrors) Visit(node ast.Node) ast.Visitor { + switch n := node.(type) { + case *ast.ExprStmt: + fCall, ok := n.X.(*ast.CallExpr) + if !ok { + return nil // not a function call + } + + funcType := w.pkg.TypeOf(fCall) + if funcType == nil { + return nil // skip, type info not available + } + + switch t := funcType.(type) { + case *types.Named: + if !w.isTypeError(t) { + return nil // func call does not return an error + } + + w.addFailure(fCall) + default: + retTypes, ok := funcType.Underlying().(*types.Tuple) + if !ok { + return nil // skip, unable to retrieve return type of the called function + } + + if w.returnsAnError(retTypes) { + w.addFailure(fCall) + } + } + } + return w +} + +func (w *lintUnhandledErrors) addFailure(n *ast.CallExpr) { + w.onFailure(lint.Failure{ + Category: "bad practice", + Confidence: 1, + Node: n, + Failure: fmt.Sprintf("Unhandled error in call to function %v", gofmt(n.Fun)), + }) +} + +func (*lintUnhandledErrors) isTypeError(t *types.Named) bool { + const errorTypeName = "_.error" + + return t.Obj().Id() == errorTypeName +} + +func (w *lintUnhandledErrors) returnsAnError(tt *types.Tuple) bool { + for i := 0; i < tt.Len(); i++ { + nt, ok := tt.At(i).Type().(*types.Named) + if ok && w.isTypeError(nt) { + return true + } + } + return false +} diff --git a/test/unhandled-error_test.go b/test/unhandled-error_test.go new file mode 100644 index 0000000..7cef462 --- /dev/null +++ b/test/unhandled-error_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestUnhandledError(t *testing.T) { + testRule(t, "unhandled-error", &rule.UnhandledErrorRule{}) +}