Skip doing any work if `Also` receives no errors (#1256)

* Skip doing any work if Also receives no errors

1. if no errors are passed skip doing stuff (checks, allocations, etc)
2. if there's one error passed and it's empty (the same)
  - this is a quite popular pattern where in API validation we do
    `x.Also(validateY())` and if that succeeded then it'll be an empty
    error. Which is in reality most of the time.
3. Improve tests by better printing
4. better test coverage shows commutativity of Also(a, b) and
   Also(a).Also(b)

* fix flow
This commit is contained in:
Victor Agababov 2020-04-28 12:03:51 -07:00 committed by GitHub
parent 6d25779288
commit 23f2fed590
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 7 deletions

View File

@ -109,6 +109,11 @@ func (fe *FieldError) ViaFieldKey(field string, key string) *FieldError {
// Also collects errors, returns a new collection of existing errors and new errors.
func (fe *FieldError) Also(errs ...*FieldError) *FieldError {
// Avoid doing any work, if we don't have to.
if l := len(errs); l == 0 || l == 1 && errs[0].isEmpty() {
return fe
}
var newErr *FieldError
// collect the current objects errors, if it has any
if !fe.isEmpty() {

View File

@ -595,10 +595,10 @@ not without this: bar.C`,
if test.want != "" {
if got, want := fe.Error(), test.want; got != want {
t.Errorf("%s: Error() = %v, wanted %v", test.name, got, want)
t.Errorf("\nGot: %v,\nwant: %v diff(-want,+got)\n%s", got, want, cmp.Diff(want, got))
}
} else if fe != nil {
t.Errorf("%s: ViaField() = %v, wanted nil", test.name, fe)
t.Errorf("ViaField() = %v, wanted nil", fe)
}
})
}
@ -614,6 +614,11 @@ func TestAlsoNil(t *testing.T) {
if got, want := errs.Error(), "original: foo"; got != want {
t.Errorf("TestAlsoNil: Also(nil).Error() = %v, wanted %v", got, want)
}
errs = errs.Also([]*FieldError{nil}...)
if got, want := errs.Error(), "original: foo"; got != want {
t.Errorf("TestAlsoNil: Also(nil).Error() = %v, wanted %v", got, want)
}
}
func TestMergeFieldErrors(t *testing.T) {
@ -723,18 +728,15 @@ more details`,
func TestAlsoStaysNil(t *testing.T) {
var err *FieldError
if err != nil {
t.Errorf("expected nil, got %v, wanted nil", err)
}
err = err.Also(nil)
if err != nil {
t.Errorf("expected nil, got %v, wanted nil", err)
t.Errorf("Got: %v, want nil", err)
}
err = err.ViaField("nil").Also(nil)
if err != nil {
t.Errorf("expected nil, got %v, wanted nil", err)
t.Errorf("Got: %v, want nil", err)
}
err = err.Also(&FieldError{})
@ -743,6 +745,30 @@ func TestAlsoStaysNil(t *testing.T) {
}
}
func TestAlsoMultiple(t *testing.T) {
var err *FieldError
errs := []*FieldError{{
Message: "1",
Paths: []string{"bar"},
}, {
Message: "2",
Paths: []string{"baz"},
}}
err = err.Also(errs...)
const want = "1: bar\n2: baz"
if got := err.Error(); got != want {
t.Errorf("Got = %q, want: %q", got, want)
}
// This shows the order of Also calls does not matter.
err = nil
err = err.Also(errs[0]).Also(errs[1])
if got := err.Error(); got != want {
t.Errorf("Got = %q, want: %q", got, want)
}
}
func TestFlatten(t *testing.T) {
tests := []struct {
name string