oxidecomputer / typify

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

`merge_so_enum_values` problem #572

Closed SRetip closed 4 months ago

SRetip commented 4 months ago

Hello, ran into this problem: When trying to process a circuit like this:

{
  "id": "https://schemas.mysite.com/schemas/defs44.json#",
  "$schema": "http://json-schema.org/draft-04/schema#",
  "definitions": {
    "UploadedUserSourceInfo": {
      "type": "object",
      "oneOf": [
        {
          "$ref": "#/definitions/SourceUploadedUserSourceInfo"
        },
        {
          "$ref": "#/definitions/NetCoreZipUploadedUserSourceInfo"
        }
      ],
      "properties": {
        "relativePath": {
          "type": "string"
        },
        "type": {
          "type": "string",
          "enum": [
            "UploadedUserSourceInfo"
          ]
        }
      },
      "required": [
        "type"
      ]
    },
    "SourceUploadedUserSourceInfo": {
      "type": "object",
      "properties": {
        "artifactSelector": {
        "type": "string"
        },
        "runtimeVersion": {
          "type": "string"
        },
        "type": {
          "type": "string",
          "enum": [
            "Source"
          ]
        }
      },
      "required": [
        "type"
      ]
    },
    "NetCoreZipUploadedUserSourceInfo": {
      "type": "object",
      "properties": {
        "netCoreMainEntryPath": {
          "type": "string"
        },
        "runtimeVersion": {
          "type": "string"
        },
        "type": {
          "type": "string",
          "enum": [
            "NetCoreZip"
          ]
        }
      },
      "required": [
        "type"
      ]
    }
  }
}

I get the following error:

The application panicked (crashed).
Message:  called `Result::unwrap()` on an `Err` value: ()
Location: typify-impl\src\[merge.rs:274](http://merge.rs:274/)

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Having analyzed where it comes from, I realized that the main problem is in this part of the code: typify-impl\src\merge.rs line 255-258

 let values = aa
                .into_iter()
                .filter(|value| bb.contains(value))
                .collect::<Vec<_>>();

As a result of execution, we get that merged_subschemas.len() is equal to 0.

Is this logic anticipated, and would it be more appropriate to substitute it with a straightforward merging of two Vec? If you approve of this approach, I'm prepared to carry out the implementation.

ahl commented 4 months ago

It's clearly not good when typify crashes! In this case it goes through these transformations:

expanding the oneOf:

    {
      "type": "object",
      "oneOf": [
        {
          "allOf": [
            {
              "$ref": "#/definitions/SourceUploadedUserSourceInfo",
            },
            {
              "properties": {
                "relativePath": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                }
              },
              "required": [
                "type"
              ]
            }
          ]
        },
        {
          "allOf": [
            {
              "$ref": "#/definitions/NetCoreZipUploadedUserSourceInfo"
            },
            {
              "properties": {
                "relativePath": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                }
              },
              "required": [
                "type"
              ]
            }
          ]
        }
      ]
    }

then inlining the references:

    {
      "type": "object",
      "oneOf": [
        {
          "allOf": [
            {
              "type": "object",
              "properties": {
                "artifactSelector": {
                  "type": "string"
                },
                "runtimeVersion": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "Source"
                  ]
                }
              },
              "required": [
                "type"
              ]
            },
            {
              "properties": {
                "relativePath": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                }
              },
              "required": [
                "type"
              ]
            }
          ]
        },
        {
          "allOf": [
            {
              "type": "object",
              "properties": {
                "netCoreMainEntryPath": {
                  "type": "string"
                },
                "runtimeVersion": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "NetCoreZip"
                  ]
                }
              },
              "required": [
                "type"
              ]
            },
            {
              "properties": {
                "relativePath": {
                  "type": "string"
                },
                "type": {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                }
              },
              "required": [
                "type"
              ]
            }
          ]
        }
      ]
    }

then resolving those inner allOfs:

    {
      "type": "object",
      "oneOf": [
        {
          "type": "object",
          "properties": {
            "artifactSelector": {
              "type": "string"
            },
            "runtimeVersion": {
              "type": "string"
            },
            "relativePath": {
              "type": "string"
            },
            "type": {
              "allOf": [
                {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                },
                {
                  "type": "string",
                  "enum": [
                    "Source"
                  ]
                }
              ]
            }
          },
          "required": [
            "type"
          ]
        },
        {
          "type": "object",
          "properties": {
            "netCoreMainEntryPath": {
              "type": "string"
            },
            "runtimeVersion": {
              "type": "string"
            },
            "relativePath": {
              "type": "string"
            },
            "type": {
              "allOf": [
                {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                },
                {
                  "type": "string",
                  "enum": [
                    "NetCoreZip"
                  ]
                }
              ]
            }
          },
          "required": [
            "type"
          ]
        }
      ]
    }

In particular, we see these unresolvable allOfs:

            "type": {
              "allOf": [
                {
                  "type": "string",
                  "enum": [
                    "UploadedUserSourceInfo"
                  ]
                },
                {
                  "type": "string",
                  "enum": [
                    "Source"
                  ]
                }
              ]
            }

So we get:

    {
      "type": "object",
      "oneOf": [
        {
          "type": "object",
          "properties": {
            "artifactSelector": {
              "type": "string"
            },
            "runtimeVersion": {
              "type": "string"
            },
            "relativePath": {
              "type": "string"
            },
            "type": false
          },
          "required": [
            "type"
          ]
        },
        {
          "type": "object",
          "properties": {
            "netCoreMainEntryPath": {
              "type": "string"
            },
            "runtimeVersion": {
              "type": "string"
            },
            "relativePath": {
              "type": "string"
            },
            "type": false
          },
          "required": [
            "type"
          ]
        }
      ]
    }

Since type is both false (i.e. unsatisfiable) and in the required list, the schema becomes simply:

    {
      "oneOf": [
        false,
        false
      ]
    }

The best we could generate for the UploadedUserSourceInfo type is--I think--an "impossible" type like:

pub enum UploadedUserSourceInfo {}