serde rename with '@' as first letter lead to broken serialization

[apatrushev]

This code breaks with InvalidIdentifier("@sender") error:

use serde::{Deserialize, Serialize};


#[derive(Debug, Deserialize, Serialize)]
struct Event {
    #[serde(rename = "@sender")]
    sender: String,
}


fn main() {
    let event = Event { sender: "test".to_string() };
    println!("{}", ron::to_string(&event).unwrap())
}

Result:

$ cargo run
   Compiling ron-bug v0.1.0 ([stripped])
    Finished dev [unoptimized + debuginfo] target(s) in 0.52s
     Running `target/debug/ron-bug`
thread 'main' panicked at src/main.rs:13:43:
called `Result::unwrap()` on an `Err` value: InvalidIdentifier("@sender")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected result:

(r#@sender:"test")

Looks like it is a regression: everything worked as expected in 0.7.1 but stopped working in 0.8.0

[apatrushev]

Additional notes: deserialization did not work in 0.7.1 (and early versions) too.

[juntyr]

I'll start with my technical answer:

This breakage is expected. Before v0.8.0, RON's grammar (https://github.com/ron-rs/ron/blob/master/docs/grammar.md#identifier) did not allow @sender as an identifier (just like it's not a valid Rust identifier), it was just never checked. In v0.8.0 such checks were added since you could otherwise do funny stuffy with identifiers, e.g. pack RON inside them which then cannot roundtrip. Now RON checks identifier at serialisation and deserialisation to ensure only valid ones can be used.

How big of a problem is this more strict parsing? If the breakage is with a large userbase we could work around it (e.g. by adding an unchecked identifier feature which turns off the checks but leaves you open to breaking serialise-deserialise roundtrips), otherwise I'd rather stick to the checks since they stick to the grammar that existed already before.

[apatrushev]

Thank you for detailed answer. I just noticed this inconsistence because I used insta+ron (insta pins 0.7.1 and use it only as serializer) and it worked fine till I tried to deserialize the data to use in tests (not only in assertion/check). So I decided that it was a regression. I personally opted out from serde/rename to serde/alias in my application so it is not a biggest problem for me and hopefully this change can be used in most cases.

I think that it will be enough to mention serde/rename limitation in the list of other limitations.