-
-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use enum values as keys for map #231
base: master
Are you sure you want to change the base?
Conversation
@GREsau I think I've got this working, I'd appreciate a review! |
for value in values { | ||
// enum values all have quotes around them, so remove them | ||
let str_value = &value.to_string(); | ||
let value = format!("{}", &str_value[1..str_value.len()-1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely feels a bit hacky, I'm open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, but unfortunately I don't think the schemas created by this change are correct.
In your example:
- according to the schema,
key
must be either an object with anOption1
property, or an object with anOption2
property (but not both). This means the schema would reject empty maps, or a map with bothOption1
andOption2
. - it probably shouldn't add
Thing
todefinitions
since it's not actually referenced anywhere in the schema. - according to the schema, the property names include double quotes (e.g.
"Option1"
instead ofOption1
), which is not correct
I think the correct schema would be something like:
{
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "EnumKeyStruct",
"type": "object",
"required": [
"key"
],
"properties": {
"key": {
"type": "object",
"properties": {
"Option1": {
"$ref": "#/definitions/SomeOtherStruct"
},
"Option2": {
"$ref": "#/definitions/SomeOtherStruct"
}
}
}
},
"definitions": {
"SomeOtherStruct": {
/* ... */
}
}
}
This PR fixes GREsau/okapi#128.
When generating a schema for a
Map
with a key that is anenum
, the generated schema only allows keys based off of theenum
options. For example,yields:
This also maintains the original behavior for maps with non-enum keys:
BTreeMap<String, String>
Code
Output
BTreeMap<SomeStruct, String>
Code
Output