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 54b3429
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 54b3429

Please sign in to comment.