diff --git a/ecs.php b/ecs.php index ea3d1db201e..21aaf901d4a 100644 --- a/ecs.php +++ b/ecs.php @@ -6,6 +6,7 @@ use PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis\AssignmentInConditionS use PhpCsFixer\Fixer\FunctionNotation\FunctionTypehintSpaceFixer; use PhpCsFixer\Fixer\Phpdoc\GeneralPhpdocAnnotationRemoveFixer; use PhpCsFixer\Fixer\Phpdoc\NoSuperfluousPhpdocTagsFixer; +use PhpCsFixer\Fixer\Phpdoc\PhpdocNoEmptyReturnFixer; use PhpCsFixer\Fixer\Phpdoc\PhpdocTypesFixer; use PhpCsFixer\Fixer\PhpUnit\PhpUnitStrictFixer; use Symplify\CodingStandard\Fixer\LineLength\DocBlockLineLengthFixer; @@ -71,5 +72,10 @@ return static function (ECSConfig $ecsConfig): void { ], AssignmentInConditionSniff::class . '.FoundInWhileCondition', + + // null on purpose as no change + PhpdocNoEmptyReturnFixer::class => [ + __DIR__ . '/rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php', + ], ]); }; diff --git a/phpstan.neon b/phpstan.neon index eb37da4e6c6..cc0a257c1bc 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -585,9 +585,13 @@ parameters: # avoid moving rule around, while re-targeting to stmt - '#Class "Rector\\(.*?)\\Rector\\(.*?) has invalid namespace category "(.*?)"\. Pick one of\: "(Expression|Class_|NodeTypeGroup)"#' + - '#Class "Rector\\DeadCode\\Rector\\ConstFetch\\RemovePhpVersionIdCheckRector" has invalid namespace category "ConstFetch"\. Pick one of\: "If_"#' - '#Cognitive complexity for "Rector\\CodingStyle\\Rector\\ClassMethod\\MakeInheritedMethodVisibilitySameAsParentRector\:\:refactorWithScope\(\)" is 11, keep it under 10#' - message: '#Cognitive complexity for "Rector\\EarlyReturn\\Rector\\If_\\ChangeAndIfToEarlyReturnRector\:\:refactor\(\)" is \d+, keep it under 10#' paths: - rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php + + # known value + - '#Method Rector\\Core\\Php\\PhpVersionProvider\:\:provide\(\) should return 50200\|50300\|50400\|50500\|50600\|70000\|70100\|70200\|70300\|70400\|80000\|80100\|80200\|100000 but returns int#' diff --git a/rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php b/rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php index 5b37600c038..cf2d899f478 100644 --- a/rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php +++ b/rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php @@ -11,25 +11,27 @@ use PhpParser\Node\Expr\BinaryOp\GreaterOrEqual; use PhpParser\Node\Expr\BinaryOp\Smaller; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Scalar\LNumber; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\If_; use Rector\Core\Contract\Rector\ConfigurableRectorInterface; use Rector\Core\Php\PhpVersionProvider; use Rector\Core\Rector\AbstractRector; -use Rector\Core\Util\PhpVersionFactory; use Rector\Core\ValueObject\PhpVersion; -use Rector\NodeTypeResolver\Node\AttributeKey; use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; +use Webmozart\Assert\Assert; /** * @see \Rector\Tests\DeadCode\Rector\ConstFetch\RemovePhpVersionIdCheckRector\RemovePhpVersionIdCheckRectorTest */ final class RemovePhpVersionIdCheckRector extends AbstractRector implements ConfigurableRectorInterface { - private string | int | null $phpVersionConstraint = null; + /** + * @var PhpVersion::*|null + */ + private int | null $phpVersion = null; public function __construct( - private readonly PhpVersionFactory $phpVersionFactory, private readonly PhpVersionProvider $phpVersionProvider, ) { } @@ -39,13 +41,19 @@ final class RemovePhpVersionIdCheckRector extends AbstractRector implements Conf */ public function configure(array $configuration): void { - $this->phpVersionConstraint = array_pop($configuration); + $phpVersion = $configuration[0]; + Assert::integer($phpVersion); + PhpVersion::assertValidValue($phpVersion); + + // ensure cast to (string) first to allow string like "8.0" value to be converted to the int value + /** @var PhpVersion::* $phpVersion */ + $this->phpVersion = $phpVersion; } public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( - 'Remove unneeded PHP_VERSION_ID check', + 'Remove unneeded PHP_VERSION_ID conditional checks', [ new ConfiguredCodeSample( <<<'CODE_SAMPLE' @@ -56,6 +64,7 @@ class SomeClass if (PHP_VERSION_ID < 80000) { return; } + echo 'do something'; } } @@ -82,195 +91,212 @@ CODE_SAMPLE */ public function getNodeTypes(): array { - return [ConstFetch::class]; + return [If_::class]; } /** - * @param ConstFetch $node + * @param If_ $node + * @return Stmt[]|null */ - public function refactor(Node $node): ?Node + public function refactor(Node $node): ?array { - if (! $this->isName($node, 'PHP_VERSION_ID')) { - return null; - } - /** * $this->phpVersionProvider->provide() fallback is here as $currentFileProvider must be accessed after initialization */ - $phpVersionConstraint = $this->phpVersionConstraint ?? $this->phpVersionProvider->provide(); + if ($this->phpVersion === null) { + $this->phpVersion = $this->phpVersionProvider->provide(); + } - // ensure cast to (string) first to allow string like "8.0" value to be converted to the int value - $this->phpVersionConstraint = $this->phpVersionFactory->createIntVersion((string) $phpVersionConstraint); - - $if = $this->betterNodeFinder->findParentType($node, If_::class); - if (! $if instanceof If_) { + if (! $node->cond instanceof BinaryOp) { return null; } - $parent = $node->getAttribute(AttributeKey::PARENT_NODE); - if ($this->shouldSkip($node, $if, $parent)) { + $binaryOp = $node->cond; + if ($binaryOp->left instanceof ConstFetch && $this->isName($binaryOp->left->name, 'PHP_VERSION_ID')) { + return $this->refactorConstFetch($binaryOp->left, $node, $binaryOp); + } + + if (! $binaryOp->right instanceof ConstFetch) { return null; } - if ($parent instanceof Smaller) { - return $this->processSmaller($node, $parent, $if); + if (! $this->isName($binaryOp->right->name, 'PHP_VERSION_ID')) { + return null; } - if ($parent instanceof GreaterOrEqual) { - return $this->processGreaterOrEqual($node, $parent, $if); - } - - if ($parent instanceof Greater) { - return $this->processGreater($node, $parent, $if); - } - - return null; + return $this->refactorConstFetch($binaryOp->right, $node, $binaryOp); } - private function shouldSkip(ConstFetch $constFetch, If_ $if, ?Node $node): bool - { - $if = $this->betterNodeFinder->findParentType($constFetch, If_::class); - if (! $if instanceof If_) { - return true; - } - - $node = $constFetch->getAttribute(AttributeKey::PARENT_NODE); - if (! $node instanceof BinaryOp) { - return true; - } - - return $if->cond !== $node; - } - - private function processSmaller(ConstFetch $constFetch, Smaller $smaller, If_ $if): ?ConstFetch + /** + * @return Stmt[]|null + */ + private function processSmaller(ConstFetch $constFetch, Smaller $smaller, If_ $if): ?array { if ($smaller->left === $constFetch) { - return $this->processSmallerLeft($constFetch, $smaller, $if); + return $this->processSmallerLeft($smaller, $if); } if ($smaller->right === $constFetch) { - return $this->processSmallerRight($constFetch, $smaller, $if); + return $this->processSmallerRight($smaller, $if); } return null; } - private function processGreaterOrEqual(ConstFetch $constFetch, GreaterOrEqual $greaterOrEqual, If_ $if): ?ConstFetch - { + /** + * @return Stmt[]|null + */ + private function processGreaterOrEqual( + ConstFetch $constFetch, + GreaterOrEqual $greaterOrEqual, + If_ $if, + ): ?array { if ($greaterOrEqual->left === $constFetch) { - return $this->processGreaterOrEqualLeft($constFetch, $greaterOrEqual, $if); + return $this->processGreaterOrEqualLeft($greaterOrEqual, $if); } if ($greaterOrEqual->right === $constFetch) { - return $this->processGreaterOrEqualRight($constFetch, $greaterOrEqual, $if); + return $this->processGreaterOrEqualRight($greaterOrEqual, $if); } return null; } - private function processSmallerLeft(ConstFetch $constFetch, Smaller $smaller, If_ $if): ?ConstFetch + /** + * @return null + */ + private function processSmallerLeft(Smaller $smaller, If_ $if) { $value = $smaller->right; if (! $value instanceof LNumber) { return null; } - if ($this->phpVersionConstraint >= $value->value) { + if ($this->phpVersion >= $value->value) { $this->removeNode($if); } - return $constFetch; + return null; } - private function processSmallerRight(ConstFetch $constFetch, Smaller $smaller, If_ $if): ?ConstFetch + /** + * @return Stmt[]|null + */ + private function processSmallerRight(Smaller $smaller, If_ $if): ?array { $value = $smaller->left; if (! $value instanceof LNumber) { return null; } - if ($this->phpVersionConstraint >= $value->value) { - $this->nodesToAddCollector->addNodesBeforeNode($if->stmts, $if); - $this->removeNode($if); - } - - return $constFetch; - } - - private function processGreaterOrEqualLeft( - ConstFetch $constFetch, - GreaterOrEqual $greaterOrEqual, - If_ $if - ): ?ConstFetch { - $value = $greaterOrEqual->right; - if (! $value instanceof LNumber) { - return null; - } - - if ($this->phpVersionConstraint >= $value->value) { - $this->nodesToAddCollector->addNodesBeforeNode($if->stmts, $if); - $this->removeNode($if); - } - - return $constFetch; - } - - private function processGreaterOrEqualRight( - ConstFetch $constFetch, - GreaterOrEqual $greaterOrEqual, - If_ $if - ): ?ConstFetch { - $value = $greaterOrEqual->left; - if (! $value instanceof LNumber) { - return null; - } - - if ($this->phpVersionConstraint >= $value->value) { - $this->removeNode($if); - } - - return $constFetch; - } - - private function processGreater(ConstFetch $constFetch, Greater $greater, If_ $if): ?ConstFetch - { - if ($greater->left === $constFetch) { - return $this->processGreaterLeft($constFetch, $greater, $if); - } - - if ($greater->right === $constFetch) { - return $this->processGreaterRight($constFetch, $greater, $if); + if ($this->phpVersion >= $value->value) { + return $if->stmts; } return null; } - private function processGreaterLeft(ConstFetch $constFetch, Greater $greater, If_ $if): ?ConstFetch + /** + * @return Stmt[]|null + */ + private function processGreaterOrEqualLeft(GreaterOrEqual $greaterOrEqual, If_ $if): array|null + { + $value = $greaterOrEqual->right; + if (! $value instanceof LNumber) { + return null; + } + + if ($this->phpVersion >= $value->value) { + return $if->stmts; + } + + return null; + } + + /** + * @return null + */ + private function processGreaterOrEqualRight(GreaterOrEqual $greaterOrEqual, If_ $if) + { + $value = $greaterOrEqual->left; + if (! $value instanceof LNumber) { + return null; + } + + if ($this->phpVersion >= $value->value) { + $this->removeNode($if); + } + + return null; + } + + /** + * @return Stmt[]|null + */ + private function processGreater(ConstFetch $constFetch, Greater $greater, If_ $if): ?array + { + if ($greater->left === $constFetch) { + return $this->processGreaterLeft($greater, $if); + } + + if ($greater->right === $constFetch) { + return $this->processGreaterRight($greater, $if); + } + + return null; + } + + /** + * @return Stmt[]|null + */ + private function processGreaterLeft(Greater $greater, If_ $if): ?array { $value = $greater->right; if (! $value instanceof LNumber) { return null; } - if ($this->phpVersionConstraint >= $value->value) { - $this->nodesToAddCollector->addNodesBeforeNode($if->stmts, $if); - $this->removeNode($if); + if ($this->phpVersion >= $value->value) { + return $if->stmts; } - return $constFetch; + return null; } - private function processGreaterRight(ConstFetch $constFetch, Greater $greater, If_ $if): ?ConstFetch + /** + * @return null + */ + private function processGreaterRight(Greater $greater, If_ $if) { $value = $greater->left; if (! $value instanceof LNumber) { return null; } - if ($this->phpVersionConstraint >= $value->value) { + if ($this->phpVersion >= $value->value) { $this->removeNode($if); } - return $constFetch; + return null; + } + + /** + * @return Stmt[]|null + */ + private function refactorConstFetch(ConstFetch $constFetch, If_ $if, BinaryOp $binaryOp): ?array + { + if ($binaryOp instanceof Smaller) { + return $this->processSmaller($constFetch, $binaryOp, $if); + } + + if ($binaryOp instanceof GreaterOrEqual) { + return $this->processGreaterOrEqual($constFetch, $binaryOp, $if); + } + + if ($binaryOp instanceof Greater) { + return $this->processGreater($constFetch, $binaryOp, $if); + } + + return null; } } diff --git a/src/Php/PhpVersionProvider.php b/src/Php/PhpVersionProvider.php index f19fed067ee..2781c34770b 100644 --- a/src/Php/PhpVersionProvider.php +++ b/src/Php/PhpVersionProvider.php @@ -29,6 +29,9 @@ final class PhpVersionProvider ) { } + /** + * @return PhpVersion::* + */ public function provide(): int { $phpVersionFeatures = $this->parameterProvider->provideParameter(Option::PHP_VERSION_FEATURES);