autorest
autorest copied to clipboard
Autorest must reject allOf that contain overlapping properties with different types
See the below examples. In both cases, the version
property is duplicated with different types. From discussion with @johanste
What you describe below is valid swagger, but very unlikely what the author intended. It effectively says that there cannot be a “version” key in the response (the value of any “version” attribute has to be both an integer and a string at the same time, which is impossible). If the authors expectation was that version could be a string or an integer, then the swagger is incorrect, and it is a prime example of why we introduced x-ms-examples (the author would have provided what they think is a valid payload, and our tooling would have said “I’m sorry, human, what you are trying to do is wrong. Your understanding of swagger is incomplete.”).
Example 1
"ExtendedProductProperties": {
"description": "Product information.",
"type": "object",
"properties": {},
"allOf": [
{
"$ref": "#/definitions/VirtualMachineExtensionProductProperties"
},
{
"$ref": "#/definitions/VirtualMachineProductProperties"
}
]
},
"VirtualMachineExtensionProductProperties": {
"description": "Product information.",
"type": "object",
"properties": {
"version": {
"description": "Specifies product version.",
"type": "integer",
"format": "int32",
"readOnly": true
}
}
},
"VirtualMachineProductProperties": {
"description": "Product information.",
"type": "object",
"properties": {
"version": {
"description": "Specifies product version.",
"type": "string",
"readOnly": true
}
}
},
Example 2
"ExtendedProductProperties": {
"description": "Product information.",
"type": "object",
"properties": {
"version": {
"description": "Specifies product version.",
"type": "integer",
"format": "int32",
"readOnly": true
}
},
"allOf": [
{
"$ref": "#/definitions/VirtualMachineProductProperties"
}
]
},
"VirtualMachineProductProperties": {
"description": "Product information.",
"type": "object",
"properties": {
"version": {
"description": "Specifies product version.",
"type": "string",
"readOnly": true
}
}
},
So this is a problem that has come before and we decided to keep autorest more on the flexible side. The problem is how do we decide what is compatible and what isn't.
- string and number are not
- raw string schema and a custom string schema(as an enum or with some validation) could be
- a int and a long are compatible(one is more restrictive so it takes over)
This basically involve creating a full subtype, parent type relation ship and compiler in autorest. This is something we'll be doing in Cadl anyway and I think might have that validation be left for the cadl compiler instead of duplicating the work. If that logic can easily be ported over from cadl then I think it could make sense to have it built-in.
Are we ever silently generating broken code as a result of lack of validation here?
By silent do you mean the SDK generate fine and compile fine(if applicable) but get published and is not usable? If so I don't think that would happen, the main issue is it would fail at compile time instead of generation.
Not even for a format change from vanilla string to date time?
While I agree that this would likely get caught when trying to compile an SDK, my concern with this approach is that we're finding this late in the process and the code generators all need to handle this in the same way. Would there always be uniformity in handling (i.e. rejecting) between statically and dynamically typed languages?