Skip to content

Commit

Permalink
Reimplement as a pretty printing option
Browse files Browse the repository at this point in the history
  • Loading branch information
juntyr committed Nov 3, 2024
1 parent dc51c7d commit 5f00364
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 72 deletions.
4 changes: 4 additions & 0 deletions fuzz/fuzz_targets/bench/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Extensions> {
Expand All @@ -154,6 +157,7 @@ impl From<ArbitraryPrettyConfig> for PrettyConfig {
.compact_structs(arbitrary.compact_structs)
.compact_maps(arbitrary.compact_maps)
.number_suffixes(arbitrary.number_suffixes)
.braced_structs(arbitrary.braced_structs)
}
}

Expand Down
86 changes: 42 additions & 44 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -236,50 +240,29 @@ impl<'de> Deserializer<'de> {
/// This method assumes there is no struct name identifier left.
fn handle_struct_after_name<V>(
&mut self,
struct_name_was_present: bool,
name_for_pretty_errors_only: &'static str,
visitor: V,
) -> Result<V::Value>
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,
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<V>(
Expand Down Expand Up @@ -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))
}
}
Expand Down
1 change: 0 additions & 1 deletion src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 35 additions & 14 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -578,26 +576,28 @@ 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() {
parser.skip_ws()?;

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(',') => {
Expand Down Expand Up @@ -694,9 +694,7 @@ impl<'a> Parser<'a> {

pub fn consume_struct_name(&mut self, ident: &'static str) -> Result<bool> {
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()));
}

Expand Down Expand Up @@ -908,6 +906,28 @@ impl<'a> Parser<'a> {
None
}

pub fn check_braced_identifier(&mut self) -> Result<Option<&'a str>> {
// 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) {
Expand Down Expand Up @@ -1617,6 +1637,7 @@ pub enum StructType {
NewtypeTuple,
NonNewtypeTuple,
Named,
BracedNamed,
Unit,
}

Expand Down
48 changes: 44 additions & 4 deletions src/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ pub struct PrettyConfig {
pub number_suffixes: bool,
/// Additional path-based field metadata to serialize
pub path_meta: Option<path_meta::Field>,
/// 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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -364,6 +397,7 @@ impl Default for PrettyConfig {
compact_maps: false,
number_suffixes: false,
path_meta: None,
braced_structs: false,
}
}
}
Expand Down Expand Up @@ -475,6 +509,12 @@ impl<W: fmt::Write> Serializer<W> {
.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
Expand Down Expand Up @@ -1039,9 +1079,9 @@ impl<'a, W: fmt::Write> ser::Serializer for &'a mut Serializer<W> {
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)?;
Expand Down Expand Up @@ -1077,8 +1117,8 @@ impl<'a, W: fmt::Write> ser::Serializer for &'a mut Serializer<W> {
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('(')?;
Expand Down
10 changes: 1 addition & 9 deletions tests/551_braced_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,7 @@
fn raw_value() {
let _: Vec<Box<ron::value::RawValue>> = ron::from_str(
r#"
[abc, 922e37, Value, [[]], None, (a: 7), {a: 7}]
"#,
)
.unwrap();

let _: Vec<Box<ron::value::RawValue>> = 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();
Expand Down

0 comments on commit 5f00364

Please sign in to comment.