Errorf: support %w anywhere in the format string Allow a single %w to appear anywhere in the format string, matched by an operand that satisfies the error interface. This is complicated by a few things: - We don't want to do full-fledged printf verb parsing. We approximate, and do not handle argument indexes on %w like "%[3]w". - There is a messy interaction with the xerrors formatting system (not adopted in Go 1.13). When the %w is at the end we can control the error string so that the wrapped error's message is formatted properly, after the wrapping errors' message. With a %w in the middle, we can't do that. In this CL, we print the wrapped error's message twice: once in its place in the format string, and then again because it's wrapped. Fixes #go/33143. Change-Id: I6192096521664476e82d5a54b80e352e47397cfe Reviewed-on: https://go-review.googlesource.com/c/xerrors/+/186957 Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/fmt.go b/fmt.go index 74c1c93..829862d 100644 --- a/fmt.go +++ b/fmt.go
@@ -7,10 +7,14 @@ import ( "fmt" "strings" + "unicode" + "unicode/utf8" "golang.org/x/xerrors/internal" ) +const percentBangString = "%!" + // Errorf formats according to a format specifier and returns the string as a // value that satisfies error. // @@ -18,29 +22,71 @@ // formatted with additional detail enabled. If the last argument is an error // the returned error's Format method will return it if the format string ends // with ": %s", ": %v", or ": %w". If the last argument is an error and the -// format string ends with ": %w", the returned error implements Wrapper -// with an Unwrap method returning it. +// format string ends with ": %w", the returned error implements an Unwrap +// method returning it. +// +// If the format specifier includes a %w verb with an error operand in a +// position other than at the end, the returned error will still implement an +// Unwrap method returning the operand, but the error's Format method will not +// return the wrapped error. +// +// It is invalid to include more than one %w verb or to supply it with an +// operand that does not implement the error interface. The %w verb is otherwise +// a synonym for %v. func Errorf(format string, a ...interface{}) error { - err, wrap := lastError(format, a) format = formatPlusW(format) - if err == nil { - return &noWrapError{fmt.Sprintf(format, a...), nil, Caller(1)} + // Support a ": %[wsv]" suffix, which works well with xerrors.Formatter. + wrap := strings.HasSuffix(format, ": %w") + idx, format2, ok := parsePercentW(format) + percentWElsewhere := !wrap && idx >= 0 + if !percentWElsewhere && (wrap || strings.HasSuffix(format, ": %s") || strings.HasSuffix(format, ": %v")) { + err := errorAt(a, len(a)-1) + if err == nil { + return &noWrapError{fmt.Sprintf(format, a...), nil, Caller(1)} + } + // TODO: this is not entirely correct. The error value could be + // printed elsewhere in format if it mixes numbered with unnumbered + // substitutions. With relatively small changes to doPrintf we can + // have it optionally ignore extra arguments and pass the argument + // list in its entirety. + msg := fmt.Sprintf(format[:len(format)-len(": %s")], a[:len(a)-1]...) + frame := Frame{} + if internal.EnableTrace { + frame = Caller(1) + } + if wrap { + return &wrapError{msg, err, frame} + } + return &noWrapError{msg, err, frame} } - - // TODO: this is not entirely correct. The error value could be - // printed elsewhere in format if it mixes numbered with unnumbered - // substitutions. With relatively small changes to doPrintf we can - // have it optionally ignore extra arguments and pass the argument - // list in its entirety. - msg := fmt.Sprintf(format[:len(format)-len(": %s")], a[:len(a)-1]...) + // Support %w anywhere. + // TODO: don't repeat the wrapped error's message when %w occurs in the middle. + msg := fmt.Sprintf(format2, a...) + if idx < 0 { + return &noWrapError{msg, nil, Caller(1)} + } + err := errorAt(a, idx) + if !ok || err == nil { + // Too many %ws or argument of %w is not an error. Approximate the Go + // 1.13 fmt.Errorf message. + return &noWrapError{fmt.Sprintf("%sw(%s)", percentBangString, msg), nil, Caller(1)} + } frame := Frame{} if internal.EnableTrace { frame = Caller(1) } - if wrap { - return &wrapError{msg, err, frame} + return &wrapError{msg, err, frame} +} + +func errorAt(args []interface{}, i int) error { + if i < 0 || i >= len(args) { + return nil } - return &noWrapError{msg, err, frame} + err, ok := args[i].(error) + if !ok { + return nil + } + return err } // formatPlusW is used to avoid the vet check that will barf at %w. @@ -48,24 +94,56 @@ return s } -func lastError(format string, a []interface{}) (err error, wrap bool) { - wrap = strings.HasSuffix(format, ": %w") - if !wrap && - !strings.HasSuffix(format, ": %s") && - !strings.HasSuffix(format, ": %v") { - return nil, false +// Return the index of the only %w in format, or -1 if none. +// Also return a rewritten format string with %w replaced by %v, and +// false if there is more than one %w. +// TODO: handle "%[N]w". +func parsePercentW(format string) (idx int, newFormat string, ok bool) { + // Loosely copied from golang.org/x/tools/go/analysis/passes/printf/printf.go. + idx = -1 + ok = true + n := 0 + sz := 0 + var isW bool + for i := 0; i < len(format); i += sz { + if format[i] != '%' { + sz = 1 + continue + } + // "%%" is not a format directive. + if i+1 < len(format) && format[i+1] == '%' { + sz = 2 + continue + } + sz, isW = parsePrintfVerb(format[i:]) + if isW { + if idx >= 0 { + ok = false + } else { + idx = n + } + // "Replace" the last character, the 'w', with a 'v'. + p := i + sz - 1 + format = format[:p] + "v" + format[p+1:] + } + n++ } + return idx, format, ok +} - if len(a) == 0 { - return nil, false +// Parse the printf verb starting with a % at s[0]. +// Return how many bytes it occupies and whether the verb is 'w'. +func parsePrintfVerb(s string) (int, bool) { + // Assume only that the directive is a sequence of non-letters followed by a single letter. + sz := 0 + var r rune + for i := 1; i < len(s); i += sz { + r, sz = utf8.DecodeRuneInString(s[i:]) + if unicode.IsLetter(r) { + return i + sz, r == 'w' + } } - - err, ok := a[len(a)-1].(error) - if !ok { - return nil, false - } - - return err, wrap + return len(s), false } type noWrapError struct {
diff --git a/fmt_test.go b/fmt_test.go index 6744b8a..7bf96a7 100644 --- a/fmt_test.go +++ b/fmt_test.go
@@ -62,6 +62,10 @@ chain("wraps:wrapw/path.TestErrorf/path.go:xxx", "chained/somefile.go:xxx"), }, { + xerrors.Errorf("wrapw %w middle", chained), + chain("wraps:wrapw chained middle/path.TestErrorf/path.go:xxx", + "chained/somefile.go:xxx"), + }, { xerrors.Errorf("not wrapped: %+v", chained), chain("not wrapped: chained: somefile.go:123/path.TestErrorf/path.go:xxx"), }} @@ -156,7 +160,7 @@ fmt: "%+v", want: "something:" + "\n golang.org/x/xerrors_test.TestErrorFormatter" + - "\n .+/fmt_test.go:97" + + "\n .+/fmt_test.go:101" + "\n something more", regexp: true, }, {
diff --git a/fmt_unexported_test.go b/fmt_unexported_test.go new file mode 100644 index 0000000..3affcae --- /dev/null +++ b/fmt_unexported_test.go
@@ -0,0 +1,51 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package xerrors + +import "testing" + +func TestParsePrintfVerb(t *testing.T) { + for _, test := range []struct { + in string + wantSize int + wantW bool + }{ + {"", 0, false}, + {"%", 1, false}, + {"%3.1", 4, false}, + {"%w", 2, true}, + {"%v", 2, false}, + {"%3.*[4]d", 8, false}, + } { + gotSize, gotW := parsePrintfVerb(test.in) + if gotSize != test.wantSize || gotW != test.wantW { + t.Errorf("parsePrintfVerb(%q) = (%d, %t), want (%d, %t)", + test.in, gotSize, gotW, test.wantSize, test.wantW) + } + } +} + +func TestParsePercentW(t *testing.T) { + for _, test := range []struct { + in string + wantIdx int + wantFormat string + wantOK bool + }{ + {"", -1, "", true}, + {"%", -1, "%", true}, + {"%w", 0, "%v", true}, + {"%w%w", 0, "%v%v", false}, + {"%3.2s %+q %% %w %#v", 2, "%3.2s %+q %% %v %#v", true}, + {"%3.2s %w %% %w %#v", 1, "%3.2s %v %% %v %#v", false}, + } { + gotIdx, gotFormat, gotOK := parsePercentW(test.in) + if gotIdx != test.wantIdx || gotFormat != test.wantFormat || gotOK != test.wantOK { + t.Errorf("parsePercentW(%q) = (%d, %q, %t), want (%d, %q, %t)", + test.in, gotIdx, gotFormat, gotOK, test.wantIdx, test.wantFormat, test.wantOK) + + } + } +}
diff --git a/wrap_113_test.go b/wrap_113_test.go new file mode 100644 index 0000000..25c3d80 --- /dev/null +++ b/wrap_113_test.go
@@ -0,0 +1,28 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//+build go1.13 + +package xerrors_test + +import ( + "errors" + "testing" + + "golang.org/x/xerrors" +) + +func TestErrorsIs(t *testing.T) { + var errSentinel = errors.New("sentinel") + + got := errors.Is(xerrors.Errorf("%w", errSentinel), errSentinel) + if !got { + t.Error("got false, want true") + } + + got = errors.Is(xerrors.Errorf("%w: %s", errSentinel, "foo"), errSentinel) + if !got { + t.Error("got false, want true") + } +}