diff --git a/abz/UnderElse.php b/abz/UnderElse.php deleted file mode 100644 index 2ec02423916f..000000000000 --- a/abz/UnderElse.php +++ /dev/null @@ -1,14 +0,0 @@ -expr = new Concat(new Node\Scalar\MagicConst\Dir(), $node->expr); + $node->expr = new Concat(new Dir(), $node->expr); return $node; } diff --git a/packages/CodeQuality/src/Rector/Ternary/ArrayKeyExistsTernaryThenValueToCoalescingRector.php b/packages/CodeQuality/src/Rector/Ternary/ArrayKeyExistsTernaryThenValueToCoalescingRector.php index bf3b2d085df9..2c3fd1ffa03c 100644 --- a/packages/CodeQuality/src/Rector/Ternary/ArrayKeyExistsTernaryThenValueToCoalescingRector.php +++ b/packages/CodeQuality/src/Rector/Ternary/ArrayKeyExistsTernaryThenValueToCoalescingRector.php @@ -95,11 +95,6 @@ private function areArrayKeysExistsArgsMatchingDimFetch(FuncCall $funcCall, Arra if (! $this->areNodesEqual($arrayDimFetch->var, $valuesExpr)) { return false; } - - if (! $this->areNodesEqual($arrayDimFetch->dim, $keyExpr)) { - return false; - } - - return true; + return $this->areNodesEqual($arrayDimFetch->dim, $keyExpr); } } diff --git a/packages/DeadCode/src/Rector/Ternary/TernaryToBooleanOrFalseToBooleanAndRector.php b/packages/DeadCode/src/Rector/Ternary/TernaryToBooleanOrFalseToBooleanAndRector.php index 6f57c1b2e579..dd78d416ea95 100644 --- a/packages/DeadCode/src/Rector/Ternary/TernaryToBooleanOrFalseToBooleanAndRector.php +++ b/packages/DeadCode/src/Rector/Ternary/TernaryToBooleanOrFalseToBooleanAndRector.php @@ -5,6 +5,7 @@ namespace Rector\DeadCode\Rector\Ternary; use PhpParser\Node; +use PhpParser\Node\Expr\BinaryOp\BooleanAnd; use PhpParser\Node\Expr\Ternary; use PHPStan\Type\BooleanType; use Rector\Rector\AbstractRector; @@ -80,6 +81,6 @@ public function refactor(Node $node): ?Node return null; } - return new Node\Expr\BinaryOp\BooleanAnd($node->cond, $node->if); + return new BooleanAnd($node->cond, $node->if); } } diff --git a/packages/Doctrine/src/Rector/Class_/AddEntityIdByConditionRector.php b/packages/Doctrine/src/Rector/Class_/AddEntityIdByConditionRector.php index 3f1336e7ec01..06556b4cab3b 100644 --- a/packages/Doctrine/src/Rector/Class_/AddEntityIdByConditionRector.php +++ b/packages/Doctrine/src/Rector/Class_/AddEntityIdByConditionRector.php @@ -125,11 +125,6 @@ private function shouldSkip(Class_ $class): bool if (! $this->isTraitMatch($class)) { return true; } - - if ($this->classManipulator->hasPropertyName($class, 'id')) { - return true; - } - - return false; + return $this->classManipulator->hasPropertyName($class, 'id'); } } diff --git a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/SluggableBehaviorRector.php b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/SluggableBehaviorRector.php index b4f0cee401b6..41a8b5586122 100644 --- a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/SluggableBehaviorRector.php +++ b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/SluggableBehaviorRector.php @@ -6,6 +6,7 @@ use PhpParser\Node; use PhpParser\Node\Identifier; +use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Return_; use PHPStan\Type\ArrayType; @@ -130,7 +131,7 @@ public function refactor(Node $node): ?Node $this->classManipulator->addAsFirstTrait($node, 'Knp\DoctrineBehaviors\Model\Sluggable\SluggableTrait'); - $node->implements[] = new Node\Name\FullyQualified('Knp\DoctrineBehaviors\Contract\Entity\SluggableInterface'); + $node->implements[] = new FullyQualified('Knp\DoctrineBehaviors\Contract\Entity\SluggableInterface'); $this->addGetSluggableFieldsClassMethod($node, $slugFields); diff --git a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TranslationBehaviorRector.php b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TranslationBehaviorRector.php index 0812dc3f9856..ec2868d28871 100644 --- a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TranslationBehaviorRector.php +++ b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TranslationBehaviorRector.php @@ -240,7 +240,7 @@ private function dumpEntityTranslation(Class_ $class, array $translatedPropertyT throw new ShouldNotHappenException(); } - $classShortName = (string) $class->name . 'Translation'; + $classShortName = $class->name . 'Translation'; $filePath = dirname($fileInfo->getRealPath()) . DIRECTORY_SEPARATOR . $classShortName . '.php'; $namespace = $class->getAttribute(AttributeKey::PARENT_NODE); diff --git a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TreeBehaviorRector.php b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TreeBehaviorRector.php index 82de66591d63..1ff37fc94df5 100644 --- a/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TreeBehaviorRector.php +++ b/packages/DoctrineGedmoToKnplabs/src/Rector/Class_/TreeBehaviorRector.php @@ -205,11 +205,6 @@ private function shouldRemoveProperty(PhpDocInfo $phpDocInfo): bool if ($phpDocInfo->hasByType(TreeParentTagValueNode::class)) { return true; } - - if ($phpDocInfo->hasByType(TreeLevelTagValueNode::class)) { - return true; - } - - return false; + return $phpDocInfo->hasByType(TreeLevelTagValueNode::class); } } diff --git a/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php b/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php index 9ea961254863..cdb5f4355335 100644 --- a/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php +++ b/packages/Laravel/src/Rector/FuncCall/HelperFunctionToConstructorInjectionRector.php @@ -277,11 +277,6 @@ private function shouldSkipFuncCall(FuncCall $funcCall): bool if ($classMethod === null) { return true; } - - if ($classMethod->isStatic()) { - return true; - } - - return false; + return $classMethod->isStatic(); } } diff --git a/packages/Nette/src/Rector/ClassMethod/TemplateMagicAssignToExplicitVariableArrayRector.php b/packages/Nette/src/Rector/ClassMethod/TemplateMagicAssignToExplicitVariableArrayRector.php index ceaf9a02be03..513a4d602417 100644 --- a/packages/Nette/src/Rector/ClassMethod/TemplateMagicAssignToExplicitVariableArrayRector.php +++ b/packages/Nette/src/Rector/ClassMethod/TemplateMagicAssignToExplicitVariableArrayRector.php @@ -7,6 +7,7 @@ use Nette\Application\UI\Control; use Nette\Application\UI\Presenter; use PhpParser\Node; +use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Expression; use Rector\Nette\NodeFactory\ActionRenderFactory; @@ -108,7 +109,7 @@ public function refactor(Node $node): ?Node private function createRenderMethodCall( ClassMethod $classMethod, MagicTemplatePropertyCalls $magicTemplatePropertyCalls - ): Node\Expr\MethodCall { + ): MethodCall { if ($this->isObjectType($classMethod, Presenter::class)) { return $this->actionRenderFactory->createThisTemplateRenderMethodCall($magicTemplatePropertyCalls); } diff --git a/packages/Nette/src/TemplatePropertyAssignCollector.php b/packages/Nette/src/TemplatePropertyAssignCollector.php index 4b0a964a2880..c80cb52a874c 100644 --- a/packages/Nette/src/TemplatePropertyAssignCollector.php +++ b/packages/Nette/src/TemplatePropertyAssignCollector.php @@ -6,6 +6,8 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt\ClassMethod; @@ -56,11 +58,11 @@ public function collectTemplateFileNameVariablesAndNodesToRemove( $this->callableNodeTraverser->traverseNodesWithCallable( (array) $classMethod->stmts, function (Node $node): void { - if ($node instanceof Expr\MethodCall) { + if ($node instanceof MethodCall) { $this->collectTemplateFileExpr($node); } - if ($node instanceof Expr\Assign) { + if ($node instanceof Assign) { $this->collectVariableFromAssign($node); } } @@ -69,7 +71,7 @@ function (Node $node): void { return new MagicTemplatePropertyCalls($this->templateFileExpr, $this->templateVariables, $this->nodesToRemove); } - private function collectTemplateFileExpr(Expr\MethodCall $methodCall): void + private function collectTemplateFileExpr(MethodCall $methodCall): void { if ($this->nameResolver->isName($methodCall->name, 'render')) { if (isset($methodCall->args[0])) { @@ -85,7 +87,7 @@ private function collectTemplateFileExpr(Expr\MethodCall $methodCall): void } } - private function collectVariableFromAssign(Expr\Assign $assign): void + private function collectVariableFromAssign(Assign $assign): void { // $this->template = x if ($assign->var instanceof PropertyFetch) { diff --git a/packages/NodeTypeResolver/src/PhpDoc/NodeAnalyzer/DocBlockManipulator.php b/packages/NodeTypeResolver/src/PhpDoc/NodeAnalyzer/DocBlockManipulator.php index 9d0159a8182b..1043551a8bd4 100644 --- a/packages/NodeTypeResolver/src/PhpDoc/NodeAnalyzer/DocBlockManipulator.php +++ b/packages/NodeTypeResolver/src/PhpDoc/NodeAnalyzer/DocBlockManipulator.php @@ -293,12 +293,12 @@ public function addReturnTag(Node $node, Type $newType): void return; } - if ($node->getDocComment()) { + if ($node->getDocComment() !== null) { $phpDocInfo = $this->createPhpDocInfoFromNode($node); $returnTagValueNode = $phpDocInfo->getByType(ReturnTagValueNode::class); // overide existing type - if ($returnTagValueNode) { + if ($returnTagValueNode !== null) { $newPHPStanPhpDocType = $this->staticTypeMapper->mapPHPStanTypeToPHPStanPhpDocTypeNode($newType); $returnTagValueNode->type = $newPHPStanPhpDocType; @@ -663,12 +663,7 @@ private function areBothSameScalarType(Type $firstType, Type $secondType): bool if ($firstType instanceof FloatType && $secondType instanceof FloatType) { return true; } - - if ($firstType instanceof BooleanType && $secondType instanceof BooleanType) { - return true; - } - - return false; + return $firstType instanceof BooleanType && $secondType instanceof BooleanType; } private function areAliasedObjectMatchingFqnObject(Type $firstType, Type $secondType): bool diff --git a/packages/Php73/src/Rector/FuncCall/ArrayKeyFirstLastRector.php b/packages/Php73/src/Rector/FuncCall/ArrayKeyFirstLastRector.php index 7309243ae752..fe5141a56db6 100644 --- a/packages/Php73/src/Rector/FuncCall/ArrayKeyFirstLastRector.php +++ b/packages/Php73/src/Rector/FuncCall/ArrayKeyFirstLastRector.php @@ -129,11 +129,6 @@ private function shouldSkip(FuncCall $funcCall): bool if ($this->isAtLeastPhpVersion(PhpVersionFeature::ARRAY_KEY_FIRST_LAST)) { return false; } - - if (function_exists(self::ARRAY_KEY_FIRST) && function_exists(self::ARRAY_KEY_LAST)) { - return false; - } - - return true; + return ! (function_exists(self::ARRAY_KEY_FIRST) && function_exists(self::ARRAY_KEY_LAST)); } } diff --git a/packages/DeadCode/src/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector.php b/packages/SOLID/src/Rector/If_/RemoveAlwaysElseRector.php similarity index 55% rename from packages/DeadCode/src/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector.php rename to packages/SOLID/src/Rector/If_/RemoveAlwaysElseRector.php index 380b1e9fde14..a46d636d4828 100644 --- a/packages/DeadCode/src/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector.php +++ b/packages/SOLID/src/Rector/If_/RemoveAlwaysElseRector.php @@ -2,10 +2,9 @@ declare(strict_types=1); -namespace Rector\DeadCode\Rector\FunctionLike; +namespace Rector\SOLID\Rector\If_; use PhpParser\Node; -use PhpParser\Node\FunctionLike; use PhpParser\Node\Stmt\If_; use Rector\PhpParser\Node\Manipulator\IfManipulator; use Rector\Rector\AbstractRector; @@ -13,9 +12,9 @@ use Rector\RectorDefinition\RectorDefinition; /** - * @see \Rector\DeadCode\Tests\Rector\FunctionLike\RemoveUnusedElseForReturnedValueRector\RemoveUnusedElseForReturnedValueRectorTest + * @see \Rector\SOLID\Tests\Rector\If_\RemoveAlwaysElseRector\RemoveAlwaysElseRectorTest */ -final class RemoveUnusedElseForReturnedValueRector extends AbstractRector +final class RemoveAlwaysElseRector extends AbstractRector { /** * @var IfManipulator @@ -29,7 +28,7 @@ public function __construct(IfManipulator $ifManipulator) public function getDefinition(): RectorDefinition { - return new RectorDefinition('Remove if for last else, if previous values were awlways returned', [ + return new RectorDefinition('Remove if for last else, if previous values were throw', [ new CodeSample( <<<'PHP' class SomeClass @@ -37,7 +36,7 @@ class SomeClass public function run($value) { if ($value) { - return 5; + throw new \InvalidStateException; } else { return 10; } @@ -51,7 +50,7 @@ class SomeClass public function run($value) { if ($value) { - return 5; + throw new \InvalidStateException; } return 10; @@ -68,29 +67,23 @@ public function run($value) */ public function getNodeTypes(): array { - return [FunctionLike::class]; + return [If_::class]; } /** - * @param FunctionLike $node + * @param If_ $node */ public function refactor(Node $node): ?Node { - $this->traverseNodesWithCallable((array) $node->getStmts(), function (Node $node) { - if (! $node instanceof If_) { - return null; - } - - if (! $this->ifManipulator->isWithElseAlwaysReturnValue($node)) { - return null; - } + if (! $this->ifManipulator->isEarlyElse($node)) { + return null; + } - foreach ($node->else->stmts as $elseStmt) { - $this->addNodeAfterNode($elseStmt, $node); - } + foreach ($node->else->stmts as $elseStmt) { + $this->addNodeAfterNode($elseStmt, $node); + } - $node->else = null; - }); + $node->else = null; return $node; } diff --git a/packages/DeadCode/tests/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector/Fixture/fixture.php.inc b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/early_else.php.inc similarity index 55% rename from packages/DeadCode/tests/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector/Fixture/fixture.php.inc rename to packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/early_else.php.inc index 0c209411818f..384b639562d5 100644 --- a/packages/DeadCode/tests/Rector/FunctionLike/RemoveUnusedElseForReturnedValueRector/Fixture/fixture.php.inc +++ b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/early_else.php.inc @@ -1,8 +1,8 @@ +----- + diff --git a/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/for_continue.php.inc b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/for_continue.php.inc new file mode 100644 index 000000000000..b16eaa92f2ed --- /dev/null +++ b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/for_continue.php.inc @@ -0,0 +1,63 @@ + +----- + diff --git a/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc new file mode 100644 index 000000000000..5620b0fe833f --- /dev/null +++ b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc @@ -0,0 +1,40 @@ + +----- + diff --git a/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/skip_nested_throw.php.inc b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/skip_nested_throw.php.inc new file mode 100644 index 000000000000..586202a5f9a9 --- /dev/null +++ b/packages/SOLID/tests/Rector/If_/RemoveAlwaysElseRector/Fixture/skip_nested_throw.php.inc @@ -0,0 +1,33 @@ +addStatementToClassBeforeTypes($class, $trait, TraitUse::class, Property::class); } @@ -417,7 +418,7 @@ public function replaceTrait(Class_ $class, string $oldTrait, string $newTrait): continue; } - $traitUse->traits[$key] = new Name\FullyQualified($newTrait); + $traitUse->traits[$key] = new FullyQualified($newTrait); break; } } diff --git a/src/PhpParser/Node/Manipulator/IfManipulator.php b/src/PhpParser/Node/Manipulator/IfManipulator.php index 89afa753c553..b807971835bb 100644 --- a/src/PhpParser/Node/Manipulator/IfManipulator.php +++ b/src/PhpParser/Node/Manipulator/IfManipulator.php @@ -8,10 +8,12 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; +use PhpParser\Node\Stmt\Continue_; use PhpParser\Node\Stmt\Do_; use PhpParser\Node\Stmt\Else_; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; +use PhpParser\Node\Stmt\Throw_; use PhpParser\Node\Stmt\While_; use PhpParser\NodeTraverser; use Rector\PhpParser\NodeTraverser\CallableNodeTraverser; @@ -19,6 +21,16 @@ final class IfManipulator { + /** + * @var string[] + */ + private const ALLOWED_BREAKING_NODE_TYPES = [Return_::class, Throw_::class, Continue_::class]; + + /** + * @var string[] + */ + private const SCOPE_CHANGING_NODE_TYPES = [Do_::class, While_::class, If_::class, Else_::class]; + /** * @var BetterStandardPrinter */ @@ -111,14 +123,14 @@ public function matchIfValueReturnValue(If_ $ifNode): ?Expr return null; } - public function isWithElseAlwaysReturnValue(If_ $if): bool + public function isEarlyElse(If_ $if): bool { - if (! $this->isAlwaysReturnValue((array) $if->stmts)) { + if (! $this->isAlwayAllowedType((array) $if->stmts, self::ALLOWED_BREAKING_NODE_TYPES)) { return false; } foreach ($if->elseifs as $elseif) { - if (! $this->isAlwaysReturnValue((array) $elseif->stmts)) { + if (! $this->isAlwayAllowedType((array) $elseif->stmts, self::ALLOWED_BREAKING_NODE_TYPES)) { return false; } } @@ -127,7 +139,7 @@ public function isWithElseAlwaysReturnValue(If_ $if): bool return false; } - return $this->isAlwaysReturnValue((array) $if->else->stmts); + return true; } private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr @@ -147,12 +159,17 @@ private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return return null; } - private function isAlwaysReturnValue(array $stmts): bool + /** + * @param Node[] $stmts + * @param string[] $allowedTypes + */ + private function isAlwayAllowedType(array $stmts, array $allowedTypes): bool { $isAlwaysReturnValue = false; $this->callableNodeTraverser->traverseNodesWithCallable($stmts, function (Node $node) use ( - &$isAlwaysReturnValue + &$isAlwaysReturnValue, + $allowedTypes ) { if ($this->isScopeChangingNode($node)) { $isAlwaysReturnValue = false; @@ -160,15 +177,21 @@ private function isAlwaysReturnValue(array $stmts): bool return NodeTraverser::STOP_TRAVERSAL; } - if ($node instanceof Return_) { - if ($node->expr === null) { - $isAlwaysReturnValue = false; + foreach ($allowedTypes as $allowedType) { + if (is_a($node, $allowedType, true)) { + if ($allowedType === Return_::class) { + if ($node->expr === null) { + $isAlwaysReturnValue = false; - return NodeTraverser::STOP_TRAVERSAL; - } + return NodeTraverser::STOP_TRAVERSAL; + } + } - $isAlwaysReturnValue = true; + $isAlwaysReturnValue = true; + } } + + return null; }); return $isAlwaysReturnValue; @@ -176,9 +199,7 @@ private function isAlwaysReturnValue(array $stmts): bool private function isScopeChangingNode(Node $node): bool { - $scopeChangingNodes = [Do_::class, While_::class, If_::class, Else_::class]; - - foreach ($scopeChangingNodes as $scopeChangingNode) { + foreach (self::SCOPE_CHANGING_NODE_TYPES as $scopeChangingNode) { if (! is_a($node, $scopeChangingNode, true)) { continue; }