fix: FunctionLikeToFirstClassCallableRector

rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_chained_calls.php.inc

@@ -0,0 +1,10 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
+
+function ($foo)
+{
+    return FooBar::foo()->bar($foo);
+};
+
+fn ($foo) => FooBar::foo()->bar($foo);

rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_using_this_outside_object.php.inc

@@ -0,0 +1,16 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
+
+class SkipUsingThisOutsideObject
+{
+    public static function boot(): void
+    {
+        self::macro('foo', fn () => $this->values());
+        self::macro('foo', fn () => $this->values()->foo());
+    }
+
+    public static function macro(string $name, callable $callback): void
+    {
+    }
+}

rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/using_this_in_instance_method.php.inc

@@ -0,0 +1,37 @@
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
+
+class UsingThisInInstanceMethod
+{
+    public function test(): callable
+    {
+        return fn () => $this->values();
+    }
+
+    public function values(): array
+    {
+        return [];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
+
+class UsingThisInInstanceMethod
+{
+    public function test(): callable
+    {
+        return $this->values(...);
+    }
+
+    public function values(): array
+    {
+        return [];
+    }
+}
+
+?>

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;
     }