[PHP 8.0] Add match expressions #672

TomasVotruba · 2020-06-22T17:21:02Z

Official RFC Wiki: https://wiki.php.net/rfc/match_expression_v2
Covers: [RFC] Match expression php/php-src#5371
Closes [PHP 8.0] Add match expression v2 #671

// After
$statement = match ($this->lexer->lookahead['type']) {
    Lexer::T_SELECT => $this->SelectStatement(),
    Lexer::T_UPDATE => $this->UpdateStatement(),
    Lexer::T_DELETE => $this->DeleteStatement(),
    default => $this->syntaxError('SELECT, UPDATE or DELETE'),
};

TomasVotruba · 2020-06-22T17:21:56Z

Ready for review

lib/PhpParser/Node/Stmt/MatchArm.php

TomasVotruba · 2020-06-22T17:55:27Z

The failing job is related to offical PHP tests fixture, that is using "match" in function name function match(): https://github.com/php/php-src/blob/dad377ba5774bf624a0f111d0eca274053124b87/ext/standard/tests/file/fnmatch_variation.phpt#L81

It was fixed in the RFC PR:

So not related to this PR.

TomasVotruba · 2020-06-22T22:34:30Z

lib/PhpParser/PrettyPrinterAbstract.php

@@ -1256,6 +1256,7 @@ protected function initializeInsertionMap() {
            'Expr_Yield->value' => [\T_YIELD, false, ' ', null],
            'Param->type' => [null, false, null, ' '],
            'Param->default' => [null, false, ' = ', null],
+            'Stmt_MatchArm->body' => [null, false, ', ', null],


I thought this is the way to add a comma after specific subnode, but it doesn't work.
Any tips?

This list only takes effect if body is changed from null to Expr, which can't happen. You need the ListInsertionMap for new match arms / new match conditions.

grammar/php7.y

nikic · 2020-06-27T16:07:38Z

lib/PhpParser/Lexer/TokenEmulator/MatchTokenEmulator.php

+    public function emulate(string $code, array $tokens): array
+    {
+        // We need to manually iterate and manage a count because we'll change
+        // the tokens array on the way


This comment doesn't make sense here (we don't maintain a manual count).

It's only copy pasted from other emulator classes. Should it be removed everywhere or just here?

lib/PhpParser/Node/Expr/Match_.php

lib/PhpParser/Node/Stmt/MatchArm.php

nikic · 2020-06-27T16:11:07Z

lib/PhpParser/Node/Stmt/Switch_.php

@@ -12,8 +12,6 @@ class Switch_ extends Node\Stmt
    public $cases;

    /**
-     * Constructs a case node.


Keep this, it's present everywhere. Makes little sense to drop it in only one case.

Makes little sense to drop it in only one case.

My bad. Intent was to clear comment without any value. Will handle the rest of comments.

nikic · 2020-06-27T16:12:19Z

lib/PhpParser/PrettyPrinterAbstract.php

@@ -1256,6 +1256,7 @@ protected function initializeInsertionMap() {
            'Expr_Yield->value' => [\T_YIELD, false, ' ', null],
            'Param->type' => [null, false, null, ' '],
            'Param->default' => [null, false, ' = ', null],
+            'Stmt_MatchArm->body' => [null, false, ', ', null],


This list only takes effect if body is changed from null to Expr, which can't happen. You need the ListInsertionMap for new match arms / new match conditions.

Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com>

TomasVotruba · 2020-06-28T18:37:16Z

lib/PhpParser/PrettyPrinterAbstract.php

@@ -1326,6 +1326,8 @@ protected function initializeListInsertionMap() {
            'Stmt_TraitUseAdaptation_Precedence->insteadof' => ', ',
            'Stmt_Unset->vars' => ', ',
            'Stmt_Use->uses' => ', ',
+            'Stmt_Arm->condList' => ', ',
+            'Stmt_Match->arms' => ', ',


Is this correct? The comma is still missing in the print :P

TomasVotruba · 2020-06-28T18:37:27Z

lib/PhpParser/PrettyPrinterAbstract.php

@@ -1326,6 +1326,8 @@ protected function initializeListInsertionMap() {
            'Stmt_TraitUseAdaptation_Precedence->insteadof' => ', ',
            'Stmt_Unset->vars' => ', ',
            'Stmt_Use->uses' => ', ',
+            'Stmt_Arm->condList' => ', ',
+            'Stmt_Match->arms' => ', ',


Is this correct? The comma is still missing in the print :/

TomasVotruba · 2020-06-28T18:39:38Z

Updated

This reverts commit b822acc. Reduce unnecessary diff.

These have been added upstream as well.

This is not a statement, and just "Arm" is a bit too generic.

This seems more in line with other names like "stmts".

This has more failures, but it's more honest about which PHP 8 features we don't support yet ...

TomasVotruba · 2020-07-15T21:07:40Z

Thanks for finishing it!

Do you recall where I messed up the grammar so the commas after arms were missing?

nikic · 2020-07-15T21:10:39Z

@TomasVotruba I think everything in the grammar was right, the problem was was the use of pStmts() instead of pCommaSeparatedMultiline() inside the pretty-printer.

TomasVotruba · 2020-07-15T21:19:18Z

I thought I tried both with same bad result. Well, maybe I overlooked it.

Thanks for letting me know 👍 , I'll be patient to it next time

ondrejmirtes · 2020-07-16T11:12:55Z

Is it expected that Emulative lexer will break on existing PHP 7-level code? I'd say that people use Emulative when they want to parse features from newer PHP versions on an older PHP version.

But this change means that the Emulative lexer will fail with parse error on interface Match which is valid on PHP 7. Which is pretty inconvenient as PHPUnit contains this interface.

nikic · 2020-07-16T12:05:01Z

@ondrejmirtes Unfortunately the emulative lexer currently only does emulation in one direction. Emulation in the other direction (and ability to pick a precise PHP version to target) would have to be implemented.

Description ----------- As of version 4.7, the PHP parser supports PHP 8 match expressions, which unfortunately [breaks PHPUnit](nikic/PHP-Parser#672 (comment)) under PHP 7.4. ``` ERROR: ParseError - vendor/phpunit/phpunit/src/Framework/MockObject/Builder/ParametersMatch.php:15:35 - Syntax error, unexpected T_MATCH on line 15 (see https://psalm.dev/173) interface ParametersMatch extends Match ERROR: ParseError - vendor/phpunit/phpunit/src/Framework/MockObject/Builder/ParametersMatch.php:34:40 - Syntax error, unexpected ';', expecting '{' on line 34 (see https://psalm.dev/173) public function with(...$arguments); ERROR: ParseError - vendor/phpunit/phpunit/src/Framework/MockObject/Builder/ParametersMatch.php:47:40 - Syntax error, unexpected ';', expecting '{' on line 47 (see https://psalm.dev/173) public function withAnyParameters(); ``` The issue has been reported in nikic/PHP-Parser#690, so I hope it will be fixed in version 4.7.1. Commits ------- 00ed9a0 Add a conflict with nikic/php-parser

TomasVotruba commented Jun 22, 2020

View reviewed changes

lib/PhpParser/Node/Stmt/MatchArm.php Outdated Show resolved Hide resolved

TomasVotruba commented Jun 22, 2020

View reviewed changes

TomasVotruba mentioned this pull request Jun 22, 2020

[PHP 8.0] Add ChangeSwitchToMatchRector rectorphp/rector#3569

Merged

nikic reviewed Jun 27, 2020

View reviewed changes

TomasVotruba and others added 14 commits June 28, 2020 20:33

[PHP 8.0] Add match expressions

e2962ba

add default

479d94b

various

4336370

Update grammar/php7.y

b43d0fd

Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com>

Update lib/PhpParser/Node/Expr/Match_.php

467fd98

Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com>

Update grammar/php7.y

38e0ec8

Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com>

Update lib/PhpParser/Node/Stmt/MatchArm.php

743e40f

Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com>

Add constructor promotion support

c7d07ad

[PHP 8.0] Add match expressions

ea990d3

re-use existing list and expr

b3ba136

rebuild parser

4b6f602

rename MatchArm to Arm

a60a195

update listInsertionMap

6be6b1b

remove all this is a _node_ class comments

b822acc

TomasVotruba commented Jun 28, 2020

View reviewed changes

lower token

b00b420

nikic added 7 commits July 15, 2020 17:44

Revert "remove all this is a _node_ class comments"

5a18cbb

This reverts commit b822acc. Reduce unnecessary diff.

Rebuild parser

f39e25c

Support trailing commas

5d4d14c

These have been added upstream as well.

Rename Node\Stmt\Arm to Node\MatchArm

832ca76

This is not a statement, and just "Arm" is a bit too generic.

Fix match pretty printing

80fe77e

Don't extend Stmt in MatchArm

2f5c980

Rename condList to conds

4927c98

This seems more in line with other names like "stmts".

nikic added 4 commits July 15, 2020 21:13

Revert unrelated change

10bb05e

More test coverage for FPPP

e85fb37

Switch run-php-src.sh to test against master

6cf39fe

This has more failures, but it's more honest about which PHP 8 features we don't support yet ...

Add changelog entry

ae09512

nikic merged commit 69c5d48 into nikic:master Jul 15, 2020

nikic mentioned this pull request Jul 15, 2020

PHP-Parser broken on PHP 8.0.0-dev #684

Closed

TomasVotruba deleted the php80-match-expression-v2 branch July 15, 2020 21:07

ondrejmirtes mentioned this pull request Jul 25, 2020

No longer to parse PHP 7.4 syntax in php parser #690

Closed

leofeyer mentioned this pull request Jul 26, 2020

Add a conflict with nikic/php-parser contao/contao#2002

Merged

celosauro mentioned this pull request Jul 27, 2020

MissingDependency in phpunit dependencies psalm/psalm-plugin-phpunit#80

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PHP 8.0] Add match expressions #672

[PHP 8.0] Add match expressions #672

TomasVotruba commented Jun 22, 2020 •

edited

Loading

TomasVotruba commented Jun 22, 2020

TomasVotruba commented Jun 22, 2020 •

edited

Loading

TomasVotruba Jun 22, 2020

nikic Jun 27, 2020

TomasVotruba Jun 28, 2020

nikic Jun 27, 2020

TomasVotruba Jun 28, 2020

nikic Jun 27, 2020

TomasVotruba Jun 28, 2020

TomasVotruba Jun 28, 2020

TomasVotruba Jun 28, 2020

nikic Jun 27, 2020

TomasVotruba Jun 28, 2020

TomasVotruba Jun 28, 2020

TomasVotruba commented Jun 28, 2020

TomasVotruba commented Jul 15, 2020 •

edited

Loading

nikic commented Jul 15, 2020

TomasVotruba commented Jul 15, 2020

ondrejmirtes commented Jul 16, 2020

nikic commented Jul 16, 2020

[PHP 8.0] Add match expressions #672

[PHP 8.0] Add match expressions #672

Conversation

TomasVotruba commented Jun 22, 2020 • edited Loading

TomasVotruba commented Jun 22, 2020

TomasVotruba commented Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomasVotruba commented Jun 28, 2020

TomasVotruba commented Jul 15, 2020 • edited Loading

nikic commented Jul 15, 2020

TomasVotruba commented Jul 15, 2020

ondrejmirtes commented Jul 16, 2020

nikic commented Jul 16, 2020

TomasVotruba commented Jun 22, 2020 •

edited

Loading

TomasVotruba commented Jun 22, 2020 •

edited

Loading

TomasVotruba commented Jul 15, 2020 •

edited

Loading