From 9f124a3a81653ce4be65175564ef5ffbbefbf86e Mon Sep 17 00:00:00 2001
From: Rain <rain@sunshowers.io>
Date: Tue, 14 Jan 2025 18:34:55 +0000
Subject: [PATCH] fix: Always overwrite with tables, even if empty

While the last commit restored behavior to mostly where it was in
v0.15.2, this commit introduces a behavior change with the goal of
treating empty tables the same as non-empty ones (in particular,
`int_to_empty` is now treated the same as `int_to_non_empty`, both of
them overwriting the integer with the table).

Treating them the same makes a lot of sense logically, and the fact that
we weren't doing so is probably a bug in the old implementation. But it
is a notable behavior change, and one worth considering carefully.
---
 src/path/mod.rs          |  5 +++--
 tests/testsuite/merge.rs | 35 +++++++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/path/mod.rs b/src/path/mod.rs
index 2fa5af2b..f02194c8 100644
--- a/src/path/mod.rs
+++ b/src/path/mod.rs
@@ -150,8 +150,9 @@ impl Expression {
         let parent = self.get_mut_forcibly(root);
         match value.kind {
             ValueKind::Table(ref incoming_map) => {
-                // If the parent is nil, treat it as an empty table
-                if matches!(parent.kind, ValueKind::Nil) {
+                // If the parent is not a table, overwrite it, treating it as a
+                // table
+                if !matches!(parent.kind, ValueKind::Table(_)) {
                     *parent = Map::<String, Value>::new().into();
                 }
 
diff --git a/tests/testsuite/merge.rs b/tests/testsuite/merge.rs
index 157254c1..abceba3e 100644
--- a/tests/testsuite/merge.rs
+++ b/tests/testsuite/merge.rs
@@ -87,13 +87,11 @@ fn test_merge_empty_maps() {
     use std::collections::BTreeMap;
 
     #[derive(Debug, Deserialize)]
-    #[allow(dead_code)] // temporary while this test is broken
     struct Settings {
         profile: BTreeMap<String, Profile>,
     }
 
     #[derive(Debug, Default, Deserialize)]
-    #[allow(dead_code)] // temporary while this test is broken
     struct Profile {
         name: Option<String>,
     }
@@ -149,10 +147,35 @@ fn test_merge_empty_maps() {
         .build()
         .unwrap();
 
-    // This is currently broken -- the next commit fixes it.
-    let error = cfg.try_deserialize::<Settings>().unwrap_err();
+    let settings = cfg.try_deserialize::<Settings>().unwrap();
+    assert_eq!(settings.profile.len(), 10);
+
+    assert_eq!(settings.profile["missing_to_empty"].name, None);
+    assert_eq!(
+        settings.profile["missing_to_non_empty"].name,
+        Some("bar".to_owned())
+    );
+    assert_eq!(settings.profile["empty_to_empty"].name, None);
+    assert_eq!(
+        settings.profile["empty_to_non_empty"].name,
+        Some("bar".to_owned())
+    );
+    assert_eq!(
+        settings.profile["non_empty_to_empty"].name,
+        Some("foo".to_owned())
+    );
+    assert_eq!(
+        settings.profile["non_empty_to_non_empty"].name,
+        Some("bar".to_owned())
+    );
+    assert_eq!(settings.profile["null_to_empty"].name, None);
+    assert_eq!(
+        settings.profile["null_to_non_empty"].name,
+        Some("bar".to_owned())
+    );
+    assert_eq!(settings.profile["int_to_empty"].name, None);
     assert_eq!(
-        error.to_string(),
-        "invalid type: unit value, expected struct 42 for key `profile.int_to_empty`"
+        settings.profile["int_to_non_empty"].name,
+        Some("bar".to_owned())
     );
 }