-
Notifications
You must be signed in to change notification settings - Fork 115
Implement union type validation for static language support #5603
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
base: main
Are you sure you want to change the base?
Conversation
|
Following you can find the validation changes against the target branch for the APIs. No changes detected. You can validate these APIs yourself by using the |
| // eslint-disable-next-line es-spec-validator/no-inline-unions -- TODO: create named alias | ||
| number_of_shards?: integer | string // TODO: should be only int |
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.
For the record, this a known issue with index settings that they can be passed as strings too.
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 should actually be Stringyfied I think.
|
Let's backport all the nice validations to 8.19 LTS branch as well, if they don't have incompatible changes 🙂 |
| // eslint-disable-next-line es-spec-validator/no-inline-unions, es-spec-validator/prefer-tagged-variants -- TODO: use tagged variant | ||
| global?: GlobalPrivilege[] | GlobalPrivilege |
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 is valid. We have two options here:
- Change this to
GlobalPrivilege | GlobalPrivilege[] - Change the logic do detect this case
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.
Vote for 2.! The validation should not care about the order.
| * Either a string or an array of strings. | ||
| */ | ||
| // eslint-disable-next-line es-spec-validator/no-inline-unions -- TODO: create named alias | ||
| input: string | Array<string> |
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.
Note that string | Array<string> is the same as string | string[]. I've pushed #5603 (6e8c9affd) to fix it.
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.
The other validation about using native types should catch and prevent this case in the future, I guess.
None of the validation work exists in 8.19, so this is out of scope. |
Closes #4737
Implements two new validation rules to improve code generation for statically-typed languages:
no-inline-unions- Forbids inline union types in propertiesprefer-tagged-variants- Enforces tagged variants for unions of class typesTotal existing violations: 91
no-inline-unions: 37prefer-tagged-variants: 54The rules automatically skips the following patterns:
Type | null/Type | undefinedType | Type[]patterns