[confmap] Return nil map if original map was nil (#13161)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Amends `ToStringMap` so that it returns `map[string]any(nil)` if the map
used to create this `Conf` was nil. Currently it returns an empty map.

I consider this a bug since, while not explicitly undocumented, I would
expect the following two properties to be true, and they were not before
this change:
1. For any map `m` without `expandedValue`s,
`NewFromStringMap(m).ToStringMap() == m`
2. For any map `m` without `expandedValue`s and any `path` referencing
an existing key in `m` with a `map[string]any` value,
`NewFromStringMap(m).Sub(path) == m[path[0]][path[1]][...][path[N]]`

<!-- Issue number if applicable -->
#### Link to tracking issue

I need this to be able to distinguish between 

```
foo:
   bar:
```

and

```
foo:
  bar: {}
```

which currently have different behaviors when mapping to pointers.

The goal is to be able to do #13168

Author: mx-psi

.chloggen/mx-psi_confmap-isnil.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Correctly distinguish between `nil` and empty map values on the `ToStringMap` method
+
+# One or more tracking issues or pull requests related to the change
+issues: [13161]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: | 
+  This means that `ToStringMap()` method can now return a nil map if the original value was `nil`.
+  If you were not doing so already, make sure to check for `nil` before writing to the map to avoid panics.
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

confmap/confmap.go

@@ -35,14 +35,18 @@ const (
 
 // New creates a new empty confmap.Conf instance.
 func New() *Conf {
-	return &Conf{k: koanf.New(KeyDelimiter)}
+	return &Conf{k: koanf.New(KeyDelimiter), isNil: false}
 }
 
 // NewFromStringMap creates a confmap.Conf from a map[string]any.
 func NewFromStringMap(data map[string]any) *Conf {
 	p := New()
-	// Cannot return error because the koanf instance is empty.
-	_ = p.k.Load(confmap.Provider(data, KeyDelimiter), nil)
+	if data == nil {
+		p.isNil = true
+	} else {
+		// Cannot return error because the koanf instance is empty.
+		_ = p.k.Load(confmap.Provider(data, KeyDelimiter), nil)
+	}
 	return p
 }
 
@@ -55,6 +59,9 @@ type Conf struct {
 	// This avoids running into an infinite recursion where Unmarshaler.Unmarshal and
 	// Conf.Unmarshal would call each other.
 	skipTopLevelUnmarshaler bool
+	// isNil is true if this Conf was created from a nil field, as opposed to an empty map.
+	// AllKeys must return an empty slice if this is true.
+	isNil bool
 }
 
 // AllKeys returns all keys holding a value, regardless of where they are set.
@@ -178,6 +185,7 @@ func (l *Conf) Merge(in *Conf) error {
 		// only use MergeAppend when enableMergeAppendOption featuregate is enabled.
 		return l.mergeAppend(in)
 	}
+	l.isNil = l.isNil && in.isNil
 	return l.k.Merge(in.k)
 }
 
@@ -196,7 +204,12 @@ func (l *Conf) Delete(key string) bool {
 // For example, if listA = [extension1, extension2] and listB = [extension1, extension3],
 // the resulting list will be [extension1, extension2, extension3].
 func (l *Conf) mergeAppend(in *Conf) error {
-	return l.k.Load(confmap.Provider(in.ToStringMap(), ""), nil, koanf.WithMergeFunc(mergeAppend))
+	err := l.k.Load(confmap.Provider(in.ToStringMap(), ""), nil, koanf.WithMergeFunc(mergeAppend))
+	if err != nil {
+		return err
+	}
+	l.isNil = l.isNil && in.isNil
+	return nil
 }
 
 // Sub returns new Conf instance representing a sub-config of this instance.
@@ -205,7 +218,9 @@ func (l *Conf) Sub(key string) (*Conf, error) {
 	// Code inspired by the koanf "Cut" func, but returns an error instead of empty map for unsupported sub-config type.
 	data := l.unsanitizedGet(key)
 	if data == nil {
-		return New(), nil
+		c := New()
+		c.isNil = true
+		return c, nil
 	}
 
 	switch v := data.(type) {
@@ -214,13 +229,23 @@ func (l *Conf) Sub(key string) (*Conf, error) {
 	case expandedValue:
 		if m, ok := v.Value.(map[string]any); ok {
 			return NewFromStringMap(m), nil
+		} else if v.Value == nil {
+			// If the value is nil, return a new empty Conf.
+			c := New()
+			c.isNil = true
+			return c, nil
 		}
+		// override data with the original value to make the error message more informative.
+		data = v.Value
 	}
 
 	return nil, fmt.Errorf("unexpected sub-config value kind for key:%s value:%v kind:%v", key, data, reflect.TypeOf(data).Kind())
 }
 
 func (l *Conf) toStringMapWithExpand() map[string]any {
+	if l.isNil {
+		return nil
+	}
 	m := maps.Unflatten(l.k.All(), KeyDelimiter)
 	return m
 }
@@ -231,6 +256,10 @@ func (l *Conf) toStringMapWithExpand() map[string]any {
 //
 // For example, for a Conf created from `foo: ${env:FOO}` and `FOO=123`
 // ToStringMap will return `map[string]any{"foo": 123}`.
+//
+// For any map `m`, `NewFromStringMap(m).ToStringMap() == m`.
+// In particular, if the Conf was created from a nil value,
+// ToStringMap will return map[string]any(nil).
 func (l *Conf) ToStringMap() map[string]any {
 	return sanitize(l.toStringMapWithExpand()).(map[string]any)
 }
@@ -481,6 +510,9 @@ func unmarshalerEmbeddedStructsHookFunc() mapstructure.DecodeHookFuncValue {
 						return nil, err
 					}
 					resultMap := conf.ToStringMap()
+					if fromAsMap == nil && len(resultMap) > 0 {
+						fromAsMap = make(map[string]any, len(resultMap))
+					}
 					for k, v := range resultMap {
 						fromAsMap[k] = v
 					}

confmap/confmap_test.go

@@ -562,6 +562,28 @@ func TestEmbeddedMarshalerError(t *testing.T) {
 	assert.EqualError(t, cfgMap.Unmarshal(tc), "error running encode hook: marshaling error")
 }
 
+type B struct {
+	String string `mapstructure:"string"`
+}
+
+func (b *B) Unmarshal(conf *Conf) error {
+	return conf.Unmarshal(b)
+}
+
+type A struct {
+	B `mapstructure:",squash"`
+}
+
+func (a *A) Unmarshal(conf *Conf) error {
+	return conf.Unmarshal(a)
+}
+
+func TestUnmarshalerEmbeddedNilMap(t *testing.T) {
+	cfg := A{}
+	nilConf := NewFromStringMap(nil)
+	require.NoError(t, nilConf.Unmarshal(&cfg))
+}
+
 func TestUnmarshalerKeepAlreadyInitialized(t *testing.T) {
 	cfgMap := NewFromStringMap(map[string]any{
 		"next": map[string]any{
@@ -1151,3 +1173,159 @@ func TestMapMerge(t *testing.T) {
 		})
 	}
 }
+
+func TestConfIsNil(t *testing.T) {
+	const subKey = "foo"
+	testCases := []struct {
+		name         string
+		input        map[string]any
+		expectIsNil  bool
+		subExpectNil bool
+		subExpectErr string
+	}{
+		{
+			name:         "nil input",
+			input:        nil,
+			expectIsNil:  true,
+			subExpectNil: true,
+		},
+		{
+			name:         "empty map",
+			input:        map[string]any{},
+			expectIsNil:  false,
+			subExpectNil: true,
+		},
+		{
+			name:         "nil subkey",
+			input:        map[string]any{subKey: nil},
+			expectIsNil:  false,
+			subExpectNil: true,
+		},
+		{
+			name:         "empty subkey",
+			input:        map[string]any{subKey: map[string]any{}},
+			expectIsNil:  false,
+			subExpectNil: false,
+		},
+		{
+			name:         "non-empty map",
+			input:        map[string]any{subKey: map[string]any{"bar": 42}},
+			expectIsNil:  false,
+			subExpectNil: false,
+		},
+		{
+			name:         "non-map subkey",
+			input:        map[string]any{subKey: 123},
+			expectIsNil:  false,
+			subExpectErr: "unexpected sub-config value kind",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			conf := NewFromStringMap(tc.input)
+			if tc.expectIsNil {
+				assert.Empty(t, conf.AllKeys())
+				assert.Equal(t, map[string]any(nil), conf.ToStringMap())
+			} else {
+				assert.NotEqual(t, map[string]any(nil), conf.ToStringMap())
+			}
+
+			sub, err := conf.Sub(subKey)
+			if tc.subExpectErr != "" {
+				assert.ErrorContains(t, err, tc.subExpectErr)
+			} else {
+				assert.NoError(t, err)
+				if tc.subExpectNil {
+					assert.Empty(t, sub.AllKeys())
+					assert.Equal(t, map[string]any(nil), sub.ToStringMap())
+				} else {
+					assert.NotEqual(t, map[string]any(nil), sub.ToStringMap())
+				}
+			}
+		})
+	}
+}
+
+func TestConfmapNilMerge(t *testing.T) {
+	tests := []struct {
+		name     string
+		left     map[string]any
+		right    map[string]any
+		expected map[string]any
+	}{
+		{
+			name:     "both nil",
+			left:     nil,
+			right:    nil,
+			expected: nil,
+		},
+		{
+			name:     "left nil",
+			left:     nil,
+			right:    map[string]any{"key": "value"},
+			expected: map[string]any{"key": "value"},
+		},
+		{
+			name:     "right nil",
+			left:     map[string]any{"key": "value"},
+			right:    nil,
+			expected: map[string]any{"key": "value"},
+		},
+		{
+			name:     "both non-nil",
+			left:     map[string]any{"key1": "value1"},
+			right:    map[string]any{"key2": "value2"},
+			expected: map[string]any{"key1": "value1", "key2": "value2"},
+		},
+		{
+			name:     "left empty, right non-empty",
+			left:     map[string]any{},
+			right:    map[string]any{"key": "value"},
+			expected: map[string]any{"key": "value"},
+		},
+		{
+			name:     "left non-empty, right empty",
+			left:     map[string]any{"key": "value"},
+			right:    map[string]any{},
+			expected: map[string]any{"key": "value"},
+		},
+		{
+			name:     "left nil, right empty",
+			left:     nil,
+			right:    map[string]any{},
+			expected: map[string]any{},
+		},
+		{
+			name:     "left empty, right nil",
+			left:     map[string]any{},
+			right:    nil,
+			expected: map[string]any{},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			leftConf := NewFromStringMap(test.left)
+			assert.Equal(t, test.left, leftConf.ToStringMap())
+			rightConf := NewFromStringMap(test.right)
+			assert.Equal(t, test.right, rightConf.ToStringMap())
+
+			err := leftConf.Merge(rightConf)
+			require.NoError(t, err)
+
+			assert.Equal(t, test.expected, leftConf.ToStringMap())
+		})
+
+		t.Run(test.name+"merge append", func(t *testing.T) {
+			leftConf := NewFromStringMap(test.left)
+			assert.Equal(t, test.left, leftConf.ToStringMap())
+			rightConf := NewFromStringMap(test.right)
+			assert.Equal(t, test.right, rightConf.ToStringMap())
+
+			err := leftConf.mergeAppend(rightConf)
+			require.NoError(t, err)
+
+			assert.Equal(t, test.expected, leftConf.ToStringMap())
+		})
+	}
+}