Skip to content

Commit

Permalink
fix: Always overwrite with tables, even if empty
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sunshowers committed Jan 14, 2025
1 parent f4b2523 commit 9f124a3
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
5 changes: 3 additions & 2 deletions src/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
35 changes: 29 additions & 6 deletions tests/testsuite/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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())
);
}

0 comments on commit 9f124a3

Please sign in to comment.