Skip to content

Conversation

@mad-briller
Copy link
Contributor

@mad-briller mad-briller commented Feb 23, 2024

Follow on from: #199 originally by @mvorisek

Making TemplateTagParser::parseTemplateTagValue() static seemed like the best approach to fix a circular dependency between TypeParser and TemplateTagParser, as it means the TypeParser must be passed for each call.

Also parseOptionalDescription was awkward to handle as it only needs to be used in PhpDocParser and calls many private functions of PhpDocParser, so i reworked that to be a callable instead of a boolean flag.

@mad-briller mad-briller force-pushed the parse_generic_callable branch 2 times, most recently from 7ce9640 to 6894d61 Compare February 23, 2024 13:38
@mad-briller mad-briller force-pushed the parse_generic_callable branch 2 times, most recently from 9e8d2b7 to cda3c21 Compare February 23, 2024 15:27
new IdentifierTypeNode('Z'),
])),
new CallableTypeTemplateNode(new IdentifierTypeNode('Ty'), new IdentifierTypeNode('Y')),
]), ''),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new IdentifierTypeNode('C'),
[
new CallableTypeTemplateNode(new IdentifierTypeNode('A'), null),
new TemplateTagValueNode('A', null, ''),
Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Member

@ondrejmirtes ondrejmirtes left a 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! 👍

@mad-briller mad-briller force-pushed the parse_generic_callable branch from cda3c21 to de6eace Compare February 23, 2024 15:57
@ondrejmirtes ondrejmirtes merged commit c23674d into phpstan:1.23.x Feb 23, 2024
@ondrejmirtes
Copy link
Member

Perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants