diff --git a/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_chained_calls.php.inc b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_chained_calls.php.inc new file mode 100644 index 00000000000..6f97a27cede --- /dev/null +++ b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_chained_calls.php.inc @@ -0,0 +1,10 @@ +bar($foo); +}; + +fn ($foo) => FooBar::foo()->bar($foo); diff --git a/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_using_this_outside_object.php.inc b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_using_this_outside_object.php.inc new file mode 100644 index 00000000000..6fe94dd58e0 --- /dev/null +++ b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_using_this_outside_object.php.inc @@ -0,0 +1,16 @@ + $this->values()); + self::macro('foo', fn () => $this->values()->foo()); + } + + public static function macro(string $name, callable $callback): void + { + } +} diff --git a/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/using_this_in_instance_method.php.inc b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/using_this_in_instance_method.php.inc new file mode 100644 index 00000000000..8ba70f768d3 --- /dev/null +++ b/rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/using_this_in_instance_method.php.inc @@ -0,0 +1,37 @@ + $this->values(); + } + + public function values(): array + { + return []; + } +} + +?> +----- +values(...); + } + + public function values(): array + { + return []; + } +} + +?> diff --git a/rules/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector.php b/rules/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector.php index 4062dff72c3..d5579044d65 100644 --- a/rules/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector.php +++ b/rules/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector.php @@ -7,17 +7,21 @@ use PhpParser\Node; use PhpParser\Node\Arg; use PhpParser\Node\Expr\ArrowFunction; +use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\Closure; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; use PhpParser\Node\Param; use PhpParser\Node\Stmt\Return_; use PhpParser\Node\VariadicPlaceholder; +use PhpParser\NodeVisitor; +use PHPStan\Analyser\Scope; +use Rector\PHPStan\ScopeFetcher; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; -use Webmozart\Assert\Assert; /** * @see \Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\FunctionLikeToFirstClassCallableRectorTest @@ -49,155 +53,176 @@ public function getNodeTypes(): array /** * @param ArrowFunction|Closure $node */ - public function refactor(Node $node): null|StaticCall|MethodCall + public function refactor(Node $node): null|CallLike { - $extractedMethodCall = $this->extractMethodCallFromFuncLike($node); + $callLike = $this->extractCallLike($node); - if (! $extractedMethodCall instanceof MethodCall && ! $extractedMethodCall instanceof StaticCall) { + if ($callLike === null) { return null; } - if ($extractedMethodCall instanceof MethodCall) { - return new MethodCall($extractedMethodCall->var, $extractedMethodCall->name, [new VariadicPlaceholder()]); + if ($this->shouldSkip($node, $callLike, ScopeFetcher::fetch($node))) { + return null; } - return new StaticCall($extractedMethodCall->class, $extractedMethodCall->name, [new VariadicPlaceholder()]); - } + $callLike->args = [new VariadicPlaceholder()]; - private function extractMethodCallFromFuncLike(Closure|ArrowFunction $node): MethodCall|StaticCall|null - { - if ($node instanceof ArrowFunction) { - if ( - ($node->expr instanceof MethodCall || $node->expr instanceof StaticCall) && - ! $node->expr->isFirstClassCallable() && - $this->notUsingNamedArgs($node->expr->getArgs()) && - $this->notUsingByRef($node->getParams()) && - $this->sameParamsForArgs($node->getParams(), $node->expr->getArgs()) && - $this->isNonDependantMethod($node->expr, $node->getParams()) - ) { - return $node->expr; - } + return $callLike; + } - return null; - } + private function shouldSkip( + ArrowFunction|Closure $node, + FuncCall|MethodCall|StaticCall $callLike, + Scope $scope + ): bool { + $params = $node->getParams(); + $args = $callLike->getArgs(); - if (count($node->stmts) != 1 || ! $node->getStmts()[0] instanceof Return_) { - return null; + if ( + $callLike->isFirstClassCallable() + || $this->isChainedCall($callLike) + || $this->isUsingNamedArgs($args) + || $this->isUsingByRef($params) + || $this->isNotUsingSameParamsForArgs($params, $args) + || $this->isDependantMethod($callLike, $params) + || $this->isUsingThisInNonObjectContext($callLike, $scope) + ) { + return true; } - $callLike = $node->getStmts()[0] - ->expr; + return false; + } - if (! $callLike instanceof MethodCall && ! $callLike instanceof StaticCall) { - return null; + private function extractCallLike(Closure|ArrowFunction $node): FuncCall|MethodCall|StaticCall|null + { + if ($node instanceof Closure) { + if (count($node->stmts) !== 1 || ! $node->stmts[0] instanceof Return_) { + return null; + } + $callLike = $node->stmts[0]->expr; + } else { + $callLike = $node->expr; } if ( - ! $callLike->isFirstClassCallable() && - $this->notUsingNamedArgs($callLike->getArgs()) && - $this->notUsingByRef($node->getParams()) && - $this->sameParamsForArgs($node->getParams(), $callLike->getArgs()) && - $this->isNonDependantMethod($callLike, $node->getParams())) { - return $callLike; + ! $callLike instanceof FuncCall + && ! $callLike instanceof MethodCall + && ! $callLike instanceof StaticCall + ) { + return null; } - return null; + return $callLike; } /** - * @param Node\Param[] $params - * @param Node\Arg[] $args + * @param Param[] $params + * @param Arg[] $args */ - private function sameParamsForArgs(array $params, array $args): bool + private function isNotUsingSameParamsForArgs(array $params, array $args): bool { - Assert::allIsInstanceOf($args, Arg::class); - Assert::allIsInstanceOf($params, Param::class); - if (count($args) > count($params)) { - return false; + return true; } if (count($args) === 1 && $args[0]->unpack) { - return $params[0]->variadic; + return ! $params[0]->variadic; } foreach ($args as $key => $arg) { if (! $this->nodeComparator->areNodesEqual($arg->value, $params[$key]->var)) { - return false; + return true; } } - return true; + return false; } /** - * Makes sure the parameter isn't used to make the call e.g. in the var or class - * * @param Param[] $params */ - private function isNonDependantMethod(StaticCall|MethodCall $expr, array $params): bool + private function isDependantMethod(StaticCall|MethodCall|FuncCall $expr, array $params): bool { - Assert::allIsInstanceOf($params, Param::class); + if ($expr instanceof FuncCall) { + return false; + } $found = false; + $parentNode = $expr instanceof MethodCall ? $expr->var : $expr->class; foreach ($params as $param) { - if ($expr instanceof MethodCall) { - $this->traverseNodesWithCallable($expr->var, function (Node $node) use ($param, &$found): null { - if ($this->nodeComparator->areNodesEqual($node, $param->var)) { - $found = true; - } - - return null; - }); + $this->traverseNodesWithCallable($parentNode, function (Node $node) use ($param, &$found) { + if ($this->nodeComparator->areNodesEqual($node, $param->var)) { + $found = true; + return NodeVisitor::STOP_TRAVERSAL; + } + }); + + if ($found) { + return true; } + } - if ($expr instanceof StaticCall) { - $this->traverseNodesWithCallable($expr->class, function (Node $node) use ($param, &$found): null { - if ($this->nodeComparator->areNodesEqual($node, $param->var)) { - $found = true; - } + return false; + } - return null; - }); - } + private function isUsingThisInNonObjectContext(FuncCall|MethodCall|StaticCall $callLike, Scope $scope): bool + { + if (! $callLike instanceof MethodCall) { + return false; + } - if ($found) { - return false; - } + if (in_array('this', $scope->getDefinedVariables(), true)) { + return false; } - return true; + $found = false; + + $this->traverseNodesWithCallable($callLike, function (Node $node) use (&$found) { + if ($this->isName($node, 'this')) { + $found = true; + return NodeVisitor::STOP_TRAVERSAL; + } + }); + + return $found; } /** * @param Param[] $params */ - private function notUsingByRef(array $params): bool + private function isUsingByRef(array $params): bool { - Assert::allIsInstanceOf($params, Param::class); - foreach ($params as $param) { if ($param->byRef) { - return false; + return true; } } - - return true; + return false; } /** * @param Arg[] $args */ - private function notUsingNamedArgs(array $args): bool + private function isUsingNamedArgs(array $args): bool { - Assert::allIsInstanceOf($args, Arg::class); - foreach ($args as $arg) { if ($arg->name instanceof Identifier) { - return false; + return true; } } + return false; + } + + private function isChainedCall(FuncCall|MethodCall|StaticCall $callLike): bool + { + if (! $callLike instanceof MethodCall) { + return false; + } + + if (! $callLike->var instanceof CallLike) { + return false; + } return true; }