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

phpdoc_types_order bug with intersection type #6238

Closed
kenjis opened this issue Jan 14, 2022 · 6 comments · Fixed by #6243
Closed

phpdoc_types_order bug with intersection type #6238

kenjis opened this issue Jan 14, 2022 · 6 comments · Fixed by #6243
Labels

Comments

@kenjis
Copy link

kenjis commented Jan 14, 2022

Bug report

v3.5.0

  • intersection type is converted to union type

Code snippet that reproduces the problem

$ tools/php-cs-fixer/vendor/bin/php-cs-fixer fix src --diff
Loaded config default from ".../php-cs-fixer-test/.php-cs-fixer.php".
Using cache file ".php-cs-fixer.cache".
Paths from configuration file have been overridden by paths provided as command arguments.
   1) src/MyPackage.php
      ---------- begin diff ----------
--- .../php-cs-fixer-test/src/MyPackage.php
+++ .../php-cs-fixer-test/src/MyPackage.php
@@ -8,7 +8,7 @@
 {
     public function test()
     {
-        /** @var MockObject&Query $query */
+        /** @var MockObject|Query $query */
         $query;
     }
 }

      ----------- end diff -----------

.php-cs-fixer.php:

$config = new PhpCsFixer\Config();
return $config->setRules([
         '@PSR12' => true,
         'phpdoc_types_order' => [
             'null_adjustment' => 'always_last',
             'sort_algorithm'  => 'alpha',
         ],
    ])
    ->setFinder($finder)
    ;
@Jean85
Copy link
Contributor

Jean85 commented Jan 14, 2022

👍 on this issue. It broke our CI sentry-php: https://github.com/getsentry/sentry-php/runs/4814501803?check_suite_focus=true#step:5:20

I'm trying to find the culprit in the changelog/diff...

@ruudk
Copy link
Contributor

ruudk commented Jan 14, 2022

Same issue here... we want to keep our intersection types. I'll postpone the upgrade for now.

@Jean85
Copy link
Contributor

Jean85 commented Jan 14, 2022

I may have found the culprit: 6bf7ba5#diff-de9d90510a6626bf0643e8978fbe25c91ff3fc48e61b0b5ccde5edd754bf70ccR389

I'll try to expand the tests to create a regression one

@Jean85
Copy link
Contributor

Jean85 commented Jan 14, 2022

Sorry I may not have all the knowledge to fix this, but I've at least ported the reproducer into a test: #6245

@Jean85
Copy link
Contributor

Jean85 commented Jan 14, 2022

Ok I've definitely found the culprit, and it's 8b8eca0
It's midnight, I'm giving up. My debugging led me to the issue anyway: the fixer calls $annotation->setTypes(...): https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/333f15e07c866e33e2765e84ba1e0b88e6a3af3b/src/Fixer/Phpdoc/PhpdocTypesOrderFixer.php#L145

which does a plain implode('|', ...) of the types, and here we have the issue: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/333f15e07c866e33e2765e84ba1e0b88e6a3af3b/src/DocBlock/Annotation.php#L207-L214

I don't know why this wasn't an issue before, it probably bailed out due to stricter regexes.

The issue is that that change is not feasible at all: changing order of types without taking account of parentheses or mixes of | and & is a no-go: it could definitely change the meaning of the type expressed in the PHPDoc. Maybe the only possible fix for now is to bail out anyway?

@kubawerlos
Copy link
Contributor

@Jean85 you were going right direction, take a look at the fix: #6243.

In deed the root of problem was implode with | - it was not a problem in 3.4.0, because intersection types were not recognized at all, this fix (see changes in src/DocBlock/TypeExpression.php) added support for intersection types, but Annotation::setTypes was assuming it always be types alternation.

alongosz added a commit to ibexa/core that referenced this issue Jan 17, 2022
alongosz added a commit to ibexa/core that referenced this issue Jan 17, 2022
sbuerk added a commit to sbuerk/testing-framework that referenced this issue Mar 4, 2022
To increase and maintain code quality has been added using
php-cs-fixer and typo3/coding-standards to this package.
CGL run changed two method return types in a way which leads
to a bunch of fails in core phpstan scans.

This is a already known bug of php-cs-fixer, reported for
version 3.5.0 but also 3.6.0 will change this the same way.

This patch reverts the return-type docblock change for

* 'BaseTestClass::getAccessibleMock()'
* 'BaseTestClass::getAccessibleMockForAbstractClass()'

and additional add following to composer.json conflict section:

    "friendsofphp/php-cs-fixer": "3.5.0 || 3.6.0"

Further reads:

PHP-CS-Fixer/PHP-CS-Fixer#6238
FriendsOfTYPO3/tea#371
FriendsOfTYPO3/tea@a999668

Releases: main, 7, 6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants