You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
sprintferrdot reports .Error() calls as "redundant" whenever the format verb is s or v, but the verb classification is wrong in both directions:
False positive: %#v is recorded as verb v (the # flag is silently skipped, not tracked), so fmt.Sprintf("%#v", err.Error()) is flagged as redundant. It is not: %#v on an error value invokes GoStringer/prints the Go-syntax struct representation (not .Error()), so removing the explicit .Error() call changes the output from a quoted message string to a struct dump.
False negative: %q, %x, %X also invoke the operand's Error() method per the fmt package's special-case rules (same family as %s/%v), but these verbs are excluded from the check (sprintferrdot.go:75), so genuinely redundant .Error() calls under these verbs are never flagged.
This is the exact bug described in #40434 ("sprintferrdot precision: verb handling {s,v} is wrong in both directions — %#v false positive, %q/%x/%X false negative"), which is closed, but the fix was never landed: the current code is unchanged in this respect and the test fixture has no case for #v, %q, %x, or %X.
Evidence
pkg/linters/sprintferrdot/sprintferrdot.go:142 — the flag-skipping loop consumes # (along with -, +, , 0) without recording it, so %#v and %v produce the identical verb rune v.
pkg/linters/sprintferrdot/sprintferrdot.go:75 — if verbs[i] != 's' && verbs[i] != 'v' { continue } excludes q/x/X, even though those verbs also trigger the error-interface special case documented in the fmt package docs ("If an operand implements the error interface, the Error method will be invoked..." — applies to %v, %s, %q, %x, %X; %#v instead uses GoStringer and does not invoke Error()).
pkg/linters/sprintferrdot/testdata/src/sprintferrdot/sprintferrdot.go has no #v/%q/%x/%X fixture, so neither direction of the bug is covered by analysistest.
Impact
Applying the suggested fix (dropping .Error()) for a %#v call site is a behavior-changing false positive: the printed output changes from the error message to a Go-syntax struct dump.
Real redundant .Error() calls under %q/%x/%X are silently missed (false negative), so the linter under-reports on a real, if less common, class of the same bug it exists to catch.
Recommendation
Track the # flag while scanning each verb (e.g. return a small struct {verb rune; sharp bool} instead of a bare []rune), and skip (don't flag) any verb with sharp && verb == 'v'.
Extend the accepted-verb set at sprintferrdot.go:75 to include q, x, X alongside s, v.
Add fixture cases to testdata/src/sprintferrdot/sprintferrdot.go: fmt.Sprintf("%#v", err.Error()) (no diagnostic expected) and fmt.Sprintf("%q", err.Error()) / %x / %X (diagnostic expected), then re-run analysistest.
Validation checklist
%#v with .Error() no longer flagged
%q, %x, %X with .Error() are flagged
existing %s/%v cases still flagged, %w/bare-error cases still clean
analysistest golden fixtures updated and passing
Effort
Small — self-contained change to parseSimpleFormatVerbs and the verb-acceptance check, plus fixture additions. No SuggestedFix is emitted by this linter, so no autofix-correctness risk.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
api.anthropic.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
Problem
sprintferrdotreports.Error()calls as "redundant" whenever the format verb issorv, but the verb classification is wrong in both directions:%#vis recorded as verbv(the#flag is silently skipped, not tracked), sofmt.Sprintf("%#v", err.Error())is flagged as redundant. It is not:%#von anerrorvalue invokesGoStringer/prints the Go-syntax struct representation (not.Error()), so removing the explicit.Error()call changes the output from a quoted message string to a struct dump.%q,%x,%Xalso invoke the operand'sError()method per thefmtpackage's special-case rules (same family as%s/%v), but these verbs are excluded from the check (sprintferrdot.go:75), so genuinely redundant.Error()calls under these verbs are never flagged.This is the exact bug described in #40434 ("sprintferrdot precision: verb handling {s,v} is wrong in both directions — %#v false positive, %q/%x/%X false negative"), which is closed, but the fix was never landed: the current code is unchanged in this respect and the test fixture has no case for
#v,%q,%x, or%X.Evidence
pkg/linters/sprintferrdot/sprintferrdot.go:142— the flag-skipping loop consumes#(along with-,+,,0) without recording it, so%#vand%vproduce the identical verb runev.pkg/linters/sprintferrdot/sprintferrdot.go:75—if verbs[i] != 's' && verbs[i] != 'v' { continue }excludesq/x/X, even though those verbs also trigger theerror-interface special case documented in thefmtpackage docs ("If an operand implements the error interface, the Error method will be invoked..." — applies to%v,%s,%q,%x,%X;%#vinstead usesGoStringerand does not invokeError()).pkg/linters/sprintferrdot/testdata/src/sprintferrdot/sprintferrdot.gohas no#v/%q/%x/%Xfixture, so neither direction of the bug is covered byanalysistest.Impact
.Error()) for a%#vcall site is a behavior-changing false positive: the printed output changes from the error message to a Go-syntax struct dump..Error()calls under%q/%x/%Xare silently missed (false negative), so the linter under-reports on a real, if less common, class of the same bug it exists to catch.Recommendation
#flag while scanning each verb (e.g. return a small struct{verb rune; sharp bool}instead of a bare[]rune), and skip (don't flag) any verb withsharp && verb == 'v'.sprintferrdot.go:75to includeq,x,Xalongsides,v.testdata/src/sprintferrdot/sprintferrdot.go:fmt.Sprintf("%#v", err.Error())(no diagnostic expected) andfmt.Sprintf("%q", err.Error())/%x/%X(diagnostic expected), then re-runanalysistest.Validation checklist
%#vwith.Error()no longer flagged%q,%x,%Xwith.Error()are flagged%s/%vcases still flagged,%w/bare-error cases still cleananalysistestgolden fixtures updated and passingEffort
Small — self-contained change to
parseSimpleFormatVerbsand the verb-acceptance check, plus fixture additions. NoSuggestedFixis emitted by this linter, so no autofix-correctness risk.Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
api.anthropic.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.