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

Fail to work with typetag during serializing #636

Open
n0b0dyCN opened this issue Oct 25, 2023 · 7 comments
Open

Fail to work with typetag during serializing #636

n0b0dyCN opened this issue Oct 25, 2023 · 7 comments
Labels
A-serde Area: Serde integration C-bug Category: Things not working as expected

Comments

@n0b0dyCN
Copy link

n0b0dyCN commented Oct 25, 2023

toml crate fail to work with typetag with vector of trait object during serializing, but serde_json works:

    use serde::{Serialize, Deserialize};

    #[typetag::serde(tag = "type")]
    trait WebEvent {
        fn inspect(&self);
    }
    
    #[derive(Serialize, Deserialize)]
    struct PageLoad(usize);
    
    #[derive(Serialize, Deserialize, Clone)]
    struct Click {
        x: i32,
        y: i32,
    }

    #[typetag::serde]
    impl WebEvent for PageLoad {
        fn inspect(&self) {
            println!("200 milliseconds or bust");
        }
    }

    #[typetag::serde]
    impl WebEvent for Click {
        fn inspect(&self) {
            println!("negative space between the ads: x={} y={}", self.x, self.y);
        }
    }

    let v = Click{ x: 1, y: 1 };
    let dynv = Box::new(v.clone()) as Box::<dyn WebEvent>;
    println!("{}", toml::to_string(&v).unwrap());
    println!("{}", toml::to_string(&dynv).unwrap());
    println!("{}", serde_json::to_string(&vec![dynv]).unwrap()); // This works
    println!("{}", toml::to_string(&vec![dynv]).unwrap()); // This does not work

JSON output:

[{"type":"Click","x":1,"y":1}]

toml Error message:

thread '' panicked at 'called `Result::unwrap()` on an `Err` value: Error { inner: UnsupportedType(None) }'

versioning:

toml = "0.8.2"
typetag = "0.2"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"

@n0b0dyCN
Copy link
Author

n0b0dyCN commented Oct 26, 2023

Also tested on toml-rs v0.20.4, still got panic.

I do dome debug and found that the value in this function is a toml_edit::Value::Array, but it should be something like a toml_edit::Value::InlineTable

@epage
Copy link
Member

epage commented Oct 27, 2023

The problem is likely that TOML doesn't support arrays as top-level items, only tables.

@n0b0dyCN
Copy link
Author

Sorry, I do not implement the minimal reproduceable case correctly. I actually find the panic in the following case, where Option<Box> is set to None:

use std::fmt::Debug;

#[typetag::serde(tag = "type")]
pub trait Value: Debug {
    fn value(&self) -> String;
}


#[typetag::serde(name = "i32")]
impl Value for i32 {
    fn value(&self) -> String {
        "i32".to_string()
    }
}

#[derive(Debug, serde::Deserialize, serde::Serialize)]
pub struct MaybeValue {
    #[serde(default)]
    pub v: Option<Box<dyn Value>>
}

#[typetag::serde(name = "maybe_value")]
impl Value for MaybeValue {
    fn value(&self) -> String {
        "maybe_value".to_string()
    }
}


#[typetag::serde(tag = "operation")]
pub trait Operation: Debug {
    fn operation(&self) -> String;
}


#[derive(Debug, serde::Deserialize, serde::Serialize)]
pub struct Operation1 {
    pub v: Vec<Box<dyn Value>>
}

#[typetag::serde(name = "operation1")]
impl Operation for Operation1 {
    fn operation(&self) -> String {
        "operation1".to_string()
    }
}

fn main() {
    let txt = r#"
    [[v]]
    type = "maybe_value"
    "#;
    let op: Operation1 = toml::from_str(txt).unwrap();
    println!("{:?}", op); // Operation1 { v: [MaybeValue { v: None }] }

    let s = serde_json::to_string(&op).unwrap();
    println!("{}", s); // {"v":[{"type":"maybe_value","v":null}]}

    // fails
    let s = toml::to_string(&op).unwrap();
    println!("{}", s);
}

@n0b0dyCN
Copy link
Author

temporarily bypassed the issue with #[serde(skip_serializing_if = "Option::is_none")] notation on Option field

@epage
Copy link
Member

epage commented Oct 30, 2023

This looks to be similar to #603, dealing with corner cases around lack of support for None.

@epage
Copy link
Member

epage commented Oct 30, 2023

Further reduced the test case and contrasted it with internal tags

Note: these tests are "passing", showing the current behavior

#[test]
fn optional_field_with_typetag() {
    #[derive(Debug, serde::Deserialize, serde::Serialize)]
    pub struct MaybeValue {
        #[serde(default)]
        pub v: Option<i32>,
    }

    #[typetag::serde(tag = "type")]
    pub trait Value: std::fmt::Debug {
        fn value(&self) -> String;
    }

    #[typetag::serde(name = "maybe_value")]
    impl Value for MaybeValue {
        fn value(&self) -> String {
            "maybe_value".to_string()
        }
    }

    #[derive(Debug, serde::Deserialize, serde::Serialize)]
    pub struct Operation1 {
        pub v: Vec<Box<dyn Value>>,
    }

    let op = Operation1 {
        v: vec![Box::new(MaybeValue { v: None })],
    };

    let txt = r#"[[v]]
type = "maybe_value"
"#;

    let s = toml::to_string(&op);
    assert!(s.is_err());

    let op: Operation1 = toml::from_str(txt).unwrap();
    assert_eq!(op.v.len(), 1);
}

#[test]
fn optional_field_with_internal_tag() {
    #[derive(Debug, serde::Deserialize, serde::Serialize)]
    pub struct MaybeValue {
        #[serde(default)]
        pub v: Option<i32>,
    }

    #[derive(Debug, serde::Deserialize, serde::Serialize)]
    #[serde(tag = "type")]
    #[serde(rename_all = "snake_case")]
    pub enum Value {
        MaybeValue(MaybeValue),
    }

    #[derive(Debug, serde::Deserialize, serde::Serialize)]
    pub struct Operation1 {
        pub v: Vec<Value>,
    }

    let op = Operation1 {
        v: vec![Value::MaybeValue(MaybeValue { v: None })],
    };

    let txt = r#"[[v]]
type = "maybe_value"
"#;

    let s = toml::to_string(&op);
    let s = s.unwrap_or_else(|e| panic!("{e}"));
    assert_eq!(s, txt);

    let op: Operation1 = toml::from_str(txt).unwrap();
    assert_eq!(op.v.len(), 1);
}

@epage
Copy link
Member

epage commented Oct 30, 2023

It looks like typetag is taking our error and then calling Error::custom on it, making it so we can't programmatically act on it anymore to identify when we should take a None and return an empty map/struct.

---- serde::optional_field_with_internal_tag stdout ----
[crates/toml_edit/src/ser/map.rs:490] 1 = 1
[crates/toml_edit/src/ser/map.rs:226] &e = UnsupportedNone


successes:
    serde::optional_field_with_internal_tag

failures:

---- serde::optional_field_with_typetag stdout ----
[crates/toml_edit/src/ser/map.rs:490] 1 = 1
[crates/toml_edit/src/ser/map.rs:189] &e = Custom(
    "unsupported None value",
)
[crates/toml_edit/src/ser/map.rs:226] &e = Custom(
    "unsupported None value",
)
thread 'serde::optional_field_with_typetag' panicked at crates/toml/tests/testsuite/serde.rs:1384:34:
unsupported None value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@epage epage added C-bug Category: Things not working as expected A-serde Area: Serde integration labels Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-serde Area: Serde integration C-bug Category: Things not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants