[Rules] Fixing Rector rules return values (#1867)

* [Rules] Fixing Rector rules return values

* check on ThrowWithPreviousExceptionRector

* [ci-review] Rector Rectify

* bug on use reference on return SimplifyUselessVariableRector

* update fixture

* update fixture

* Fixed closure use not by reference on SimplifyUselessVariableRector

* [ci-review] Rector Rectify

* remove unneeded ALREADY_SORTED flag on OptionalParametersAfterRequiredRector

* phpstan

* phpstan

* phpstan

* phpstan

* check on ArrayMergeOfNonArraysToSimpleArrayRector

* check return null on SimplifyFuncGetArgsCountRector

Co-authored-by: GitHub Action <action@github.com>

rules-tests/CodeQuality/Rector/Return_/SimplifyUselessVariableRector/Fixture/closure_use_not_by_reference.php.inc

@@ -0,0 +1,40 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Return_\SimplifyUselessVariableRector\Fixture;
+
+function ClosureUseNOTByReference()
+{
+    $isChanged = false;
+    (function ($x) use ($isChanged) {
+        $isChanged = true;
+        return $isChanged;
+    })('x');
+
+    if ($isChanged) {
+        return true;
+    }
+
+    return false;
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Return_\SimplifyUselessVariableRector\Fixture;
+
+function ClosureUseNOTByReference()
+{
+    $isChanged = false;
+    (function ($x) use ($isChanged) {
+        return true;
+    })('x');
+
+    if ($isChanged) {
+        return true;
+    }
+
+    return false;
+}
+
+?>

rules-tests/CodeQuality/Rector/Return_/SimplifyUselessVariableRector/Fixture/skip_closure_use_by_reference.php.inc

@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Return_\SimplifyUselessVariableRector\Fixture;
+
+function SkipClosureUseByReference()
+{
+    $isChanged = false;
+    (function ($x) use (&$isChanged) {
+        $isChanged = true;
+        return $isChanged;
+    })('x');
+
+    if ($isChanged) {
+        return true;
+    }
+
+    return false;
+}

rules/CodeQuality/Rector/Catch_/ThrowWithPreviousExceptionRector.php

@@ -96,14 +96,23 @@ CODE_SAMPLE
             return null;
         }
 
-        $this->traverseNodesWithCallable($node->stmts, function (Node $node) use ($caughtThrowableVariable): ?int {
+        $isChanged = false;
+        $this->traverseNodesWithCallable($node->stmts, function (Node $node) use (
+            $caughtThrowableVariable,
+            &$isChanged
+        ): ?int {
             if (! $node instanceof Throw_) {
                 return null;
             }
 
-            return $this->refactorThrow($node, $caughtThrowableVariable);
+            $isChanged = $this->refactorThrow($node, $caughtThrowableVariable);
+            return $isChanged;
         });
 
+        if (! (bool) $isChanged) {
+            return null;
+        }
+
         return $node;
     }
 

rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php

@@ -25,11 +25,6 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
  */
 final class OptionalParametersAfterRequiredRector extends AbstractRector
 {
-    /**
-     * @var string
-     */
-    private const ALREADY_SORTED = 'already_sorted';
-
     public function __construct(
         private readonly RequireOptionalParamResolver $requireOptionalParamResolver,
         private readonly ArgumentSorter $argumentSorter,
@@ -77,11 +72,6 @@ CODE_SAMPLE
      */
     public function refactor(Node $node): ?Node
     {
-        $isAlreadySorted = (bool) $node->getAttribute(self::ALREADY_SORTED, false);
-        if ($isAlreadySorted) {
-            return null;
-        }
-
         if ($node instanceof ClassMethod) {
             return $this->refactorClassMethod($node);
         }
@@ -115,7 +105,6 @@ CODE_SAMPLE
         );
 
         $classMethod->params = $newParams;
-        $classMethod->setAttribute(self::ALREADY_SORTED, true);
 
         return $classMethod;
     }
@@ -137,7 +126,6 @@ CODE_SAMPLE
         }
 
         $new->args = $this->argumentSorter->sortArgsByExpectedParamOrder($new->args, $expectedArgOrParamOrder);
-        $new->setAttribute(self::ALREADY_SORTED, true);
 
         return $new;
     }
@@ -164,8 +152,6 @@ CODE_SAMPLE
         }
 
         $methodCall->args = $newArgs;
-        $methodCall->setAttribute(self::ALREADY_SORTED, true);
-
         return $methodCall;
     }
 

rules/CodeQuality/Rector/FuncCall/ArrayMergeOfNonArraysToSimpleArrayRector.php

@@ -76,6 +76,7 @@ CODE_SAMPLE
         }
 
         $array = new Array_();
+        $isAssigned = false;
         foreach ($node->args as $arg) {
             // found non Arg? return early
             if (! $arg instanceof Arg) {
@@ -93,9 +94,14 @@ CODE_SAMPLE
                 }
 
                 $array->items[] = new ArrayItem($nestedArrayItemItem->value, $nestedArrayItemItem->key);
+                $isAssigned = true;
             }
         }
 
+        if (! $isAssigned) {
+            return null;
+        }
+
         return $array;
     }
 }

rules/CodeQuality/Rector/FuncCall/SimplifyFuncGetArgsCountRector.php

@@ -53,7 +53,7 @@ final class SimplifyFuncGetArgsCountRector extends AbstractRector
         $innerFuncCall = $node->args[0]->value;
 
         if (! $this->isName($innerFuncCall, 'func_get_args')) {
-            return $node;
+            return null;
         }
 
         return $this->nodeFactory->createFuncCall('func_num_args');

rules/CodeQuality/Rector/Return_/SimplifyUselessVariableRector.php

@@ -157,7 +157,11 @@ CODE_SAMPLE
             return true;
         }
 
-        return $this->callAnalyzer->isNewInstance($previousNode->var);
+        if ($this->callAnalyzer->isNewInstance($previousNode->var)) {
+            return true;
+        }
+
+        return $this->variableAnalyzer->isUsedByReference($variableNode);
     }
 
     private function hasSomeComment(Expr $expr): bool

rules/DowngradePhp81/Rector/FunctionLike/DowngradePureIntersectionTypeRector.php

@@ -65,6 +65,7 @@ CODE_SAMPLE
      */
     public function refactor(Node $node): ?Node
     {
+        $paramDecorated = false;
         foreach ($node->getParams() as $param) {
             if (! $param->type instanceof IntersectionType) {
                 continue;
@@ -75,9 +76,14 @@ CODE_SAMPLE
                 $node,
                 [\PHPStan\Type\IntersectionType::class]
             );
+            $paramDecorated = true;
         }
 
         if (! $node->returnType instanceof IntersectionType) {
+            if ($paramDecorated) {
+                return $node;
+            }
+
             return null;
         }
 

src/NodeAnalyzer/VariableAnalyzer.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Rector\Core\NodeAnalyzer;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\ClosureUse;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt\Global_;
 use PhpParser\Node\Stmt\Static_;
@@ -53,6 +54,28 @@ final class VariableAnalyzer
         });
     }
 
+    public function isUsedByReference(Variable $variable): bool
+    {
+        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $subNode) use (
+            $variable
+        ): bool {
+            if (! $subNode instanceof Variable) {
+                return false;
+            }
+
+            if (! $this->nodeComparator->areNodesEqual($subNode, $variable)) {
+                return false;
+            }
+
+            $parent = $subNode->getAttribute(AttributeKey::PARENT_NODE);
+            if (! $parent instanceof ClosureUse) {
+                return false;
+            }
+
+            return $parent->byRef;
+        });
+    }
+
     private function isParentStaticOrGlobal(Variable $variable): bool
     {
         $parentNode = $variable->getAttribute(AttributeKey::PARENT_NODE);