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

Fix roundtrip of flatten externally tagged enum unit variant #2786

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 3, 2024

Unit variant of externally tagged enum can be deserialized when enum is flatten, but cannot be serialized -- ser test failed:

#![cfg(test)]

use serde::{Deserialize, Serialize}; // 1.0.204;
use serde_json::{from_str, to_string}; // 1.0.121

#[derive(Debug, PartialEq, Serialize, Deserialize)]
struct Flatten {
    #[serde(flatten)]
    data: Enum,
}

#[derive(Debug, PartialEq, Serialize, Deserialize)]
enum Enum {
    Unit,
}

const JSON: &str = r#"{"Unit":null}"#;

#[test]
fn de() {
    // ok
    assert_eq!(from_str::<Flatten>(JSON).unwrap(), Flatten { data: Enum::Unit });
}

#[test]
fn ser() {
    // called `Result::unwrap()` on an `Err` value: Error("can only flatten structs and maps (got an enum)", line: 0, column: 0)
    assert_eq!(to_string(&Flatten { data: Enum::Unit }).unwrap(), JSON);
}

This PR fixes this inconsistency

@Mingun Mingun force-pushed the serialize-flatten-unit-variant branch from 87cf403 to 7fc1636 Compare August 3, 2024 12:04
@Mingun
Copy link
Contributor Author

Mingun commented Aug 4, 2024

This PR does not conflicts with #2608, because changed files not moved in #2608, it has pretty obvious test case and reproduction example, so is it seems it shouldn't take too much time for review.

- deserialization of flatten unit variant is possible
- serialization of such variant gives Err("can only flatten structs and maps (got an enum)")
@Mingun Mingun force-pushed the serialize-flatten-unit-variant branch from 7fc1636 to c17b139 Compare October 25, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant