diff --git a/crates/toml_edit/src/inline_table.rs b/crates/toml_edit/src/inline_table.rs index 9b8e9c42..43a32e82 100644 --- a/crates/toml_edit/src/inline_table.rs +++ b/crates/toml_edit/src/inline_table.rs @@ -3,6 +3,7 @@ use std::iter::FromIterator; use crate::key::Key; use crate::repr::Decor; use crate::table::{Iter, IterMut, KeyValuePairs, TableLike}; +use crate::internal_table::{GetTableValues, SortTable, SortTableBy}; use crate::{InternalString, Item, KeyMut, RawString, Table, Value}; /// Type representing a TOML inline table, @@ -51,30 +52,7 @@ impl InlineTable { /// /// For example, this will return dotted keys pub fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> { - let mut values = Vec::new(); - let root = Vec::new(); - self.append_values(&root, &mut values); - values - } - - pub(crate) fn append_values<'s>( - &'s self, - parent: &[&'s Key], - values: &mut Vec<(Vec<&'s Key>, &'s Value)>, - ) { - for (key, value) in self.items.iter() { - let mut path = parent.to_vec(); - path.push(key); - match value { - Item::Value(Value::InlineTable(table)) if table.is_dotted() => { - table.append_values(&path, values); - } - Item::Value(value) => { - values.push((path, value)); - } - _ => {} - } - } + GetTableValues::get_values(self) } /// Auto formats the table. @@ -84,52 +62,18 @@ impl InlineTable { /// Sorts the key/value pairs by key. pub fn sort_values(&mut self) { - // Assuming standard tables have their position set and this won't negatively impact them - self.items.sort_keys(); - for value in self.items.values_mut() { - match value { - Item::Value(Value::InlineTable(table)) if table.is_dotted() => { - table.sort_values(); - } - _ => {} - } - } + SortTable::sort_values(self) } /// Sort Key/Value Pairs of the table using the using the comparison function `compare`. /// /// The comparison function receives two key and value pairs to compare (you can sort by keys or /// values or their combination as needed). - pub fn sort_values_by(&mut self, mut compare: F) + pub fn sort_values_by(&mut self, compare: F) where F: FnMut(&Key, &Value, &Key, &Value) -> std::cmp::Ordering, { - self.sort_values_by_internal(&mut compare); - } - - fn sort_values_by_internal(&mut self, compare: &mut F) - where - F: FnMut(&Key, &Value, &Key, &Value) -> std::cmp::Ordering, - { - let modified_cmp = - |key1: &Key, val1: &Item, key2: &Key, val2: &Item| -> std::cmp::Ordering { - match (val1.as_value(), val2.as_value()) { - (Some(v1), Some(v2)) => compare(key1, v1, key2, v2), - (Some(_), None) => std::cmp::Ordering::Greater, - (None, Some(_)) => std::cmp::Ordering::Less, - (None, None) => std::cmp::Ordering::Equal, - } - }; - - self.items.sort_by(modified_cmp); - for value in self.items.values_mut() { - match value { - Item::Value(Value::InlineTable(table)) if table.is_dotted() => { - table.sort_values_by_internal(compare); - } - _ => {} - } - } + SortTableBy::::sort_values_by(self, compare) } /// If a table has no key/value pairs and implicit, it will not be displayed. diff --git a/crates/toml_edit/src/internal_table.rs b/crates/toml_edit/src/internal_table.rs new file mode 100644 index 00000000..ed25a867 --- /dev/null +++ b/crates/toml_edit/src/internal_table.rs @@ -0,0 +1,203 @@ +use crate::{InlineTable, Item, Key, Table, Value}; +/// a place for helper methods supporting the table-like impls + +use crate::table::KeyValuePairs; + +/// GetTableValues provides the logic for displaying a table's items in their parsed order +pub(crate) trait GetTableValues { + fn items(&self) -> &KeyValuePairs; + + fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> { + let mut values = Vec::new(); + let root = Vec::new(); + self.append_values(&root, &mut values); + values + } + + fn append_values<'s>( + &'s self, + parent: &[&'s Key], + values: &mut Vec<(Vec<&'s Key>, &'s Value)>, + ) { + for (key, item) in self.items().iter() { + let mut path = parent.to_vec(); + path.push(key); + match item { + Item::Table(table) if table.is_dotted() => { + GetTableValues::append_values(table, &path, values) + } + Item::Value(Value::InlineTable(table)) if table.is_dotted() => { + GetTableValues::append_values(table, &path, values) + } + Item::Value(value) => { + values.push((path, value)) + } + _ => {} + } + } + sort_values_by_position(values); + } +} + +/// SortTable provides the logic for sorting a table's items by its keys +pub(crate) trait SortTable { + fn items_mut(&mut self) -> &mut KeyValuePairs; + + fn sort_values(&mut self) { + // Assuming standard tables have their doc_position set and this won't negatively impact them + self.items_mut().sort_keys(); + assign_sequential_key_positions(self.items_mut(), |item| { + match item { + Item::Table(table) if table.is_dotted() => { + SortTable::sort_values(table) + } + Item::Value(Value::InlineTable(table)) if table.is_dotted() => { + SortTable::sort_values(table) + } + _ => {} + } + }); + } +} + +/// SortTableBy provides the logic for sorting a table by a custom comparison +pub(crate) trait SortTableBy : SortTable +where + It: for<'a> TryFrom<&'a Item> +{ + fn sort_values_by(&mut self, compare: F) + where + F: FnMut(&Key, &It, &Key, &It) -> std::cmp::Ordering, + { + // intended for `InlineTable`s, where some `Item`s might not be `Value`s, + // in the case of dotted keys mostly I expect. + // but for `Table`s the `(Some,Some)` will be the only one used. + self.sort_vals_by_direct( + &mut Self::generalize(compare) + ) + } + + /// no modification to the comparing Fn in this one, + /// allows for slightly improved recursion that does not continuously + /// re-modify the comparison function. + fn sort_vals_by_direct(&mut self, compare: &mut F) + where + F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering + { + self.items_mut().sort_by(|key1, val1, key2, val2| { + compare(key1, val1, key2, val2) + }); + + assign_sequential_key_positions(self.items_mut(), |value| { + match value { + Item::Table(table) if table.is_dotted() => { + SortTableBy::::sort_values_by( + table, + |k1, i1, k2, i2| { + compare(k1, i1, k2, i2) + } + ) + }, + Item::Value(Value::InlineTable(table)) if table.is_dotted() => { + SortTableBy::::sort_values_by( + table, + |k1, i1, k2, i2| { + let s1 = &Item::from(i1); + let s2 = &Item::from(i2); + compare(k1, s1, k2, s2) + } + ) + }, + _ => {} + }; + }); + } + + fn generalize<'a, F>(mut compare: F) -> Box std::cmp::Ordering + 'a> + where + F: FnMut(&Key, &It, &Key, &It) -> std::cmp::Ordering + 'a, + { + Box::new(move |key1, s1, key2, s2| { + match (It::try_from(s1).ok(), It::try_from(s2).ok()) { + (Some(v1), Some(v2)) => compare(key1, &v1, key2, &v2), + (Some(_), None) => std::cmp::Ordering::Greater, + (None, Some(_)) => std::cmp::Ordering::Less, + (None, None) => std::cmp::Ordering::Equal, + } + }) + } +} + +fn assign_sequential_key_positions(items: &mut KeyValuePairs, mut recursive_step: F) +where + F: FnMut(&mut Item), +{ + use indexmap::map::MutableKeys; + for (pos, (key, value)) in items.iter_mut2().enumerate() { + key.set_position(Some(pos)); + recursive_step(value); + } +} + +fn sort_values_by_position<'s>(values: &mut [(Vec<&'s Key>, &'s Value)]) { + /* + `Vec::sort_by_key` works because we add the position to _every_ item's key during parsing, + so keys without positions would be either: + 1. non-leaf keys (i.e. "foo" or "bar" in dotted key "foo.bar.baz") + 2. custom keys added to the doc without an explicit position + In the case of (1), we'd never see it since we only look at the last + key in a dotted-key. So, we can safely return a constant value for these cases. + + To support the most intuitive behavior, we return the maximum usize, placing + position=None items at the end, so when you insert it without position, it + appends it to the end. + */ + values.sort_by_key(|(key_path, _)| { + return match key_path.last().map(|x| x.position) { + // unwrap "last()" -> unwrap "position" + Some(Some(pos)) => pos, + // either last() = None, or position = None + _ => usize::MAX + }; + }); +} + +impl TryFrom<&Item> for Value { + type Error = String; + + fn try_from(value: &Item) -> Result { + let err = "cannot extract Value from Non-Value Item:"; + match value { + Item::Value(v) => Ok((*v).clone()), + it => it.as_value().map(|v| v.clone()).ok_or( + format!("{err}: {it:?}") + ), + } + } + +} + +impl GetTableValues for Table { + fn items(&self) -> &KeyValuePairs { + &self.items + } +} +impl GetTableValues for InlineTable { + fn items(&self) -> &KeyValuePairs { + &self.items + } +} + +impl SortTable for Table { + fn items_mut(&mut self) -> &mut KeyValuePairs { + &mut self.items + } +} +impl SortTable for InlineTable { + fn items_mut(&mut self) -> &mut KeyValuePairs { + &mut self.items + } +} + +impl SortTableBy for Table {} +impl SortTableBy for InlineTable {} diff --git a/crates/toml_edit/src/key.rs b/crates/toml_edit/src/key.rs index 314870f5..75ae83d3 100644 --- a/crates/toml_edit/src/key.rs +++ b/crates/toml_edit/src/key.rs @@ -32,6 +32,7 @@ pub struct Key { pub(crate) repr: Option, pub(crate) leaf_decor: Decor, pub(crate) dotted_decor: Decor, + pub(crate) position: Option, } impl Key { @@ -42,6 +43,7 @@ impl Key { repr: None, leaf_decor: Default::default(), dotted_decor: Default::default(), + position: Default::default(), } } @@ -76,6 +78,12 @@ impl Key { self } + /// While creating the `Key`, add a table position to it + pub fn with_position(mut self, position: Option) -> Self { + self.position = position; + self + } + /// Access a mutable proxy for the `Key`. pub fn as_mut(&mut self) -> KeyMut<'_> { KeyMut { key: self } @@ -158,6 +166,16 @@ impl Key { } } + /// Get the position relative to other keys in parent table + pub fn position(&self) -> Option { + self.position + } + + /// Set the position relative to other keys in parent table + pub fn set_position(&mut self, position: Option) { + self.position = position; + } + /// Auto formats the key. pub fn fmt(&mut self) { self.repr = None; @@ -190,6 +208,7 @@ impl Clone for Key { repr: self.repr.clone(), leaf_decor: self.leaf_decor.clone(), dotted_decor: self.dotted_decor.clone(), + position: self.position, } } } diff --git a/crates/toml_edit/src/lib.rs b/crates/toml_edit/src/lib.rs index c47b902a..966e001c 100644 --- a/crates/toml_edit/src/lib.rs +++ b/crates/toml_edit/src/lib.rs @@ -85,6 +85,7 @@ mod error; mod index; mod inline_table; mod internal_string; +mod internal_table; mod item; mod key; #[cfg(feature = "parse")] diff --git a/crates/toml_edit/src/parser/inline_table.rs b/crates/toml_edit/src/parser/inline_table.rs index 6eb06a31..21e1cc20 100644 --- a/crates/toml_edit/src/parser/inline_table.rs +++ b/crates/toml_edit/src/parser/inline_table.rs @@ -40,7 +40,8 @@ fn table_from_pairs( // Assuming almost all pairs will be directly in `root` root.items.reserve(v.len()); - for (path, (key, value)) in v { + for (position, (path, (mut key, value))) in v.into_iter().enumerate() { + key.set_position(Some(position)); let table = descend_path(&mut root, &path)?; // "Likewise, using dotted keys to redefine tables already defined in [table] form is not allowed" @@ -162,6 +163,7 @@ mod test { r#"{a = 1e165}"#, r#"{ hello = "world", a = 1}"#, r#"{ hello.world = "a" }"#, + r#"{ hello.world = "a", goodbye = "b", hello.moon = "c" }"#, ]; for input in inputs { dbg!(input); diff --git a/crates/toml_edit/src/parser/mod.rs b/crates/toml_edit/src/parser/mod.rs index e0d9e4f0..da7a7d68 100644 --- a/crates/toml_edit/src/parser/mod.rs +++ b/crates/toml_edit/src/parser/mod.rs @@ -216,6 +216,20 @@ that key = "value" "#, r#"hello.world = "a" +"#, + r#" +hello.world = "a" +goodbye = "b" +hello.moon = "c" +"#, + r#" +tool1.featureA = "foo" +# note this must match the above line, ask Dave why +tool2.featureA = "foo" + +# these two values must always add to 9 because reasons +tool1.featureB = -1 +tool3.featureB = 10 "#, r#"foo = 1979-05-27 # Comment "#, diff --git a/crates/toml_edit/src/parser/state.rs b/crates/toml_edit/src/parser/state.rs index 6a234136..de126df9 100644 --- a/crates/toml_edit/src/parser/state.rs +++ b/crates/toml_edit/src/parser/state.rs @@ -7,6 +7,8 @@ pub(crate) struct ParseState { root: Table, trailing: Option>, current_table_position: usize, + // Current position within a table, to help order dotted keys + current_value_position: usize, current_table: Table, current_is_array: bool, current_table_path: Vec, @@ -20,6 +22,7 @@ impl ParseState { root: Table::new(), trailing: None, current_table_position: 0, + current_value_position: 0, current_table: root, current_is_array: false, current_table_path: Vec::new(), @@ -74,6 +77,9 @@ impl ParseState { if let (Some(existing), Some(value)) = (self.current_table.span(), value.span()) { self.current_table.span = Some((existing.start)..(value.end)); } + key.set_position(Some(self.current_value_position)); + self.current_value_position += 1; + let table = &mut self.current_table; let table = Self::descend_path(table, &path, true)?; @@ -161,6 +167,7 @@ impl ParseState { } self.current_table_position += 1; + self.current_value_position = 0; self.current_table.decor = decor; self.current_table.set_implicit(false); self.current_table.set_dotted(false); diff --git a/crates/toml_edit/src/table.rs b/crates/toml_edit/src/table.rs index 1ae02310..94b4cdcf 100644 --- a/crates/toml_edit/src/table.rs +++ b/crates/toml_edit/src/table.rs @@ -5,6 +5,7 @@ use indexmap::map::IndexMap; use crate::key::Key; use crate::repr::Decor; use crate::value::DEFAULT_VALUE_DECOR; +use crate::internal_table::{GetTableValues, SortTable, SortTableBy}; use crate::{InlineTable, InternalString, Item, KeyMut, Value}; /// Type representing a TOML non-inline table @@ -64,38 +65,7 @@ impl Table { /// /// For example, this will return dotted keys pub fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> { - let mut values = Vec::new(); - let root = Vec::new(); - self.append_values(&root, &mut values); - values - } - - fn append_values<'s>( - &'s self, - parent: &[&'s Key], - values: &mut Vec<(Vec<&'s Key>, &'s Value)>, - ) { - for (key, value) in self.items.iter() { - let mut path = parent.to_vec(); - path.push(key); - match value { - Item::Table(table) if table.is_dotted() => { - table.append_values(&path, values); - } - Item::Value(value) => { - if let Some(table) = value.as_inline_table() { - if table.is_dotted() { - table.append_values(&path, values); - } else { - values.push((path, value)); - } - } else { - values.push((path, value)); - } - } - _ => {} - } - } + GetTableValues::get_values(self) } /// Auto formats the table. @@ -107,48 +77,18 @@ impl Table { /// /// Doesn't affect subtables or subarrays. pub fn sort_values(&mut self) { - // Assuming standard tables have their doc_position set and this won't negatively impact them - self.items.sort_keys(); - for value in self.items.values_mut() { - match value { - Item::Table(table) if table.is_dotted() => { - table.sort_values(); - } - _ => {} - } - } + SortTable::sort_values(self) } /// Sort Key/Value Pairs of the table using the using the comparison function `compare`. /// /// The comparison function receives two key and value pairs to compare (you can sort by keys or /// values or their combination as needed). - pub fn sort_values_by(&mut self, mut compare: F) + pub fn sort_values_by(&mut self, compare: F) where F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering, { - self.sort_values_by_internal(&mut compare); - } - - fn sort_values_by_internal(&mut self, compare: &mut F) - where - F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering, - { - let modified_cmp = - |key1: &Key, val1: &Item, key2: &Key, val2: &Item| -> std::cmp::Ordering { - compare(key1, val1, key2, val2) - }; - - self.items.sort_by(modified_cmp); - - for value in self.items.values_mut() { - match value { - Item::Table(table) if table.is_dotted() => { - table.sort_values_by_internal(compare); - } - _ => {} - } - } + SortTableBy::::sort_values_by(self, compare) } /// If a table has no key/value pairs and implicit, it will not be displayed. diff --git a/crates/toml_edit/tests/testsuite/edit.rs b/crates/toml_edit/tests/testsuite/edit.rs index 84c53d05..59e7ea6e 100644 --- a/crates/toml_edit/tests/testsuite/edit.rs +++ b/crates/toml_edit/tests/testsuite/edit.rs @@ -26,8 +26,10 @@ struct Test { fn given(input: &str) -> Test { let doc = input.parse::(); - assert!(doc.is_ok()); - Test { doc: doc.unwrap() } + match doc { + Err(e) => panic!("{}", e), + _ => Test { doc: doc.unwrap() } + } } impl Test { @@ -579,6 +581,42 @@ fn test_sort_values() { "#]]); } +#[test] +fn test_sort_dotted_values() { + given( + r#" + [a.z] + + [a] + a.b = 2 + # this comment is attached to b + b = 3 # as well as this + a.a = 1 + c = 4 + + [a.y]"#, + ) + .running(|root| { + let a = root.get_mut("a").unwrap(); + let a = as_table!(a); + a.sort_values(); + }) + .produces_display(str![[r#" + + [a.z] + + [a] + a.a = 1 + a.b = 2 + # this comment is attached to b + b = 3 # as well as this + c = 4 + + [a.y] + +"#]]); +} + #[test] fn test_sort_values_by() { given( @@ -1026,3 +1064,118 @@ name = "test.swf" "#]] ); } + +#[test] +fn modify_dotted_keys() { + given(r#" +tool1.featureA = "foo" +# note this must match the above line, ask Dave why +tool2.featureA = "foo" + +# these two values must always add to 9 because reasons +tool1.featureB = -1 +tool3.featureB = 10 +"#) + .running_on_doc(|doc| { + let first_tool_k = "tool1"; + let second_tool_k = "tool3"; + let feature_k = "featureB"; + let root = doc.as_table_mut(); + + let tool1_v = root.get_mut(first_tool_k).unwrap(); + let tool1: &mut Table = as_table!(tool1_v); + let tool1_b = tool1.get_mut(feature_k).unwrap().as_integer().unwrap(); + + let tool3_v = root.get_mut(second_tool_k).unwrap(); + let tool3: &mut Table = as_table!(tool3_v); + let tool3_b = tool3.get_mut(feature_k).unwrap().as_integer().unwrap(); + + let total = (tool1_b + tool3_b) as f64; + let split_val = (total / 2.0) as i64; + let mut split_val2 = split_val; + if split_val * 2 != total as i64 { + split_val2 += 1 + } + + root[first_tool_k][feature_k] = Item::from(split_val); + root[second_tool_k][feature_k] = Item::from(split_val2); + }) + .produces_display(str![[ + r#" + +tool1.featureA = "foo" +# note this must match the above line, ask Dave why +tool2.featureA = "foo" + +# these two values must always add to 9 because reasons +tool1.featureB = 4 +tool3.featureB = 5 + +"# + ]]); +} + +#[test] +fn insert_key_with_position() { + given(r#"foo.bar = 1 + +foo.baz.qwer = "a" +goodbye = { forever="no" } + +foo.pub = 2 +foo.baz.asdf = "b" +"#) + .running_on_doc(|doc| { + let root = doc.as_table_mut(); + let (_, foo_v) = root.get_key_value_mut("foo").unwrap(); + let foo = as_table!(foo_v); + let (_, foo_baz_v) = foo.get_key_value_mut("baz").unwrap(); + let foo_baz = as_table!(foo_baz_v); + + let foo_baz_asdf_v = foo_baz.remove("asdf").unwrap(); + let foo_baz_asdf_k = Key::new("asdf").with_position(Some(0)); + + foo_baz.insert_formatted(&foo_baz_asdf_k, foo_baz_asdf_v); + }) + .produces_display(str![[r#"foo.bar = 1 +foo.baz.asdf = "b" + +foo.baz.qwer = "a" +goodbye = { forever="no" } + +foo.pub = 2 + +"#]]); +} + +#[test] +fn insert_key_with_position_none() { + given(r#"foo.bar = 1 + +foo.baz.qwer = "a" +goodbye = { forever="no" } + +foo.pub = 2 +foo.baz.asdf = "b" +"#) + .running_on_doc(|doc| { + let root = doc.as_table_mut(); + let (_, foo_v) = root.get_key_value_mut("foo").unwrap(); + let foo = as_table!(foo_v); + let (_, foo_baz_v) = foo.get_key_value_mut("baz").unwrap(); + let foo_baz = as_table!(foo_baz_v); + + let foo_baz_asdf_v = foo_baz.remove("qwer").unwrap(); + let foo_baz_asdf_k = Key::new("qwer").with_position(None); + + foo_baz.insert_formatted(&foo_baz_asdf_k, foo_baz_asdf_v); + }) + .produces_display(str![[r#"foo.bar = 1 +goodbye = { forever="no" } + +foo.pub = 2 +foo.baz.asdf = "b" +foo.baz.qwer = "a" + +"#]]); +} diff --git a/crates/toml_edit/tests/testsuite/parse.rs b/crates/toml_edit/tests/testsuite/parse.rs index 39e00cfd..e437ca4e 100644 --- a/crates/toml_edit/tests/testsuite/parse.rs +++ b/crates/toml_edit/tests/testsuite/parse.rs @@ -1605,6 +1605,25 @@ clippy.exhaustive_enums = "warn" assert_data_eq!(actual, expected.raw()); } +#[test] +fn dotted_key_interspersed_roundtrip() { + let input = r###" +rust.unsafe_op_in_unsafe_fn = "deny" + +clippy.cast_lossless = "warn" +rust.explicit_outlives_requirements = "warn" + +clippy.doc_markdown = "warn" +clippy.exhaustive_enums = "warn" +"###; + let expected = input; + + let manifest: DocumentMut = input.parse().unwrap(); + let actual = manifest.to_string(); + + assert_data_eq!(actual, expected.raw()); +} + #[test] fn string_repr_roundtrip() { assert_string_repr_roundtrip(r#""""#, str![[r#""""#]]);