Skip to content

sprintferrdot precision: verb handling {s,v} is wrong in both directions — %#v false positive, %q/%x/%X false negative #40434

Description

@github-actions

Summary

sprintferrdot (the newly-landed 30th custom linter; not yet in pkg/linters/doc.go) flags redundant .Error() calls on error arguments passed to fmt format functions, because fmt invokes Error() automatically. Its verb gate only recognizes %s and %v (pkg/linters/sprintferrdot/sprintferrdot.go:89), which is wrong in both directions relative to the fmt specification.

The fmt rule (authoritative)

Per the fmt package docs: the Error() method is invoked when the format “is valid for a string (%s %q %x %X), or is %v but not %#v.” So Error() is auto-called for %s %q %v %x %X, and is not called for %#v (which uses the GoStringer/Go-syntax representation instead).

Two defects, one root cause

The verb allow-list is {s, v} and parseSimpleFormatVerbs discards the # flag (it is skipped along with the other flags at pkg/linters/sprintferrdot/sprintferrdot.go:156), so %#v collapses to bare verb v.

1. False positive on %#vfmt.Sprintf("%#v", err.Error()) is flagged as redundant, but %#v does not invoke Error(). Dropping .Error() changes output: %#v on the string prints a quoted Go string literal, while %#v on the error value prints the Go-syntax representation (e.g. &errors.errorString{s:"..."}). The suggestion is behavior-changing and incorrect.

2. False negatives on %q, %x, %Xfmt.Errorf("%q", err.Error()) (and %x/%X) are genuinely redundant per the same rule, but the verb gate skips them (verbs[i] != 's' && verbs[i] != 'v' at :89).

Evidence

  • pkg/linters/sprintferrdot/sprintferrdot.go:89if verbs[i] != 's' && verbs[i] != 'v' { continue }
  • pkg/linters/sprintferrdot/sprintferrdot.go:156 — flag-skip loop consumes #, so the # of %#v is lost before the verb rune is captured at :183
  • parseSimpleFormatVerbs (:139-187) stores only the verb rune, never the flag, so %#v and %v are indistinguishable downstream

Impact

The .Error() detection itself is sound (isErrorDotCall uses pass.TypesInfo + types.Implements, :191-208). The defect is purely in verb classification. The FP is the higher-severity half: if this linter is ever added to the CI-enforced set, a %#v site becomes a build-blocker with an incorrect fix. No production sites of either case exist today (grep of pkg/ non-test for %#v ... .Error() and %q/%x/%X ... .Error() returns zero), so this is latent — but it is a correctness gap on documented fmt semantics.

Recommendation

  1. Preserve the # flag in parseSimpleFormatVerbs (track sharp-v distinctly from plain v).
  2. Widen the auto-Error() verb set to {s, q, v, x, X} while excluding %#v.
  3. Add testdata cases: %#v (no report), and %q/%x/%X (report), mirroring the existing %s/%v cases.

Validation checklist

  • %#v with err.Error() produces no diagnostic
  • %q, %x, %X with err.Error() each produce a diagnostic
  • %s, %v behavior unchanged
  • %T, %p never flagged (already correct — they bypass the verb gate)

Effort: small (single file + testdata).

References: §27860925409

Generated by 🤖 Sergo - Serena Go Expert · 276.8 AIC · ⌖ 11.6 AIC · ⊞ 5.8K ·

  • expires on Jun 26, 2026, 9:14 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions