Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust bug: number keys are not properly handled #5

Closed
ahdinosaur opened this issue May 13, 2023 · 0 comments · Fixed by #7
Closed

Rust bug: number keys are not properly handled #5

ahdinosaur opened this issue May 13, 2023 · 0 comments · Fixed by #7

Comments

@ahdinosaur
Copy link
Owner

ahdinosaur commented May 13, 2023

failing test:

macro_rules! treemap {
    () => {
        BTreeMap::new()
    };
    ($($k:expr => $v:expr),+) => {
        {
            let mut m = BTreeMap::new();
            $(
                m.insert($k, $v);
            )+
            m
        }
    };
}

#[test]
fn test_object_with_wacky_keys() {
    #[derive(PartialEq, Eq, PartialOrd, Ord, serde_derive::Serialize)]
    #[serde(untagged)]
    enum Key<'a> {
        Str(&'a str),
        Num(u32),
    }
    let expected = r#"{"\n":"Newline","1":"One","2":"Two","3":"Three","4":"Four"}"#;
    let input = treemap![
        Key::Num(2) => "Two",
        Key::Str("4") => "Four",
        Key::Str("1") => "One",
        Key::Num(3) => "Three",
        Key::Str("\u{000a}") => "Newline"
    ];
    let actual = to_string(&input).unwrap();
    assert_eq!(actual, expected);
}
failures:

---- test_object_with_wacky_keys stdout ----
thread 'test_object_with_wacky_keys' panicked at 'assertion failed: `(left == right)`
  left: `"{\"2\":\"Two\",\"3\":\"Three\",\"\\n\":\"Newline\",\"1\":\"One\",\"4\":\"Four\"}"`,
 right: `"{\"\\n\":\"Newline\",\"1\":\"One\",\"2\":\"Two\",\"3\":\"Three\",\"4\":\"Four\"}"`', 

i'll admit i didn't realize numbers could be keys, but that's because i keep thinking that the Rust is deserialized to serde_json::Value (where keys can only be strings) before being serialized, which is just not the case.

ahdinosaur added a commit that referenced this issue May 13, 2023
- handle number keys
  - closes #5
- handle numbers greater than JavaScript's Number.MAX_SAFE_INTEGER
  - closes #6
- add more unit tests
- add more code docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant