From 5f003640e2ed2c62a44c37b391f6ae5a435c608c Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Sun, 3 Nov 2024 05:29:42 +0000 Subject: [PATCH] Reimplement as a pretty printing option --- fuzz/fuzz_targets/bench/lib.rs | 4 ++ src/de/mod.rs | 86 +++++++++++++++++----------------- src/extensions.rs | 1 - src/parse.rs | 49 +++++++++++++------ src/ser/mod.rs | 48 +++++++++++++++++-- tests/551_braced_struct.rs | 10 +--- 6 files changed, 126 insertions(+), 72 deletions(-) diff --git a/fuzz/fuzz_targets/bench/lib.rs b/fuzz/fuzz_targets/bench/lib.rs index d8ed9104d..ef10b77b5 100644 --- a/fuzz/fuzz_targets/bench/lib.rs +++ b/fuzz/fuzz_targets/bench/lib.rs @@ -134,6 +134,9 @@ struct ArbitraryPrettyConfig { compact_maps: bool, /// Enable explicit number type suffixes like `1u16` number_suffixes: bool, + /// Use braced struct syntax like `Person { age: 42 }` instead of the + /// parenthesised syntax `Person(age: 42)` or just `(age: 42)` + braced_structs: bool, } fn arbitrary_ron_extensions(u: &mut Unstructured) -> arbitrary::Result { @@ -154,6 +157,7 @@ impl From for PrettyConfig { .compact_structs(arbitrary.compact_structs) .compact_maps(arbitrary.compact_maps) .number_suffixes(arbitrary.number_suffixes) + .braced_structs(arbitrary.braced_structs) } } diff --git a/src/de/mod.rs b/src/de/mod.rs index 1087ce6a1..a1c4070c2 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -202,7 +202,11 @@ impl<'de> Deserializer<'de> { } (StructType::Named, _) => { // giving no name results in worse errors but is necessary here - self.handle_struct_after_name("", visitor) + self.handle_struct_after_name(ident.is_some(), "", visitor) + } + (StructType::BracedNamed, _) => { + // giving no name results in worse errors but is necessary here + self.handle_struct_after_name(true, "", visitor) } (StructType::NewtypeTuple, _) if old_serde_content_newtype => { // deserialize a newtype struct or variant @@ -236,50 +240,29 @@ impl<'de> Deserializer<'de> { /// This method assumes there is no struct name identifier left. fn handle_struct_after_name( &mut self, + struct_name_was_present: bool, name_for_pretty_errors_only: &'static str, visitor: V, ) -> Result where V: Visitor<'de>, { - if self.extensions().contains(Extensions::BRACED_STRUCTS) { - self.newtype_variant = false; - - if self.parser.consume_char('{') { - let value = guard_recursion! { self => - visitor - .visit_map(CommaSeparated::new(Terminator::BracedStruct, self)) - .map_err(|err| { - struct_error_name( - err, - if name_for_pretty_errors_only.is_empty() { - None - } else { - Some(name_for_pretty_errors_only) - }, - ) - })? - }; + let open_brace = self.parser.check_char('{'); - self.parser.skip_ws()?; - - if self.parser.consume_char('}') { - Ok(value) - } else { - Err(Error::ExpectedStructLikeEnd) - } - } else if name_for_pretty_errors_only.is_empty() { - Err(Error::ExpectedStructLike) - } else { - Err(Error::ExpectedNamedStructLike(name_for_pretty_errors_only)) - } - } else if self.newtype_variant || self.parser.consume_char('(') { + if self.newtype_variant + || self.parser.consume_char('(') + || (struct_name_was_present && self.parser.consume_char('{')) + { let old_newtype_variant = self.newtype_variant; self.newtype_variant = false; let value = guard_recursion! { self => visitor - .visit_map(CommaSeparated::new(Terminator::Struct, self)) + .visit_map(CommaSeparated::new(if open_brace { + Terminator::BracedStruct + } else { + Terminator::Struct + }, self)) .map_err(|err| { struct_error_name( err, @@ -294,7 +277,13 @@ impl<'de> Deserializer<'de> { self.parser.skip_ws()?; - if old_newtype_variant || self.parser.consume_char(')') { + if old_newtype_variant + || (if open_brace { + self.parser.consume_char('}') + } else { + self.parser.consume_char(')') + }) + { Ok(value) } else { Err(Error::ExpectedStructLikeEnd) @@ -326,10 +315,15 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { TupleMode::DifferentiateNewtype, false, )? { + StructType::BracedNamed => { + let _ident = self.parser.identifier()?; + self.parser.skip_ws()?; + return self.handle_struct_after_name(true, "", visitor); + } StructType::Named => { // newtype variant wraps a named struct // giving no name results in worse errors but is necessary here - return self.handle_struct_after_name("", visitor); + return self.handle_struct_after_name(false, "", visitor); } StructType::EmptyTuple | StructType::NonNewtypeTuple => { // newtype variant wraps a tuple (struct) @@ -752,17 +746,21 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { where V: Visitor<'de>, { - if self.extensions().contains(Extensions::BRACED_STRUCTS) { - self.newtype_variant = false; - } - - if !self.newtype_variant { - self.parser.consume_struct_name(name)?; - } + let struct_name_was_present = if self.newtype_variant { + if self.parser.check_braced_identifier()?.is_some() { + // braced structs disable newtype variants + self.newtype_variant = false; + self.parser.consume_struct_name(name)? + } else { + false + } + } else { + self.parser.consume_struct_name(name)? + }; self.parser.skip_ws()?; - self.handle_struct_after_name(name, visitor) + self.handle_struct_after_name(struct_name_was_present, name, visitor) } fn deserialize_enum( @@ -1025,7 +1023,7 @@ impl<'de, 'a> de::VariantAccess<'de> for Enum<'a, 'de> { self.de.parser.skip_ws()?; self.de - .handle_struct_after_name("", visitor) + .handle_struct_after_name(true, "", visitor) .map_err(|err| struct_error_name(err, struct_variant)) } } diff --git a/src/extensions.rs b/src/extensions.rs index a37f07688..c4862b791 100644 --- a/src/extensions.rs +++ b/src/extensions.rs @@ -11,7 +11,6 @@ bitflags::bitflags! { /// /// During deserialization, this extension requires that structs' names are stated explicitly. const EXPLICIT_STRUCT_NAMES = 0x8; - const BRACED_STRUCTS = 0x10; } } // GRCOV_EXCL_STOP diff --git a/src/parse.rs b/src/parse.rs index 04fe96e9b..defeb4c01 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -560,9 +560,7 @@ impl<'a> Parser<'a> { if matches!(newtype, NewtypeMode::NoParensMeanUnit) && !parser.consume_char('(') - && !(parser.exts.contains(Extensions::BRACED_STRUCTS) - && has_name - && parser.consume_char('{')) + && !(has_name && parser.consume_char('{')) { return Ok(StructType::Unit); } @@ -578,12 +576,11 @@ impl<'a> Parser<'a> { // Check for `Ident {}` which is a braced struct if matches!(newtype, NewtypeMode::NoParensMeanUnit) - && parser.exts.contains(Extensions::BRACED_STRUCTS) && has_name && open_brace && parser.check_char('}') { - return Ok(StructType::Named); + return Ok(StructType::BracedNamed); } if parser.skip_identifier().is_some() { @@ -591,13 +588,16 @@ impl<'a> Parser<'a> { match parser.peek_char() { // Definitely a struct with named fields - Some(':') => return Ok(StructType::Named), + Some(':') => { + return if has_name && open_brace { + Ok(StructType::BracedNamed) + } else { + Ok(StructType::Named) + } + } // Definitely a braced struct inside a newtype - Some('{') - if matches!(newtype, NewtypeMode::InsideNewtype) - && parser.exts.contains(Extensions::BRACED_STRUCTS) => - { - return Ok(StructType::Named) + Some('{') if matches!(newtype, NewtypeMode::InsideNewtype) => { + return Ok(StructType::BracedNamed) } // Definitely a tuple-like struct with fields Some(',') => { @@ -694,9 +694,7 @@ impl<'a> Parser<'a> { pub fn consume_struct_name(&mut self, ident: &'static str) -> Result { if self.check_ident("") { - if self.exts.contains(Extensions::EXPLICIT_STRUCT_NAMES) - || self.exts.contains(Extensions::BRACED_STRUCTS) - { + if self.exts.contains(Extensions::EXPLICIT_STRUCT_NAMES) { return Err(Error::ExpectedStructName(ident.to_string())); } @@ -908,6 +906,28 @@ impl<'a> Parser<'a> { None } + pub fn check_braced_identifier(&mut self) -> Result> { + // Create a temporary working copy + let backup_cursor = self.cursor; + + let ident = if let Some(ident) = self.skip_identifier() { + self.skip_ws()?; + + if self.peek_char() == Some('{') { + Some(ident) + } else { + None + } + } else { + None + }; + + // Revert the parser to before the peek + self.set_cursor(backup_cursor); + + Ok(ident) + } + pub fn identifier(&mut self) -> Result<&'a str> { let first = self.peek_char_or_eof()?; if !is_ident_first_char(first) { @@ -1617,6 +1637,7 @@ pub enum StructType { NewtypeTuple, NonNewtypeTuple, Named, + BracedNamed, Unit, } diff --git a/src/ser/mod.rs b/src/ser/mod.rs index 1892c210f..e5aaf10a2 100644 --- a/src/ser/mod.rs +++ b/src/ser/mod.rs @@ -113,6 +113,9 @@ pub struct PrettyConfig { pub number_suffixes: bool, /// Additional path-based field metadata to serialize pub path_meta: Option, + /// Use braced struct syntax like `Person { age: 42 }` instead of the + /// parenthesised syntax `Person(age: 42)` or just `(age: 42)` + pub braced_structs: bool, } impl PrettyConfig { @@ -341,6 +344,36 @@ impl PrettyConfig { self } + + /// Configures whether structs or struct variants with named fields should + /// be printed using braced syntax (`true`) or parenthesised syntax + /// (`false`). + /// + /// When `false`, the struct `Person { age: 42 }` will serialize to + /// ```ignore + /// Person(age: 42) + /// # ; + /// ``` + /// if printing struct names is turned on, or + /// ```ignore + /// (age: 42) + /// # ; + /// ``` + /// if struct names are turned off. + /// + /// When `true`, the struct `Person { age: 42 }` will serialize to + /// ```ignore + /// Person {age: 42} + /// # ; + /// ``` + /// + /// Default: `false` + #[must_use] + pub fn braced_structs(mut self, braced_structs: bool) -> Self { + self.braced_structs = braced_structs; + + self + } } impl Default for PrettyConfig { @@ -364,6 +397,7 @@ impl Default for PrettyConfig { compact_maps: false, number_suffixes: false, path_meta: None, + braced_structs: false, } } } @@ -475,6 +509,12 @@ impl Serializer { .map_or(false, |(ref config, _)| config.number_suffixes) } + fn braced_structs(&self) -> bool { + self.pretty + .as_ref() + .map_or(false, |(ref config, _)| config.braced_structs) + } + fn extensions(&self) -> Extensions { self.default_extensions | self @@ -1039,9 +1079,9 @@ impl<'a, W: fmt::Write> ser::Serializer for &'a mut Serializer { self.newtype_variant = false; self.implicit_some_depth = 0; - let closing = if self.extensions().contains(Extensions::BRACED_STRUCTS) { + let closing = if self.braced_structs() { self.write_identifier(name)?; - self.output.write_char('{')?; + self.output.write_str(" {")?; Some('}') } else if old_newtype_variant { self.validate_identifier(name)?; @@ -1077,8 +1117,8 @@ impl<'a, W: fmt::Write> ser::Serializer for &'a mut Serializer { self.validate_identifier(name)?; self.write_identifier(variant)?; - let closing = if self.extensions().contains(Extensions::BRACED_STRUCTS) { - self.output.write_char('{')?; + let closing = if self.braced_structs() { + self.output.write_str(" {")?; '}' } else { self.output.write_char('(')?; diff --git a/tests/551_braced_struct.rs b/tests/551_braced_struct.rs index 66819a204..3a3257a93 100644 --- a/tests/551_braced_struct.rs +++ b/tests/551_braced_struct.rs @@ -2,15 +2,7 @@ fn raw_value() { let _: Vec> = ron::from_str( r#" -[abc, 922e37, Value, [[]], None, (a: 7), {a: 7}] -"#, - ) - .unwrap(); - - let _: Vec> = ron::from_str( - r#" -#![enable(braced_structs)] -[abc, 922e37, Value, [[]], None, {a: 7}] +[abc, 922e37, Value, [[]], None, (a: 7), {a: 7}, Person { age: 42 }] "#, ) .unwrap();