From eec13d91b369a80ca249cceb900ba733e74376f5 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Mon, 22 Jun 2020 20:18:58 +0200 Subject: [PATCH] [PHP 8.0] Add ChangeSwitchToMatchRector --- .../conflict_phpstan_rector_php_parser.yaml | 22 ++ composer.json | 7 +- config/set/php80.php | 3 + docs/nodes_overview.md | 3 + docs/rector_rules_overview.md | 49 +++++ phpstan.neon | 3 +- .../Switch_/ChangeSwitchToMatchRector.php | 199 ++++++++++++++++++ .../ChangeSwitchToMatchRectorTest.php | 31 +++ .../Fixture/fixture.php.inc | 38 ++++ .../Fixture/skip_2_and_more_stmts.php.inc | 16 ++ .../skip_different_variable_assign.php.inc | 17 ++ .../Fixture/skip_missing_break.php.inc | 14 ++ .../src/Command/DumpNodesCommand.php | 8 + 13 files changed, 407 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/conflict_phpstan_rector_php_parser.yaml create mode 100644 rules/php80/src/Rector/Switch_/ChangeSwitchToMatchRector.php create mode 100644 rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/ChangeSwitchToMatchRectorTest.php create mode 100644 rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/fixture.php.inc create mode 100644 rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_2_and_more_stmts.php.inc create mode 100644 rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_different_variable_assign.php.inc create mode 100644 rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_missing_break.php.inc diff --git a/.github/workflows/conflict_phpstan_rector_php_parser.yaml b/.github/workflows/conflict_phpstan_rector_php_parser.yaml new file mode 100644 index 000000000000..e1b4fa4adde0 --- /dev/null +++ b/.github/workflows/conflict_phpstan_rector_php_parser.yaml @@ -0,0 +1,22 @@ +# see https://github.com/rectorphp/rector/pull/3788#issue-456723123 +name: PHP-Parser conflict + +on: + push: + branches: + - master + +jobs: + code_coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: shivammathur/setup-php@v1 + with: + php-version: 7.4 + coverage: none + + - run: composer install --no-progress --ansi + + # https://kizu514.com/blog/pcov-is-better-than-phpdbg-and-xdebug-for-code-coverage/ + - run: bin/rector diff --git a/composer.json b/composer.json index 4a923688602d..8b90ece97941 100644 --- a/composer.json +++ b/composer.json @@ -18,9 +18,9 @@ "jean85/pretty-package-versions": "^1.2", "nette/robot-loader": "^3.2", "nette/utils": "^3.1", - "nikic/php-parser": "4.6", + "nikic/php-parser": "^4.7", "ondram/ci-detector": "^3.4", - "phpstan/phpdoc-parser": "^0.4.7", + "phpstan/phpdoc-parser": "^0.4.8", "phpstan/phpstan-phpunit": "^0.12.10", "psr/simple-cache": "^1.0", "sebastian/diff": "^3.0|^4.0", @@ -55,6 +55,9 @@ "symplify/phpstan-extensions": "^8.1.15", "thecodingmachine/phpstan-strict-rules": "^0.12" }, + "replace": { + "rector/rector-prefixed": "self.version" + }, "autoload": { "psr-4": { "Rector\\Architecture\\": "rules/architecture/src", diff --git a/config/set/php80.php b/config/set/php80.php index 07ab70e8371e..65c32947baf5 100644 --- a/config/set/php80.php +++ b/config/set/php80.php @@ -12,6 +12,7 @@ use Rector\Php80\Rector\Identical\StrEndsWithRector; use Rector\Php80\Rector\Identical\StrStartsWithRector; use Rector\Php80\Rector\NotIdentical\StrContainsRector; +use Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector; use Rector\Php80\Rector\Ternary\GetDebugTypeRector; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -39,4 +40,6 @@ $services->set(RemoveUnusedVariableInCatchRector::class); $services->set(ClassPropertyAssignToConstructorPromotionRector::class); + + $services->set(ChangeSwitchToMatchRector::class); }; diff --git a/docs/nodes_overview.md b/docs/nodes_overview.md index b7bc39bad165..3c55ee6b54a0 100644 --- a/docs/nodes_overview.md +++ b/docs/nodes_overview.md @@ -1,3 +1,4 @@ +<<<<<<< HEAD # Node Overview * [Expressions](#expressions) @@ -2562,3 +2563,5 @@ string|null * `$types` - `/** @var (Identifier|Name)[] Types */`
+======= +>>>>>>> [PHP 8.0] Add ChangeSwitchToMatchRector diff --git a/docs/rector_rules_overview.md b/docs/rector_rules_overview.md index 5560d3afcd19..30cdf8928aa0 100644 --- a/docs/rector_rules_overview.md +++ b/docs/rector_rules_overview.md @@ -1,4 +1,8 @@ +<<<<<<< HEAD # All 541 Rectors Overview +======= +# All 534 Rectors Overview +>>>>>>> [PHP 8.0] Add ChangeSwitchToMatchRector - [Projects](#projects) --- @@ -9492,7 +9496,52 @@ Changes if/else to spaceship <=> where useful

+<<<<<<< HEAD ### `MultiDirnameRector` +======= +### `ChangeSwitchToMatchRector` + +- class: [`Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector`](/../master/rules/php80/src/Rector/Switch_/ChangeSwitchToMatchRector.php) +- [test fixtures](/../master/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture) + +Change `switch()` to `match()` + +```diff + class SomeClass + { + public function run() + { +- $statement = switch ($this->lexer->lookahead['type']) { +- case Lexer::T_SELECT: +- $statement = $this->SelectStatement(); +- break; +- +- case Lexer::T_UPDATE: +- $statement = $this->UpdateStatement(); +- break; +- +- case Lexer::T_DELETE: +- $statement = $this->DeleteStatement(); +- break; +- +- default: +- $this->syntaxError('SELECT, UPDATE or DELETE'); +- break; +- } ++ $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'), ++ }; + } + } +``` + +

+ +### `ClassOnObjectRector` +>>>>>>> [PHP 8.0] Add ChangeSwitchToMatchRector - class: [`Rector\Php70\Rector\FuncCall\MultiDirnameRector`](/../master/rules/php70/src/Rector/FuncCall/MultiDirnameRector.php) - [test fixtures](/../master/rules/php70/tests/Rector/FuncCall/MultiDirnameRector/Fixture) diff --git a/phpstan.neon b/phpstan.neon index e4e672f79463..e800ec3b97e5 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -318,7 +318,7 @@ parameters: - '#Method (.*?) specified in iterable type Symfony\\Component\\Process\\Process#' - '#Cannot cast PhpParser\\Node\\Expr\\Error\|PhpParser\\Node\\Identifier to string#' - - '#Class cognitive complexity for "DumpNodesCommand" is 103, keep it under 50#' + - '#Class cognitive complexity for "DumpNodesCommand" is \d+, keep it under 50#' - '#Cognitive complexity for "Rector\\Utils\\DocumentationGenerator\\Command\\DumpNodesCommand\:\:execute\(\)" is \d+, keep it under 9#' - '#Method Rector\\Utils\\DocumentationGenerator\\Node\\NodeClassProvider\:\:getNodeClasses\(\) should return array but returns array#' @@ -379,3 +379,4 @@ parameters: - '#Static property Symplify\\PackageBuilder\\Tests\\AbstractKernelTestCase\:\:\$container \(Psr\\Container\\ContainerInterface\) does not accept Rector\\Naming\\Tests\\Rector\\Class_\\RenamePropertyToMatchTypeRector\\Source\\ContainerInterface\|Symfony\\Component\\DependencyInjection\\Container#' - '#Static property Rector\\Core\\Testing\\PHPUnit\\AbstractGenericRectorTestCase\:\:\$allRectorContainer \(Rector\\Naming\\Tests\\Rector\\Class_\\RenamePropertyToMatchTypeRector\\Source\\ContainerInterface\|Symfony\\Component\\DependencyInjection\\Container\|null\) does not accept Psr\\Container\\ContainerInterface#' - '#Separate function "Symfony\\Component\\DependencyInjection\\Loader\\Configurator\\ref\(\)" in method call to standalone row to improve readability#' + - '#Class with base "(.*?)" name is already used in "_HumbugBox(.*?)#' diff --git a/rules/php80/src/Rector/Switch_/ChangeSwitchToMatchRector.php b/rules/php80/src/Rector/Switch_/ChangeSwitchToMatchRector.php new file mode 100644 index 000000000000..c2f37b083a7c --- /dev/null +++ b/rules/php80/src/Rector/Switch_/ChangeSwitchToMatchRector.php @@ -0,0 +1,199 @@ +lexer->lookahead['type']) { + case Lexer::T_SELECT: + $statement = $this->SelectStatement(); + break; + + case Lexer::T_UPDATE: + $statement = $this->UpdateStatement(); + break; + + case Lexer::T_DELETE: + $statement = $this->DeleteStatement(); + break; + + default: + $this->syntaxError('SELECT, UPDATE or DELETE'); + break; + } + } +} +PHP +, + <<<'PHP' +class SomeClass +{ + public function run() + { + $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'), + }; + } +} +PHP + + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Switch_::class]; + } + + /** + * @param Switch_ $node + */ + public function refactor(Node $node): ?Node + { + $this->assignVariable = null; + + if (! $this->hasEachCaseBreak($node)) { + return null; + } + + if (! $this->hasSingleStmtCases($node)) { + return null; + } + + if (! $this->hasSingleAssignVariableInStmtCase($node)) { + return null; + } + + $matchArms = $this->createMatchArmsFromCases($node->cases); + $match = new Match_($node->cond, $matchArms); + if ($this->assignVariable) { + return new Assign($this->assignVariable, $match); + } + + return $match; + } + + private function hasEachCaseBreak(Switch_ $switch): bool + { + foreach ($switch->cases as $case) { + foreach ($case->stmts as $caseStmt) { + if (! $caseStmt instanceof Break_) { + continue; + } + + return true; + } + } + + return false; + } + + /** + * @param Case_[] $cases + * @return MatchArm[] + */ + private function createMatchArmsFromCases(array $cases): array + { + $matchArms = []; + foreach ($cases as $case) { + $stmt = $case->stmts[0]; + if (! $stmt instanceof Expression) { + throw new ShouldNotHappenException(); + } + + $expr = $stmt->expr; + + if ($expr instanceof Assign) { + $this->assignVariable = $expr->var; + $expr = $expr->expr; + } + + if ($case->cond === null) { + $condList = null; + } else { + $condList = [$case->cond]; + } + + $matchArms[] = new MatchArm($condList, $expr); + } + + return $matchArms; + } + + private function hasSingleStmtCases(Switch_ $switch): bool + { + foreach ($switch->cases as $case) { + $stmtsWithoutBreak = array_filter($case->stmts, function (Node $node) { + return ! $node instanceof Break_; + }); + + if (count($stmtsWithoutBreak) !== 1) { + return false; + } + } + + return true; + } + + private function hasSingleAssignVariableInStmtCase(Switch_ $switch): bool + { + $assignVariableNames = []; + + foreach ($switch->cases as $case) { + /** @var Expression $onlyStmt */ + $onlyStmt = $case->stmts[0]; + $expr = $onlyStmt->expr; + + if (! $expr instanceof Assign) { + continue; + } + + $assignVariableNames[] = $this->getName($expr->var); + } + + $assignVariableNames = array_unique($assignVariableNames); + + return count($assignVariableNames) <= 1; + } +} diff --git a/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/ChangeSwitchToMatchRectorTest.php b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/ChangeSwitchToMatchRectorTest.php new file mode 100644 index 000000000000..f9ea41dee5b8 --- /dev/null +++ b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/ChangeSwitchToMatchRectorTest.php @@ -0,0 +1,31 @@ +doTestFileInfo($fileInfo); + } + + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + protected function getRectorClass(): string + { + return ChangeSwitchToMatchRector::class; + } +} diff --git a/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/fixture.php.inc b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..38de7e1d0b4b --- /dev/null +++ b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/fixture.php.inc @@ -0,0 +1,38 @@ +lexer->lookahead['type']) { + case Lexer::T_DELETE: + $statement = $this->DeleteStatement(); + break; + + default: + $this->syntaxError('SELECT, UPDATE or DELETE'); + break; + } + } +} + +?> +----- +lexer->lookahead['type']) { + Lexer::T_DELETE => $this->DeleteStatement(), + default => $this->syntaxError('SELECT, UPDATE or DELETE'), + }; + } +} + +?> diff --git a/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_2_and_more_stmts.php.inc b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_2_and_more_stmts.php.inc new file mode 100644 index 000000000000..8e9e69872a37 --- /dev/null +++ b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_2_and_more_stmts.php.inc @@ -0,0 +1,16 @@ +lexer->lookahead['type']) { + case Lexer::T_DELETE: + $statement = $this->DeleteStatement(); + $statement = $this->DeleteStatement(); + break; + } + } +} diff --git a/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_different_variable_assign.php.inc b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_different_variable_assign.php.inc new file mode 100644 index 000000000000..815d93c63fe0 --- /dev/null +++ b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_different_variable_assign.php.inc @@ -0,0 +1,17 @@ +lexer->lookahead['type']) { + case 'a': + $statementA = $this->DeleteStatement(); + case 'b': + $statementB = $this->DeleteStatement(); + break; + } + } +} diff --git a/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_missing_break.php.inc b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_missing_break.php.inc new file mode 100644 index 000000000000..67958dbd4ed6 --- /dev/null +++ b/rules/php80/tests/Rector/Switch_/ChangeSwitchToMatchRector/Fixture/skip_missing_break.php.inc @@ -0,0 +1,14 @@ +lexer->lookahead['type']) { + case Lexer::T_DELETE: + $statement = $this->DeleteStatement(); + } + } +} diff --git a/utils/documentation-generator/src/Command/DumpNodesCommand.php b/utils/documentation-generator/src/Command/DumpNodesCommand.php index c9d09e384c19..04b3fd9d55c3 100644 --- a/utils/documentation-generator/src/Command/DumpNodesCommand.php +++ b/utils/documentation-generator/src/Command/DumpNodesCommand.php @@ -31,6 +31,7 @@ use PhpParser\Node\Expr\Instanceof_; use PhpParser\Node\Expr\Isset_; use PhpParser\Node\Expr\List_; +use PhpParser\Node\Expr\Match_; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\New_; use PhpParser\Node\Expr\PostDec; @@ -48,6 +49,7 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\Expr\YieldFrom; use PhpParser\Node\Identifier; +use PhpParser\Node\MatchArm; use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\NullableType; @@ -244,6 +246,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $useUseNode = new UseUse(new Name('UsedNamespace')); $someVariableNode = new Variable(self::SOME_VARIABLE); + $matchArm = new MatchArm([new LNumber(1)], new String_('yes')); + if ($nodeClass === NullableType::class) { $node = new NullableType('SomeType'); } elseif ($nodeClass === Const_::class) { @@ -270,6 +274,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $node = new TraitUse([new Name('trait')]); } elseif ($nodeClass === Switch_::class) { $node = new Switch_(new Variable(self::VARIABLE), [new Case_(new LNumber(1))]); + } elseif ($nodeClass === Match_::class) { + $node = new Match_(new Variable(self::VARIABLE), [$matchArm]); + } elseif ($nodeClass === MatchArm::class) { + $node = $matchArm; } elseif ($nodeClass === Echo_::class) { $node = new Echo_([new String_('hello')]); } elseif ($nodeClass === StaticVar::class) {