diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..b97cd6e --- /dev/null +++ b/Makefile @@ -0,0 +1,31 @@ +TEST?=./... + +default: test + +# test runs the test suite and vets the code. +test: generate + @echo "==> Running tests..." + @go list $(TEST) \ + | grep -v "/vendor/" \ + | xargs -n1 go test -timeout=60s -parallel=10 ${TESTARGS} + +# testrace runs the race checker +testrace: generate + @echo "==> Running tests (race)..." + @go list $(TEST) \ + | grep -v "/vendor/" \ + | xargs -n1 go test -timeout=60s -race ${TESTARGS} + +# updatedeps installs all the dependencies needed to run and build. +updatedeps: + @sh -c "'${CURDIR}/scripts/deps.sh' '${NAME}'" + +# generate runs `go generate` to build the dynamically generated source files. +generate: + @echo "==> Generating..." + @find . -type f -name '.DS_Store' -delete + @go list ./... \ + | grep -v "/vendor/" \ + | xargs -n1 go generate + +.PHONY: default test testrace updatedeps generate diff --git a/README.md b/README.md index e81be50..e744938 100644 --- a/README.md +++ b/README.md @@ -8,16 +8,12 @@ be a list of errors. If the caller knows this, they can unwrap the list and access the errors. If the caller doesn't know, the error formats to a nice human-readable format. -`go-multierror` implements the -[errwrap](https://github.com/hashicorp/errwrap) interface so that it can -be used with that library, as well. +This is a fork of the hashicorp `go-multierror` library. In this +fork, nil error values are handled transparently. ## Installation and Docs -Install using `go get github.com/hashicorp/go-multierror`. - -Full documentation is available at -http://godoc.org/github.com/hashicorp/go-multierror +Install using `go get github.com/mspiegel/go-multierror`. ## Usage @@ -32,14 +28,12 @@ if the first argument is nil, a `multierror.Error`, or any other `error`, the function behaves as you would expect. ```go -var result error +var err, result error -if err := step1(); err != nil { - result = multierror.Append(result, err) -} -if err := step2(); err != nil { - result = multierror.Append(result, err) -} +err = step1() +result = multierror.Append(result, err) +err = step2() +result = multierror.Append(result, err) return result ``` @@ -73,19 +67,4 @@ if err := something(); err != nil { // Use merr.Errors } } -``` - -**Returning a multierror only if there are errors** - -If you build a `multierror.Error`, you can use the `ErrorOrNil` function -to return an `error` implementation only if there are errors to return: - -```go -var result *multierror.Error - -// ... accumulate errors here - -// Return the `error` only if errors were added to the multierror, otherwise -// return nil since there are no errors. -return result.ErrorOrNil() -``` +``` \ No newline at end of file diff --git a/append.go b/append.go index 00afa9b..6826205 100644 --- a/append.go +++ b/append.go @@ -1,12 +1,61 @@ package multierror +import ( + "reflect" +) + +// removeNils preserves the non-nil elements +// of a slice. This is a destructive operation +// the contents of the original slice are modified. +func removeNils(errs []error) []error { + view := errs[:0] + for _, err := range errs { + if err == nil { + continue + } + add := true + switch reflect.TypeOf(err).Kind() { + case reflect.Chan, reflect.Func, + reflect.Interface, reflect.Map, + reflect.Ptr, reflect.Slice: + add = !reflect.ValueOf(err).IsNil() + } + if add { + view = append(view, err) + } + } + return view +} + // Append is a helper function that will append more errors // onto an Error in order to create a larger multi-error. // // If err is not a multierror.Error, then it will be turned into // one. If any of the errs are multierr.Error, they will be flattened // one level into err. -func Append(err error, errs ...error) *Error { +// +// nil values in errs are filtered out. If the err is nil and +// the length of filtered errs is zero then the function returns nil. +func Append(err error, errs ...error) error { + + errs = removeNils(errs) + // Preserve input value when no errors have occurred + // Preserve output value when only one error is produced + if len(errs) == 0 { + return err + } else if (err == nil) && len(errs) == 1 { + return errs[0] + } + return appendInternal(err, errs...) +} + +// appendInternal is a helper function that will append more errors +// onto an Error in order to create a larger multi-error. +// +// If err is not a multierror.Error, then it will be turned into +// one. If any of the errs are multierr.Error, they will be flattened +// one level into err. +func appendInternal(err error, errs ...error) *Error { switch err := err.(type) { case *Error: // Typed nils can reach here, so initialize if we are nil @@ -32,6 +81,6 @@ func Append(err error, errs ...error) *Error { } newErrs = append(newErrs, errs...) - return Append(&Error{}, newErrs...) + return appendInternal(&Error{}, newErrs...) } } diff --git a/append_test.go b/append_test.go index dfa79e2..9ccc3fc 100644 --- a/append_test.go +++ b/append_test.go @@ -5,25 +5,37 @@ import ( "testing" ) +type TestError struct{} + +func (e TestError) Error() string { return "TestError" } + +func TestRemoveNils(t *testing.T) { + errs := []error{errors.New("foo"), nil, nil, errors.New("foo"), nil} + errs = removeNils(errs) + if len(errs) != 2 { + t.Fatalf("wrong len: %d", len(errs)) + } +} + func TestAppend_Error(t *testing.T) { original := &Error{ Errors: []error{errors.New("foo")}, } - result := Append(original, errors.New("bar")) + result := Append(original, errors.New("bar")).(*Error) if len(result.Errors) != 2 { t.Fatalf("wrong len: %d", len(result.Errors)) } original = &Error{} - result = Append(original, errors.New("bar")) + result = Append(original, errors.New("bar")).(*Error) if len(result.Errors) != 1 { t.Fatalf("wrong len: %d", len(result.Errors)) } // Test when a typed nil is passed var e *Error - result = Append(e, errors.New("baz")) + result = Append(e, errors.New("baz")).(*Error) if len(result.Errors) != 1 { t.Fatalf("wrong len: %d", len(result.Errors)) } @@ -33,7 +45,7 @@ func TestAppend_Error(t *testing.T) { Errors: []error{errors.New("foo")}, } - result = Append(original, Append(nil, errors.New("foo"), errors.New("bar"))) + result = Append(original, Append(nil, errors.New("foo"), errors.New("bar"))).(*Error) if len(result.Errors) != 3 { t.Fatalf("wrong len: %d", len(result.Errors)) } @@ -42,14 +54,50 @@ func TestAppend_Error(t *testing.T) { func TestAppend_NilError(t *testing.T) { var err error result := Append(err, errors.New("bar")) - if len(result.Errors) != 1 { - t.Fatalf("wrong len: %d", len(result.Errors)) + if result.Error() != "bar" { + t.Fatalf("wrong error: %s", result.Error()) + } +} + +func TestAppend_NilNil(t *testing.T) { + var err error + result := Append(err, nil) + if result != nil { + t.Fatalf("non-nil errors: %s", result.Error()) + } +} + +func TestAppendNonNil(t *testing.T) { + var err1 error + var err2 *Error + result := Append(err1, err2, nil, nil) + if result != nil { + t.Fatalf("non-nil errors: %s", result.Error()) + } + err1 = errors.New("foo") + result = Append(err1, err2, nil, nil) + if result != err1 { + t.Fatalf("input error modified: %s", result.Error()) + } + err1 = errors.New("foo") + result = Append(nil, err1, nil, nil) + if result != err1 { + t.Fatalf("input error modified: %s", result.Error()) + } +} + +func TestAppendNonNilStruct(t *testing.T) { + var err1 error + var err3 TestError + result := Append(err1, err3, nil) + if result == nil { + t.Fatalf("TestError was not appended") } } func TestAppend_NonError(t *testing.T) { original := errors.New("foo") - result := Append(original, errors.New("bar")) + result := Append(original, errors.New("bar")).(*Error) if len(result.Errors) != 2 { t.Fatalf("wrong len: %d", len(result.Errors)) } @@ -57,7 +105,7 @@ func TestAppend_NonError(t *testing.T) { func TestAppend_NonError_Error(t *testing.T) { original := errors.New("foo") - result := Append(original, Append(nil, errors.New("bar"))) + result := Append(original, Append(nil, errors.New("bar"))).(*Error) if len(result.Errors) != 2 { t.Fatalf("wrong len: %d", len(result.Errors)) } diff --git a/multierror.go b/multierror.go index 2ea0827..70f0593 100644 --- a/multierror.go +++ b/multierror.go @@ -20,21 +20,6 @@ func (e *Error) Error() string { return fn(e.Errors) } -// ErrorOrNil returns an error interface if this Error represents -// a list of errors, or returns nil if the list of errors is empty. This -// function is useful at the end of accumulation to make sure that the value -// returned represents the existence of errors. -func (e *Error) ErrorOrNil() error { - if e == nil { - return nil - } - if len(e.Errors) == 0 { - return nil - } - - return e -} - func (e *Error) GoString() string { return fmt.Sprintf("*%#v", *e) } diff --git a/multierror_test.go b/multierror_test.go index 3e78079..60cbc18 100644 --- a/multierror_test.go +++ b/multierror_test.go @@ -43,20 +43,6 @@ func TestErrorError_default(t *testing.T) { } } -func TestErrorErrorOrNil(t *testing.T) { - err := new(Error) - if err.ErrorOrNil() != nil { - t.Fatalf("bad: %#v", err.ErrorOrNil()) - } - - err.Errors = []error{errors.New("foo")} - if v := err.ErrorOrNil(); v == nil { - t.Fatal("should not be nil") - } else if !reflect.DeepEqual(v, err) { - t.Fatalf("bad: %#v", v) - } -} - func TestErrorWrappedErrors(t *testing.T) { errors := []error{ errors.New("foo"), diff --git a/prefix.go b/prefix.go deleted file mode 100644 index 5c477ab..0000000 --- a/prefix.go +++ /dev/null @@ -1,37 +0,0 @@ -package multierror - -import ( - "fmt" - - "github.com/hashicorp/errwrap" -) - -// Prefix is a helper function that will prefix some text -// to the given error. If the error is a multierror.Error, then -// it will be prefixed to each wrapped error. -// -// This is useful to use when appending multiple multierrors -// together in order to give better scoping. -func Prefix(err error, prefix string) error { - if err == nil { - return nil - } - - format := fmt.Sprintf("%s {{err}}", prefix) - switch err := err.(type) { - case *Error: - // Typed nils can reach here, so initialize if we are nil - if err == nil { - err = new(Error) - } - - // Wrap each of the errors - for i, e := range err.Errors { - err.Errors[i] = errwrap.Wrapf(format, e) - } - - return err - default: - return errwrap.Wrapf(format, err) - } -} diff --git a/prefix_test.go b/prefix_test.go deleted file mode 100644 index 1d4a6f6..0000000 --- a/prefix_test.go +++ /dev/null @@ -1,33 +0,0 @@ -package multierror - -import ( - "errors" - "testing" -) - -func TestPrefix_Error(t *testing.T) { - original := &Error{ - Errors: []error{errors.New("foo")}, - } - - result := Prefix(original, "bar") - if result.(*Error).Errors[0].Error() != "bar foo" { - t.Fatalf("bad: %s", result) - } -} - -func TestPrefix_NilError(t *testing.T) { - var err error - result := Prefix(err, "bar") - if result != nil { - t.Fatalf("bad: %#v", result) - } -} - -func TestPrefix_NonError(t *testing.T) { - original := errors.New("foo") - result := Prefix(original, "bar") - if result.Error() != "bar foo" { - t.Fatalf("bad: %s", result) - } -}