diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/array_param_truthy_check.php.inc b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/array_param_truthy_check.php.inc new file mode 100644 index 00000000000..241ff58e2d6 --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/array_param_truthy_check.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_int_from_method_call.php.inc b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_int_from_method_call.php.inc new file mode 100644 index 00000000000..ebaa284cea5 --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_int_from_method_call.php.inc @@ -0,0 +1,47 @@ +resolveCount(); + if ($count === null || ! is_int($count)) { + return; + } + + echo $count; + } + + private function resolveCount(): ?int + { + return 1; + } +} + +?> +----- +resolveCount(); + if ($count === null) { + return; + } + + echo $count; + } + + private function resolveCount(): ?int + { + return 1; + } +} + +?> diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string.php.inc b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string.php.inc new file mode 100644 index 00000000000..8663e36715b --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string.php.inc @@ -0,0 +1,35 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string_truthy_check.php.inc b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string_truthy_check.php.inc new file mode 100644 index 00000000000..25f6d0bccb9 --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/nullable_string_truthy_check.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/skip_another_variable.php.inc b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/skip_another_variable.php.inc new file mode 100644 index 00000000000..fe67608e39a --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/Fixture/skip_another_variable.php.inc @@ -0,0 +1,15 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/config/configured_rule.php b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/config/configured_rule.php new file mode 100644 index 00000000000..79e6bd5ffcd --- /dev/null +++ b/rules-tests/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(RemoveRedundantTypeCheckRector::class); +}; diff --git a/rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/FixtureAddToInterfaceMethods/interface_implement.php b/rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/FixtureAddToInterfaceMethods/interface_implement.php deleted file mode 100644 index 1d2157809f8..00000000000 --- a/rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/FixtureAddToInterfaceMethods/interface_implement.php +++ /dev/null @@ -1,17 +0,0 @@ - diff --git a/rules/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector.php b/rules/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector.php new file mode 100644 index 00000000000..f2e86be0866 --- /dev/null +++ b/rules/DeadCode/Rector/BinaryOp/RemoveRedundantTypeCheckRector.php @@ -0,0 +1,226 @@ +() check that can never fail on already known type', [ + new CodeSample( + <<<'CODE_SAMPLE' +class SomeClass +{ + public function run(?string $value, array $items) + { + if ($value === null || ! is_string($value)) { + return; + } + + if ($items && is_array($items)) { + return; + } + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function run(?string $value, array $items) + { + if ($value === null) { + return; + } + + if ($items) { + return; + } + } +} +CODE_SAMPLE + ), + ]); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [BooleanOr::class, BooleanAnd::class]; + } + + /** + * @param BooleanOr|BooleanAnd $node + */ + public function refactor(Node $node): ?Expr + { + if ($node instanceof BooleanOr) { + return $this->refactorBooleanOr($node); + } + + return $this->refactorBooleanAnd($node); + } + + /** + * Handles "null === $value || ! is_string($value)", where is_string() can never fail once null is excluded + */ + private function refactorBooleanOr(BooleanOr $booleanOr): ?Expr + { + if (! $booleanOr->left instanceof Identical) { + return null; + } + + $nullComparedExpr = $this->matchNullComparedExpr($booleanOr->left); + if (! $nullComparedExpr instanceof Variable) { + return null; + } + + // the docblock type can be wider than the real value + if ($this->exprAnalyzer->isNonTypedFromParam($nullComparedExpr)) { + return null; + } + + if (! $booleanOr->right instanceof BooleanNot) { + return null; + } + + $funcCall = $this->matchTypeCheckFuncCall($booleanOr->right->expr, $nullComparedExpr); + if (! $funcCall instanceof FuncCall) { + return null; + } + + $funcCallName = $this->getName($funcCall); + if ($funcCallName === null) { + return null; + } + + $comparedType = $this->getNativeType($nullComparedExpr); + if (! TypeCombinator::containsNull($comparedType)) { + return null; + } + + if (! $this->isAlwaysMatchingType($funcCallName, TypeCombinator::removeNull($comparedType))) { + return null; + } + + return $booleanOr->left; + } + + /** + * Handles "$items && is_array($items)", where is_array() can never fail on an array type + */ + private function refactorBooleanAnd(BooleanAnd $booleanAnd): ?Expr + { + if (! $booleanAnd->left instanceof Variable) { + return null; + } + + // the docblock type can be wider than the real value + if ($this->exprAnalyzer->isNonTypedFromParam($booleanAnd->left)) { + return null; + } + + $funcCall = $this->matchTypeCheckFuncCall($booleanAnd->right, $booleanAnd->left); + if (! $funcCall instanceof FuncCall) { + return null; + } + + $funcCallName = $this->getName($funcCall); + if ($funcCallName === null) { + return null; + } + + // the type is already narrowed by the truthy check on the left + $checkedType = $this->getNativeType($funcCall->getArgs()[0]->value); + if (! $this->isAlwaysMatchingType($funcCallName, $checkedType)) { + return null; + } + + return $booleanAnd->left; + } + + /** + * Matches "is_($expectedExpr)" single arg function call + */ + private function matchTypeCheckFuncCall(Expr $expr, Expr $expectedExpr): ?FuncCall + { + if (! $expr instanceof FuncCall) { + return null; + } + + if ($expr->isFirstClassCallable()) { + return null; + } + + if (count($expr->getArgs()) !== 1) { + return null; + } + + if (! $this->nodeComparator->areNodesEqual($expr->getArgs()[0]->value, $expectedExpr)) { + return null; + } + + return $expr; + } + + private function matchNullComparedExpr(Identical $identical): ?Expr + { + if ($this->valueResolver->isNull($identical->left)) { + return $identical->right; + } + + if ($this->valueResolver->isNull($identical->right)) { + return $identical->left; + } + + return null; + } + + private function isAlwaysMatchingType(string $funcCallName, Type $type): bool + { + return match ($funcCallName) { + 'is_string' => $type->isString() + ->yes(), + 'is_int', 'is_integer', 'is_long' => $type->isInteger() + ->yes(), + 'is_float', 'is_double' => $type->isFloat() + ->yes(), + 'is_bool' => $type->isBoolean() + ->yes(), + 'is_array' => $type->isArray() + ->yes(), + 'is_object' => $type->isObject() + ->yes(), + default => false, + }; + } +} diff --git a/src/Config/Level/DeadCodeLevel.php b/src/Config/Level/DeadCodeLevel.php index 81cded04455..6a9a706dc04 100644 --- a/src/Config/Level/DeadCodeLevel.php +++ b/src/Config/Level/DeadCodeLevel.php @@ -14,6 +14,7 @@ use Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector; use Rector\DeadCode\Rector\Assign\RemoveDoubleSelfAssignRector; use Rector\DeadCode\Rector\Assign\RemoveUnusedVariableAssignRector; +use Rector\DeadCode\Rector\BinaryOp\RemoveRedundantTypeCheckRector; use Rector\DeadCode\Rector\Block\ReplaceBlockToItsStmtsRector; use Rector\DeadCode\Rector\BooleanAnd\RemoveAndTrueRector; use Rector\DeadCode\Rector\Cast\RecastingRemovalRector; @@ -148,6 +149,7 @@ final class DeadCodeLevel RemoveAlwaysTrueIfConditionRector::class, ReduceAlwaysFalseIfOrRector::class, + RemoveRedundantTypeCheckRector::class, RemoveUnusedPrivateClassConstantRector::class, RemoveUnusedPrivatePropertyRector::class, RemoveUnusedClosureVariableUseRector::class,