Add initial oneOf schema code gen support #27
Conversation
This uses shrubbery untagged unions to implement oneOf code gen.
This adds better support for handling $refs inside the components/schemas
section of the spec, allowing for parse errors on missing ref names.
- This converts additionalProperties refs to the new approach used by
oneOf, since previously a missing ref name in additionalProperties
under components/schemas would be a compile instead of parse error
- This does not fix nullable for inline arrays under the operations
section (or implement them correctly for inline oneOf), though we
could as part of this pr if deemed important enough
- This does not implement openapi discriminators, which would
use tagged unions
- This does not implement inline objects as choices for `oneOf`
- If we run into `anyOf` being used, it would probably be simple to
have it just call out to the `oneOf` code (though hopefully no one
ever uses `anyOf`...)
Co-Authored-By: Tyler <[email protected]>
Co-Authored-By: Janus <[email protected]>
| Nothing -> | ||
| pure [] | ||
| Just additionalPropsTypeInfo -> do | ||
| pure |
There was a problem hiding this comment.
Either we should do the mapM above here, or change this case back to a let and remove the extraneous pure calls that are inside it.
|
|
||
| taggedUnionTypeList :: [TypeExpression] -> TypeExpression | ||
| taggedUnionTypeList = | ||
| prefixedTypeList (Just $ \member -> quote member <> " @= ") |
There was a problem hiding this comment.
We're not using tagged unions anywhere in the codegen yet, right? I'm surprised this is here. We should remove it until we actually implement something that uses it so we don't have to maintain it in the meantime.
| import qualified TestCases.Types.MixedInJustAdditionalPropertiesSchemaInline as MixedInJustAdditionalPropertiesSchemaInline | ||
| import qualified TestCases.Types.Num2SchemaStartingWithNumber as Num2SchemaStartingWithNumber | ||
|
|
||
| newtype TopLevelOneOf = TopLevelOneOf (Shrubbery.Union '[T.Text, Integer, [T.Text], AStringType.AStringType, Num2SchemaStartingWithNumber.Num2SchemaStartingWithNumber, [FieldDescriptions.FieldDescriptions], [[MixedInJustAdditionalPropertiesSchemaInline.MixedInJustAdditionalPropertiesSchemaInline]]]) |
There was a problem hiding this comment.
I think we should put newlines in the type list in the generate code to make it more readable
| @@ -241,24 +242,24 @@ lookupRequestBody operationKey operation = | |||
| pure Nothing | |||
|
|
|||
| data SchemaTypeInfoWithDeps = SchemaTypeInfoWithDeps | |||
| { schemaTypeInfoDependent :: CGU.SchemaTypeInfo | |||
| { schemaTypeInfoDependent :: CGU.SchemaTypeInfoOrRef | |||
There was a problem hiding this comment.
This seem suspicious to me... the type SchemaTypeInfoWithDeps is supposed to be exactly that -- some type info along with some dependencies. It seems weird that we would end up with a ref in there.
There was a problem hiding this comment.
I agree there could probably be some further simplification in the abstractions here around SchemaTypeInfoWithDeps (actually SchemaTypeInfoOrRef was one initial attempt in that direction). The tricky thing is we have refs which need to be resolved later (at code generation time, once we have all the top level names, since refs under components could refer to other refs in components), but we also have a lot of code (especially the operations section) that directly produces SchemaTypeInfo (ex: lookupRequestBodySchema). I think it might be feasible to move all the SchemaTypeInfo resolution into generateCodeGenDataFormat by expanding resolveRefTypeInfo to work on more than just refs for instance, but this would be a pretty significant change that I'd want to discuss through first. I also think it may be reasonable to wait and do this refactoring at a later stage, as implementing some of the unsupported features (like nested inline objects) might change how we approach the abstractions.
| case OA._schemaItems schema of | ||
| Just (OA.OpenApiItemsObject (OA.Ref ref)) -> do | ||
| pure $ | ||
| -- We don't currently have a good way to get the nullability of the referenced schema |
There was a problem hiding this comment.
This is probably an issues we could fix because it leaves things in state that will generate code that doesn't match the OpenAPI description in a very subtle and non-obvious way.
There was a problem hiding this comment.
I added several tests and support for inline nullability, there are still some lingering edge cases (with other features we haven't fully covered yet like inline nested objects, or nested oneOf), but stuff like nullable: true with a $ref sibling doesn't even appear to be valid according to the spec, which I didn't realize at first.
This uses shrubbery untagged unions to implement oneOf code gen. This adds better support for handling $refs inside the components/schemas section of the spec, allowing for parse errors on missing ref names.
oneOfanyOfbeing used, it would probably be simple to have it just call out to theoneOfcode (though hopefully no one ever usesanyOf...)