fix: deficiencies in the EloquentMagicMethodToQueryBuilderRector

src/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector.php

@@ -4,17 +4,18 @@
 
 namespace RectorLaravel\Rector\StaticCall;
 
-use Illuminate\Database\Eloquent\Builder as EloquentQueryBuilder;
+use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Query\Builder as QueryBuilder;
 use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Identifier;
-use PhpParser\Node\Name;
+use PHPStan\Analyser\OutOfClassScope;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\ReflectionProvider;
 use Rector\Contract\Rector\ConfigurableRectorInterface;
 use RectorLaravel\AbstractRector;
-use ReflectionException;
-use ReflectionMethod;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 use Webmozart\Assert\Assert;
@@ -31,6 +32,10 @@ final class EloquentMagicMethodToQueryBuilderRector extends AbstractRector imple
      */
     private array $excludeMethods = [];
 
+    public function __construct(
+        private readonly ReflectionProvider $reflectionProvider
+    ) {}
+
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition(
@@ -40,13 +45,15 @@ public function getRuleDefinition(): RuleDefinition
                     <<<'CODE_SAMPLE'
 use App\Models\User;
 
+$user = User::first();
 $user = User::find(1);
 CODE_SAMPLE
                     ,
                     <<<'CODE_SAMPLE'
 use App\Models\User;
 
-$user = User::query()->find(1);
+$user = User::query()->first();
+$user = User::find(1);
 CODE_SAMPLE
                     , [
                         self::EXCLUDE_METHODS => ['find'],
@@ -68,60 +75,50 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        $resolvedType = $this->nodeTypeResolver->getType($node->class);
-
-        $classNames = $resolvedType->getObjectClassNames();
-
-        if ($classNames === []) {
+        if (! $node->name instanceof Identifier) {
             return null;
         }
 
-        $className = $classNames[0];
-
-        $originalClassName = $this->getName($node->class); // like "self" or "App\Models\User"
+        $methodName = $node->name->toString();
 
-        if ($originalClassName === null) {
+        if (
+            $methodName === 'query' // short circuit
+            || in_array($methodName, $this->excludeMethods, true)
+        ) {
             return null;
         }
 
-        // does not extend Eloquent Model
-        if (! is_subclass_of($className, Model::class)) {
-            return null;
-        }
+        $resolvedType = $this->nodeTypeResolver->getType($node->class);
 
-        if (! $node->name instanceof Identifier) {
-            return null;
-        }
+        $classNames = $resolvedType->isClassString()->yes()
+            ? $resolvedType->getClassStringObjectType()->getObjectClassNames()
+            : $resolvedType->getObjectClassNames();
 
-        $methodName = $node->name->toString();
+        $classReflection = null;
 
-        // if not a magic method
-        if (! $this->isMagicMethod($className, $methodName)) {
-            return null;
-        }
+        foreach ($classNames as $className) {
+            if (! $this->reflectionProvider->hasClass($className)) {
+                continue;
+            }
 
-        // if method belongs to Eloquent Query Builder or Query Builder
-        if (! $this->isPublicMethod(EloquentQueryBuilder::class, $methodName) && ! $this->isPublicMethod(
-            QueryBuilder::class,
-            $methodName
-        )) {
-            return null;
-        }
+            $classReflection = $this->reflectionProvider->getClass($className);
 
-        if ($node->class instanceof Name) {
-            $staticCall = $this->nodeFactory->createStaticCall($originalClassName, 'query');
+            if (! $classReflection->is(Model::class)) {
+                continue;
+            }
+
+            break;
         }
 
-        if (! $node->class instanceof Name) {
-            $staticCall = new StaticCall($node->class, 'query');
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
         }
 
-        $methodCall = $this->nodeFactory->createMethodCall($staticCall, $methodName);
-        foreach ($node->args as $arg) {
-            $methodCall->args[] = $arg;
+        if (! $this->isMagicMethod($classReflection, $methodName)) {
+            return null;
         }
 
-        return $methodCall;
+        return new MethodCall(new StaticCall($node->class, 'query'), $node->name, $node->args);
     }
 
     /**
@@ -136,39 +133,45 @@ public function configure(array $configuration): void
         $this->excludeMethods = $excludeMethods;
     }
 
-    public function isMagicMethod(string $className, string $methodName): bool
+    private function isMagicMethod(ClassReflection $classReflection, string $methodName): bool
     {
-        if (in_array($methodName, $this->excludeMethods, true)) {
-            return false;
+        if (! $classReflection->hasMethod($methodName)) {
+            // if the class doesn't have the method then check if the method is a scope
+            if ($classReflection->hasMethod('scope' . ucfirst($methodName))) {
+                return true;
+            }
+
+            // otherwise, need to check if the method is directly on the EloquentBuilder or QueryBuilder
+            return $this->isPublicMethod(EloquentBuilder::class, $methodName)
+                || $this->isPublicMethod(QueryBuilder::class, $methodName);
         }
 
-        try {
-            $reflectionMethod = new ReflectionMethod($className, $methodName);
-        } catch (ReflectionException) {
-            return true; // method does not exist => is magic method
+        $extendedMethodReflection = $classReflection->getMethod($methodName, new OutOfClassScope);
+
+        if (! $extendedMethodReflection->isPublic() || $extendedMethodReflection->isStatic()) {
+            return false;
         }
 
-        return false; // not a magic method
+        $declaringClass = $extendedMethodReflection->getDeclaringClass();
+
+        // finally, make sure the method is on the builders or a subclass
+        return $declaringClass->is(EloquentBuilder::class) || $declaringClass->is(QueryBuilder::class);
     }
 
-    public function isPublicMethod(string $className, string $methodName): bool
+    private function isPublicMethod(string $className, string $methodName): bool
     {
-        try {
-            $reflectionMethod = new ReflectionMethod($className, $methodName);
+        if (! $this->reflectionProvider->hasClass($className)) {
+            return false;
+        }
 
-            // if not public
-            if (! $reflectionMethod->isPublic()) {
-                return false;
-            }
+        $classReflection = $this->reflectionProvider->getClass($className);
 
-            // if static
-            if ($reflectionMethod->isStatic()) {
-                return false;
-            }
-        } catch (ReflectionException) {
-            return false; // method does not exist => is magic method
+        if (! $classReflection->hasMethod($methodName)) {
+            return false;
         }
 
-        return true; // method exist
+        $extendedMethodReflection = $classReflection->getMethod($methodName, new OutOfClassScope);
+
+        return $extendedMethodReflection->isPublic() && ! $extendedMethodReflection->isStatic();
     }
 }

stubs/Illuminate/Database/Eloquent/Factories/Factory.php

@@ -6,4 +6,9 @@
     return;
 }
 
-abstract class Factory {}
+/** @template TModel of \Illuminate\Database\Eloquent\Model */
+abstract class Factory
+{
+    /** @var class-string<TModel> */
+    protected $model;
+}

stubs/Illuminate/Database/Query/Builder.php

@@ -22,6 +22,12 @@ public function orderBy($column, string $direction): static {}
      */
     public function orderByDesc($column): static {}
 
+    /**
+     * @param  \Illuminate\Contracts\Database\Query\Expression|string  $column
+     * @return mixed
+     */
+    public function max($column) {}
+
     protected function protectedMethodBelongsToQueryBuilder(): void {}
 
     private function privateMethodBelongsToQueryBuilder(): void {}

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/EloquentMagicMethodToQueryBuilderRectorTest.php

@@ -4,13 +4,10 @@
 
 namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector;
 
-use Illuminate\Database\Eloquent\Model;
 use Iterator;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 
-final class User extends Model {}
-
 final class EloquentMagicMethodToQueryBuilderRectorTest extends AbstractRectorTestCase
 {
     public static function provideData(): Iterator

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/factory_model.php.inc

@@ -0,0 +1,37 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use Illuminate\Database\Eloquent\Factories\Factory;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+/** @extends Factory<User> */
+class UserFactory extends Factory
+{
+    public function definition(): array
+    {
+        return [
+            'sort' => $this->model::max('sort') + 1,
+        ];
+    }
+}
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use Illuminate\Database\Eloquent\Factories\Factory;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+/** @extends Factory<User> */
+class UserFactory extends Factory
+{
+    public function definition(): array
+    {
+        return [
+            'sort' => $this->model::query()->max('sort') + 1,
+        ];
+    }
+}
+?>

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/fixture.php.inc

@@ -2,7 +2,7 @@
 
 namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
 
-use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\User;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
 
 class SomeController
 {
@@ -37,7 +37,7 @@ class SomeController
 
 namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
 
-use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\User;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
 
 class SomeController
 {

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/model_interface_intersection.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\FooInterface;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+function (User&FooInterface $user) {
+    $user::max('sort') + 1;
+};
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\FooInterface;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+function (User&FooInterface $user) {
+    $user::query()->max('sort') + 1;
+};
+
+?>

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/model_scope.php.inc

@@ -0,0 +1,17 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+User::foo();
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+User::query()->foo();
+?>

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/skip_method_matching_scope.php.inc

@@ -0,0 +1,7 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
+
+User::conflict();

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/unknown_model.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+
+function (string $class, Model $model) {
+    /** @var class-string<Model> $class */
+    $class::max('sort') + 1;
+    $model::max('sort') + 1;
+};
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+
+function (string $class, Model $model) {
+    /** @var class-string<Model> $class */
+    $class::query()->max('sort') + 1;
+    $model::query()->max('sort') + 1;
+};
+?>

tests/Rector/StaticCall/EloquentMagicMethodToQueryBuilderRector/Fixture/with_exclude_methods.php.inc

@@ -2,7 +2,7 @@
 
 namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
 
-use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\User;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
 
 class SomeController
 {
@@ -29,7 +29,7 @@ class SomeController
 
 namespace RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Fixture;
 
-use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\User;
+use RectorLaravel\Tests\Rector\StaticCall\EloquentMagicMethodToQueryBuilderRector\Source\User;
 
 class SomeController
 {