Skip to content

fmterrorfnoverbs false negative: escaped %% is treated as a real verb, missing no-verb fmt.Errorf calls #41845

Description

@github-actions

Summary

fmterrorfnoverbs decides whether a fmt.Errorf format string contains a real verb with a bare strings.Contains(val, "%") substring test. An escaped literal percent %% contains a % but is not a format verb, so fmt.Errorf("... %% ...") with no other verb is wrongly treated as "has a verb" and is not flagged — a false negative. The idiomatic rewrite errors.New("... % ...") is missed.

Location

pkg/linters/fmterrorfnoverbs/fmterrorfnoverbs.go:58-69

val := lit.Value
if len(val) >= 2 {
    val = val[1 : len(val)-1]
}
if !strings.Contains(val, "%") {   // %% contains '%' => skipped, but %% is NOT a verb
    ... report ...
}

Evidence / repro

// not flagged today, but has no real verb — equivalent to errors.New("disk usage exceeds 90% limit")
return fmt.Errorf("disk usage exceeds 90%% limit")

val is disk usage exceeds 90%% limit; strings.Contains(val, "%") is true, so the linter skips it. There is no real verb — %% renders as a literal %.

A secondary, lower-frequency miss: the check runs on the raw literal (quotes stripped, escapes not decoded), so a verb expressed as an escape (e.g. "\u0025d") is also mis-classified. The %% case is the realistic one.

Impact

False negative only (no false positive, no autofix risk — the linter emits no SuggestedFix). fmterrorfnoverbs is one of the CI-enforced linters, so closing the gap improves enforcement precision. Frequency is low (no-verb fmt.Errorf whose only % is an escaped %%), but the fix is small and removes a clear correctness gap.

Recommendation

Replace the strings.Contains(val, "%") heuristic with a scan that skips %% pairs before deciding whether a real verb is present — e.g. walk the decoded string and treat %% as two literal characters, reporting when no non-%% %-directive remains. The same %%-aware scan already exists in spirit in the verb-handling logic referenced by #40434 (sprintferrdot) and #38788 (hardcodedfilepath); reuse that approach for consistency.

Before / after intent

  • Before: Contains("%") → any % (including %%) suppresses the report.
  • After: strip %% pairs first; report when the remaining string has no %.

Validation checklist

  • fmt.Errorf("90%% done") (no other verb) is flagged.
  • fmt.Errorf("%d%% done", n) (real verb present) is NOT flagged.
  • fmt.Errorf("plain message") still flagged; fmt.Errorf("value=%s", v) still skipped.
  • testdata cases for each of the above.

Effort

Low — single-function change in run, ~5 lines plus testdata.

Sergo R49 — fresh deep-audit of fmterrorfnoverbs (type-resolved IsFmtErrorf and nolint are correct; only the verb-presence test is too coarse).

Generated by 🤖 Sergo - Serena Go Expert · 231.1 AIC · ⌖ 14.3 AIC · ⊞ 5.9K ·

  • expires on Jul 3, 2026, 9:09 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