Merge pull request #43 from ilyakaznacheev/bugfix/40-env-rewrite

Fix env-default overwrite issue

cleanenv.go

@@ -19,6 +19,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"math"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -176,6 +177,11 @@ type structMeta struct {
 	updatable   bool
 }
 
+// isFieldValueZero determines if fieldValue empty or not
+func (sm *structMeta) isFieldValueZero() bool {
+	return isZero(sm.fieldValue)
+}
+
 // readStructMetadata reads structure metadata (types, tags, etc.)
 func readStructMetadata(cfgRoot interface{}) ([]structMeta, error) {
 	cfgStack := []interface{}{cfgRoot}
@@ -277,7 +283,7 @@ func readEnvVars(cfg interface{}, update bool) error {
 			continue
 		}
 
-		rawValue := meta.defValue
+		var rawValue *string
 
 		for _, env := range meta.envList {
 			if value, ok := os.LookupEnv(env); ok {
@@ -286,6 +292,10 @@ func readEnvVars(cfg interface{}, update bool) error {
 			}
 		}
 
+		if rawValue == nil && meta.isFieldValueZero() {
+			rawValue = meta.defValue
+		}
+
 		if rawValue == nil {
 			continue
 		}
@@ -515,3 +525,42 @@ func FUsage(w io.Writer, cfg interface{}, headerText *string, usageFuncs ...func
 		fmt.Fprintln(w, text)
 	}
 }
+
+// isZero is a backport of reflect.Value.IsZero()
+func isZero(v reflect.Value) bool {
+	switch v.Kind() {
+	case reflect.Bool:
+		return !v.Bool()
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		return v.Int() == 0
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		return v.Uint() == 0
+	case reflect.Float32, reflect.Float64:
+		return math.Float64bits(v.Float()) == 0
+	case reflect.Complex64, reflect.Complex128:
+		c := v.Complex()
+		return math.Float64bits(real(c)) == 0 && math.Float64bits(imag(c)) == 0
+	case reflect.Array:
+		for i := 0; i < v.Len(); i++ {
+			if !isZero(v.Index(i)) {
+				return false
+			}
+		}
+		return true
+	case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice, reflect.UnsafePointer:
+		return v.IsNil()
+	case reflect.String:
+		return v.Len() == 0
+	case reflect.Struct:
+		for i := 0; i < v.NumField(); i++ {
+			if !isZero(v.Field(i)) {
+				return false
+			}
+		}
+		return true
+	default:
+		// This should never happens, but will act as a safeguard for
+		// later, as a default value doesn't makes sense here.
+		panic(fmt.Sprintf("Value.IsZero: %v", v.Kind()))
+	}
+}

cleanenv_test.go

@@ -855,6 +855,7 @@ func TestReadConfig(t *testing.T) {
 		Number    int64  `yaml:"number" env:"TEST_NUMBER" env-default:"1"`
 		String    string `yaml:"string" env:"TEST_STRING" env-default:"default"`
 		NoDefault string `yaml:"no-default" env:"TEST_NO_DEFAULT"`
+		NoEnv     string `yaml:"no-env" env-default:"default"`
 	}
 
 	tests := []struct {
@@ -871,13 +872,15 @@ func TestReadConfig(t *testing.T) {
 number: 2
 string: test
 no-default: NoDefault
+no-env: this
 `,
 			ext: "yaml",
 			env: nil,
 			want: &config{
-				Number:    1,
-				String:    "default",
+				Number:    2,
+				String:    "test",
 				NoDefault: "NoDefault",
+				NoEnv:     "this",
 			},
 			wantErr: false,
 		},
@@ -894,6 +897,29 @@ no-default: NoDefault
 				Number:    2,
 				String:    "test",
 				NoDefault: "",
+				NoEnv:     "default",
+			},
+			wantErr: false,
+		},
+
+		{
+			name: "yaml_and_env",
+			file: `
+number: 2
+string: test
+no-default: NoDefault
+no-env: this
+`,
+			ext: "yaml",
+			env: map[string]string{
+				"TEST_NUMBER": "3",
+				"TEST_STRING": "fromEnv",
+			},
+			want: &config{
+				Number:    3,
+				String:    "fromEnv",
+				NoDefault: "NoDefault",
+				NoEnv:     "this",
 			},
 			wantErr: false,
 		},
@@ -907,6 +933,7 @@ no-default: NoDefault
 				Number:    1,
 				String:    "default",
 				NoDefault: "",
+				NoEnv:     "default",
 			},
 			wantErr: false,
 		},