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")
+ }
+}
