refactor: work-in-progress proposed refactor of composed schema restrictions #506
Conversation
This commit introduces the new spectral-style 'composed-schema-restrictions' rule, which will verify that oneOf and anyOf compositions comply with the restrictions imposed by the SDK generator. Signed-off-by: Phil Adams <[email protected]>
Signed-off-by: Phil Adams <[email protected]>
…ictions Signed-off-by: Dan Hudlow <[email protected]>
There was a problem hiding this comment.
Okay after re-reading, my only feedback is to add a little detail to one of the comments for overall readability.
The other feedback I had turned out to be irrelevant on second look.
I still think it would be helpful to 1) validate that all schemas in a composed schema are objects and 2) add some additional test cases
| } | ||
| // Only object schemas have properties | ||
| if (isObjectSchema(schema)) { | ||
| // Collects all composed property schemas indexed by the name of the property they define |
There was a problem hiding this comment.
Would be easier to read (for me, at least) if this comment elaborated slightly more on the data structure returned by getPropertySchemasByName. Something to let the reader know the map values are lists of schemas all defining properties of common names.
Actually, if we encounter a primitive schema that also happens to contain a oneOf/anyOf (perhaps to define different sets of contstraints), we don't really care what is contained in the oneOf/anyOf list because we'll end up just refactoring the oneOf/anyOf list away during pre-processing. |
Yes, I summarized around that nuance - thanks for clarifying! |
|
|
Currently one failing test and some missing tests for the new utility, but posting this PR for feedback on the approach.