Marwes / schemafy

Crate for generating rust types from a json schema
MIT License
242 stars 51 forks source link

Type is Option<serde_json::Value> instead of Option<String> #10

Open waywardmonkeys opened 6 years ago

waywardmonkeys commented 6 years ago

In the Vega Lite schema, there is this property on an Axis:

        "title": {
          "description": "A title for the field. If `null`, the title will be removed.\n\n__Default value:__  derived from the field's name and transformation function (`aggregate`, `bin` and `timeUnit`).  If the field has an aggregate function, the function is displayed as a part of the title (e.g., `\"Sum of Profit\"`). If the field is binned or has a time unit applied, the applied function will be denoted in parentheses (e.g., `\"Profit (binned)\"`, `\"Transaction Date (year-month)\"`).  Otherwise, the title is simply the field name.\n\n__Note__: You can customize the default field title format by providing the [`fieldTitle` property in the [config](config.html) or [`fieldTitle` function via the `compile` function's options](compile.html#field-title).",
          "type": [
            "string",
            "null"
          ]
        },

This is coming out in the generated Rust as:

    /// A title for the field. If `null`, the title will be removed.
    ///
    /// __Default value:__  derived from the field's name and transformation function (`aggregate`,
    /// `bin` and `timeUnit`).  If the field has an aggregate function, the function is displayed
    /// as a part of the title (e.g., `"Sum of Profit"`). If the field is binned or has a time unit
    /// applied, the applied function will be denoted in parentheses (e.g., `"Profit (binned)"`,
    /// `"Transaction Date (year-month)"`).  Otherwise, the title is simply the field name.
    ///
    /// __Note__: You can customize the default field title format by providing the [`fieldTitle`
    /// property in the [config](config.html) or [`fieldTitle` function via the `compile`
    /// function's options](compile.html#field-title).
    pub title: Option<serde_json::Value>,

I really think that should be Option<String> though?

waywardmonkeys commented 6 years ago

Perhaps related to this is this:

        "legend": {
          "anyOf": [
            {
              "$ref": "#/definitions/Legend"
            },
            {
              "type": "null"
            }
          ],
          "description": "An object defining properties of the legend.\nIf `null`, the legend for the encoding channel will be removed.\n\n__Default value:__ If undefined, default [legend properties](legend.html) are applied."
        },

Which gets translated to:

pub legend: Option<serde_json::Value>,

But I think that should be Option<Legend>.

Marwes commented 6 years ago

There is a bit of code that handles anyOf where there can either be just an element of some type or an array of that type

https://github.com/Marwes/schemafy/blob/7871e2152e0f827ce8c0f6901fb1220eb5b9f35d/src/lib.rs#L307-L324

or it falls back to emitting serde_json::Value

You would want to extend that bit to check for "type": "null" + some other type and then emit an Option<some other type>

waywardmonkeys commented 6 years ago

Thanks! I started looking at this this evening. I will poke at it more and submit a PR this week.

waywardmonkeys commented 6 years ago

FYI: as we've sort of discussed elsewhere, QuickType handles all of this pretty well.