Add error rules

fixtures/error-strings.go

@@ -0,0 +1,20 @@
+// Package foo ...
+package foo
+
+import (
+	"errors"
+	"fmt"
+)
+
+// Check for the error strings themselves.
+
+func g(x int) error {
+	var err error
+	err = fmt.Errorf("This %d is too low", x)     // MATCH /error strings should not be capitalized or end with punctuation or a newline/
+	err = fmt.Errorf("XML time")                  // ok
+	err = fmt.Errorf("newlines are fun\n")        // MATCH /error strings should not be capitalized or end with punctuation or a newline/
+	err = fmt.Errorf("Newlines are really fun\n") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
+	err = errors.New(`too much stuff.`)           // MATCH /error strings should not be capitalized or end with punctuation or a newline/
+	err = errors.New("This %d is too low", x)     // MATCH /error strings should not be capitalized or end with punctuation or a newline/
+	return err
+}

fixtures/errors.go

@@ -0,0 +1,25 @@
+// Test for naming errors.
+
+// Package foo ...
+package foo
+
+import (
+	"errors"
+	"fmt"
+)
+
+var unexp = errors.New("some unexported error") // MATCH /error var unexp should have name of the form errFoo/
+
+// Exp ...
+var Exp = errors.New("some exported error") // MATCH /error var Exp should have name of the form ErrFoo/
+
+var (
+	e1 = fmt.Errorf("blah %d", 4) // MATCH /error var e1 should have name of the form errFoo/
+	// E2 ...
+	E2 = fmt.Errorf("blah %d", 5) // MATCH /error var E2 should have name of the form ErrFoo/
+)
+
+func f() {
+	var whatever = errors.New("ok") // ok
+	_ = whatever
+}

rule/error-strings.go

@@ -0,0 +1,99 @@
+package rule
+
+import (
+	"go/ast"
+	"go/token"
+	"strconv"
+	"unicode"
+	"unicode/utf8"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// ErrorStringsRule lints given else constructs.
+type ErrorStringsRule struct{}
+
+// Apply applies the rule to given file.
+func (r *ErrorStringsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	fileAst := file.AST
+	walker := lintErrorStrings{
+		file:    file,
+		fileAst: fileAst,
+		onFailure: func(failure lint.Failure) {
+			failures = append(failures, failure)
+		},
+	}
+
+	ast.Walk(walker, fileAst)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *ErrorStringsRule) Name() string {
+	return "errorf"
+}
+
+type lintErrorStrings struct {
+	file      *lint.File
+	fileAst   *ast.File
+	onFailure func(lint.Failure)
+}
+
+func (w lintErrorStrings) Visit(n ast.Node) ast.Visitor {
+	ce, ok := n.(*ast.CallExpr)
+	if !ok {
+		return w
+	}
+	if !isPkgDot(ce.Fun, "errors", "New") && !isPkgDot(ce.Fun, "fmt", "Errorf") {
+		return w
+	}
+	if len(ce.Args) < 1 {
+		return w
+	}
+	str, ok := ce.Args[0].(*ast.BasicLit)
+	if !ok || str.Kind != token.STRING {
+		return w
+	}
+	s, _ := strconv.Unquote(str.Value) // can assume well-formed Go
+	if s == "" {
+		return w
+	}
+	clean, conf := lintErrorString(s)
+	if clean {
+		return w
+	}
+
+	w.onFailure(lint.Failure{
+		Node:       str,
+		Confidence: conf,
+		URL:        "#error-strings",
+		Category:   "errors",
+		Failure:    "error strings should not be capitalized or end with punctuation or a newline",
+	})
+	return w
+}
+
+func lintErrorString(s string) (isClean bool, conf float64) {
+	const basicConfidence = 0.8
+	const capConfidence = basicConfidence - 0.2
+	first, firstN := utf8.DecodeRuneInString(s)
+	last, _ := utf8.DecodeLastRuneInString(s)
+	if last == '.' || last == ':' || last == '!' || last == '\n' {
+		return false, basicConfidence
+	}
+	if unicode.IsUpper(first) {
+		// People use proper nouns and exported Go identifiers in error strings,
+		// so decrease the confidence of warnings for capitalization.
+		if len(s) <= firstN {
+			return false, capConfidence
+		}
+		// Flag strings starting with something that doesn't look like an initialism.
+		if second, _ := utf8.DecodeRuneInString(s[firstN:]); !unicode.IsUpper(second) {
+			return false, capConfidence
+		}
+	}
+	return true, 0
+}

rule/errors.go

@@ -0,0 +1,79 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"strings"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// ErrorsRule lints given else constructs.
+type ErrorsRule struct{}
+
+// Apply applies the rule to given file.
+func (r *ErrorsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	fileAst := file.AST
+	walker := lintErrors{
+		file:    file,
+		fileAst: fileAst,
+		onFailure: func(failure lint.Failure) {
+			failures = append(failures, failure)
+		},
+	}
+
+	ast.Walk(walker, fileAst)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *ErrorsRule) Name() string {
+	return "errors"
+}
+
+type lintErrors struct {
+	file      *lint.File
+	fileAst   *ast.File
+	onFailure func(lint.Failure)
+}
+
+func (w lintErrors) Visit(n ast.Node) ast.Visitor {
+	for _, decl := range w.fileAst.Decls {
+		gd, ok := decl.(*ast.GenDecl)
+		if !ok || gd.Tok != token.VAR {
+			continue
+		}
+		for _, spec := range gd.Specs {
+			spec := spec.(*ast.ValueSpec)
+			if len(spec.Names) != 1 || len(spec.Values) != 1 {
+				continue
+			}
+			ce, ok := spec.Values[0].(*ast.CallExpr)
+			if !ok {
+				continue
+			}
+			if !isPkgDot(ce.Fun, "errors", "New") && !isPkgDot(ce.Fun, "fmt", "Errorf") {
+				continue
+			}
+
+			id := spec.Names[0]
+			prefix := "err"
+			if id.IsExported() {
+				prefix = "Err"
+			}
+			if !strings.HasPrefix(id.Name, prefix) {
+				w.onFailure(lint.Failure{
+					Node:       id,
+					Confidence: 0.9,
+					Category:   "naming",
+					Failure:    fmt.Sprintf("error var %s should have name of the form %sFoo", id.Name, prefix),
+				})
+			}
+		}
+	}
+	return nil
+}

testutil/lint_test.go

@@ -43,6 +43,8 @@ var rules = []lint.Rule{
 	&rule.IfReturnRule{},
 	&rule.RangeRule{},
 	&rule.ErrorfRule{},
+	&rule.ErrorsRule{},
+	&rule.ErrorStringsRule{},
 }
 
 func TestAll(t *testing.T) {