Skip to content

Commit 3c16f8f

Browse files
committed
fix: treat nil parents as empty tables
Encountered this regression via nextest's test suite (nextest-rs/nextest#2001). If a table is empty, it would be treated as a unit value. If attempted to be deserialized via a `Default` impl, it would lead to deserialization failing with an error like ```console profile.default-miri: invalid type: unit value, expected struct CustomProfileImpl ``` A bisect appears to indicate that ec36bff is responsible. For empty tables where the Default impl is supposed to work, it no longer would. I've attempted to restore the old behavior of putting in an empty table, specifically this section: ec36bff#diff-c5423e2d2d6c87501239c0304c0f496742e00440defdd20368cf548ba42ab184L175-L178 I'm happy to make changes if there's a better approach. I've also added some tests for a few situations around empty tables. In this case only the last one (`profile.baz`) regressed, but I've also added tests for a couple other cases.
1 parent a3ff970 commit 3c16f8f

File tree

2 files changed

+61
-1
lines changed

2 files changed

+61
-1
lines changed

src/path/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,16 @@ impl Expression {
150150
let parent = self.get_mut_forcibly(root);
151151
match value.kind {
152152
ValueKind::Table(ref incoming_map) => {
153+
// If the parent is nil, treat it as an empty table
154+
if matches!(parent.kind, ValueKind::Nil) {
155+
*parent = Map::<String, Value>::new().into();
156+
}
157+
153158
// Continue the deep merge
154159
for (key, val) in incoming_map {
155160
Self::root(key.clone()).set(parent, val.clone());
156161
}
157162
}
158-
159163
_ => {
160164
*parent = value;
161165
}

tests/testsuite/file_toml.rs

+56
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![cfg(feature = "toml")]
22

3+
use std::collections::BTreeMap;
4+
35
use chrono::{DateTime, TimeZone, Utc};
46
use float_cmp::ApproxEqUlps;
57
use serde_derive::Deserialize;
@@ -422,6 +424,60 @@ bar = "bar is a lowercase param"
422424
);
423425
}
424426

427+
#[test]
428+
fn test_override_empty_tables() {
429+
#[derive(Debug, Deserialize)]
430+
struct Settings {
431+
profile: BTreeMap<String, Profile>,
432+
}
433+
434+
#[derive(Debug, Default, Deserialize)]
435+
struct Profile {
436+
name: Option<String>,
437+
}
438+
439+
// Test a few scenarios with empty tables:
440+
// * foo: empty table -> table with k/v
441+
// * bar: table with k/v -> empty table
442+
// * baz: empty table relying on Default impl
443+
let cfg = Config::builder()
444+
.add_source(File::from_str(
445+
r#"
446+
[profile.foo]
447+
"#,
448+
FileFormat::Toml,
449+
))
450+
.add_source(File::from_str(
451+
r#"
452+
[profile.foo]
453+
name = "foo"
454+
"#,
455+
FileFormat::Toml,
456+
))
457+
.add_source(File::from_str(
458+
r#"
459+
[profile.bar]
460+
name = "bar"
461+
"#,
462+
FileFormat::Toml,
463+
))
464+
.add_source(File::from_str(
465+
r#"
466+
[profile.bar]
467+
[profile.baz]
468+
"#,
469+
FileFormat::Toml,
470+
))
471+
.build()
472+
.unwrap();
473+
474+
let settings: Settings = cfg.try_deserialize().unwrap();
475+
assert_eq!(settings.profile.len(), 3);
476+
assert_eq!(settings.profile["foo"].name.as_deref(), Some("foo"));
477+
assert_eq!(settings.profile["bar"].name.as_deref(), Some("bar"));
478+
assert_eq!(settings.profile["baz"].name.as_deref(), None);
479+
}
480+
425481
#[test]
426482
fn toml() {
427483
let s = Config::builder()

0 commit comments

Comments
 (0)