feat: add linter for error/trace/log messages

cmd/lintroller/lintroller.go

@@ -14,6 +14,7 @@ import (
 	"github.com/getoutreach/lintroller/internal/config"
 	"github.com/getoutreach/lintroller/internal/copyright"
 	"github.com/getoutreach/lintroller/internal/doculint"
+	"github.com/getoutreach/lintroller/internal/errorlint"
 	"github.com/getoutreach/lintroller/internal/header"
 	"github.com/getoutreach/lintroller/internal/todo"
 	"github.com/getoutreach/lintroller/internal/why"
@@ -76,6 +77,19 @@ func main() {
 			}
 		}
 
+		warnTable := []struct {
+			Warn     bool
+			Analyzer *analysis.Analyzer
+		}{
+			{cfg.Errorlint.Warn, &errorlint.Analyzer},
+		}
+
+		for i := range warnTable {
+			if warnTable[i].Warn {
+				analyzers = append(analyzers, table[i].Analyzer)
+			}
+		}
+
 		if len(analyzers) > 0 {
 			multichecker.Main(analyzers...)
 		}
@@ -88,5 +102,6 @@ func main() {
 		&copyright.Analyzer,
 		&todo.Analyzer,
 		&why.Analyzer,
+		&errorlint.Analyzer,
 	)
 }

internal/config/config.go

@@ -59,6 +59,7 @@ type Lintroller struct {
 	Doculint  Doculint  `yaml:"doculint"`
 	Todo      Todo      `yaml:"todo"`
 	Why       Why       `yaml:"why"`
+	Errorlint Errorlint `yaml:"errorlint"`
 }
 
 // MarshalLog implements the log.Marshaler interface.
@@ -68,6 +69,7 @@ func (lr *Lintroller) MarshalLog(addField func(key string, value interface{})) {
 	addField("doculint", lr.Doculint)
 	addField("todo", lr.Todo)
 	addField("why", lr.Why)
+	addField("errorlint", lr.Errorlint)
 }
 
 // Header is the configuration type that matches the flags exposed by the header
@@ -174,3 +176,14 @@ type Why struct {
 func (w *Why) MarshalLog(addField func(key string, value interface{})) {
 	addField("enabled", w.Enabled)
 }
+
+// Errorlint is the configuration type that matches the flags exposed by the errorlint linter.
+type Errorlint struct {
+	// Warn denotes whether or not this linter is enabled as a warning. Defaults to true.
+	Warn bool `yaml:"warn"`
+}
+
+// MarshalLog implements the log.Marshaler interface.
+func (e *Errorlint) MarshalLog(addField func(key string, value interface{})) {
+	addField("warn", e.Warn)
+}

internal/config/tiers.go

@@ -49,19 +49,19 @@ func (l *Lintroller) ValidateTier() error {
 	switch strings.ToLower(*l.Tier) {
 	case TierBronze:
 		if err := l.EnsureMinimums(&TierBronzeConfiguration); err != nil {
-			return errors.Wrap(err, "ensure given configuration meets minimum requirments for bronze tier")
+			return errors.Wrap(err, "ensure given configuration meets minimum requirements for bronze tier")
 		}
 	case TierSilver:
 		if err := l.EnsureMinimums(&TierSilverConfiguration); err != nil {
-			return errors.Wrap(err, "ensure given configuration meets minimum requirments for silver tier")
+			return errors.Wrap(err, "ensure given configuration meets minimum requirements for silver tier")
 		}
 	case TierGold:
 		if err := l.EnsureMinimums(&TierGoldConfiguration); err != nil {
-			return errors.Wrap(err, "ensure given configuration meets minimum requirments for gold tier")
+			return errors.Wrap(err, "ensure given configuration meets minimum requirements for gold tier")
 		}
 	case TierPlatinum:
 		if err := l.EnsureMinimums(&TierPlatinumConfiguration); err != nil {
-			return errors.Wrap(err, "ensure given configuration meets minimum requirments for platinum tier")
+			return errors.Wrap(err, "ensure given configuration meets minimum requirements for platinum tier")
 		}
 	default:
 		log.Warn(context.Background(),
@@ -164,7 +164,7 @@ func (l *Lintroller) EnsureMinimums(desired *Lintroller) error { //nolint:funlen
 				l.Doculint.MinFunLen = desired.Doculint.MinFunLen
 			} else if l.Doculint.MinFunLen > desired.Doculint.MinFunLen || l.Doculint.MinFunLen < 0 {
 				return fmt.Errorf(
-					"deviation detected from tier minimum defaults in lintroller.doculint.minFunLen, minFunLen must be set within (0, %d]",
+					"deviation detected from tier minimum defaults in lintroller.doculint.minfunlen, minfunlen must be set within (0, %d]",
 					desired.Doculint.MinFunLen)
 			}
 		}
@@ -205,6 +205,9 @@ var TierBronzeConfiguration = Lintroller{
 	Why: Why{
 		Enabled: false,
 	},
+	Errorlint: Errorlint{
+		Warn: false,
+	},
 }
 
 // TierSilverConfiguration is the Lintroller configuration minumums that correspond
@@ -234,6 +237,9 @@ var TierSilverConfiguration = Lintroller{
 	Why: Why{
 		Enabled: true,
 	},
+	Errorlint: Errorlint{
+		Warn: false,
+	},
 }
 
 // TierGoldConfiguration is the Lintroller configuration minumums that correspond
@@ -263,6 +269,9 @@ var TierGoldConfiguration = Lintroller{
 	Why: Why{
 		Enabled: true,
 	},
+	Errorlint: Errorlint{
+		Warn: true,
+	},
 }
 
 // TierPlatinumConfiguration is the Lintroller configuration minumums that correspond
@@ -292,4 +301,7 @@ var TierPlatinumConfiguration = Lintroller{
 	Why: Why{
 		Enabled: true,
 	},
+	Errorlint: Errorlint{
+		Warn: true,
+	},
 }

internal/errorlint/errorlint.go

@@ -0,0 +1,169 @@
+// Copyright 2023 Outreach Corporation. All Rights Reserved.
+
+// Description: See package comment for this one file package.
+
+// Package errorlint contains the necessary logic for the error/log/trace linter. This validates that error
+// messages follow Google's go error guidelines
+// (https://google.github.io/styleguide/go/decisions.html#error-strings).
+// Specifically, this requires that error messages start with a lower-case letter, and do not end in
+// punctuation.
+//
+// This validates the following functions:
+// errors.New, errors.WithMessage, errors.WithMessagef, trace.StartCall, trace.StartSpan, log.Warn,
+// log.Error, log.Info, errors.Wrap, errors.Wrapf, fmt.Errorf, errors.Errorf
+package errorlint
+
+import (
+	"go/ast"
+	"go/token"
+	"strconv"
+	"unicode"
+	"unicode/utf8"
+
+	"github.com/getoutreach/lintroller/internal/common"
+	"github.com/getoutreach/lintroller/internal/reporter"
+	"golang.org/x/tools/go/analysis"
+)
+
+// name defines the name for the errorlint linter.
+const name = "errorlint"
+
+// doc defines the help text for the error linter.
+const doc = `Ensures that each error message starts with a lower-case letter and does not end in puctuation.
+// Bad:
+err := fmt.Errorf("Something bad happened.")
+// Good:
+err := fmt.Errorf("something bad happened")`
+
+// Analyzer exports the errorlint analyzer (linter).
+var Analyzer = analysis.Analyzer{
+	Name: name,
+	Doc:  doc,
+	Run:  errorlint,
+}
+
+// errorlint defines linter for error/trace/log messages
+func errorlint(_pass *analysis.Pass) (interface{}, error) {
+	// Ignore test packages.
+	if common.IsTestPackage(_pass) {
+		return nil, nil
+	}
+
+	// Wrap _pass with reporter.Pass to take nolint directives into account.
+	pass := reporter.NewPass(name, _pass, reporter.Warn())
+	for _, file := range pass.Files {
+		// Ignore generated files and test files.
+		if common.IsGenerated(file) || common.IsTestFile(pass.Pass, file) {
+			continue
+		}
+		lintMessageStrings(file, pass)
+	}
+
+	return nil, nil
+}
+
+// lintMessageStrings examines error/trace/log message strings for capitalization and valid ending
+func lintMessageStrings(file *ast.File, pass *reporter.Pass) {
+	ast.Inspect(file, func(node ast.Node) bool {
+		call, ok := node.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+
+		if isNotErrorPackage(call.Fun) || len(call.Args) < 1 {
+			return true
+		}
+
+		msgIndex := 1
+		if isDotInPkg(call.Fun, "errors", "New") || isDotInPkg(call.Fun, "fmt", "Errorf") ||
+			isDotInPkg(call.Fun, "errors", "Errorf") {
+			msgIndex = 0
+		}
+
+		msg, ok := call.Args[msgIndex].(*ast.BasicLit)
+		if !ok || msg.Kind != token.STRING {
+			return true
+		}
+
+		msgString, err := strconv.Unquote(msg.Value)
+		if err != nil {
+			return false
+		}
+
+		if msgString == "" {
+			return true
+		}
+		errormsg := getErrorMessage(msgString)
+		if errormsg != "" {
+			pkgName := getPkgName(call.Fun)
+			pass.Reportf(node.Pos(), "%s "+errormsg, pkgName)
+		}
+		return true
+	})
+}
+
+// isNotErrorPackage checks if the ast.Expr package matches the error/fmt/trace/log packages for linter
+func isNotErrorPackage(expr ast.Expr) bool {
+	return !isDotInPkg(expr, "errors", "New") && !isDotInPkg(expr, "errors", "Wrap") &&
+		!isDotInPkg(expr, "errors", "Wrapf") && !isDotInPkg(expr, "log", "Warn") &&
+		!isDotInPkg(expr, "log", "Info") && !isDotInPkg(expr, "log", "Error") &&
+		!isDotInPkg(expr, "trace", "StartSpan") && !isDotInPkg(expr, "trace", "StartCall") &&
+		!isDotInPkg(expr, "fmt", "Errorf") && !isDotInPkg(expr, "errors", "Errorf") &&
+		!isDotInPkg(expr, "errors", "WithMessage") && !isDotInPkg(expr, "errors", "WithMessagef")
+}
+
+// getErrorMessage returns message based on whether it has capitalization, punctuation or not
+func getErrorMessage(msg string) string {
+	isCap, isPunct := isStringFormatted(msg)
+	var errormsg string
+	switch {
+	case isCap && isPunct:
+		errormsg = "message should not be capitalized and should not end with punctuation"
+	case isCap:
+		errormsg = "message should not be capitalized"
+	case isPunct:
+		errormsg = "message should not end with punctuation"
+	}
+
+	return errormsg
+}
+
+// isIdent checks if ident string is equal to ast.Ident name
+func isIdent(expr ast.Expr, ident string) bool {
+	id, ok := expr.(*ast.Ident)
+	return ok && id.Name == ident
+}
+
+// hasDotInPkg checks if pkg.function format is followed
+func isDotInPkg(expr ast.Expr, pkg, name string) bool {
+	sel, ok := expr.(*ast.SelectorExpr)
+	return ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name)
+}
+
+// getPkgName returns package name errors/fmt/log/trace
+func getPkgName(expr ast.Expr) string {
+	sel, ok := expr.(*ast.SelectorExpr)
+	if !ok {
+		return ""
+	}
+	for _, pkg := range []string{"errors", "log", "fmt", "trace"} {
+		if isIdent(sel.X, pkg) {
+			return pkg
+		}
+	}
+
+	return ""
+}
+
+// isStringFormatted examines error/trace/log strings for incorrect ending and capitalization
+func isStringFormatted(msg string) (isCap, isPunct bool) {
+	last, _ := utf8.DecodeLastRuneInString(msg)
+	isPunct = last == '.' || last == ':' || last == '!' || last == '\n'
+	for _, ch := range msg {
+		if unicode.IsUpper(ch) {
+			isCap = true
+		}
+	}
+
+	return
+}

internal/reporter/reporter.go

@@ -122,7 +122,7 @@ func (p *Pass) Reportf(pos token.Pos, format string, args ...interface{}) {
 	}
 
 	if p.warn {
-		fmt.Printf("%s: %s (%s) [WARNING]", p.Fset.PositionFor(pos, false).String(), fmt.Sprintf(format, args...), p.linter)
+		fmt.Printf("%s: %s (%s) [WARNING]\n", p.Fset.PositionFor(pos, false).String(), fmt.Sprintf(format, args...), p.linter)
 		return
 	}
 	p.Pass.Reportf(pos, format+" (%s)", append(args, p.linter)...)