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

Specific docblock not parsed after upgrading to 5.0 #1002

Closed
antiftw opened this issue Jun 9, 2024 · 2 comments
Closed

Specific docblock not parsed after upgrading to 5.0 #1002

antiftw opened this issue Jun 9, 2024 · 2 comments

Comments

@antiftw
Copy link

antiftw commented Jun 9, 2024

Like the title says, when bumping the version, suddenly a very specific Docblock is no longer being recognized by the parser.

I'm assisting in bumping this repository to Symfony 7, for another project that I'm trying to assist in a version bump.

Long story short, I need to bump the version of this repo to 5 because of another dependency using it.

However, when doing so, the extracter repo has a test that suddenly fails.

The only change I've made to the code is to allow the version bump, which was to replace

$parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7);

with

$parser = (new ParserFactory())->createForVersion(PhpVersion::fromComponents(7, 4, 0));
or
$parser = (new ParserFactory())->createForVersion(PhpVersion::getHostVersion());
or
$parser = (new ParserFactory())->createForVersion(PhpVersion::fromString('7.4'));
or
$parser = (new ParserFactory())->createForVersion(PhpVersion::fromString('8.3'));

But the Docblock wont be parsed. When switching back to 4.19.1 it appears again like expected. The weird thing is, that there are multiple other tests with similar setup (a custom annotation to indicate some functionality), and they are passing without issues, so they must be properly parsed. Even in the same case, inside the configureOptions() function, there is an Ignore Annotation, which apparently gets parsed properly since the test regarding that case passes without issues.

Since its a test case, so an isolated, well described situation, it would seem like a perfect spot to inspect this peculiarity. It's regarding this test which is regarding this case.

To reproduce:

  • Get my fork of the repo
  • Switch to the parserBump branch
  • composer install
  • vendor/bin/simple-phpunit
  • I've var_dumped the node where you can see the docblocks not being parsed.

To see the Docblocks appear again:

  • update "nikic/php-parser": "^3.0 || ^4.0 || ^5.0", to "nikic/php-parser": "^3.0 || ^4.0",`
  • composer update
  • vendor/bin/simple-phpunit
@nikic
Copy link
Owner

nikic commented Jun 9, 2024

The problem is this line: https://github.com/php-translation/extractor/blob/d26fba7475e1adca7d14483a3cf6d3aad3b60d39/src/Visitor/Php/Symfony/FormTypeChoices.php#L102 It checks for the doc comment on $item->key. With PHP Parser 5 it's necessary to check it on $item instead. (Checking it on $item should also work with PHP Parser 4). This is related to this change: https://github.com/nikic/PHP-Parser/blob/master/UPGRADE-5.0.md#changes-to-comment-assignment

@antiftw
Copy link
Author

antiftw commented Jun 9, 2024

@nikic

First of all, thanks for the quick response, it is appreciated!

And your answer was spot on (but I have some feeling you already knew that ;)) so the issue is resolved.

Thanks again!

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