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

[PHP 8.0] Add ChangeSwitchToMatchRector #3569

Merged
merged 1 commit into from
Jul 30, 2020
Merged

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Jun 22, 2020

Covers nikic/PHP-Parser#672, https://wiki.php.net/rfc/match_expression_v2

 class SomeClass
 {
     public function run()
     {
-        $statement = switch ($this->lexer->lookahead['type']) {
-            case Lexer::T_SELECT:
-                $statement = $this->SelectStatement();
-                break;
-            default:
-                $this->syntaxError('SELECT, UPDATE or DELETE');
-                break;
-        }
+        $statement = match ($this->lexer->lookahead['type']) {
+            Lexer::T_SELECT => $this->SelectStatement(),
+            default => $this->syntaxError('SELECT, UPDATE or DELETE'),
+        };
     }
 }

Ref #3127

@TomasVotruba TomasVotruba marked this pull request as draft June 22, 2020 22:53
@TomasVotruba TomasVotruba marked this pull request as ready for review July 20, 2020 10:01
@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch 4 times, most recently from bb8ef27 to 7c4e72a Compare July 26, 2020 09:36
@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch 6 times, most recently from eec13d9 to 8738a99 Compare July 30, 2020 14:56
@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch from 8738a99 to 9de05c9 Compare July 30, 2020 14:57
@TomasVotruba
Copy link
Member Author

@ondrejmirtes Hi Ondra, here is the failing CI job I made just for this issue:
https://github.com/rectorphp/rector/pull/3569/checks?check_run_id=928156765

Only running bin/rector causes it.

@TomasVotruba
Copy link
Member Author

TomasVotruba commented Jul 30, 2020

I've just added separated jobs for:

Everything else in the code is the same

@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch from 54a8582 to 9a0598c Compare July 30, 2020 15:05
@ondrejmirtes
Copy link
Contributor

ondrejmirtes commented Jul 30, 2020

I probably know what this is about:

$composerAutoloader = require 'phar://' . __DIR__ . '/phpstan.phar/vendor/autoload.php';
  • Which means that potentially the PHP-Parser library from inside the PHAR will be loaded some time later.
  • But it has to be this way as PHPStan internals also rely on PHP-Parser.
  • My conclusion is that if a project uses PHPStan internals but also requires PHP-Parser on its own, the PHP-Parser has to be the same version as it's bundled in the PHPStan's PHAR.

I really don't know the way out of this, it's a hard problem.

@TomasVotruba
Copy link
Member Author

Great! I came to exact same conclussion.

@TomasVotruba
Copy link
Member Author

The only solution that worked for me in the past is to use the same php-parser as PHPStan does.
Last bug was with property promotion, so we had to wait till new version of phpstan uses 4.6. It does now, but there is already 4.7 with more breaks.

@ondrejmirtes
Copy link
Contributor

I'll be releasing PHPStan 0.12.34 with PHP-Parser 4.7 later today 🚀

@TomasVotruba
Copy link
Member Author

That's way over my expectations. Epic, thank you 👏 ❤️

@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch 2 times, most recently from 7b4cbaa to 7796219 Compare July 30, 2020 16:13
@TomasVotruba TomasVotruba mentioned this pull request Jul 30, 2020
@TomasVotruba TomasVotruba force-pushed the php80-switch-to-match branch from 909a974 to dc14ddd Compare July 30, 2020 16:20
@kodiakhq kodiakhq bot merged commit 82d3304 into master Jul 30, 2020
@TomasVotruba TomasVotruba deleted the php80-switch-to-match branch July 30, 2020 16:42
@TomasVotruba
Copy link
Member Author

@ondrejmirtes I've just updated Rector and can confirm 0.12.34 works purfectly! Thanks 👏 👏

@ondrejmirtes
Copy link
Contributor

Cool :)

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

Successfully merging this pull request may close these issues.

2 participants