[linter-miner] feat(linters): add fmterrorfnoverbs linter

cmd/linters/main.go

@@ -22,6 +22,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/errstringmatch"
 	"github.com/github/gh-aw/pkg/linters/excessivefuncparams"
 	"github.com/github/gh-aw/pkg/linters/fileclosenotdeferred"
+	"github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs"
 	"github.com/github/gh-aw/pkg/linters/fprintlnsprintf"
 	"github.com/github/gh-aw/pkg/linters/jsonmarshalignoredeerror"
 	"github.com/github/gh-aw/pkg/linters/largefunc"
@@ -45,6 +46,7 @@ func main() {
 		errstringmatch.Analyzer,
 		excessivefuncparams.Analyzer,
 		fileclosenotdeferred.Analyzer,
+		fmterrorfnoverbs.Analyzer,
 		largefunc.Analyzer,
 		manualmutexunlock.Analyzer,
 		osexitinlibrary.Analyzer,

docs/adr/36146-add-fmterrorfnoverbs-linter.md

@@ -0,0 +1,40 @@
+# ADR-36146: Add a dedicated `fmterrorfnoverbs` analysis-pass linter
+
+**Date**: 2026-05-31
+**Status**: Draft
+
+## Context
+
+The codebase already maintains a set of custom `golang.org/x/tools/go/analysis` linters under `pkg/linters/`, each as its own package and registered centrally in `cmd/linters/main.go`. A recurring anti-pattern was observed: `fmt.Errorf` is called with a format string that contains no `%` verbs (14 production occurrences across `pkg/console/*`, `pkg/workflow`, and `pkg/parser`), where `errors.New` is the idiomatic and allocation-cheaper alternative. The same issue surfaced independently from the `lint-go` CI step in PR #36140. There was no automated guard preventing this pattern from recurring.
+
+## Decision
+
+We will add a new standalone analysis-pass linter, `fmterrorfnoverbs`, in its own package `pkg/linters/fmterrorfnoverbs/`, and register it in the central analyzer list in `cmd/linters/main.go`. The analyzer inspects `*ast.CallExpr` nodes, resolves the callee to `fmt.Errorf` via type information (`pass.TypesInfo`), and reports any call whose first argument is a string literal containing no `%` character. This follows the existing per-linter package convention and uses `analysistest`-based test fixtures, consistent with sibling linters.
+
+## Alternatives Considered
+
+### Alternative 1: Rely on an off-the-shelf linter (e.g. `perfsprint` / golangci-lint)
+Existing third-party linters such as `perfsprint` cover this exact case. It was not chosen because the repository has deliberately standardized on a homegrown `pkg/linters/` analysis-pass framework, giving full control over rule semantics, messaging, and CI integration without adding an external linter aggregator dependency. *[TODO: verify whether adopting an existing linter was explicitly evaluated and rejected.]*
+
+### Alternative 2: Add the check to an existing linter package
+The verb check could have been folded into a broader existing analyzer rather than introducing a new package. It was not chosen because each rule in `pkg/linters/` is intentionally a single-responsibility, independently testable package, which keeps rules discoverable and individually toggleable.
+
+## Consequences
+
+### Positive
+- Automated, repeatable prevention of the no-verb `fmt.Errorf` pattern, removing reliance on ad-hoc review.
+- Consistent with the established one-package-per-linter convention, so the addition is low-friction for maintainers.
+- Encourages idiomatic, marginally cheaper `errors.New` usage across the codebase.
+
+### Negative
+- Detection is limited to direct string-literal first arguments; format strings built from constants, concatenation, or variables are not flagged (false negatives).
+- The naive unquoting (stripping first/last byte and a raw `strings.Contains(val, "%")` check) does not account for escaped `%%` or raw string literals, which could in edge cases mis-handle the literal. *[TODO: verify escaped-percent and backtick-literal handling.]*
+- Adds yet another custom linter to maintain in-house rather than leveraging an upstream-maintained equivalent.
+
+### Neutral
+- The existing 14 production occurrences are not auto-fixed by this PR; they must be remediated separately once the linter is enforced in CI.
+- The analyzer requires the `inspect` pass and type information, matching the resource profile of sibling linters.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26720516192) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/linters/fmterrorfnoverbs/fmterrorfnoverbs.go

@@ -0,0 +1,91 @@
+// Package fmterrorfnoverbs implements a Go analysis linter that flags calls to
+// fmt.Errorf where the format string contains no format verbs, in which case
+// errors.New is the idiomatic and cheaper alternative.
+package fmterrorfnoverbs
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// Analyzer is the fmterrorfnoverbs analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "fmterrorfnoverbs",
+	Doc:      "reports fmt.Errorf calls whose format string contains no verbs, preferring errors.New",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/fmterrorfnoverbs",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, nil
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.CallExpr)(nil),
+	}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return
+		}
+
+		if !isFmtErrorf(pass, call) {
+			return
+		}
+
+		if len(call.Args) == 0 {
+			return
+		}
+
+		lit, ok := call.Args[0].(*ast.BasicLit)
+		if !ok || lit.Kind != token.STRING {
+			return
+		}
+
+		// Unquote the string value
+		val := lit.Value
+		if len(val) >= 2 {
+			val = val[1 : len(val)-1]
+		}
+
+		if !strings.Contains(val, "%") {
+			pass.ReportRangef(call, "fmt.Errorf called with no format verbs; use errors.New(%s) instead", lit.Value)
+		}
+	})
+
+	return nil, nil
+}
+
+// isFmtErrorf returns true if the call expression is a call to fmt.Errorf.
+func isFmtErrorf(pass *analysis.Pass, call *ast.CallExpr) bool {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+	if sel.Sel.Name != "Errorf" {
+		return false
+	}
+	pkgIdent, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return false
+	}
+	obj := pass.TypesInfo.ObjectOf(pkgIdent)
+	if obj == nil {
+		return false
+	}
+	pkgName, ok := obj.(*types.PkgName)
+	if !ok {
+		return false
+	}
+	return pkgName.Imported().Path() == "fmt"
+}

pkg/linters/fmterrorfnoverbs/fmterrorfnoverbs_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package fmterrorfnoverbs_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs"
+)
+
+func TestFmtErrorfNoVerbs(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, fmterrorfnoverbs.Analyzer, "fmterrorfnoverbs")
+}

pkg/linters/fmterrorfnoverbs/testdata/src/fmterrorfnoverbs/fmterrorfnoverbs.go

@@ -0,0 +1,27 @@
+// Package fmterrorfnoverbs contains test fixtures for the fmterrorfnoverbs linter.
+package fmterrorfnoverbs
+
+import (
+	"errors"
+	"fmt"
+)
+
+func bad() error {
+	return fmt.Errorf("something went wrong") // want `fmt\.Errorf called with no format verbs; use errors\.New`
+}
+
+func badMultiLine() error {
+	return fmt.Errorf("interactive input not available in Wasm") // want `fmt\.Errorf called with no format verbs; use errors\.New`
+}
+
+func good() error {
+	return errors.New("something went wrong")
+}
+
+func goodWithVerb(name string) error {
+	return fmt.Errorf("failed to process %s", name)
+}
+
+func goodWithWrap(err error) error {
+	return fmt.Errorf("wrapper: %w", err)
+}