[linter-miner] feat(linters): add sprintferrdot — flag redundant .Error() calls in fmt format functions

cmd/linters/main.go

@@ -39,6 +39,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/regexpcompileinfunction"
 	"github.com/github/gh-aw/pkg/linters/seenmapbool"
 	"github.com/github/gh-aw/pkg/linters/sortslice"
+	"github.com/github/gh-aw/pkg/linters/sprintferrdot"
 	"github.com/github/gh-aw/pkg/linters/ssljson"
 	"github.com/github/gh-aw/pkg/linters/strconvparseignorederror"
 	"github.com/github/gh-aw/pkg/linters/timeafterleak"
@@ -71,6 +72,7 @@ func main() {
 		ssljson.Analyzer,
 		seenmapbool.Analyzer,
 		sortslice.Analyzer,
+		sprintferrdot.Analyzer,
 		strconvparseignorederror.Analyzer,
 		jsonmarshalignoredeerror.Analyzer,
 		lenstringzero.Analyzer,

docs/adr/40371-flag-redundant-error-calls-in-fmt-linter.md

@@ -0,0 +1,38 @@
+# ADR-40371: Flag Redundant `.Error()` Calls in `fmt` Format Functions via a Dedicated Linter
+
+**Date**: 2026-06-19
+**Status**: Draft
+
+## Context
+
+The codebase maintains a suite of custom `go/analysis` analyzers under `pkg/linters/`, each registered in `cmd/linters/main.go` and targeting a single, narrowly-scoped code smell that the default `golangci-lint`/`staticcheck`/`govet` rule sets do not catch. A source scan found 6 live production instances of `err.Error()` being passed to `fmt` format functions (`Sprintf`, `Errorf`, `Fprintf`) under `%s`/`%v` verbs, where `fmt` already invokes `.Error()` implicitly through the error/`Stringer` interface. The explicit call is redundant noise that obscures error-chain inspection intent, and no existing lint rule flags it. The project needs a mechanical, CI-enforceable way to detect and prevent this pattern.
+
+## Decision
+
+We will add a new dedicated `go/analysis` analyzer package, `pkg/linters/sprintferrdot`, that reports redundant zero-argument `.Error()` calls on error-typed values passed to recognized `fmt` format functions when the positionally-mapped verb is `%s` or `%v`. The analyzer follows the established `pkg/linters/` pattern — type-based error-interface detection via `go/types`, shared helpers from `internal/astutil` and `internal/filecheck`, and a fixture-driven `analysistest` suite — and is registered in `cmd/linters/main.go` alongside the other analyzers. Format strings using explicit argument indices (`%[n]v`) or `*` width/precision are deliberately skipped to avoid unreliable positional mapping.
+
+## Alternatives Considered
+
+### Alternative 1: Rely on existing `golangci-lint` / `staticcheck` configuration
+We could attempt to express this rule through existing linter configuration. Rejected because no rule in `errcheck`, `govet`, or default `staticcheck` flags this specific redundancy; there is no configuration knob that captures it, so a custom analyzer is required.
+
+### Alternative 2: Extend the existing `errorfwrapv` linter instead of a new package
+Since `errorfwrapv` already inspects `fmt.Errorf` error-formatting, we could fold this check into it. Rejected to preserve the project's one-analyzer-per-smell convention: the two checks have different scopes (`errorfwrapv` covers `%v`→`%w` in `Errorf` only; `sprintferrdot` covers `.Error()` redundancy across multiple `fmt` functions), and keeping them separate keeps each analyzer's diagnostics and tests focused.
+
+## Consequences
+
+### Positive
+- Mechanically catches a redundancy class that no existing linter detects, across `Sprintf`/`Errorf`/`Printf`/`Fprintf`.
+- Improves readability and consistency by encouraging callers to pass the error value directly; complements `errorfwrapv`.
+
+### Negative
+- Adds another analyzer to maintain, register, and keep fast in the lint pass.
+- Positional verb parsing is heuristic and intentionally bails on complex format strings (`%[n]v`, `*` widths), producing false negatives in those cases rather than risking false positives.
+
+### Neutral
+- The analyzer reports only; it does not auto-fix. Flagged sites must be remediated manually (or by a follow-up fixer).
+- Test files are skipped via `filecheck.IsTestFile`, matching the convention of sibling linters.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27842164958) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/linters/sprintferrdot/sprintferrdot.go

@@ -0,0 +1,208 @@
+// Package sprintferrdot implements a Go analysis linter that flags redundant
+// .Error() calls on error values passed to fmt format functions.
+//
+// When an error is formatted with %s or %v, the fmt package calls .Error()
+// automatically, so calling .Error() explicitly before passing the value
+// to the format function is redundant.
+package sprintferrdot
+
+import (
+	"go/ast"
+	"go/types"
+	"strconv"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+
+	"github.com/github/gh-aw/pkg/linters/internal/astutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+)
+
+var errorIface = universeErrorInterface()
+
+// universeErrorInterface returns the built-in error interface type, or nil.
+func universeErrorInterface() *types.Interface {
+	errorObj := types.Universe.Lookup("error")
+	if errorObj == nil {
+		return nil
+	}
+	iface, ok := errorObj.Type().Underlying().(*types.Interface)
+	if !ok {
+		return nil
+	}
+	return iface
+}
+
+// Analyzer is the sprintf-err-dot analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "sprintferrdot",
+	Doc:      "reports redundant .Error() calls on error arguments passed to fmt format functions",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/sprintferrdot",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, err := astutil.Inspector(pass)
+	if err != nil {
+		return nil, err
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.CallExpr)(nil),
+	}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return
+		}
+
+		pos := pass.Fset.PositionFor(call.Pos(), false)
+		if filecheck.IsTestFile(pos.Filename) {
+			return
+		}
+
+		formatArgIdx, variadicStart, ok := fmtFormatCallInfo(pass, call)
+		if !ok {
+			return
+		}
+		if formatArgIdx >= len(call.Args) || variadicStart > len(call.Args) {
+			return
+		}
+
+		formatStr, ok := extractStringLit(call.Args[formatArgIdx])
+		if !ok {
+			return
+		}
+
+		verbs := parseSimpleFormatVerbs(formatStr)
+		if verbs == nil {
+			return
+		}
+
+		variadicArgs := call.Args[variadicStart:]
+		for i, arg := range variadicArgs {
+			if i >= len(verbs) {
+				break
+			}
+			if verbs[i] != 's' && verbs[i] != 'v' {
+				continue
+			}
+			if isErrorDotCall(pass, arg) {
+				pass.Reportf(arg.Pos(),
+					"redundant .Error() call: pass the error value directly with %%%c", verbs[i])
+			}
+		}
+	})
+
+	return nil, nil
+}
+
+// fmtFormatCallInfo returns the format-string argument index and the
+// variadic-args start index for recognised fmt format functions.
+func fmtFormatCallInfo(pass *analysis.Pass, call *ast.CallExpr) (formatArgIdx, variadicStart int, ok bool) {
+	sel, isSel := call.Fun.(*ast.SelectorExpr)
+	if !isSel {
+		return 0, 0, false
+	}
+	if !astutil.IsPkgSelector(pass, sel, "fmt") {
+		return 0, 0, false
+	}
+	switch sel.Sel.Name {
+	case "Sprintf", "Errorf", "Printf":
+		return 0, 1, true
+	case "Fprintf", "Fscanf":
+		return 1, 2, true
+	default:
+		return 0, 0, false
+	}
+}
+
+// extractStringLit returns the unquoted value of a string literal expression.
+func extractStringLit(expr ast.Expr) (string, bool) {
+	lit, ok := expr.(*ast.BasicLit)
+	if !ok {
+		return "", false
+	}
+	s, err := strconv.Unquote(lit.Value)
+	if err != nil {
+		return "", false
+	}
+	return s, true
+}
+
+// parseSimpleFormatVerbs returns the list of format verbs in sequential order.
+// It returns nil when the format string uses complex features such as explicit
+// argument indices (%[n]verb) or * widths/precisions, since those prevent
+// reliable positional mapping.
+func parseSimpleFormatVerbs(format string) []rune {
+	var verbs []rune
+	i := 0
+	for i < len(format) {
+		if format[i] != '%' {
+			i++
+			continue
+		}
+		i++
+		if i >= len(format) {
+			break
+		}
+		if format[i] == '%' {
+			i++
+			continue
+		}
+		// Skip flags: -, +, #, space, 0
+		for i < len(format) && (format[i] == '-' || format[i] == '+' || format[i] == '#' || format[i] == ' ' || format[i] == '0') {
+			i++
+		}
+		if i >= len(format) {
+			break
+		}
+		// Explicit arg index or * width/precision: too complex to analyse.
+		if format[i] == '[' || format[i] == '*' {
+			return nil
+		}
+		// Skip width digits.
+		for i < len(format) && format[i] >= '0' && format[i] <= '9' {
+			i++
+		}
+		// Skip precision.
+		if i < len(format) && format[i] == '.' {
+			i++
+			if i < len(format) && format[i] == '*' {
+				return nil
+			}
+			for i < len(format) && format[i] >= '0' && format[i] <= '9' {
+				i++
+			}
+		}
+		if i >= len(format) {
+			break
+		}
+		verbs = append(verbs, rune(format[i]))
+		i++
+	}
+	return verbs
+}
+
+// isErrorDotCall reports whether expr is a zero-argument .Error() call on a
+// value that implements the error interface.
+func isErrorDotCall(pass *analysis.Pass, expr ast.Expr) bool {
+	call, ok := expr.(*ast.CallExpr)
+	if !ok || len(call.Args) != 0 {
+		return false
+	}
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != "Error" {
+		return false
+	}
+	if errorIface == nil {
+		return false
+	}
+	t := pass.TypesInfo.TypeOf(sel.X)
+	if t == nil {
+		return false
+	}
+	return types.Implements(t, errorIface) || types.Implements(types.NewPointer(t), errorIface)
+}

pkg/linters/sprintferrdot/sprintferrdot_test.go

@@ -0,0 +1,17 @@
+//go:build !integration
+
+// Package sprintferrdot_test provides tests for the sprintferrdot analyzer.
+package sprintferrdot_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/sprintferrdot"
+)
+
+func TestSprintfErrDot(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, sprintferrdot.Analyzer, "sprintferrdot")
+}

pkg/linters/sprintferrdot/testdata/src/sprintferrdot/sprintferrdot.go

@@ -0,0 +1,62 @@
+// Package sprintferrdot is the test fixture for the sprintferrdot analyzer.
+package sprintferrdot
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"os"
+)
+
+var sentinel = errors.New("sentinel")
+
+// badSprintfS calls fmt.Sprintf with err.Error() and a %s verb — should be flagged.
+func badSprintfS(err error) string {
+	return fmt.Sprintf("operation failed: %s", err.Error()) // want `redundant \.Error\(\) call: pass the error value directly with %s`
+}
+
+// badSprintfV calls fmt.Sprintf with err.Error() and a %v verb — should be flagged.
+func badSprintfV(err error) string {
+	return fmt.Sprintf("operation failed: %v", err.Error()) // want `redundant \.Error\(\) call: pass the error value directly with %v`
+}
+
+// badErrorf calls fmt.Errorf with err.Error() and a %s verb — should be flagged.
+func badErrorf(err error) error {
+	return fmt.Errorf("wrapped: %s", err.Error()) // want `redundant \.Error\(\) call: pass the error value directly with %s`
+}
+
+// badFprintf calls fmt.Fprintf with err.Error() and a %s verb — should be flagged.
+func badFprintf(w io.Writer, err error) {
+	fmt.Fprintf(w, "error: %s\n", err.Error()) // want `redundant \.Error\(\) call: pass the error value directly with %s`
+}
+
+// goodSprintfS passes the error value directly — no diagnostic expected.
+func goodSprintfS(err error) string {
+	return fmt.Sprintf("operation failed: %s", err)
+}
+
+// goodSprintfW wraps with %w — no diagnostic expected.
+func goodSprintfW(err error) error {
+	return fmt.Errorf("wrapped: %w", err)
+}
+
+// goodStandaloneError calls .Error() outside of a format function — no diagnostic expected.
+func goodStandaloneError(err error) string {
+	return err.Error()
+}
+
+// goodMultiVerb has a %d verb for the error position — no diagnostic expected.
+func goodMultiVerb(n int, err error) string {
+	return fmt.Sprintf("code %d: %T", n, err.Error())
+}
+
+// goodNonErrorDot calls .SomeMethod() that is not Error() — no diagnostic expected.
+func goodNonErrorDot(f *os.File) string {
+	name := f.Name()
+	return fmt.Sprintf("file: %s", name)
+}
+
+// goodNilErr explicitly avoids the pattern.
+func goodNilErr() string {
+	return fmt.Sprintf("sentinel: %s", sentinel)
+}