-
Notifications
You must be signed in to change notification settings - Fork 319
Fix propertyNames keyword auto-completion
#1162
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
Fix propertyNames keyword auto-completion
#1162
Conversation
|
In this case the property name can only be The property name cannot be anything, since it must be both "One" and "Two" and that's impossible. |
|
@datho7561 Thanks for catching this, David. I tested them individually and simply I combined them in the integration test without noticing the issue. I’ll get it fixed! |
Signed-off-by: Morgan Chang <[email protected]>
Signed-off-by: Morgan Chang <[email protected]>
dd2c8dd to
32079b6
Compare
|
@datho7561 The {
"$schema": "https://json-schema.org/draft-07/schema#",
"properties": {
"propertyNamesConst": {
"type": "object",
"propertyNames": {
"const": "Event1"
},
"additionalProperties": {
"type": "integer"
}
},
"propertyNamesEnum": {
"type": "object",
"propertyNames": {
"enum": [
"Event1",
"Event2",
"Event3"
]
},
"additionalProperties": {
"type": "integer"
}
},
"propertyNamesOneOf": {
"type": "object",
"propertyNames": {
"oneOf": [
{ "const": "Event1" },
{ "const": "Event2" }
]
},
"additionalProperties": {
"type": "integer"
}
},
"propertyNamesAnyOf": {
"type": "object",
"propertyNames": {
"anyOf": [
{ "const": "Event1" },
{ "enum": ["Event2", "Event3"] }
]
},
"additionalProperties": {
"type": "integer"
}
},
"propertyNamesAllOf": {
"type": "object",
"propertyNames": {
"allOf": [
{ "const": "One" },
{ "enum": ["One", "Two"] }
]
},
"additionalProperties": {
"type": "integer"
}
},
"propertyNamesAllOfImpossible": {
"type": "object",
"propertyNames": {
"allOf": [
{ "const": "One" },
{ "const": "Two" }
]
},
"additionalProperties": {
"type": "integer"
}
}
},
"required": [
"propertyNamesConst",
"propertyNamesEnum",
"propertyNamesOneOf",
"propertyNamesAnyOf",
"propertyNamesAllOf",
"propertyNamesAllOfImpossible"
],
"title": "FooContainer",
"type": "object"
}Expected completion behavior: propertyNamesConst:
| # auto-complete gets option Event1
propertyNamesEnum:
| # auto-complete gets options Event1, Event2, Event3
propertyNamesOneOf:
| # auto-complete gets options Event1, Event2
propertyNamesAnyOf:
| # auto-complete gets options Event1, Event2, Event3
propertyNamesAllOf:
| # auto-complete gets option One only (your case 1)
propertyNamesAllOfImpossible:
| # impossible case, no suggestion from auto-completion (your case 2)Also updated the automated test cases as well. |
datho7561
left a comment
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.
Looks good and works well. Thanks, Morgan!
What does this PR do?
Implementation for
propertyNameskeyword auto-completion was only providing the title of thepropertyNamesschema as completion and did not correctly handle schemas that used enum, const, oneOf, anyOf, and allOf, and definition via$ref.This PR improves
propertyNameskeyword auto-completion to support definition via$refand enum/const/oneOf/anyOf/allOf.What issues does this PR fix or reference?
Fixes #1141
Is it tested? How?
Manual test: test with the schema provided in Support for propertyNames schema is incorrect #1141.
Automation test: added tests for
propertyNameswith$refto definitions andpropertyNamesusing enum/const/oneOf/ anyOf/allOf