From ab894fa8e9de691d1f8eee2698b9948e6dbd961a Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Tue, 31 Oct 2023 03:41:57 -0700
Subject: [PATCH 1/7] Work on convert.FromTyped

---
 libs/config/convert/from_typed.go      | 214 +++++++++++++++++++++
 libs/config/convert/from_typed_test.go | 247 +++++++++++++++++++++++++
 libs/config/convert/struct_info.go     |  26 +++
 3 files changed, 487 insertions(+)
 create mode 100644 libs/config/convert/from_typed.go
 create mode 100644 libs/config/convert/from_typed_test.go

diff --git a/libs/config/convert/from_typed.go b/libs/config/convert/from_typed.go
new file mode 100644
index 00000000000..d2c166ff8c1
--- /dev/null
+++ b/libs/config/convert/from_typed.go
@@ -0,0 +1,214 @@
+package convert
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/databricks/cli/libs/config"
+)
+
+// FromTyped converts changes made in the typed structure w.r.t. the configuration value
+// back to the configuration value, retaining existing location information where possible.
+func FromTyped(src any, ref config.Value) (config.Value, error) {
+	srcv := reflect.ValueOf(src)
+
+	// Dereference pointer if necessary
+	for srcv.Kind() == reflect.Pointer {
+		if srcv.IsNil() {
+			return config.NilValue, nil
+		}
+		srcv = srcv.Elem()
+	}
+
+	switch srcv.Kind() {
+	case reflect.Struct:
+		return fromTypedStruct(srcv, ref)
+	case reflect.Map:
+		return fromTypedMap(srcv, ref)
+	case reflect.Slice:
+		return fromTypedSlice(srcv, ref)
+	case reflect.String:
+		return fromTypedString(srcv, ref)
+	case reflect.Bool:
+		return fromTypedBool(srcv, ref)
+	case reflect.Int, reflect.Int32, reflect.Int64:
+		return fromTypedInt(srcv, ref)
+	case reflect.Float32, reflect.Float64:
+		// return fromTypedFloat(srcv, dst)
+	}
+
+	return config.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
+}
+
+func fromTypedStruct(src reflect.Value, ref config.Value) (config.Value, error) {
+	switch ref.Kind() {
+	case config.KindMap, config.KindNil:
+		// Nothing to do.
+	default:
+		panic("type error")
+	}
+
+	out := make(map[string]config.Value)
+	info := getStructInfo(src.Type())
+	for k, v := range info.FieldValues(src) {
+		// Convert the field taking into account the reference value (may be equal to config.NilValue).
+		nv, err := FromTyped(v.Interface(), ref.Get(k))
+		if err != nil {
+			return config.Value{}, err
+		}
+
+		if nv != config.NilValue {
+			out[k] = nv
+		}
+	}
+
+	// If the struct was equal to its zero value, emit a nil.
+	if len(out) == 0 {
+		return config.NilValue, nil
+	}
+
+	return config.NewValue(out, ref.Location()), nil
+
+	// what are my options
+	// totyped / fromtyped at every mutator boundary
+	// pro's -- minimal changes to existing mutators
+	// con's -- doesn't hold for all mutators, so we need different interface ANYWAY
+	// (e.g. get/set config.Value instances)
+	// cons -- lossy (cannot do all to/from conversions, lose location, lose variables)
+
+	// explicit mutator interface
+	// pro's -- very clear what's happening
+	// cons -- all code + tests need to be changed
+	//
+
+	// need an incremental approach
+	// thus, we run totyped + fromtyped at mutator boundary
+	// can eventually move this into the mutators themselves?
+	// can treat the typed structure as read-only, perhaps?
+	// can generate wrapper type that exposes a Get + GetValue at every node
+
+}
+
+func fromTypedMap(src reflect.Value, ref config.Value) (config.Value, error) {
+	switch ref.Kind() {
+	case config.KindMap, config.KindNil:
+		// Nothing to do.
+	default:
+		panic("type error")
+	}
+
+	out := make(map[string]config.Value)
+	iter := src.MapRange()
+	for iter.Next() {
+		k := iter.Key().String()
+		v := iter.Value()
+
+		// Convert entry taking into account the reference value (may be equal to config.NilValue).
+		nv, err := FromTyped(v.Interface(), ref.Get(k))
+		if err != nil {
+			return config.Value{}, err
+		}
+
+		// Every entry is represented, even if it is a nil.
+		// Otherwise, a map with zero-valued structs would yield a nil as well.
+		out[k] = nv
+	}
+
+	// If the map has no entries, emit a nil.
+	if len(out) == 0 {
+		return config.NilValue, nil
+	}
+
+	return config.NewValue(out, ref.Location()), nil
+}
+
+func fromTypedSlice(src reflect.Value, ref config.Value) (config.Value, error) {
+	switch ref.Kind() {
+	case config.KindSequence, config.KindNil:
+		// Nothing to do.
+	default:
+		panic("type error")
+	}
+
+	out := make([]config.Value, src.Len())
+	for i := 0; i < src.Len(); i++ {
+		v := src.Index(i)
+
+		// Convert entry taking into account the reference value (may be equal to config.NilValue).
+		nv, err := FromTyped(v.Interface(), ref.Index(i))
+		if err != nil {
+			return config.Value{}, err
+		}
+
+		out[i] = nv
+	}
+
+	// If the slice has no entries, emit a nil.
+	if len(out) == 0 {
+		return config.NilValue, nil
+	}
+
+	return config.NewValue(out, ref.Location()), nil
+}
+
+func fromTypedString(src reflect.Value, ref config.Value) (config.Value, error) {
+	if src.IsZero() {
+		return config.NilValue, nil
+	}
+
+	switch ref.Kind() {
+	case config.KindString:
+		value := src.String()
+		if value == ref.MustString() {
+			return ref, nil
+		}
+
+		return config.V(value), nil
+	case config.KindNil:
+		return config.V(src.String()), nil
+	}
+
+	return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+}
+
+func fromTypedBool(src reflect.Value, ref config.Value) (config.Value, error) {
+	// Note: this means it's not possible to flip a boolean to false on a typed
+	// structure and see it reflected in the dynamic configuration.
+	// This case is not handled as is, so we punt on it until the mutaotrs
+	// modify the dynamic configuration directly.
+	if src.IsZero() {
+		return config.NilValue, nil
+	}
+
+	switch ref.Kind() {
+	case config.KindBool:
+		value := src.Bool()
+		if value == ref.MustBool() {
+			return ref, nil
+		}
+		return config.V(value), nil
+	case config.KindNil:
+		return config.V(src.Bool()), nil
+	}
+
+	return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+}
+
+func fromTypedInt(src reflect.Value, ref config.Value) (config.Value, error) {
+	if src.IsZero() {
+		return config.NilValue, nil
+	}
+
+	switch ref.Kind() {
+	case config.KindInt:
+		value := src.Int()
+		if value == ref.MustInt() {
+			return ref, nil
+		}
+		return config.V(value), nil
+	case config.KindNil:
+		return config.V(src.Bool()), nil
+	}
+
+	return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+}
diff --git a/libs/config/convert/from_typed_test.go b/libs/config/convert/from_typed_test.go
new file mode 100644
index 00000000000..cb5400b9fd8
--- /dev/null
+++ b/libs/config/convert/from_typed_test.go
@@ -0,0 +1,247 @@
+package convert
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/libs/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFromTypedStructZeroFields(t *testing.T) {
+	type Tmp struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+
+	src := Tmp{}
+	ref := config.V(map[string]config.Value{
+		"foo": config.V("bar"),
+		"bar": config.V("baz"),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedStructSetFields(t *testing.T) {
+	type Tmp struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+
+	src := Tmp{
+		Foo: "foo",
+		Bar: "bar",
+	}
+
+	ref := config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(map[string]config.Value{
+		"foo": config.V("foo"),
+		"bar": config.V("bar"),
+	}), nv)
+}
+
+func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
+	type Tmp struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+
+	src := Tmp{
+		Foo: "bar",
+		Bar: "qux",
+	}
+
+	ref := config.V(map[string]config.Value{
+		"foo": config.NewValue("bar", config.Location{File: "foo"}),
+		"bar": config.NewValue("baz", config.Location{File: "bar"}),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+
+	// Assert foo has retained its location.
+	assert.Equal(t, config.NewValue("bar", config.Location{File: "foo"}), nv.Get("foo"))
+
+	// Assert bar lost its location (because it was overwritten).
+	assert.Equal(t, config.NewValue("qux", config.Location{}), nv.Get("bar"))
+}
+
+func TestFromTypedMapEmpty(t *testing.T) {
+	var src = map[string]string{}
+
+	ref := config.V(map[string]config.Value{
+		"foo": config.V("bar"),
+		"bar": config.V("baz"),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedMapNonEmpty(t *testing.T) {
+	var src = map[string]string{
+		"foo": "foo",
+		"bar": "bar",
+	}
+
+	ref := config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(map[string]config.Value{
+		"foo": config.V("foo"),
+		"bar": config.V("bar"),
+	}), nv)
+}
+
+func TestFromTypedMapNonEmptyRetainLocationIfUnchanged(t *testing.T) {
+	var src = map[string]string{
+		"foo": "bar",
+		"bar": "qux",
+	}
+
+	ref := config.V(map[string]config.Value{
+		"foo": config.NewValue("bar", config.Location{File: "foo"}),
+		"bar": config.NewValue("baz", config.Location{File: "bar"}),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+
+	// Assert foo has retained its location.
+	assert.Equal(t, config.NewValue("bar", config.Location{File: "foo"}), nv.Get("foo"))
+
+	// Assert bar lost its location (because it was overwritten).
+	assert.Equal(t, config.NewValue("qux", config.Location{}), nv.Get("bar"))
+}
+
+func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
+	var src = map[string]string{
+		"foo": "",
+	}
+
+	ref := config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(map[string]config.Value{
+		"foo": config.NilValue,
+	}), nv)
+}
+
+func TestFromTypedSliceEmpty(t *testing.T) {
+	var src = []string{}
+
+	ref := config.V([]config.Value{
+		config.V("bar"),
+		config.V("baz"),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedSliceNonEmpty(t *testing.T) {
+	var src = []string{
+		"foo",
+		"bar",
+	}
+
+	ref := config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V([]config.Value{
+		config.V("foo"),
+		config.V("bar"),
+	}), nv)
+}
+
+func TestFromTypedSliceNonEmptyRetainLocationIfUnchanged(t *testing.T) {
+	var src = []string{
+		"foo",
+		"bar",
+	}
+
+	ref := config.V([]config.Value{
+		config.NewValue("foo", config.Location{File: "foo"}),
+		config.NewValue("baz", config.Location{File: "baz"}),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+
+	// Assert foo has retained its location.
+	assert.Equal(t, config.NewValue("foo", config.Location{File: "foo"}), nv.Index(0))
+
+	// Assert bar lost its location (because it was overwritten).
+	assert.Equal(t, config.NewValue("bar", config.Location{}), nv.Index(1))
+}
+
+func TestFromTypedStringEmpty(t *testing.T) {
+	var src string
+	var ref = config.V("string")
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedStringNonEmpty(t *testing.T) {
+	var src = "new"
+	var ref = config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V("new"), nv)
+}
+
+func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
+	var src = "new"
+	var ref = config.V("old")
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V("new"), nv)
+}
+
+func TestFromTypedStringRetainsLocationsIfUnchanged(t *testing.T) {
+	var src = "foo"
+	var ref = config.NewValue("foo", config.Location{File: "foo"})
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NewValue("foo", config.Location{File: "foo"}), nv)
+}
+
+func TestFromTypedBoolEmpty(t *testing.T) {
+	var src bool
+	var ref = config.V(true)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedBoolNonEmpty(t *testing.T) {
+	var src bool = true
+	var ref = config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(true), nv)
+}
+
+func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
+	var src bool = true
+	var ref = config.V(false)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(true), nv)
+}
+
+func TestFromTypedBoolRetainsLocationsIfUnchanged(t *testing.T) {
+	var src = true
+	var ref = config.NewValue(true, config.Location{File: "foo"})
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NewValue(true, config.Location{File: "foo"}), nv)
+}
diff --git a/libs/config/convert/struct_info.go b/libs/config/convert/struct_info.go
index 367b9ecdc47..cc899542c0c 100644
--- a/libs/config/convert/struct_info.go
+++ b/libs/config/convert/struct_info.go
@@ -85,3 +85,29 @@ func buildStructInfo(typ reflect.Type) structInfo {
 
 	return out
 }
+
+func (s *structInfo) FieldValues(v reflect.Value) map[string]reflect.Value {
+	var out = make(map[string]reflect.Value)
+
+	for k, index := range s.Fields {
+		fv := v
+
+		// Locate value in struct (it could be an embedded type).
+		for i, x := range index {
+			if i > 0 {
+				if fv.Kind() == reflect.Pointer && fv.Type().Elem().Kind() == reflect.Struct {
+					if fv.IsNil() {
+						fv = reflect.Value{}
+						break
+					}
+					fv = fv.Elem()
+				}
+			}
+			fv = fv.Field(x)
+		}
+
+		out[k] = fv
+	}
+
+	return out
+}

From d23e201b61cf8b5cfc89f33e9e53e37f093b8c53 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Tue, 31 Oct 2023 04:57:38 -0700
Subject: [PATCH 2/7] More work on ToTyped

---
 libs/config/convert/from_typed.go      |  23 +++++-
 libs/config/convert/from_typed_test.go | 100 ++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 6 deletions(-)

diff --git a/libs/config/convert/from_typed.go b/libs/config/convert/from_typed.go
index d2c166ff8c1..852c479ca09 100644
--- a/libs/config/convert/from_typed.go
+++ b/libs/config/convert/from_typed.go
@@ -34,7 +34,7 @@ func FromTyped(src any, ref config.Value) (config.Value, error) {
 	case reflect.Int, reflect.Int32, reflect.Int64:
 		return fromTypedInt(srcv, ref)
 	case reflect.Float32, reflect.Float64:
-		// return fromTypedFloat(srcv, dst)
+		return fromTypedFloat(srcv, ref)
 	}
 
 	return config.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
@@ -207,7 +207,26 @@ func fromTypedInt(src reflect.Value, ref config.Value) (config.Value, error) {
 		}
 		return config.V(value), nil
 	case config.KindNil:
-		return config.V(src.Bool()), nil
+		return config.V(src.Int()), nil
+	}
+
+	return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+}
+
+func fromTypedFloat(src reflect.Value, ref config.Value) (config.Value, error) {
+	if src.IsZero() {
+		return config.NilValue, nil
+	}
+
+	switch ref.Kind() {
+	case config.KindFloat:
+		value := src.Float()
+		if value == ref.MustFloat() {
+			return ref, nil
+		}
+		return config.V(value), nil
+	case config.KindNil:
+		return config.V(src.Float()), nil
 	}
 
 	return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
diff --git a/libs/config/convert/from_typed_test.go b/libs/config/convert/from_typed_test.go
index cb5400b9fd8..f07c75e83c2 100644
--- a/libs/config/convert/from_typed_test.go
+++ b/libs/config/convert/from_typed_test.go
@@ -191,7 +191,7 @@ func TestFromTypedStringEmpty(t *testing.T) {
 }
 
 func TestFromTypedStringNonEmpty(t *testing.T) {
-	var src = "new"
+	var src string = "new"
 	var ref = config.NilValue
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
@@ -199,7 +199,7 @@ func TestFromTypedStringNonEmpty(t *testing.T) {
 }
 
 func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
-	var src = "new"
+	var src string = "new"
 	var ref = config.V("old")
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
@@ -207,13 +207,20 @@ func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
 }
 
 func TestFromTypedStringRetainsLocationsIfUnchanged(t *testing.T) {
-	var src = "foo"
+	var src string = "foo"
 	var ref = config.NewValue("foo", config.Location{File: "foo"})
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NewValue("foo", config.Location{File: "foo"}), nv)
 }
 
+func TestFromTypedStringTypeError(t *testing.T) {
+	var src string = "foo"
+	var ref = config.V(1234)
+	_, err := FromTyped(src, ref)
+	require.Error(t, err)
+}
+
 func TestFromTypedBoolEmpty(t *testing.T) {
 	var src bool
 	var ref = config.V(true)
@@ -239,9 +246,94 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
 }
 
 func TestFromTypedBoolRetainsLocationsIfUnchanged(t *testing.T) {
-	var src = true
+	var src bool = true
 	var ref = config.NewValue(true, config.Location{File: "foo"})
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NewValue(true, config.Location{File: "foo"}), nv)
 }
+
+func TestFromTypedBoolTypeError(t *testing.T) {
+	var src bool = true
+	var ref = config.V("string")
+	_, err := FromTyped(src, ref)
+	require.Error(t, err)
+}
+
+func TestFromTypedIntEmpty(t *testing.T) {
+	var src int
+	var ref = config.V(true)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedIntNonEmpty(t *testing.T) {
+	var src int = 1234
+	var ref = config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(int64(1234)), nv)
+}
+
+func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
+	var src int = 1234
+	var ref = config.V(1233)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(int64(1234)), nv)
+}
+
+func TestFromTypedIntRetainsLocationsIfUnchanged(t *testing.T) {
+	var src int = 1234
+	var ref = config.NewValue(1234, config.Location{File: "foo"})
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NewValue(1234, config.Location{File: "foo"}), nv)
+}
+
+func TestFromTypedIntTypeError(t *testing.T) {
+	var src int = 1234
+	var ref = config.V("string")
+	_, err := FromTyped(src, ref)
+	require.Error(t, err)
+}
+
+func TestFromTypedFloatEmpty(t *testing.T) {
+	var src float64
+	var ref = config.V(1.23)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
+func TestFromTypedFloatNonEmpty(t *testing.T) {
+	var src float64 = 1.23
+	var ref = config.NilValue
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(1.23), nv)
+}
+
+func TestFromTypedFloatNonEmptyOverwrite(t *testing.T) {
+	var src float64 = 1.23
+	var ref = config.V(1.24)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(1.23), nv)
+}
+
+func TestFromTypedFloatRetainsLocationsIfUnchanged(t *testing.T) {
+	var src float64 = 1.23
+	var ref = config.NewValue(1.23, config.Location{File: "foo"})
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NewValue(1.23, config.Location{File: "foo"}), nv)
+}
+
+func TestFromTypedFloatTypeError(t *testing.T) {
+	var src float64 = 1.23
+	var ref = config.V("string")
+	_, err := FromTyped(src, ref)
+	require.Error(t, err)
+}

From 36ea807339c0184517ba49697bbcb98677f367c0 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Tue, 31 Oct 2023 05:10:11 -0700
Subject: [PATCH 3/7] More tests

---
 libs/config/convert/struct_info.go      |   4 +-
 libs/config/convert/struct_info_test.go | 107 ++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/libs/config/convert/struct_info.go b/libs/config/convert/struct_info.go
index cc899542c0c..2457b3c2971 100644
--- a/libs/config/convert/struct_info.go
+++ b/libs/config/convert/struct_info.go
@@ -106,7 +106,9 @@ func (s *structInfo) FieldValues(v reflect.Value) map[string]reflect.Value {
 			fv = fv.Field(x)
 		}
 
-		out[k] = fv
+		if fv.IsValid() {
+			out[k] = fv
+		}
 	}
 
 	return out
diff --git a/libs/config/convert/struct_info_test.go b/libs/config/convert/struct_info_test.go
index 3079958b2b7..2e31adac162 100644
--- a/libs/config/convert/struct_info_test.go
+++ b/libs/config/convert/struct_info_test.go
@@ -87,3 +87,110 @@ func TestStructInfoAnonymousByPointer(t *testing.T) {
 	assert.Equal(t, []int{0, 0}, si.Fields["foo"])
 	assert.Equal(t, []int{0, 1, 0}, si.Fields["bar"])
 }
+
+func TestStructInfoFieldValues(t *testing.T) {
+	type Tmp struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+
+	var src = Tmp{
+		Foo: "foo",
+		Bar: "bar",
+	}
+
+	si := getStructInfo(reflect.TypeOf(Tmp{}))
+	fv := si.FieldValues(reflect.ValueOf(src))
+	assert.Len(t, fv, 2)
+	assert.Equal(t, "foo", fv["foo"].String())
+	assert.Equal(t, "bar", fv["bar"].String())
+}
+
+func TestStructInfoFieldValuesAnonymousByValue(t *testing.T) {
+	type Bar struct {
+		Bar string `json:"bar"`
+	}
+
+	type Foo struct {
+		Foo string `json:"foo"`
+		Bar
+	}
+
+	type Tmp struct {
+		Foo
+	}
+
+	var src = Tmp{
+		Foo: Foo{
+			Foo: "foo",
+			Bar: Bar{
+				Bar: "bar",
+			},
+		},
+	}
+
+	si := getStructInfo(reflect.TypeOf(Tmp{}))
+	fv := si.FieldValues(reflect.ValueOf(src))
+	assert.Len(t, fv, 2)
+	assert.Equal(t, "foo", fv["foo"].String())
+	assert.Equal(t, "bar", fv["bar"].String())
+}
+
+func TestStructInfoFieldValuesAnonymousByPointer(t *testing.T) {
+	type Bar struct {
+		Bar string `json:"bar"`
+	}
+
+	type Foo struct {
+		Foo string `json:"foo"`
+		*Bar
+	}
+
+	type Tmp struct {
+		*Foo
+	}
+
+	// Test that the embedded fields are dereferenced properly.
+	t.Run("all are set", func(t *testing.T) {
+		src := Tmp{
+			Foo: &Foo{
+				Foo: "foo",
+				Bar: &Bar{
+					Bar: "bar",
+				},
+			},
+		}
+
+		si := getStructInfo(reflect.TypeOf(Tmp{}))
+		fv := si.FieldValues(reflect.ValueOf(src))
+		assert.Len(t, fv, 2)
+		assert.Equal(t, "foo", fv["foo"].String())
+		assert.Equal(t, "bar", fv["bar"].String())
+	})
+
+	// Test that fields of embedded types are skipped if the embedded type is nil.
+	t.Run("top level is set", func(t *testing.T) {
+		src := Tmp{
+			Foo: &Foo{
+				Foo: "foo",
+				Bar: nil,
+			},
+		}
+
+		si := getStructInfo(reflect.TypeOf(Tmp{}))
+		fv := si.FieldValues(reflect.ValueOf(src))
+		assert.Len(t, fv, 1)
+		assert.Equal(t, "foo", fv["foo"].String())
+	})
+
+	// Test that fields of embedded types are skipped if the embedded type is nil.
+	t.Run("none are set", func(t *testing.T) {
+		src := Tmp{
+			Foo: nil,
+		}
+
+		si := getStructInfo(reflect.TypeOf(Tmp{}))
+		fv := si.FieldValues(reflect.ValueOf(src))
+		assert.Empty(t, fv)
+	})
+}

From b9191a404748240db05f3801ae3180ff6f3d3f9c Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Tue, 31 Oct 2023 05:24:06 -0700
Subject: [PATCH 4/7] Cleanup

---
 libs/config/convert/from_typed.go | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/libs/config/convert/from_typed.go b/libs/config/convert/from_typed.go
index 852c479ca09..8f247853e1b 100644
--- a/libs/config/convert/from_typed.go
+++ b/libs/config/convert/from_typed.go
@@ -41,11 +41,11 @@ func FromTyped(src any, ref config.Value) (config.Value, error) {
 }
 
 func fromTypedStruct(src reflect.Value, ref config.Value) (config.Value, error) {
+	// Check that the reference value is compatible or nil.
 	switch ref.Kind() {
 	case config.KindMap, config.KindNil:
-		// Nothing to do.
 	default:
-		panic("type error")
+		return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	out := make(map[string]config.Value)
@@ -68,33 +68,14 @@ func fromTypedStruct(src reflect.Value, ref config.Value) (config.Value, error)
 	}
 
 	return config.NewValue(out, ref.Location()), nil
-
-	// what are my options
-	// totyped / fromtyped at every mutator boundary
-	// pro's -- minimal changes to existing mutators
-	// con's -- doesn't hold for all mutators, so we need different interface ANYWAY
-	// (e.g. get/set config.Value instances)
-	// cons -- lossy (cannot do all to/from conversions, lose location, lose variables)
-
-	// explicit mutator interface
-	// pro's -- very clear what's happening
-	// cons -- all code + tests need to be changed
-	//
-
-	// need an incremental approach
-	// thus, we run totyped + fromtyped at mutator boundary
-	// can eventually move this into the mutators themselves?
-	// can treat the typed structure as read-only, perhaps?
-	// can generate wrapper type that exposes a Get + GetValue at every node
-
 }
 
 func fromTypedMap(src reflect.Value, ref config.Value) (config.Value, error) {
+	// Check that the reference value is compatible or nil.
 	switch ref.Kind() {
 	case config.KindMap, config.KindNil:
-		// Nothing to do.
 	default:
-		panic("type error")
+		return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	out := make(map[string]config.Value)
@@ -123,11 +104,11 @@ func fromTypedMap(src reflect.Value, ref config.Value) (config.Value, error) {
 }
 
 func fromTypedSlice(src reflect.Value, ref config.Value) (config.Value, error) {
+	// Check that the reference value is compatible or nil.
 	switch ref.Kind() {
 	case config.KindSequence, config.KindNil:
-		// Nothing to do.
 	default:
-		panic("type error")
+		return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	out := make([]config.Value, src.Len())

From 90eef48a2991250647133c24b44856833a0ead91 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Fri, 3 Nov 2023 13:42:29 -0700
Subject: [PATCH 5/7] Retain zero-valued primitive types if they are present in
 the reference

---
 libs/config/convert/from_typed.go      | 40 +++++++++++------------
 libs/config/convert/from_typed_test.go | 45 +++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/libs/config/convert/from_typed.go b/libs/config/convert/from_typed.go
index 8f247853e1b..2c23a2e9ea5 100644
--- a/libs/config/convert/from_typed.go
+++ b/libs/config/convert/from_typed.go
@@ -133,10 +133,6 @@ func fromTypedSlice(src reflect.Value, ref config.Value) (config.Value, error) {
 }
 
 func fromTypedString(src reflect.Value, ref config.Value) (config.Value, error) {
-	if src.IsZero() {
-		return config.NilValue, nil
-	}
-
 	switch ref.Kind() {
 	case config.KindString:
 		value := src.String()
@@ -146,6 +142,11 @@ func fromTypedString(src reflect.Value, ref config.Value) (config.Value, error)
 
 		return config.V(value), nil
 	case config.KindNil:
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
+		// Otherwise, we would always include all zero valued fields.
+		if src.IsZero() {
+			return config.NilValue, nil
+		}
 		return config.V(src.String()), nil
 	}
 
@@ -153,14 +154,6 @@ func fromTypedString(src reflect.Value, ref config.Value) (config.Value, error)
 }
 
 func fromTypedBool(src reflect.Value, ref config.Value) (config.Value, error) {
-	// Note: this means it's not possible to flip a boolean to false on a typed
-	// structure and see it reflected in the dynamic configuration.
-	// This case is not handled as is, so we punt on it until the mutaotrs
-	// modify the dynamic configuration directly.
-	if src.IsZero() {
-		return config.NilValue, nil
-	}
-
 	switch ref.Kind() {
 	case config.KindBool:
 		value := src.Bool()
@@ -169,6 +162,11 @@ func fromTypedBool(src reflect.Value, ref config.Value) (config.Value, error) {
 		}
 		return config.V(value), nil
 	case config.KindNil:
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
+		// Otherwise, we would always include all zero valued fields.
+		if src.IsZero() {
+			return config.NilValue, nil
+		}
 		return config.V(src.Bool()), nil
 	}
 
@@ -176,10 +174,6 @@ func fromTypedBool(src reflect.Value, ref config.Value) (config.Value, error) {
 }
 
 func fromTypedInt(src reflect.Value, ref config.Value) (config.Value, error) {
-	if src.IsZero() {
-		return config.NilValue, nil
-	}
-
 	switch ref.Kind() {
 	case config.KindInt:
 		value := src.Int()
@@ -188,6 +182,11 @@ func fromTypedInt(src reflect.Value, ref config.Value) (config.Value, error) {
 		}
 		return config.V(value), nil
 	case config.KindNil:
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
+		// Otherwise, we would always include all zero valued fields.
+		if src.IsZero() {
+			return config.NilValue, nil
+		}
 		return config.V(src.Int()), nil
 	}
 
@@ -195,10 +194,6 @@ func fromTypedInt(src reflect.Value, ref config.Value) (config.Value, error) {
 }
 
 func fromTypedFloat(src reflect.Value, ref config.Value) (config.Value, error) {
-	if src.IsZero() {
-		return config.NilValue, nil
-	}
-
 	switch ref.Kind() {
 	case config.KindFloat:
 		value := src.Float()
@@ -207,6 +202,11 @@ func fromTypedFloat(src reflect.Value, ref config.Value) (config.Value, error) {
 		}
 		return config.V(value), nil
 	case config.KindNil:
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
+		// Otherwise, we would always include all zero valued fields.
+		if src.IsZero() {
+			return config.NilValue, nil
+		}
 		return config.V(src.Float()), nil
 	}
 
diff --git a/libs/config/convert/from_typed_test.go b/libs/config/convert/from_typed_test.go
index f07c75e83c2..b1ff0bdde91 100644
--- a/libs/config/convert/from_typed_test.go
+++ b/libs/config/convert/from_typed_test.go
@@ -15,10 +15,7 @@ func TestFromTypedStructZeroFields(t *testing.T) {
 	}
 
 	src := Tmp{}
-	ref := config.V(map[string]config.Value{
-		"foo": config.V("bar"),
-		"bar": config.V("baz"),
-	})
+	ref := config.NilValue
 
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
@@ -184,12 +181,20 @@ func TestFromTypedSliceNonEmptyRetainLocationIfUnchanged(t *testing.T) {
 
 func TestFromTypedStringEmpty(t *testing.T) {
 	var src string
-	var ref = config.V("string")
+	var ref = config.NilValue
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NilValue, nv)
 }
 
+func TestFromTypedStringEmptyOverwrite(t *testing.T) {
+	var src string
+	var ref = config.V("old")
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(""), nv)
+}
+
 func TestFromTypedStringNonEmpty(t *testing.T) {
 	var src string = "new"
 	var ref = config.NilValue
@@ -223,12 +228,20 @@ func TestFromTypedStringTypeError(t *testing.T) {
 
 func TestFromTypedBoolEmpty(t *testing.T) {
 	var src bool
-	var ref = config.V(true)
+	var ref = config.NilValue
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NilValue, nv)
 }
 
+func TestFromTypedBoolEmptyOverwrite(t *testing.T) {
+	var src bool
+	var ref = config.V(true)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(false), nv)
+}
+
 func TestFromTypedBoolNonEmpty(t *testing.T) {
 	var src bool = true
 	var ref = config.NilValue
@@ -262,12 +275,20 @@ func TestFromTypedBoolTypeError(t *testing.T) {
 
 func TestFromTypedIntEmpty(t *testing.T) {
 	var src int
-	var ref = config.V(true)
+	var ref = config.NilValue
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NilValue, nv)
 }
 
+func TestFromTypedIntEmptyOverwrite(t *testing.T) {
+	var src int
+	var ref = config.V(1234)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(int64(0)), nv)
+}
+
 func TestFromTypedIntNonEmpty(t *testing.T) {
 	var src int = 1234
 	var ref = config.NilValue
@@ -301,12 +322,20 @@ func TestFromTypedIntTypeError(t *testing.T) {
 
 func TestFromTypedFloatEmpty(t *testing.T) {
 	var src float64
-	var ref = config.V(1.23)
+	var ref = config.NilValue
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, config.NilValue, nv)
 }
 
+func TestFromTypedFloatEmptyOverwrite(t *testing.T) {
+	var src float64
+	var ref = config.V(1.23)
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.V(0.0), nv)
+}
+
 func TestFromTypedFloatNonEmpty(t *testing.T) {
 	var src float64 = 1.23
 	var ref = config.NilValue

From 62a8b3fdc0992bc4a12a33b157a51058e7d3cd5e Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Wed, 8 Nov 2023 16:32:46 +0100
Subject: [PATCH 6/7] Differentiate between nil and empty maps and sequences

---
 libs/config/convert/from_typed.go      | 20 ++++++++---------
 libs/config/convert/from_typed_test.go | 30 ++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/libs/config/convert/from_typed.go b/libs/config/convert/from_typed.go
index 2c23a2e9ea5..e3911a9e5ab 100644
--- a/libs/config/convert/from_typed.go
+++ b/libs/config/convert/from_typed.go
@@ -78,6 +78,11 @@ func fromTypedMap(src reflect.Value, ref config.Value) (config.Value, error) {
 		return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
+	// Return nil if the map is nil.
+	if src.IsNil() {
+		return config.NilValue, nil
+	}
+
 	out := make(map[string]config.Value)
 	iter := src.MapRange()
 	for iter.Next() {
@@ -95,11 +100,6 @@ func fromTypedMap(src reflect.Value, ref config.Value) (config.Value, error) {
 		out[k] = nv
 	}
 
-	// If the map has no entries, emit a nil.
-	if len(out) == 0 {
-		return config.NilValue, nil
-	}
-
 	return config.NewValue(out, ref.Location()), nil
 }
 
@@ -111,6 +111,11 @@ func fromTypedSlice(src reflect.Value, ref config.Value) (config.Value, error) {
 		return config.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
+	// Return nil if the slice is nil.
+	if src.IsNil() {
+		return config.NilValue, nil
+	}
+
 	out := make([]config.Value, src.Len())
 	for i := 0; i < src.Len(); i++ {
 		v := src.Index(i)
@@ -124,11 +129,6 @@ func fromTypedSlice(src reflect.Value, ref config.Value) (config.Value, error) {
 		out[i] = nv
 	}
 
-	// If the slice has no entries, emit a nil.
-	if len(out) == 0 {
-		return config.NilValue, nil
-	}
-
 	return config.NewValue(out, ref.Location()), nil
 }
 
diff --git a/libs/config/convert/from_typed_test.go b/libs/config/convert/from_typed_test.go
index b1ff0bdde91..2b28f549cd8 100644
--- a/libs/config/convert/from_typed_test.go
+++ b/libs/config/convert/from_typed_test.go
@@ -68,6 +68,19 @@ func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
 	assert.Equal(t, config.NewValue("qux", config.Location{}), nv.Get("bar"))
 }
 
+func TestFromTypedMapNil(t *testing.T) {
+	var src map[string]string = nil
+
+	ref := config.V(map[string]config.Value{
+		"foo": config.V("bar"),
+		"bar": config.V("baz"),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
 func TestFromTypedMapEmpty(t *testing.T) {
 	var src = map[string]string{}
 
@@ -78,7 +91,7 @@ func TestFromTypedMapEmpty(t *testing.T) {
 
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
-	assert.Equal(t, config.NilValue, nv)
+	assert.Equal(t, config.V(map[string]config.Value{}), nv)
 }
 
 func TestFromTypedMapNonEmpty(t *testing.T) {
@@ -130,6 +143,19 @@ func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
 	}), nv)
 }
 
+func TestFromTypedSliceNil(t *testing.T) {
+	var src []string = nil
+
+	ref := config.V([]config.Value{
+		config.V("bar"),
+		config.V("baz"),
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, config.NilValue, nv)
+}
+
 func TestFromTypedSliceEmpty(t *testing.T) {
 	var src = []string{}
 
@@ -140,7 +166,7 @@ func TestFromTypedSliceEmpty(t *testing.T) {
 
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
-	assert.Equal(t, config.NilValue, nv)
+	assert.Equal(t, config.V([]config.Value{}), nv)
 }
 
 func TestFromTypedSliceNonEmpty(t *testing.T) {

From 157cb9f64dafca624aff0e6d109295e215d0f6cf Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Wed, 15 Nov 2023 10:14:47 +0100
Subject: [PATCH 7/7] Retain nil pointers

---
 libs/config/convert/end_to_end_test.go | 61 ++++++++++++++++++++++++++
 libs/config/convert/to_typed.go        |  6 +++
 2 files changed, 67 insertions(+)
 create mode 100644 libs/config/convert/end_to_end_test.go

diff --git a/libs/config/convert/end_to_end_test.go b/libs/config/convert/end_to_end_test.go
new file mode 100644
index 00000000000..c06830e83ea
--- /dev/null
+++ b/libs/config/convert/end_to_end_test.go
@@ -0,0 +1,61 @@
+package convert
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/libs/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func assertFromTypedToTypedEqual[T any](t *testing.T, src T) {
+	nv, err := FromTyped(src, config.NilValue)
+	require.NoError(t, err)
+
+	var dst T
+	err = ToTyped(&dst, nv)
+	require.NoError(t, err)
+	assert.Equal(t, src, dst)
+}
+
+func TestAdditional(t *testing.T) {
+	type StructType struct {
+		Str string `json:"str"`
+	}
+
+	type Tmp struct {
+		MapToPointer   map[string]*string `json:"map_to_pointer"`
+		SliceOfPointer []*string          `json:"slice_of_pointer"`
+		NestedStruct   StructType         `json:"nested_struct"`
+	}
+
+	t.Run("nil", func(t *testing.T) {
+		assertFromTypedToTypedEqual(t, Tmp{})
+	})
+
+	t.Run("empty map", func(t *testing.T) {
+		assertFromTypedToTypedEqual(t, Tmp{
+			MapToPointer: map[string]*string{},
+		})
+	})
+
+	t.Run("map with nil value", func(t *testing.T) {
+		assertFromTypedToTypedEqual(t, Tmp{
+			MapToPointer: map[string]*string{
+				"key": nil,
+			},
+		})
+	})
+
+	t.Run("empty slice", func(t *testing.T) {
+		assertFromTypedToTypedEqual(t, Tmp{
+			SliceOfPointer: []*string{},
+		})
+	})
+
+	t.Run("slice with nil value", func(t *testing.T) {
+		assertFromTypedToTypedEqual(t, Tmp{
+			SliceOfPointer: []*string{nil},
+		})
+	})
+}
diff --git a/libs/config/convert/to_typed.go b/libs/config/convert/to_typed.go
index 9915d30a6ed..ca09fce42b3 100644
--- a/libs/config/convert/to_typed.go
+++ b/libs/config/convert/to_typed.go
@@ -13,6 +13,12 @@ func ToTyped(dst any, src config.Value) error {
 
 	// Dereference pointer if necessary
 	for dstv.Kind() == reflect.Pointer {
+		// If the source value is nil and the destination is a settable pointer,
+		// set the destination to nil. Also see `end_to_end_test.go`.
+		if dstv.CanSet() && src == config.NilValue {
+			dstv.SetZero()
+			return nil
+		}
 		if dstv.IsNil() {
 			dstv.Set(reflect.New(dstv.Type().Elem()))
 		}