Preserve nil value when no errors have occurred.

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

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()
-```
+```

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...)
 	}
 }

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,22 +54,58 @@ 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))
 	}
 }
 
 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))
 	}

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

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"),