-
Notifications
You must be signed in to change notification settings - Fork 230
Allow support for JSON arrays within annotations #122
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
Conversation
Ocramius
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.
Code looks good, but we need to improve the testing of nested values, optional additional values.
This should be done both in the parser tests, and in the annotation reader afterwards, in my opinion.
@guilhermeblanco any opinion on supporting this?
| array('array', '{@AnnotationExtendsAnnotationTargetAll}'), | ||
| array('array', '{@AnnotationExtendsAnnotationTargetAll,@AnnotationExtendsAnnotationTargetAll}'), | ||
| array('array', '[@AnnotationExtendsAnnotationTargetAll]'), | ||
| array('array', '[@AnnotationExtendsAnnotationTargetAll,@AnnotationExtendsAnnotationTargetAll]'), |
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.
Would need a few tests around parsing array elements, their order, and different types of sub-elements
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.
Also nested arrays
| const T_TRUE = 110; | ||
| const T_NULL = 111; | ||
| const T_COLON = 112; | ||
| const T_OPEN_SQR_BRACKETS = 113; |
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.
SQUARE
| const T_NULL = 111; | ||
| const T_COLON = 112; | ||
| const T_OPEN_SQR_BRACKETS = 113; | ||
| const T_CLOSE_SQR_BRACKETS = 114; |
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.
SQUARE
|
This is something that should be relatively easy with the new parser in Annotations 2.0. Milestoning, since there is no relevant issue, but it will require different implementation. |
As said, added square brackets to complete support for JSON.