diff --git a/src/de.rs b/src/de.rs index c3e3570..288bd5c 100644 --- a/src/de.rs +++ b/src/de.rs @@ -23,7 +23,9 @@ use crate::{ Error, ErrorPathSegment, Mapping, Node, NodeValue, Number, Span, Tag, TaggedValue, Value, ast::MergePolicy, error::utf8_error_span, - key_identity::{DuplicateKeyTracker, check_duplicate_with_tracker_at_depth_limit}, + key_identity::{ + DuplicateKeyTracker, check_duplicate_with_tracker_at_depth_limit, same_key_identity, + }, schema::{DEFAULT_MAX_NESTING_DEPTH, LoadOptions}, yaml11, }; @@ -551,7 +553,7 @@ fn merged_node_ref_entries_at_depth( .filter_map(|(index, (key, value))| (index != merge_index).then_some((key, value))) .collect::>(); let merge_entries = node_ref_merge_entries(&entries[merge_index].1, depth)?; - insert_missing_node_ref_entries(&mut merged, merge_entries); + insert_missing_node_ref_entries(&mut merged, merge_entries)?; Ok(Some(merged)) } @@ -575,7 +577,7 @@ fn node_ref_merge_entries(merge: &Node, depth: usize) -> Result { return Err(value_merge_error( @@ -604,12 +606,20 @@ fn node_ref_merge_entries(merge: &Node, depth: usize) -> Result( entries: &mut Vec<(&'a Node, &'a Node)>, merge_entries: Vec<(&'a Node, &'a Node)>, -) { +) -> Result<(), Error> { for (key, value) in merge_entries { - if entries.iter().all(|(existing_key, _)| *existing_key != key) { + let mut already_present = false; + for (existing_key, _) in entries.iter() { + if same_key_identity(existing_key, key)? { + already_present = true; + break; + } + } + if !already_present { entries.push((key, value)); } } + Ok(()) } fn apply_default_value_merges(mut value: Value) -> Result { diff --git a/tests/serde_value_api.rs b/tests/serde_value_api.rs index 2e8deaa..7cd9e5c 100644 --- a/tests/serde_value_api.rs +++ b/tests/serde_value_api.rs @@ -271,6 +271,14 @@ fn unsigned_node(value: u128) -> Node { Node::new(NodeValue::Number(Number::Unsigned(value)), Span::default()) } +fn string_node_at(value: &str, span: Span) -> Node { + Node::new(NodeValue::String(value.to_string()), span) +} + +fn unsigned_node_at(value: u128, span: Span) -> Node { + Node::new(NodeValue::Number(Number::Unsigned(value)), span) +} + fn mapping_node(entries: Vec<(Node, Node)>) -> Node { Node::new(NodeValue::Mapping(entries), Span::default()) } @@ -291,6 +299,38 @@ fn caller_built_merge_node() -> Node { mapping_node(vec![(string_node("job"), job)]) } +fn caller_built_merge_node_with_keys(source_key: Node, local_key: Node) -> Node { + let defaults = mapping_node(vec![(source_key, string_node("from-merge"))]); + let job = mapping_node(vec![ + (string_node("<<"), defaults), + (local_key, string_node("local")), + ]); + mapping_node(vec![(string_node("job"), job)]) +} + +fn assert_borrowed_node_merge_key_identity(source_key: Node, local_key: Node, expected_key: Value) { + assert_ne!( + source_key, local_key, + "regression keys must differ as complete Nodes" + ); + let node = caller_built_merge_node_with_keys(source_key, local_key); + + let from_node: Value = saneyaml::from_node(&node).expect("from_node expands merge"); + let borrowed: Value = Value::deserialize(&node).expect("&Node expands merge"); + let owned: Value = Value::deserialize(node.clone()).expect("owned Node expands merge"); + + for value in [&from_node, &borrowed, &owned] { + let mapping = value["job"].as_mapping().expect("merged job mapping"); + assert_eq!(mapping.len(), 1, "semantic duplicate must be omitted"); + let (key, value) = mapping.iter().next().expect("local merged entry"); + assert_eq!(key, &expected_key, "the explicit key must be retained"); + assert_eq!(value.as_str(), Some("local"), "local value wins the merge"); + } + + assert_eq!(from_node, borrowed); + assert_eq!(borrowed, owned); +} + fn caller_built_enum_merge_value() -> Value { let mut action_payload = Mapping::new(); action_payload.insert(Value::from("run"), Value::from("cargo test")); @@ -5719,6 +5759,50 @@ fn serde_api_caller_built_node_deserializers_expand_merge_keys_by_default() { } } +#[test] +fn serde_api_borrowed_node_merge_uses_semantic_key_identity() { + assert_borrowed_node_merge_key_identity( + string_node_at("shared", Span::new(10, 16, 2, 3)), + string_node_at("shared", Span::new(30, 36, 4, 3)), + Value::from("shared"), + ); + + assert_borrowed_node_merge_key_identity( + Node::new( + NodeValue::Number(Number::Integer(7)), + Span::new(40, 41, 5, 3), + ), + unsigned_node_at(7, Span::new(50, 51, 6, 3)), + Value::from(7u64), + ); + + let source_key = mapping_node(vec![ + ( + string_node_at("a", Span::new(60, 61, 7, 4)), + unsigned_node_at(1, Span::new(62, 63, 7, 6)), + ), + ( + string_node_at("b", Span::new(64, 65, 7, 8)), + unsigned_node_at(2, Span::new(66, 67, 7, 10)), + ), + ]); + let local_key = mapping_node(vec![ + ( + string_node_at("b", Span::new(70, 71, 8, 4)), + unsigned_node_at(2, Span::new(72, 73, 8, 6)), + ), + ( + string_node_at("a", Span::new(74, 75, 8, 8)), + unsigned_node_at(1, Span::new(76, 77, 8, 10)), + ), + ]); + let mut expected_key = Mapping::new(); + expected_key.insert(Value::from("b"), Value::from(2u64)); + expected_key.insert(Value::from("a"), Value::from(1u64)); + + assert_borrowed_node_merge_key_identity(source_key, local_key, Value::Mapping(expected_key)); +} + /// Builds a `Value` mapping whose `<<` merge source nests `depth` levels deep. /// Each level carries a unique sibling key so the merge resolves to a single /// flattened mapping, exercising real merge expansion at the requested depth.