oxidecomputer / typify

compiler from JSON Schema into idiomatic Rust types
Apache License 2.0
393 stars 57 forks source link

Panic due to missing/incorrect metadata after subschema merge #573

Closed miwig closed 4 months ago

miwig commented 4 months ago

Processing this schema:

{
    "title": "My root schema",
    "properties": {
        "foo": {
            "type": "integer"
        }
    },
    "allOf": [
        {
            "title": "My subschema",
            "properties": {
                "bar": {
                    "type": "integer"
                }
            }
        }
    ]
}

will panic with

The application panicked (crashed).
Message:  called `Option::unwrap()` on a `None` value
Location: typify-impl/src/type_entry.rs:289

due to metadata getting lost somewhere in the // Subschemas with other stuff. branch of TypeSpace::convert_schema_object.

Reducing the schema to:

{
    "title": "My root schema",
    "allOf": [
        {
            "title": "My subschema",
            "properties": {
                "bar": {
                    "type": "integer"
                }
            }
        }
    ]
}

will go into the convert_all_of branch and (in my opinion incorrectly) generate:

pub struct MySubschema {
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub bar: Option<i64>,
}

anyOf also shows the same behaviour.

For an example of such a schema in the wild, see (after resolving references):

ahl commented 4 months ago

will go into the convert_all_of branch and (in my opinion incorrectly) generate:

pub struct MySubschema {
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub bar: Option<i64>,
}

Why do you feel this would be incorrect? What type would you expect or what type would more faithfully model the schema?

miwig commented 4 months ago

will go into the convert_all_of branch and (in my opinion incorrectly) generate:

pub struct MySubschema {
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub bar: Option<i64>,
}

Why do you feel this would be incorrect? What type would you expect or what type would more faithfully model the schema?

The outer metadata should be preserved, so the struct name should be MyRootSchema. I think #575 did not fix this case. However I can confirm that the panic in the other case has been fixed. :tada: Thanks for the swift response :+1: