Skip to content

Commit ac2ed98

Browse files
committed
go/analysis/passes/printf: warn against using non-error interface values with %w
CL 217180 permits passing interface values to any format verb, since we have no way of knowing if the underlying type implements fmt.Formatter. Restore the previous behavior of reporting an error for passing a non-error value to %w. The %w verb requires an error argument, and the risk of false positives is small. Fixes golang/go#48931 Change-Id: I83a6c9ed252976476f12372a490779336a1f536f Reviewed-on: https://go-review.googlesource.com/c/tools/+/355730 Trust: Damien Neil <[email protected]> Run-TryBot: Damien Neil <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 378b9e1 commit ac2ed98

File tree

2 files changed

+13
-1
lines changed

2 files changed

+13
-1
lines changed

go/analysis/passes/printf/printf.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -834,8 +834,9 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
834834
}
835835

836836
// Could current arg implement fmt.Formatter?
837+
// Skip check for the %w verb, which requires an error.
837838
formatter := false
838-
if state.argNum < len(call.Args) {
839+
if v.typ != argError && state.argNum < len(call.Args) {
839840
if tv, ok := pass.TypesInfo.Types[call.Args[state.argNum]]; ok {
840841
formatter = isFormatter(tv.Type)
841842
}

go/analysis/passes/printf/testdata/src/a/a.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ type errorTest5 int
5151
func (errorTest5) error() { // niladic; don't complain if no args (was bug)
5252
}
5353

54+
type errorTestOK int
55+
56+
func (errorTestOK) Error() string { return "" }
57+
5458
// This function never executes, but it serves as a simple test for the program.
5559
// Test with make test.
5660
func PrintfTests() {
@@ -327,12 +331,19 @@ func PrintfTests() {
327331
dbg("", 1) // no error "call has arguments but no formatting directive"
328332

329333
// %w
334+
var errSubset interface {
335+
Error() string
336+
A()
337+
}
330338
_ = fmt.Errorf("%w", err) // OK
331339
_ = fmt.Errorf("%#w", err) // OK
332340
_ = fmt.Errorf("%[2]w %[1]s", "x", err) // OK
333341
_ = fmt.Errorf("%[2]w %[1]s", e, "x") // want `fmt.Errorf format %\[2\]w has arg "x" of wrong type string`
334342
_ = fmt.Errorf("%w", "x") // want `fmt.Errorf format %w has arg "x" of wrong type string`
335343
_ = fmt.Errorf("%w %w", err, err) // want `fmt.Errorf call has more than one error-wrapping directive %w`
344+
_ = fmt.Errorf("%w", interface{}(nil)) // want `fmt.Errorf format %w has arg interface{}\(nil\) of wrong type interface{}`
345+
_ = fmt.Errorf("%w", errorTestOK(0)) // concrete value implements error
346+
_ = fmt.Errorf("%w", errSubset) // interface value implements error
336347
fmt.Printf("%w", err) // want `fmt.Printf does not support error-wrapping directive %w`
337348
var wt *testing.T
338349
wt.Errorf("%w", err) // want `\(\*testing.common\).Errorf does not support error-wrapping directive %w`

0 commit comments

Comments
 (0)