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.
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.
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.
changing TemplateTagValueNode would be a BC break though, and any other solution would require not reusing what already exists
ondrejmirtes
left a comment
There was a problem hiding this comment.
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.