[CodeQuality] Add Function_ and FuncCall support on OptionalParameter…

…sAfterRequiredRector (#5835) * [CodeQuality] Add Function_ and FuncCall support on OptionalParametersAfterRequiredRector * rename method * clean up * cs fix * [ci-review] Rector Rectify * fix phpstan * test skip after 2nd try * skip native func call --------- Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Apr 20, 2024 · 1374e15 · 1374e15
1 parent b81ef2f
commit 1374e15
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 46 deletions.
diff --git a/...lity/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc b/...lity/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc
@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
+
+function foo($optional = 1, $required)
+{
+}
+
+foo($optional, $required);
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
+
+function foo($required, $optional = 1)
+{
+}
+
+foo($required, $optional);
+
+?>
diff --git a/...ptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc b/...ptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc
@@ -0,0 +1,9 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
+
+function skip_correct_optional_after_required($required, $optional = 1)
+{
+}
+
+skip_correct_optional_after_required($required, $optional);
diff --git a/...r/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_native_func_call.php.inc b/...r/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_native_func_call.php.inc
@@ -0,0 +1,5 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
+
+implode(['foo'], '');
diff --git a/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php b/rules/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector.php
@@ -5,11 +5,14 @@
 namespace Rector\CodeQuality\Rector\ClassMethod;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
 use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\MethodReflection;
 use Rector\CodingStyle\Reflection\VendorLocationDetector;
 use Rector\NodeTypeResolver\PHPStan\ParametersAcceptorSelectorVariantsWrapper;
@@ -71,59 +74,68 @@ public function run($required, $optional = 1)
      */
     public function getNodeTypes(): array
     {
-        return [ClassMethod::class, New_::class, MethodCall::class, StaticCall::class];
+        return [
+            ClassMethod::class,
+            Function_::class,
+            New_::class,
+            MethodCall::class,
+            StaticCall::class,
+            FuncCall::class,
+        ];
     }
 
     /**
-     * @param ClassMethod|New_|MethodCall|StaticCall $node
+     * @param ClassMethod|Function_|New_|MethodCall|StaticCall|FuncCall $node
      */
-    public function refactorWithScope(Node $node, Scope $scope): ClassMethod|null|New_|MethodCall|StaticCall
-    {
-        if ($node instanceof ClassMethod) {
-            return $this->refactorClassMethod($node, $scope);
+    public function refactorWithScope(
+        Node $node,
+        Scope $scope
+    ): ClassMethod|Function_|null|New_|MethodCall|StaticCall|FuncCall {
+        if ($node instanceof ClassMethod || $node instanceof Function_) {
+            return $this->refactorClassMethodOrFunction($node, $scope);
         }
 
         if ($node instanceof New_) {
             return $this->refactorNew($node, $scope);
         }
 
-        return $this->refactorMethodCall($node, $scope);
+        return $this->refactorMethodCallOrFuncCall($node, $scope);
     }
 
-    private function refactorClassMethod(ClassMethod $classMethod, Scope $scope): ?ClassMethod
-    {
-        if ($classMethod->params === []) {
+    private function refactorClassMethodOrFunction(
+        ClassMethod|Function_ $node,
+        Scope $scope
+    ): ClassMethod|Function_|null {
+        if ($node->params === []) {
             return null;
         }
 
-        if ($classMethod->getAttribute(self::HAS_SWAPPED_PARAMS, false) === true) {
+        if ($node->getAttribute(self::HAS_SWAPPED_PARAMS, false) === true) {
             return null;
         }
 
-        $classMethodReflection = $this->reflectionResolver->resolveMethodReflectionFromClassMethod(
-            $classMethod,
-            $scope
-        );
-        if (! $classMethodReflection instanceof MethodReflection) {
+        if ($node instanceof ClassMethod) {
+            $reflection = $this->reflectionResolver->resolveMethodReflectionFromClassMethod($node, $scope);
+        } else {
+            $reflection = $this->reflectionResolver->resolveFunctionReflectionFromFunction($node, $scope);
+        }
+
+        if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) {
             return null;
         }
 
-        $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent(
-            $classMethodReflection,
-            $classMethod,
-            $scope
-        );
+        $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope);
         if ($expectedArgOrParamOrder === null) {
             return null;
         }
 
-        $classMethod->params = $this->argumentSorter->sortArgsByExpectedParamOrder(
-            $classMethod->params,
+        $node->params = $this->argumentSorter->sortArgsByExpectedParamOrder(
+            $node->params,
             $expectedArgOrParamOrder
         );
 
-        $classMethod->setAttribute(self::HAS_SWAPPED_PARAMS, true);
-        return $classMethod;
+        $node->setAttribute(self::HAS_SWAPPED_PARAMS, true);
+        return $node;
     }
 
     private function refactorNew(New_ $new, Scope $scope): ?New_
@@ -151,52 +163,51 @@ private function refactorNew(New_ $new, Scope $scope): ?New_
         return $new;
     }
 
-    private function refactorMethodCall(MethodCall|StaticCall $methodCall, Scope $scope): MethodCall|StaticCall|null
-    {
-        if ($methodCall->isFirstClassCallable()) {
+    private function refactorMethodCallOrFuncCall(
+        MethodCall|StaticCall|FuncCall $node,
+        Scope $scope
+    ): MethodCall|StaticCall|FuncCall|null {
+        if ($node->isFirstClassCallable()) {
             return null;
         }
 
-        $methodReflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($methodCall);
-        if (! $methodReflection instanceof MethodReflection) {
+        $reflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node);
+        if (! $reflection instanceof MethodReflection && ! $reflection instanceof FunctionReflection) {
             return null;
         }
 
-        $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent(
-            $methodReflection,
-            $methodCall,
-            $scope
-        );
+        $expectedArgOrParamOrder = $this->resolveExpectedArgParamOrderIfDifferent($reflection, $node, $scope);
         if ($expectedArgOrParamOrder === null) {
             return null;
         }
 
-        $newArgs = $this->argumentSorter->sortArgsByExpectedParamOrder(
-            $methodCall->getArgs(),
-            $expectedArgOrParamOrder
-        );
+        $newArgs = $this->argumentSorter->sortArgsByExpectedParamOrder($node->getArgs(), $expectedArgOrParamOrder);
 
-        if ($methodCall->args === $newArgs) {
+        if ($node->args === $newArgs) {
             return null;
         }
 
-        $methodCall->args = $newArgs;
-        return $methodCall;
+        $node->args = $newArgs;
+        return $node;
     }
 
     /**
      * @return int[]|null
      */
     private function resolveExpectedArgParamOrderIfDifferent(
-        MethodReflection $methodReflection,
-        New_|MethodCall|ClassMethod|StaticCall $node,
+        MethodReflection|FunctionReflection $reflection,
+        New_|MethodCall|ClassMethod|Function_|StaticCall|FuncCall $node,
         Scope $scope
     ): ?array {
-        if ($this->vendorLocationDetector->detectMethodReflection($methodReflection)) {
+        if ($reflection instanceof MethodReflection && $this->vendorLocationDetector->detectMethodReflection($reflection)) {
+            return null;
+        }
+
+        if ($reflection instanceof FunctionReflection && $this->vendorLocationDetector->detectFunctionReflection($reflection)) {
             return null;
         }
 
-        $parametersAcceptor = ParametersAcceptorSelectorVariantsWrapper::select($methodReflection, $node, $scope);
+        $parametersAcceptor = ParametersAcceptorSelectorVariantsWrapper::select($reflection, $node, $scope);
         $expectedParameterReflections = $this->requireOptionalParamResolver->resolveFromParametersAcceptor(
             $parametersAcceptor
         );

diff --git a/rules/CodingStyle/Reflection/VendorLocationDetector.php b/rules/CodingStyle/Reflection/VendorLocationDetector.php
@@ -4,6 +4,7 @@
 
 namespace Rector\CodingStyle\Reflection;
 
+use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\MethodReflection;
 use Rector\FileSystem\FilePathHelper;
 
@@ -19,6 +20,18 @@ public function detectMethodReflection(MethodReflection $methodReflection): bool
         $declaringClassReflection = $methodReflection->getDeclaringClass();
         $fileName = $declaringClassReflection->getFileName();
 
+        return $this->detect($fileName);
+    }
+
+    public function detectFunctionReflection(FunctionReflection $functionReflection): bool
+    {
+        $fileName = $functionReflection->getFileName();
+
+        return $this->detect($fileName);
+    }
+
+    private function detect(?string $fileName = null): bool
+    {
         // probably internal
         if ($fileName === null) {
             return false;

diff --git a/src/Reflection/ReflectionResolver.php b/src/Reflection/ReflectionResolver.php
@@ -16,6 +16,7 @@
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\FunctionReflection;
@@ -180,6 +181,21 @@ public function resolveMethodReflectionFromClassMethod(ClassMethod $classMethod,
         return $this->resolveMethodReflection($className, $methodName, $scope);
     }
 
+    public function resolveFunctionReflectionFromFunction(Function_ $function, Scope $scope): ?FunctionReflection
+    {
+        $name = $this->nodeNameResolver->getName($function);
+        if ($name === null) {
+            return null;
+        }
+
+        $functionName = new Name($name);
+        if ($this->reflectionProvider->hasFunction($functionName, $scope)) {
+            return $this->reflectionProvider->getFunction($functionName, $scope);
+        }
+
+        return null;
+    }
+
     public function resolveMethodReflectionFromNew(New_ $new): ?MethodReflection
     {
         $newClassType = $this->nodeTypeResolver->getType($new->class);