From a37a7d1dad664c91c7b41044cefec6d44d7f1bec Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Mon, 6 Jan 2020 12:54:01 +0100 Subject: [PATCH] [Polyfill] Add UnwrapFutureCompatibleIfRector --- composer.json | 6 +- config/set/polyfill/unwrap-compat.yaml | 2 + packages/Polyfill/config/config.yaml | 9 ++ .../FunctionSupportResolver.php | 46 +++++++ .../If_/UnwrapFutureCompatibleIfRector.php | 113 ++++++++++++++++++ .../Fixture/fixture.php.inc | 33 +++++ .../UnwrapFutureCompatibleIfRectorTest.php | 30 +++++ .../Node/Manipulator/IfManipulator.php | 34 +++++- 8 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 config/set/polyfill/unwrap-compat.yaml create mode 100644 packages/Polyfill/config/config.yaml create mode 100644 packages/Polyfill/src/FeatureSupport/FunctionSupportResolver.php create mode 100644 packages/Polyfill/src/Rector/If_/UnwrapFutureCompatibleIfRector.php create mode 100644 packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/Fixture/fixture.php.inc create mode 100644 packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/UnwrapFutureCompatibleIfRectorTest.php diff --git a/composer.json b/composer.json index 47b452cbaa35..d26667743256 100644 --- a/composer.json +++ b/composer.json @@ -105,7 +105,8 @@ "Rector\\PhpDeglobalize\\": "packages/PhpDeglobalize/src", "Rector\\Phalcon\\": "packages/Phalcon/src", "Rector\\DoctrineGedmoToKnplabs\\": "packages/DoctrineGedmoToKnplabs/src", - "Rector\\MinimalScope\\": "packages/MinimalScope/src" + "Rector\\MinimalScope\\": "packages/MinimalScope/src", + "Rector\\Polyfill\\": "packages/Polyfill/src" } }, "autoload-dev": { @@ -166,7 +167,8 @@ "Rector\\PhpDeglobalize\\Tests\\": "packages/PhpDeglobalize/tests", "Rector\\Phalcon\\Tests\\": "packages/Phalcon/tests", "Rector\\DoctrineGedmoToKnplabs\\Tests\\": "packages/DoctrineGedmoToKnplabs/tests", - "Rector\\MinimalScope\\Tests\\": "packages/MinimalScope/tests" + "Rector\\MinimalScope\\Tests\\": "packages/MinimalScope/tests", + "Rector\\Polyfill\\Tests\\": "packages/Polyfill/tests" }, "classmap": [ "packages/Symfony/tests/Rector/FrameworkBundle/AbstractToConstructorInjectionRectorSource", diff --git a/config/set/polyfill/unwrap-compat.yaml b/config/set/polyfill/unwrap-compat.yaml new file mode 100644 index 000000000000..1e513f60849f --- /dev/null +++ b/config/set/polyfill/unwrap-compat.yaml @@ -0,0 +1,2 @@ +services: + Rector\Polyfill\Rector\If_\UnwrapFutureCompatibleIfRector: null diff --git a/packages/Polyfill/config/config.yaml b/packages/Polyfill/config/config.yaml new file mode 100644 index 000000000000..7930a618da94 --- /dev/null +++ b/packages/Polyfill/config/config.yaml @@ -0,0 +1,9 @@ +services: + _defaults: + public: true + autowire: true + + Rector\Polyfill\: + resource: '../src' + exclude: + - '../src/Rector/**/*Rector.php' diff --git a/packages/Polyfill/src/FeatureSupport/FunctionSupportResolver.php b/packages/Polyfill/src/FeatureSupport/FunctionSupportResolver.php new file mode 100644 index 000000000000..f531a340455d --- /dev/null +++ b/packages/Polyfill/src/FeatureSupport/FunctionSupportResolver.php @@ -0,0 +1,46 @@ + ['session_abort'], + ]; + + /** + * @var PhpVersionProvider + */ + private $phpVersionProvider; + + public function __construct(PhpVersionProvider $phpVersionProvider) + { + $this->phpVersionProvider = $phpVersionProvider; + } + + public function isFunctionSupported(string $desiredFunction): bool + { + foreach (self::FUNCTIONS_BY_VERSION as $version => $functions) { + foreach ($functions as $function) { + if ($desiredFunction !== $function) { + continue; + } + + if (! $this->phpVersionProvider->isAtLeast($version)) { + continue; + } + + return true; + } + } + + return false; + } +} diff --git a/packages/Polyfill/src/Rector/If_/UnwrapFutureCompatibleIfRector.php b/packages/Polyfill/src/Rector/If_/UnwrapFutureCompatibleIfRector.php new file mode 100644 index 000000000000..057f1c89ec2c --- /dev/null +++ b/packages/Polyfill/src/Rector/If_/UnwrapFutureCompatibleIfRector.php @@ -0,0 +1,113 @@ +ifManipulator = $ifManipulator; + $this->functionSupportResolver = $functionSupportResolver; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Remove functions exists if with else for always existing', [ + new CodeSample( + <<<'PHP' +class SomeClass +{ + public function run() + { + // session locking trough other addons + if (function_exists('session_abort')) { + session_abort(); + } else { + session_write_close(); + } + } +} +PHP +, + <<<'PHP' +class SomeClass +{ + public function run() + { + // session locking trough other addons + session_abort(); + } +} +PHP + + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [If_::class]; + } + + /** + * @param If_ $node + */ + public function refactor(Node $node): ?Node + { + $match = $this->ifManipulator->isIfElseWithFunctionCondition($node, 'function_exists'); + if ($match === false) { + return null; + } + + /** @var Node\Expr\FuncCall $funcCall */ + $funcCall = $node->cond; + + $functionToExistName = $this->getValue($funcCall->args[0]->value); + if (! is_string($functionToExistName)) { + return null; + } + + if (! $this->functionSupportResolver->isFunctionSupported($functionToExistName)) { + return null; + } + + foreach ($node->stmts as $key => $ifStmt) { + if ($key === 0) { + // move comment from if to first element to keep it + $ifStmt->setAttribute('comments', $node->getComments()); + } + + $this->addNodeAfterNode($ifStmt, $node); + } + + $this->removeNode($node); + + return null; + } +} diff --git a/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/Fixture/fixture.php.inc b/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..473d8b91dc7c --- /dev/null +++ b/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/Fixture/fixture.php.inc @@ -0,0 +1,33 @@ + +----- + diff --git a/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/UnwrapFutureCompatibleIfRectorTest.php b/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/UnwrapFutureCompatibleIfRectorTest.php new file mode 100644 index 000000000000..0c7e7b1055cb --- /dev/null +++ b/packages/Polyfill/tests/Rector/If_/UnwrapFutureCompatibleIfRector/UnwrapFutureCompatibleIfRectorTest.php @@ -0,0 +1,30 @@ +doTestFile($file); + } + + public function provideDataForTest(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + protected function getRectorClass(): string + { + return UnwrapFutureCompatibleIfRector::class; + } +} diff --git a/src/PhpParser/Node/Manipulator/IfManipulator.php b/src/PhpParser/Node/Manipulator/IfManipulator.php index 98ad4b19892b..c37653bfe110 100644 --- a/src/PhpParser/Node/Manipulator/IfManipulator.php +++ b/src/PhpParser/Node/Manipulator/IfManipulator.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\Continue_; use PhpParser\Node\Stmt\Do_; @@ -18,6 +19,7 @@ use PhpParser\Node\Stmt\Throw_; use PhpParser\Node\Stmt\While_; use PhpParser\NodeTraverser; +use Rector\PhpParser\Node\Resolver\NameResolver; use Rector\PhpParser\NodeTraverser\CallableNodeTraverser; use Rector\PhpParser\Printer\BetterStandardPrinter; @@ -53,16 +55,23 @@ final class IfManipulator */ private $stmtsManipulator; + /** + * @var NameResolver + */ + private $nameResolver; + public function __construct( BetterStandardPrinter $betterStandardPrinter, ConstFetchManipulator $constFetchManipulator, CallableNodeTraverser $callableNodeTraverser, - StmtsManipulator $stmtsManipulator + StmtsManipulator $stmtsManipulator, + NameResolver $nameResolver ) { $this->betterStandardPrinter = $betterStandardPrinter; $this->constFetchManipulator = $constFetchManipulator; $this->callableNodeTraverser = $callableNodeTraverser; $this->stmtsManipulator = $stmtsManipulator; + $this->nameResolver = $nameResolver; } /** @@ -233,6 +242,29 @@ public function isIfAndElseWithSameVariableAssignAsLastStmts(If_ $if, Expr $desi return true; } + /** + * Matches: + * if () { + * } else { + * } + */ + public function isIfElseWithFunctionCondition(If_ $if, string $functionName): bool + { + if (! $this->isIfWithElse($if)) { + return false; + } + + if (! $if->cond instanceof FuncCall) { + return false; + } + + if (! $this->nameResolver->isName($if->cond, $functionName)) { + return false; + } + + return true; + } + private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr { if ($this->betterStandardPrinter->areNodesEqual($notIdentical->left, $returnNode->expr)) {