Skip to content

[PropertyInfo] fix support for phpstan/phpdoc-parser 2 #58800

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

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Nov 7, 2024

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #58796
License MIT

@derrabus
Copy link
Member

derrabus commented Nov 7, 2024

Thank you Christian.

@derrabus derrabus merged commit 05ab010 into symfony:5.4 Nov 7, 2024
11 of 12 checks passed
@xabbuh xabbuh deleted the issue-58798 branch November 7, 2024 16:41
@bnf
Copy link
Contributor

bnf commented Nov 12, 2024

Hey, can you estimate when this fix is going to be released?

https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0 was released today which added support for phpstan/phpdoc-parser v2 in phpDocumentor/ReflectionDocBlock#388
That means a lot of instances start updating to phpstan/phpdoc-parser v2 now, and run into conflicts with symfony/type-info 7.1.6

In TYPO3, where symfony/property-info (pulls in symfony/type-info) and phpdocumentor/reflection-docblock are both used, there is now an implicit conflict, because symfony/type-info 7.1.6 does not declare an explicit dependency to phpstan/phpdoc-parser 1.x – it only defines a conflict to <1.0 and a require-dev dependency. Is that on purpose or a mistake?

Also see: https://forge.typo3.org/issues/105582

@derrabus
Copy link
Member

Hey, can you estimate when this fix is going to be released?

We do monthly releases, so this change will be released by the end of the month.

@bnf
Copy link
Contributor

bnf commented Nov 12, 2024

Thanks for the answer.
Can you elaborate on this aspect:

symfony/type-info 7.1.6 does not declare an explicit dependency to phpstan/phpdoc-parser 1.x – it only defines a conflict to <1.0 and a require-dev dependency. Is that on purpose or a mistake?

@stof
Copy link
Member

stof commented Nov 12, 2024

We don't declare an explicit dependency because this is an optional dependency of the component. You could totally configure the type-info component without enabling support for parsing phpdoc (relying only on native types) if you don't need the more precise types supported only in phpdoc (which is what FrameworkBundle does if the dependency is not available, btw).

@bnf
Copy link
Contributor

bnf commented Nov 12, 2024

Sorry for discussing in here.
I'm just wondering if we were wrong (and should release a fix ourselves) or not:

https://symfony.com/doc/current/components/property_info.html#phpdocextractor states that

This extractor depends on the phpdocumentor/reflection-docblock library.

We're using PhpDocExtractor from symfony/property-info in https://github.com/TYPO3/typo3/blob/04fb1190b9d/typo3/sysext/extbase/Classes/Reflection/ClassSchema.php#L113 and therefore require your optional dependency in https://github.com/TYPO3/typo3/blob/04fb1190b9d/typo3/sysext/extbase/composer.json#L23

We'not using PhpStanExtract (which is documented to require phpstan/phpdoc-parser) and therefore do not define a phpstan/phpdoc-parser dependency.

Therefore phpstan/phpdoc-parser was pulled in implicitly by phpdocumentor/reflection-docblock.

Also we do not require symfony/type-info directly.

Since https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0 was released today and now pulls in phpstan/phpdoc-parser 2.x, effectively all users of symfony/property-info:PhpDocExtractor run into this issue.

The problem I see is:

  • symfony/property-info:PhpDocExtractor implicitly pulls in phpstan/phpdoc-parser and can not be used without
  • users could pin phpstan/phpdoc-parser to 1.x, without actually using it, which is only "ok" to be done until november release and mean they define a dependency they are not supposed to define.

We'll now ask our community to apply a hotfix like the following do force a downgrade without specifying an unneeded dependency:

composer require phpstan/phpdoc-parser:^1
composer remove --no-update phpstan/phpdoc-parser

…but we already received complains for composer-updates in the meantime.
Any chance to fix this earlier ?

@stof
Copy link
Member

stof commented Nov 12, 2024

FrameworkBundle does not check which requirements you put in your vendor folder. It checks what is installed in the vendor folder.

We'll now ask our community to apply a hotfix like the following do force a downgrade without specifying an unneeded dependency

you can also ask them to run composer update phpstan/phpdoc-parser --with "phpstan/phpdoc-parser:^1" if you want to run a resolution using a constraint forcing phpdoc-parser 1.x without putting it in the composer.json

@bnf
Copy link
Contributor

bnf commented Nov 12, 2024

Thanks a lot. That oneline is much better.

I'm still sure that this conflict is wrong:

https://github.com/symfony/type-info/blob/51535dde21c7abf65c9d000a30bb15f6478195e6/composer.json#L36-L37

    "conflict": {
        "phpstan/phpdoc-parser": "<1.0",

It should have been:

    "conflict": {
        "phpstan/phpdoc-parser": "<1.0 || >=2.0",

…and now that v2 compatibility has been assured it should of course be:

    "conflict": {
        "phpstan/phpdoc-parser": "<1.0 || >=3.0",

@bnf
Copy link
Contributor

bnf commented Nov 12, 2024

That is why I'm asking for a hotfix release.

It seems everyone is affected who installs or updates symfony/property-info from now on until december, as #58841 shows.

CC @jaapio (whom I do not see as responsible – we just talked via direct messages – but revealed this with his https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0 update today). Maybe he can share his take on this situation.

@IT-Cru
Copy link

IT-Cru commented Nov 12, 2024

Also in api-platform/core project they run in issues from new phpstan/phpdoc-parser release.

See: api-platform/core#6789

As workaround projects could add following to their own composer.json file:

    "conflict": {
        ...,
        "phpstan/phpdoc-parser": "<1.0 || >=2.0"
    }

@jaapio
Copy link
Contributor

jaapio commented Nov 12, 2024

There is not much I can do.

The version constraint seems to be missing as the package is not compatible with the new version of phpstan/phpdoc-parser. We might want to prevent this in the future.

For now the only reasonable fix would be a hotfix release from Symfony. Even reverting the changes on my side would not solve anything as it would block symfony 's upgrade path. And composer would still be able to resolve a set containing the V2 version of the docblock parser.

To be fair, the actual origin of issues are the devs who just randomly update all their packages without reviewing what they actually do. We as maintainers cannot prevent all stupidity. But we can try to prevent it. It would at least reduce the number of emails and issues reported to us.

bnf added a commit to bnf/cms-cli that referenced this pull request Nov 13, 2024
… for composer tests

`symfony/type-info` is currently not compatible to
`phpstan/phpdoc-parser` 2.x which results in a PHP error in extbase
modules as shown in https://forge.typo3.org/issues/105582.
A fix has been merged upstream, but has not been released yet and
it seems there will be no immediate hotfix release by symfony – that
means it will only be released by the end of the month [2]).

Explanation of the issue:

`phpdocumentor/reflection-docblock` 5.6.0 [1] was released yesterday
which allows `composer install` and `composer update` to raise
`phpstan/phpdoc-parser` to 2.x.

Since symfony/type-info has an optional dependency to
`phpstan/phpdoc-parser`, a conflict is used to express the
supported version. This upstream conflict has a weak range [3]
as it only forbids incompatible old versions, but not new upcoming
*major*(!) versions.
The statement `"conflict": "<1.0"`[3] forbids to install any
version before 1.0, but missed to opt out from future major releases
(which are allowed to be breaking per semver).

Now that `phpstan/phpdoc-parser` 2.x has been released and is allowed
to be installed by other dependencies like
`phpdocumentor/reflection-docblock` (which is perfectly fine), this
range flaw got revealed.

This workaround will be removed once `symfony/type-info` has been
updated to allow an upgrade to `phpstan/phpdoc-parser`. For that
reason this conflict is only added to the testing instance and not
to extbase (extbase is incompatible, `symfony/type-info` is).

[1] https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0
[2] symfony/symfony#58800 (comment)
[3] https://github.com/symfony/type-info/blob/v7.1.6/composer.json#L36-L37
[4] symfony/symfony#58800 (comment)
bnf added a commit to bnf/cms-cli that referenced this pull request Nov 13, 2024
`symfony/type-info` is currently not compatible to
`phpstan/phpdoc-parser` 2.x which results in a PHP error in extbase
modules as shown in https://forge.typo3.org/issues/105582.
A fix has been merged upstream, but has not been released yet and
it seems there will be no immediate hotfix release by symfony – that
means it will only be released by the end of the month [2]).

Explanation of the issue:

`phpdocumentor/reflection-docblock` 5.6.0 [1] was released yesterday
which allows `composer install` and `composer update` to raise
`phpstan/phpdoc-parser` to 2.x.

Since symfony/type-info has an optional dependency to
`phpstan/phpdoc-parser`, a conflict is used to express the
supported version. This upstream conflict has a weak range [3]
as it only forbids incompatible old versions, but not new upcoming
*major*(!) versions.
The statement `"conflict": "<1.0"`[3] forbids to install any
version before 1.0, but missed to opt out from future major releases
(which are allowed to be breaking per semver).

Now that `phpstan/phpdoc-parser` 2.x has been released and is allowed
to be installed by other dependencies like
`phpdocumentor/reflection-docblock` (which is perfectly fine), this
range flaw got revealed.

This workaround will be removed once `symfony/type-info` has been
updated to allow an upgrade to `phpstan/phpdoc-parser`. For that
reason this conflict is only added to this composer-only package
to avoid a comlete TYPO3 release and is not added to extbase
(extbase is not incompatible, `symfony/type-info` is).

[1] https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0
[2] symfony/symfony#58800 (comment)
[3] https://github.com/symfony/type-info/blob/v7.1.6/composer.json#L36-L37
[4] symfony/symfony#58800 (comment)
bnf added a commit to bnf/cms-cli that referenced this pull request Nov 13, 2024
`symfony/type-info` is currently not compatible to
`phpstan/phpdoc-parser` 2.x which results in a PHP error in extbase
modules as shown in https://forge.typo3.org/issues/105582.
A fix has been merged upstream, but has not been released yet and
it seems there will be no immediate hotfix release by symfony – that
means it will only be released by the end of the month [2]).

Explanation of the issue:

`phpdocumentor/reflection-docblock` 5.6.0 [1] was released yesterday
which allows `composer install` and `composer update` to raise
`phpstan/phpdoc-parser` to 2.x.

Since symfony/type-info has an optional dependency to
`phpstan/phpdoc-parser`, a conflict is used to express the
supported version. This upstream conflict has a weak range [3]
as it only forbids incompatible old versions, but not new upcoming
*major*(!) versions.
The statement `"conflict": "<1.0"`[3] forbids to install any
version before 1.0, but missed to opt out from future major releases
(which are allowed to be breaking per semver).

Now that `phpstan/phpdoc-parser` 2.x has been released and is allowed
to be installed by other dependencies like
`phpdocumentor/reflection-docblock` (which is perfectly fine), this
range flaw got revealed.

This workaround will be removed once `symfony/type-info` has been
updated to allow an upgrade to `phpstan/phpdoc-parser`. For that
reason this conflict is only added to this composer-only package
to avoid a comlete TYPO3 release and is not added to extbase
(extbase is not incompatible, `symfony/type-info` is).

[1] https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0
[2] symfony/symfony#58800 (comment)
[3] https://github.com/symfony/type-info/blob/v7.1.6/composer.json#L36-L37
[4] symfony/symfony#58800 (comment)
bnf added a commit to bnf/cms-cli that referenced this pull request Nov 13, 2024
`symfony/type-info` is currently not compatible to
`phpstan/phpdoc-parser` 2.x which results in a PHP error in extbase
modules as shown in https://forge.typo3.org/issues/105582.
A fix has been merged upstream, but has not been released yet and
it seems there will be no immediate hotfix release by symfony – that
means it will only be released by the end of the month [2]).

Explanation of the issue:

`phpdocumentor/reflection-docblock` 5.6.0 [1] was released yesterday
which allows `composer install` and `composer update` to raise
`phpstan/phpdoc-parser` to 2.x.

Since symfony/type-info has an optional dependency to
`phpstan/phpdoc-parser`, a conflict is used to express the
supported version. This upstream conflict has a weak range [3]
as it only forbids incompatible old versions, but not new upcoming
*major*(!) versions.
The statement `"conflict": "<1.0"`[3] forbids to install any
version before 1.0, but missed to opt out from future major releases
(which are allowed to be breaking per semver).

Now that `phpstan/phpdoc-parser` 2.x has been released and is allowed
to be installed by other dependencies like
`phpdocumentor/reflection-docblock` (which is perfectly fine), this
range flaw got revealed.

This workaround will be removed once `symfony/type-info` has been
updated to allow an upgrade to `phpstan/phpdoc-parser`. For that
reason this conflict is only added to this composer-only package
to avoid a comlete TYPO3 release and is not added to extbase
(extbase is not incompatible, `symfony/type-info` is).

[1] https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0
[2] symfony/symfony#58800 (comment)
[3] https://github.com/symfony/type-info/blob/v7.1.6/composer.json#L36-L37
[4] symfony/symfony#58800 (comment)
bnf added a commit to bnf/cms-cli that referenced this pull request Nov 13, 2024
`symfony/type-info` is currently not compatible to
`phpstan/phpdoc-parser` 2.x which results in a PHP error in extbase
modules as shown in https://forge.typo3.org/issues/105582.
A fix has been merged upstream, but has not been released yet and
it seems there will be no immediate hotfix release by symfony – that
means it will only be released by the end of the month [2]).

Explanation of the issue:

`phpdocumentor/reflection-docblock` 5.6.0 [1] was released yesterday
which allows `composer install` and `composer update` to raise
`phpstan/phpdoc-parser` to 2.x.

Since symfony/type-info has an optional dependency to
`phpstan/phpdoc-parser`, a conflict is used to express the
supported version. This upstream conflict has a weak range [3]
as it only forbids incompatible old versions, but not new upcoming
*major*(!) versions.
The statement `"conflict": "<1.0"`[3] forbids to install any
version before 1.0, but missed to opt out from future major releases
(which are allowed to be breaking per semver).

Now that `phpstan/phpdoc-parser` 2.x has been released and is allowed
to be installed by other dependencies like
`phpdocumentor/reflection-docblock` (which is perfectly fine), this
range flaw got revealed.

This workaround will be removed once `symfony/type-info` has been
updated to allow an upgrade to `phpstan/phpdoc-parser`. For that
reason this conflict is only added to this composer-only package
to avoid a comlete TYPO3 release and is not added to extbase
(extbase is not incompatible, `symfony/type-info` is).

[1] https://github.com/phpDocumentor/ReflectionDocBlock/releases/tag/5.6.0
[2] symfony/symfony#58800 (comment)
[3] https://github.com/symfony/type-info/blob/v7.1.6/composer.json#L36-L37
[4] symfony/symfony#58800 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants