- 
                Notifications
    You must be signed in to change notification settings 
- Fork 66
Parse generic callable #232
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
Parse generic callable #232
Conversation
7ce9640    to
    6894d61      
    Compare
  
    9e8d2b7    to
    cda3c21      
    Compare
  
    | new IdentifierTypeNode('Z'), | ||
| ])), | ||
| new CallableTypeTemplateNode(new IdentifierTypeNode('Ty'), new IdentifierTypeNode('Y')), | ||
| ]), ''), | 
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.
https://github.com/phpstan/phpdoc-parser/blob/1.25.0/src/Ast/PhpDoc/TemplateTagValueNode.php#L26 description should be made optional
| new IdentifierTypeNode('C'), | ||
| [ | ||
| new CallableTypeTemplateNode(new IdentifierTypeNode('A'), null), | ||
| new TemplateTagValueNode('A', null, ''), | 
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.
I would prefer to change https://github.com/phpstan/phpdoc-parser/blob/1.25.0/src/Ast/PhpDoc/TemplateTagValueNode.php#L15 first to keep IdentifierTypeNode instead of string to allow to store enriched attributes like position in the source phpdoc.
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.
changing TemplateTagValueNode would be a BC break though, and any other solution would require not reusing what already exists
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 looks really solid! 👍
cda3c21    to
    de6eace      
    Compare
  
    | Perfect, thank you! | 
Follow on from: #199 originally by @mvorisek
MakingTemplateTagParser::parseTemplateTagValue()static seemed like the best approach to fix a circular dependency betweenTypeParserandTemplateTagParser, as it means theTypeParsermust be passed for each call.Also
parseOptionalDescriptionwas awkward to handle as it only needs to be used inPhpDocParserand calls many private functions ofPhpDocParser, so i reworked that to be a callable instead of a boolean flag.