From 1374e15d742f4f36f0bef45db9f8920a589865b3 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Apr 2024 17:59:14 +0700 Subject: [PATCH] [CodeQuality] Add Function_ and FuncCall support on OptionalParametersAfterRequiredRector (#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 --- .../Fixture/on_function.php.inc | 23 ++++ ...ip_correct_optional_after_required.php.inc | 9 ++ .../Fixture/skip_native_func_call.php.inc | 5 + .../OptionalParametersAfterRequiredRector.php | 103 ++++++++++-------- .../Reflection/VendorLocationDetector.php | 13 +++ src/Reflection/ReflectionResolver.php | 16 +++ 6 files changed, 123 insertions(+), 46 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_native_func_call.php.inc diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc new file mode 100644 index 00000000000..f1f4a248b76 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/on_function.php.inc @@ -0,0 +1,23 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc new file mode 100644 index 00000000000..499019a6c0e --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_correct_optional_after_required.php.inc @@ -0,0 +1,9 @@ +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 index 043864ccda2..cc0b665cadb 100644 --- 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 index 66b2b517172..c09582eb387 100644 --- 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);