Fix unmarshal text on missing value. (#56)

We invoke TextUnmarshaler.UnmarshalText even if a value is missing
from a config, which will lead to errors like #55. We should keep a zero
initialized value instead of unmarshalling an empty string.

Author: Alex

CHANGELOG.md

@@ -1,10 +1,10 @@
 # Changelog
 
-## v1.0.1 (unreleased)
+## v1.0.1 (2017-08-04)
 
-- No changes yet
+- Fixed unmarshal text on missing value.
 
-## v1.0.0 (07-31-2017)
+## v1.0.0 (2017-07-31)
 
 First stable release: no breaking changes will be made in the 1.x series.
 
@@ -23,6 +23,6 @@ First stable release: no breaking changes will be made in the 1.x series.
 - **[Breaking]** Unexport NewYAMLProviderFromReader* functions.
 - **[Breaking]** `NewProviderGroup` returns an error.
 
-## v1.0.0-rc1 (06-26-2017)
+## v1.0.0-rc1 (2017-06-26)
 
 - **[Breaking]** `Provider` interface was trimmed down to 2 methods: `Name` and `Get`

decoder.go

@@ -529,6 +529,11 @@ func jsonMap(v interface{}) interface{} {
 	return v
 }
 
+// Skip unmarshaling if there is no value in provider and default is empty.
+func shouldSkip(value *Value, def string) bool {
+	return !value.HasValue() && def == ""
+}
+
 // tryUnmarshallers checks if the value's type implements either one of standard
 // interfaces in order:
 // 1. `json.Unmarshaler`
@@ -548,10 +553,15 @@ func (d *decoder) tryUnmarshalers(key string, value reflect.Value, def string) (
 	switch t := value.Addr().Interface().(type) {
 	case json.Unmarshaler:
 		// Skip unmarshaling if there is no value.
-		if !v.HasValue() {
+		if shouldSkip(&v, def) {
 			return true, nil
 		}
 
+		// Use default if a value wasn't found.
+		if !v.HasValue() {
+			return true, errorWithKey(t.UnmarshalJSON([]byte(def)), key)
+		}
+
 		// Marshal the value first.
 		b, err := json.Marshal(jsonMap(v.Value()))
 		if err != nil {
@@ -561,18 +571,26 @@ func (d *decoder) tryUnmarshalers(key string, value reflect.Value, def string) (
 		// Unmarshal corresponding json.
 		return true, errorWithKey(t.UnmarshalJSON(b), key)
 	case encoding.TextUnmarshaler:
-		// check if we need to use custom defaults
+		if shouldSkip(&v, def) {
+			return true, nil
+		}
+
+		// Use default if a value wasn't found.
 		if v.HasValue() {
 			def = v.String()
 		}
 
 		return true, errorWithKey(t.UnmarshalText([]byte(def)), key)
 	case yaml.Unmarshaler:
-		// Skip unmarshaling if there is no value.
-		if !v.HasValue() {
+		if shouldSkip(&v, def) {
 			return true, nil
 		}
 
+		// Use default if a value wasn't found.
+		if !v.HasValue() {
+			return true, errorWithKey(yaml.Unmarshal([]byte(def), value.Addr().Interface()), key)
+		}
+
 		b, err := yaml.Marshal(v.Value())
 		if err != nil {
 			return true, err

static_provider_test.go

@@ -404,3 +404,12 @@ func TestStaticProviderConstructorErrors(t *testing.T) {
 		assert.Contains(t, err.Error(), `default is empty for "Email"`)
 	})
 }
+
+func TestTextUnmarshalerOnMissingValue(t *testing.T) {
+	t.Parallel()
+
+	p, err := NewStaticProvider(nil)
+	require.NoError(t, err)
+	ds := duckTales{}
+	require.NoError(t, p.Get(Root).Populate(&ds))
+}

yaml_test.go

@@ -921,6 +921,7 @@ func (m *unmarshallerFunc) UnmarshalText(text []byte) error {
 
 func TestHappyUnmarshallerChannelFunction(t *testing.T) {
 	t.Parallel()
+
 	type Chart struct {
 		Band unmarshallerChan `default:"Beatles"`
 		Song unmarshallerFunc `default:"back"`
@@ -1336,6 +1337,35 @@ func (y *yamlUnmarshal) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return nil
 }
 
+func TestPopulateUnmarshalersWithDefaultsFromTags(t *testing.T) {
+	t.Parallel()
+
+	t.Run("JSON", func(t *testing.T) {
+		var j struct {
+			J jsonUnmarshaller `default:"explode"`
+		}
+
+		p, err := NewStaticProvider(nil)
+		require.NoError(t, err)
+
+		err = p.Get(Root).Populate(&j)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "boom")
+	})
+
+	t.Run("YAML", func(t *testing.T) {
+		var y struct {
+			Y yamlUnmarshal `default:"Fake: fake"`
+		}
+
+		p, err := NewStaticProvider(nil)
+		require.NoError(t, err)
+
+		err = p.Get(Root).Populate(&y)
+		require.NoError(t, err)
+		assert.Equal(t, yamlUnmarshal{Name: "Fake"}, y.Y)
+	})
+}
 func TestPopulateNotAppropriateTypes(t *testing.T) {
 	t.Parallel()
 
@@ -1673,6 +1703,7 @@ func TestArrayElementInDifferentPositions(t *testing.T) {
 a:
 - protagonist: Scrooge
   pilot: LaunchpadMcQuack
+- protagonist:
 `))
 
 		require.NoError(t, err, "Can't create a YAML provider")