unhandled-error (new rule) (#124)

* unhandled-error (new rule)

* better failure msg

* encapsulates error type detection

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
 

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.

config.go

@@ -77,6 +77,7 @@ var allRules = append([]lint.Rule{
 	&rule.ImportShadowingRule{},
 	&rule.BareReturnRule{},
 	&rule.UnusedReceiverRule{},
+	&rule.UnhandledErrorRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

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
+}

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
+}

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{})
+}