Skip to content
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

Access PHP Parser nodes #224

Closed
arogachev opened this issue Nov 23, 2021 · 6 comments
Closed

Access PHP Parser nodes #224

arogachev opened this issue Nov 23, 2021 · 6 comments

Comments

@arogachev
Copy link
Contributor

Needed for getting:

They were available in version 3.

@jaapio
Copy link
Member

jaapio commented Nov 30, 2021

Hi,

We designed this library with caching in mind. All relations in this library are non recurring. Which allows us to cache each file separate. Without having any serializing issues. As the php parser nodes do contain references to their parent nodes this behavior would break by storing the full node in each object. Could you please explain a bit more what is needed for you use case? Because this library isn't perfect, we do have missing features.

Based on the information you provided right now.

End line of a block.

It sounds reasonable to have the end to a block in our elements. I would be happy to accept a pr for this to add an end location in all applicable elements.

Is return by reference

Looks like we are missing this option on methods and functions. This should be added to the model.

Custom pretty print

We do allow everyone to create their own set of ProjectFactoryStrategies. This will allow you to inject a custom pretty print. However if you think your custom pretty print does a better job than ours. I would be happy to review a PR to see if it could replace the current version. Because we also had some issues in phpDocumentor with the print values.

@arogachev
Copy link
Contributor Author

Hello again! Thanks for detailed response. Sorry for such a delay.

We use this library in yii2-apidoc. All 3 issues reflect problems after migrating from version 3 to 5, so it's sort of regression. We parse and display this kind of info for the end users.

I understood your position about caching. I'll redo my implementation then. I suggest to decompose it into separate tasks accordingly.

The first two are pretty clear. As for the third, I'll try to provide more detailed example later on.

@arogachev
Copy link
Contributor Author

arogachev commented Dec 28, 2021

As for pretty print, we experienced these problems:

  1. Single slash becomes double in values of properties and class constants.

For example:

Before:

public string|array $serializer = 'yii\rest\Serializer'

After:

public string|array $serializer = 'yii\\rest\\Serializer'

So unexpected escaping is applied.

I made a fix based on this suggestion - nikic/PHP-Parser#447 (comment). But still it's better to dig deeper to understand author's intent for changing previous behavior.

  1. Inline comments in values are shifted to the next line (can be confusing). Thus we decided to completely remove all comments. See Inline comment belong to the next line nikic/PHP-Parser#791 for details. Probably not the best solution, but acceptable for us.

Can be more for sure, but these two are the main ones.

I hope it's more clear now. Should I create PR with fixes for any of these problems?

And, by the way, I figured it out how to inject custom pretty printer through strategies. Thanks for your advice! So it's not critical now, however default behavior still can be improved.

Just in case, our custom pretty printer version - https://github.com/yiisoft/yii2-apidoc/blob/f1405704b188beefb36d740201c54c45f0ec4ef3/helpers/PrettyPrinter.php. getRepresentationOfValue will be deprecated after merging this PR (yiisoft/yii2-apidoc#264).

@jaapio
Copy link
Member

jaapio commented Dec 28, 2021

Thanks for explaining this! I remember we had some issues in phpDocumentor itself rendering values of properties and constants, but never digged in to that this far. I would be happy to get another small pr. If that improved the default behavior of this library.

Thanks for all your hard work to improve this library!

@arogachev
Copy link
Contributor Author

👌 Will try to submit both fixes then later.

@arogachev
Copy link
Contributor Author

arogachev commented Dec 29, 2021

This can be closed now. Thanks! 👍 Improving pretty print moved to #232.

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

No branches or pull requests

2 participants