Skip to content

Commit

Permalink
Add explicit_struct_names Extension (#522)
Browse files Browse the repository at this point in the history
* prepare adding `explicit_struct_names` extension

* add error handling, prepare to implement it, prepare to add test

* implement extension, add test

* update CHANGELOG.md

* add documentation, improve comments

* add example using the Options method

* fix wording in ExpectedStructName error message

Co-authored-by: Juniper Tyree <[email protected]>

* add tuple struct test, use all codepaths

* in test: rename each foo variable to correspond with their phase (formatting)

* fix extensions test

* add check for Error::ExpectedStructName message, suppress unnecessary compiler warning

* resolve redundant else block

* add "see also" blurbs, OR the `explicit_struct_names` value in `PrettyConfig::struct_names`

* fix formatting

* fix coverage

* move OR operation to Serializer::struct_names

* fix pretty print in Error::ExpectedStructName message

* fix Serializer::struct_names

* add trunk config files to .gitignore (for those who use it)

* add .DS_Store to the .gitignore (you'll thank me later)

* add test for enum variants, clean up `Extensions::from_ident` code

* Fix grammar error in extensions.rs

Co-authored-by: Juniper Tyree <[email protected]>

* add `unwrap_variant_newtypes` to test

* add to phase 5 a deserialize test

* add GRCOV_EXCL

---------

Co-authored-by: Juniper Tyree <[email protected]>
  • Loading branch information
sylv256 and juntyr authored Jan 7, 2024
1 parent e27d1d5 commit d509208
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 35 deletions.
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@
# Generated by Cargo
/target/
/Cargo.lock

# VSCode Extensions
/.trunk

# MacOS Shenanigans
.DS_Store
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Allow `ron::value::RawValue` to capture any whitespace to the left and right of a ron value ([#487](https://github.com/ron-rs/ron/pull/487))
- Breaking: Enforce that ron always writes valid UTF-8 ([#488](https://github.com/ron-rs/ron/pull/488))
- Add convenient `Value::from` impls ([#498](https://github.com/ron-rs/ron/pull/498))
- Add new extension `explicit_struct_names` which requires that struct names are included during deserialization ([#522](https://github.com/ron-rs/ron/pull/522))

### Format Changes

Expand Down
44 changes: 44 additions & 0 deletions docs/extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,47 @@ With the `unwrap_variant_newtypes` extension, the first structural layer inside
```

Note that when the `unwrap_variant_newtypes` extension is enabled, the first layer inside a newtype variant will **always** be unwrapped, i.e. it is no longer possible to write `A(Inner(a: 4, b: true))` or `A((a: 4, b: true))`.

# explicit_struct_names
During serialization, this extension emits struct names. For instance, this would be emitted:
```ron
Foo(
bar: Bar(42),
)
```

During deserialization, this extension requires that all structs have names attached to them. For example, the following deserializes perfectly fine:
```ron
Foo(
bar: Bar(42),
)
```

However, with the `explicit_struct_names` extension enabled, the following will throw an `ExpectedStructName` error:
```ron
(
bar: Bar(42),
)
```

Similarly, the following will throw the same error:
```ron
Foo(
bar: (42),
)
```

Note that if what you are parsing is spread across many files, you would likely use `Options::with_default_extension` to enable `Extensions::EXPLICIT_STRUCT_NAMES` before the parsing stage. This is because prepending `#![enable(explicit_struct_names)]` to the contents of every file you parse would violate DRY (Don't Repeat Yourself).

Here is an example of how to enable `explicit_struct_names` using this method:
```rust
use ron::extensions::Extensions;
use ron::options::Options;

// Setup the options
let options = Options::default().with_default_extension(Extensions::EXPLICIT_STRUCT_NAMES);
// Retrieve the contents of the file
let file_contents: &str = /* ... */;
// Parse the file's contents
let foo: Foo = options.from_str(file_contents)?;
```
11 changes: 11 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ pub enum Error {
SuggestRawIdentifier(String),
ExpectedRawValue,
ExceededRecursionLimit,
ExpectedStructName(String),
}

impl fmt::Display for SpannedError {
Expand Down Expand Up @@ -281,6 +282,11 @@ impl fmt::Display for Error {
"Exceeded recursion limit, try increasing `ron::Options::recursion_limit` \
and using `serde_stacker` to protect against a stack overflow",
),
Error::ExpectedStructName(ref name) => write!(
f,
"Expected the explicit struct name {}, but none was found",
Identifier(name)
),
}
}
}
Expand Down Expand Up @@ -568,6 +574,7 @@ mod tests {
"Unexpected leading underscore in a number",
);
check_error_message(&Error::UnexpectedChar('🦀'), "Unexpected char \'🦀\'");
#[allow(invalid_from_utf8)]
check_error_message(
&Error::Utf8Error(std::str::from_utf8(b"error: \xff\xff\xff\xff").unwrap_err()),
"invalid utf-8 sequence of 1 bytes from index 7",
Expand Down Expand Up @@ -666,6 +673,10 @@ mod tests {
"Exceeded recursion limit, try increasing `ron::Options::recursion_limit` \
and using `serde_stacker` to protect against a stack overflow",
);
check_error_message(
&Error::ExpectedStructName(String::from("Struct")),
"Expected the explicit struct name `Struct`, but none was found",
);
}

fn check_error_message<T: std::fmt::Display>(err: &T, msg: &str) {
Expand Down
34 changes: 17 additions & 17 deletions src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ bitflags::bitflags! {
const UNWRAP_NEWTYPES = 0x1;
const IMPLICIT_SOME = 0x2;
const UNWRAP_VARIANT_NEWTYPES = 0x4;
/// During serialization, this extension emits struct names. See also [`PrettyConfig::struct_names`](crate::ser::PrettyConfig::struct_names) for the [`PrettyConfig`](crate::ser::PrettyConfig) equivalent.
///
/// During deserialization, this extension requires that structs' names are stated explicitly.
const EXPLICIT_STRUCT_NAMES = 0x8;
}
}
// GRCOV_EXCL_STOP
Expand All @@ -15,20 +19,23 @@ impl Extensions {
/// Creates an extension flag from an ident.
#[must_use]
pub fn from_ident(ident: &str) -> Option<Extensions> {
match ident {
"unwrap_newtypes" => Some(Extensions::UNWRAP_NEWTYPES),
"implicit_some" => Some(Extensions::IMPLICIT_SOME),
"unwrap_variant_newtypes" => Some(Extensions::UNWRAP_VARIANT_NEWTYPES),
_ => None,
for (name, extension) in Extensions::all().iter_names() {
if ident == name.to_lowercase() {
return Some(extension);
}
}

None
}
}

// GRCOV_EXCL_START
impl Default for Extensions {
fn default() -> Self {
Extensions::empty()
}
}
// GRCOV_EXCL_STOP

#[cfg(test)]
mod tests {
Expand All @@ -42,17 +49,10 @@ mod tests {

#[test]
fn test_extension_serde() {
roundtrip_extensions(Extensions::default());
roundtrip_extensions(Extensions::UNWRAP_NEWTYPES);
roundtrip_extensions(Extensions::IMPLICIT_SOME);
roundtrip_extensions(Extensions::UNWRAP_VARIANT_NEWTYPES);
roundtrip_extensions(Extensions::UNWRAP_NEWTYPES | Extensions::IMPLICIT_SOME);
roundtrip_extensions(Extensions::UNWRAP_NEWTYPES | Extensions::UNWRAP_VARIANT_NEWTYPES);
roundtrip_extensions(Extensions::IMPLICIT_SOME | Extensions::UNWRAP_VARIANT_NEWTYPES);
roundtrip_extensions(
Extensions::UNWRAP_NEWTYPES
| Extensions::IMPLICIT_SOME
| Extensions::UNWRAP_VARIANT_NEWTYPES,
);
// iterate over the powerset of all extensions (i.e. every possible combination of extensions)
for bits in Extensions::empty().bits()..=Extensions::all().bits() {
let extensions = Extensions::from_bits_retain(bits);
roundtrip_extensions(extensions);
}
}
}
4 changes: 4 additions & 0 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,10 @@ 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) {
return Err(Error::ExpectedStructName(ident.to_string()));
}

return Ok(false);
}

Expand Down
30 changes: 14 additions & 16 deletions src/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ impl PrettyConfig {

/// Configures whether to emit struct names.
///
/// See also [`Extensions::EXPLICIT_STRUCT_NAMES`] for the extension equivalent.
///
/// Default: `false`
#[must_use]
pub fn struct_names(mut self, struct_names: bool) -> Self {
Expand Down Expand Up @@ -413,20 +415,10 @@ impl<W: fmt::Write> Serializer<W> {

let non_default_extensions = !options.default_extensions;

if (non_default_extensions & conf.extensions).contains(Extensions::IMPLICIT_SOME) {
writer.write_str("#![enable(implicit_some)]")?;
writer.write_str(&conf.new_line)?;
};
if (non_default_extensions & conf.extensions).contains(Extensions::UNWRAP_NEWTYPES) {
writer.write_str("#![enable(unwrap_newtypes)]")?;
writer.write_str(&conf.new_line)?;
};
if (non_default_extensions & conf.extensions)
.contains(Extensions::UNWRAP_VARIANT_NEWTYPES)
{
writer.write_str("#![enable(unwrap_variant_newtypes)]")?;
for (extension_name, _) in (non_default_extensions & conf.extensions).iter_names() {
write!(writer, "#![enable({})]", extension_name.to_lowercase())?;
writer.write_str(&conf.new_line)?;
};
}
};
Ok(Serializer {
output: writer,
Expand Down Expand Up @@ -636,10 +628,16 @@ impl<W: fmt::Write> Serializer<W> {
Ok(())
}

/// Checks if struct names should be emitted
///
/// Note that when using the `explicit_struct_names` extension, this method will use an OR operation on the extension and the [`PrettyConfig::struct_names`] option. See also [`Extensions::EXPLICIT_STRUCT_NAMES`] for the extension equivalent.
fn struct_names(&self) -> bool {
self.pretty
.as_ref()
.map_or(false, |(pc, _)| pc.struct_names)
self.extensions()
.contains(Extensions::EXPLICIT_STRUCT_NAMES)
|| self
.pretty
.as_ref()
.map_or(false, |(pc, _)| pc.struct_names)
}
}

Expand Down
109 changes: 109 additions & 0 deletions tests/522_explicit_struct_names.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use ron::{
extensions::Extensions,
from_str,
ser::{to_string_pretty, PrettyConfig},
Error, Options,
};
use serde_derive::{Deserialize, Serialize};

#[derive(Debug, PartialEq, Serialize, Deserialize)]
struct Id(u32);

#[derive(Debug, PartialEq, Serialize, Deserialize)]
struct Position(f32, f32);

#[derive(Debug, PartialEq, Serialize, Deserialize)]
enum Query {
None,
Creature(Id),
Location(Position),
}

#[derive(Debug, PartialEq, Serialize, Deserialize)]
struct Foo {
#[allow(unused)]
pub id: Id,
#[allow(unused)]
pub position: Position,
#[allow(unused)]
pub query: Query,
}

const EXPECT_ERROR_MESSAGE: &'static str =
"expected `Err(Error::ExpectedStructName)`, deserializer returned `Ok`";

#[test]
fn explicit_struct_names() {
let options = Options::default().with_default_extension(Extensions::EXPLICIT_STRUCT_NAMES);
let foo_ser = Foo {
id: Id(3),
position: Position(0.0, 8.72),
query: Query::Creature(Id(4)),
};

// phase 1 (regular structs)
let content_regular = r#"(
id: Id(3),
position: Position(0.0, 8.72),
query: None,
)"#;
let foo = options.from_str::<Foo>(content_regular);
assert_eq!(
foo.expect_err(EXPECT_ERROR_MESSAGE).code,
Error::ExpectedStructName("Foo".to_string())
);

// phase 2 (newtype structs)
let content_newtype = r#"Foo(
id: (3),
position: Position(0.0, 8.72),
query: None,
)"#;
let foo = options.from_str::<Foo>(content_newtype);
assert_eq!(
foo.expect_err(EXPECT_ERROR_MESSAGE).code,
Error::ExpectedStructName("Id".to_string())
);

// phase 3 (tuple structs)
let content_tuple = r#"Foo(
id: Id(3),
position: (0.0, 8.72),
query: None,
)"#;
let foo = options.from_str::<Foo>(content_tuple);
assert_eq!(
foo.expect_err(EXPECT_ERROR_MESSAGE).code,
Error::ExpectedStructName("Position".to_string())
);

// phase 4 (test without this extension)
let _foo1 = from_str::<Foo>(content_regular).unwrap();
let _foo2 = from_str::<Foo>(content_newtype).unwrap();
let _foo3 = from_str::<Foo>(content_tuple).unwrap();

// phase 5 (test serialization)
let pretty_config = PrettyConfig::new()
.extensions(Extensions::EXPLICIT_STRUCT_NAMES | Extensions::UNWRAP_VARIANT_NEWTYPES);
let content = to_string_pretty(&foo_ser, pretty_config).unwrap();
assert_eq!(
content,
r#"#![enable(unwrap_variant_newtypes)]
#![enable(explicit_struct_names)]
Foo(
id: Id(3),
position: Position(0.0, 8.72),
query: Creature(4),
)"#
);
let foo_de = from_str::<Foo>(&content);
match foo_de {
// GRCOV_EXCL_START
Err(err) => panic!(
"failed to deserialize with `explicit_struct_names` and `unwrap_variant_newtypes`: {}",
err
),
// GRCOV_EXCL_STOP
Ok(foo_de) => assert_eq!(foo_de, foo_ser),
}
}
4 changes: 2 additions & 2 deletions tests/non_string_tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ macro_rules! test_non_tag {
}
}

#[derive(Debug)]
#[derive(Debug)] // GRCOV_EXCL_LINE
struct InternallyTagged;

impl<'de> Deserialize<'de> for InternallyTagged {
Expand Down Expand Up @@ -112,7 +112,7 @@ macro_rules! test_tag {
}
}

#[derive(Debug)]
#[derive(Debug)] // GRCOV_EXCL_LINE
struct InternallyTagged;

impl<'de> Deserialize<'de> for InternallyTagged {
Expand Down

0 comments on commit d509208

Please sign in to comment.