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

[FPDP] Format preserving doc printer #4334

Closed
TomasVotruba opened this issue Sep 30, 2020 · 8 comments
Closed

[FPDP] Format preserving doc printer #4334

TomasVotruba opened this issue Sep 30, 2020 · 8 comments
Assignees
Labels

Comments

@TomasVotruba
Copy link
Member

TomasVotruba commented Sep 30, 2020

Mirror of nikic/PHP-Parser#344, just for phpdoc

It might be worth to create https://github.com/phpstan/phpdoc-parser which actually preserver format

Here is knowledge snippet from php-parser: nikic/PHP-Parser#487

Related Issues

@staabm
Copy link
Contributor

staabm commented Nov 27, 2020

It might be worth to create phpstan/phpdoc-parser which actually preserver format

what does this mean? do you have a plan/a idea of what needs to be done?

@TomasVotruba
Copy link
Member Author

TomasVotruba commented Nov 27, 2020

We need a new package that builds on https://github.com/phpstan/phpdoc-parser,
and adds printing the docblock back to code.

I don't have any ETA on that.

@staabm
Copy link
Contributor

staabm commented Nov 27, 2020

Whats the job of this package? What should it provide on top of what phpstan/phpdoc-parser already can?

@TomasVotruba
Copy link
Member Author

TomasVotruba commented Nov 27, 2020

Whats the job of this package?

Current job of existing package is getting docs block to types.

E.g.

/**
 * @var int
 */

Very rougly:

use PHPStan\PhpDocParser\VarTagNode;
use PHPStan\PhpDocParser\IntegerTypeNode;

new VarTagNode(new IntegerTypeNode());

The problem is, all these cases:

/**
 * @var int
 */

/**
 * @var integer
 */

/**
 *  @var     int
 */

Are printed back as:

/**
 * @var int
 */



What should it provide on top of what phpstan/phpdoc-parser already can?

Keep the format in it's original form.

/**
 * @var integer
 */

/**
 * @var integer
 */

and

/**
 *  @var     int
 */

/**
 *  @var     int
 */

@TomasVotruba
Copy link
Member Author

@TomasVotruba
Copy link
Member Author

Big jump forward via:

@TomasVotruba
Copy link
Member Author

TomasVotruba commented Apr 5, 2021

We've just added another jump with static doctrine annotatoin parsing #5974

Before, to parse class-based annotations the https://github.com/doctrine/annotations was needed:

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity
 */
class Product
{
}

That required the Doctrine\ORM\Mapping\Entity class to be present and autoloaded. That kind-of kills the advantage of static reflection added in Rector 0.10 for annotations.

Further more, this approach required every such annotation to have own php doc node class and factory to make it. In total, Rector contained ~50 such classes + factories.


The #5974 PR solves this using https://github.com/phpstan/phpdoc-parser and copying the doctrine/annotation behavior 1:1 without autoloading used classes and constants.

Now, Rector is now using static for annotations making it more accessible to wide PHP comunity 🤗 👍

@TomasVotruba
Copy link
Member Author

Since last 2 comments and full month of testing we have exactly 0 reported issues with docblocks 👍

I think we can close this issue as resolved 🙂

TomasVotruba added a commit that referenced this issue Jun 24, 2023
rectorphp/rector-src@d6db548 [NodeTypeResolver] Remove parent attribute on VariableTypeResolver (#4334)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants