diff --git a/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Fixture/attribute.php.inc b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Fixture/attribute.php.inc new file mode 100644 index 00000000000..e632e542822 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Fixture/attribute.php.inc @@ -0,0 +1,27 @@ + +----- + diff --git a/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/SortAttributeNamedArgsRectorTest.php b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/SortAttributeNamedArgsRectorTest.php new file mode 100644 index 00000000000..66c21fe2de1 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/SortAttributeNamedArgsRectorTest.php @@ -0,0 +1,28 @@ +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/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Source/MyAttribute.php b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Source/MyAttribute.php new file mode 100644 index 00000000000..489494211a3 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector/Source/MyAttribute.php @@ -0,0 +1,11 @@ +withRules([SortAttributeNamedArgsRector::class]); diff --git a/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/attribute.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/attribute.php.inc deleted file mode 100644 index 924133c1dce..00000000000 --- a/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/attribute.php.inc +++ /dev/null @@ -1,27 +0,0 @@ - ------ - diff --git a/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/skip_attribute.php.inc b/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/skip_attribute.php.inc new file mode 100644 index 00000000000..722559061f8 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/FuncCall/SortNamedParamRector/Fixture/skip_attribute.php.inc @@ -0,0 +1,11 @@ + + */ + public function sortArgsToMatchReflectionParameters( + array $currentArgs, + FunctionReflection | MethodReflection $functionLikeReflection, + ): array { + $extendedParametersAcceptor = ParametersAcceptorSelector::combineAcceptors( + $functionLikeReflection->getVariants() + ); + + $parameters = $extendedParametersAcceptor->getParameters(); + + $order = []; + foreach ($parameters as $key => $parameter) { + $order[$parameter->getName()] = $key; + } + + $sortedArgs = []; + $toSortArgs = []; + foreach ($currentArgs as $currentArg) { + if (! $currentArg->name instanceof Identifier) { + $sortedArgs[] = $currentArg; + continue; + } + + $toSortArgs[] = $currentArg; + } + + usort( + $toSortArgs, + static function (Arg $arg1, Arg $arg2) use ($order): int { + /** @var Identifier $argName1 */ + $argName1 = $arg1->name; + /** @var Identifier $argName2 */ + $argName2 = $arg2->name; + + $order1 = $order[$argName1->name] ?? PHP_INT_MAX; + $order2 = $order[$argName2->name] ?? PHP_INT_MAX; + + return $order1 <=> $order2; + } + ); + + return [...$sortedArgs, ...$toSortArgs]; + } +} diff --git a/rules/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector.php b/rules/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector.php new file mode 100644 index 00000000000..3610c9659a9 --- /dev/null +++ b/rules/CodeQuality/Rector/Attribute/SortAttributeNamedArgsRector.php @@ -0,0 +1,102 @@ +args; + if (count($args) <= 1) { + return null; + } + + if (! $this->argsAnalyzer->hasNamedArg($args)) { + return null; + } + + $functionLikeReflection = $this->reflectionResolver->resolveConstructorReflectionFromAttribute($node); + if (! $functionLikeReflection instanceof MethodReflection) { + return null; + } + + $args = $this->namedArgsSorter->sortArgsToMatchReflectionParameters($args, $functionLikeReflection); + if ($node->args === $args) { + return null; + } + + $node->args = $args; + + return $node; + + } +} diff --git a/rules/CodeQuality/Rector/FuncCall/SortNamedParamRector.php b/rules/CodeQuality/Rector/FuncCall/SortNamedParamRector.php index ceefebd412d..1605646a474 100644 --- a/rules/CodeQuality/Rector/FuncCall/SortNamedParamRector.php +++ b/rules/CodeQuality/Rector/FuncCall/SortNamedParamRector.php @@ -5,17 +5,13 @@ namespace Rector\CodeQuality\Rector\FuncCall; use PhpParser\Node; -use PhpParser\Node\Arg; -use PhpParser\Node\Attribute; -use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\New_; use PhpParser\Node\Expr\StaticCall; -use PhpParser\Node\Identifier; use PHPStan\Reflection\FunctionReflection; use PHPStan\Reflection\MethodReflection; -use PHPStan\Reflection\ParametersAcceptorSelector; +use Rector\CodeQuality\NodeManipulator\NamedArgsSorter; use Rector\NodeAnalyzer\ArgsAnalyzer; use Rector\Rector\AbstractRector; use Rector\Reflection\ReflectionResolver; @@ -29,30 +25,27 @@ final class SortNamedParamRector extends AbstractRector { public function __construct( private readonly ReflectionResolver $reflectionResolver, - private readonly ArgsAnalyzer $argsAnalyzer + private readonly ArgsAnalyzer $argsAnalyzer, + private readonly NamedArgsSorter $namedArgsSorter, ) { } public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( - 'Sort named parameters usage in a function or method call', + 'Sort named arguments to match their order in a function or method call or class constructors', [ new CodeSample( <<<'CODE_SAMPLE' function run($foo = null, $bar = null, $baz = null) {} run(bar: $bar, foo: $foo); - -run($foo, baz: $baz, bar: $bar); CODE_SAMPLE , <<<'CODE_SAMPLE' function run($foo = null, $bar = null, $baz = null) {} run(foo: $foo, bar: $bar); - -run($foo, bar: $bar, baz: $baz); CODE_SAMPLE ), ] @@ -64,20 +57,19 @@ function run($foo = null, $bar = null, $baz = null) {} */ public function getNodeTypes(): array { - return [MethodCall::class, StaticCall::class, New_::class, FuncCall::class, Attribute::class]; + return [MethodCall::class, StaticCall::class, New_::class, FuncCall::class]; } /** - * @param MethodCall|StaticCall|New_|FuncCall|Attribute $node + * @param MethodCall|StaticCall|New_|FuncCall $node */ public function refactor(Node $node): ?Node { - if ($node instanceof CallLike && $node->isFirstClassCallable()) { + if ($node->isFirstClassCallable()) { return null; } - $args = $node instanceof Attribute ? $node->args : $node->getArgs(); - + $args = $node->getArgs(); if (count($args) <= 1) { return null; } @@ -88,8 +80,6 @@ public function refactor(Node $node): ?Node if ($node instanceof New_) { $functionLikeReflection = $this->reflectionResolver->resolveMethodReflectionFromNew($node); - } elseif ($node instanceof Attribute) { - $functionLikeReflection = $this->reflectionResolver->resolveMethodReflectionFromAttribute($node); } else { $functionLikeReflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($node); } @@ -98,7 +88,7 @@ public function refactor(Node $node): ?Node return null; } - $args = $this->sortNamedArguments($functionLikeReflection, $args); + $args = $this->namedArgsSorter->sortArgsToMatchReflectionParameters($args, $functionLikeReflection); if ($node->args === $args) { return null; } @@ -107,52 +97,4 @@ public function refactor(Node $node): ?Node return $node; } - - /** - * @param Arg[] $currentArgs - * @return Arg[] - */ - public function sortNamedArguments( - FunctionReflection | MethodReflection $functionLikeReflection, - array $currentArgs - ): array { - $extendedParametersAcceptor = ParametersAcceptorSelector::combineAcceptors( - $functionLikeReflection->getVariants() - ); - - $parameters = $extendedParametersAcceptor->getParameters(); - - $order = []; - foreach ($parameters as $key => $parameter) { - $order[$parameter->getName()] = $key; - } - - $sortedArgs = []; - $toSortArgs = []; - foreach ($currentArgs as $currentArg) { - if (! $currentArg->name instanceof Identifier) { - $sortedArgs[] = $currentArg; - continue; - } - - $toSortArgs[] = $currentArg; - } - - usort( - $toSortArgs, - static function (Arg $arg1, Arg $arg2) use ($order): int { - /** @var Identifier $argName1 */ - $argName1 = $arg1->name; - /** @var Identifier $argName2 */ - $argName2 = $arg2->name; - - $order1 = $order[$argName1->name] ?? PHP_INT_MAX; - $order2 = $order[$argName2->name] ?? PHP_INT_MAX; - - return $order1 <=> $order2; - } - ); - - return [...$sortedArgs, ...$toSortArgs]; - } } diff --git a/rules/CodingStyle/Guard/ArrowFunctionAndClosureFirstClassCallableGuard.php b/rules/CodingStyle/Guard/ArrowFunctionAndClosureFirstClassCallableGuard.php index 7b709d5e3f7..344361c3540 100644 --- a/rules/CodingStyle/Guard/ArrowFunctionAndClosureFirstClassCallableGuard.php +++ b/rules/CodingStyle/Guard/ArrowFunctionAndClosureFirstClassCallableGuard.php @@ -196,7 +196,9 @@ private function isChainedCall(FuncCall|MethodCall|StaticCall $callLike): bool { if ($callLike instanceof MethodCall) { return $callLike->var instanceof CallLike; - } elseif ($callLike instanceof StaticCall) { + } + + if ($callLike instanceof StaticCall) { return $callLike->class instanceof CallLike; } diff --git a/src/PhpParser/NodeVisitor/ParamDefaultNodeVisitor.php b/src/PhpParser/NodeVisitor/ParamDefaultNodeVisitor.php index 9f48c62aac0..68822bf50f1 100644 --- a/src/PhpParser/NodeVisitor/ParamDefaultNodeVisitor.php +++ b/src/PhpParser/NodeVisitor/ParamDefaultNodeVisitor.php @@ -24,11 +24,7 @@ public function enterNode(Node $node): ?Node return null; } - SimpleNodeTraverser::decorateWithAttributeValue( - $node->default, - AttributeKey::IS_PARAM_DEFAULT, - true - ); + SimpleNodeTraverser::decorateWithAttributeValue($node->default, AttributeKey::IS_PARAM_DEFAULT, true); return null; } diff --git a/src/Reflection/ReflectionResolver.php b/src/Reflection/ReflectionResolver.php index deb23cddf9f..dc7b2dd9b4c 100644 --- a/src/Reflection/ReflectionResolver.php +++ b/src/Reflection/ReflectionResolver.php @@ -244,7 +244,7 @@ public function resolveMethodReflectionFromNew(New_ $new): ?MethodReflection return $this->resolveMethodReflection($className, MethodName::CONSTRUCT, $scope); } - public function resolveMethodReflectionFromAttribute(Attribute $attribute): ?MethodReflection + public function resolveConstructorReflectionFromAttribute(Attribute $attribute): ?MethodReflection { $attributeClassType = $this->nodeTypeResolver->getType($attribute->name); $className = ClassNameFromObjectTypeResolver::resolve($attributeClassType);