fix: skip more scope methods that aren't scopes based on types

Author: mnastalski

src/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector.php

@@ -5,6 +5,7 @@
 namespace RectorLaravel\Rector\ClassMethod;
 
 use PhpParser\Node;
+use PhpParser\Node\Identifier;
 use PhpParser\Node\Stmt\ClassMethod;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
@@ -142,12 +143,28 @@ private function isAttributeMethod(ClassMethod $classMethod): bool
 
     private function isScopeMethod(ClassMethod $classMethod): bool
     {
+        if ($this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Illuminate\Database\Eloquent\Attributes\Scope')) {
+            return true;
+        }
+
         $name = $this->getName($classMethod);
 
-        if ((bool) preg_match('/^scope.+$/', $name)) {
+        if (! (bool) preg_match('/^scope.+$/', $name)) {
+            return false;
+        }
+
+        if (! isset($classMethod->params[0]) || ! $classMethod->params[0] instanceof Node) {
+            return false;
+        }
+
+        if (! $this->isObjectType($classMethod->params[0], new ObjectType('Illuminate\Database\Eloquent\Builder'))) {
+            return false;
+        }
+
+        if ($classMethod->returnType instanceof Identifier && $classMethod->returnType->toString() === 'void') {
             return true;
         }
 
-        return $this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Illuminate\Database\Eloquent\Attributes\Scope');
+        return $classMethod->returnType instanceof Node && $this->isObjectType($classMethod->returnType, new ObjectType('Illuminate\Database\Eloquent\Builder'));
     }
 }

tests/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector/Fixture/final_model.php.inc

@@ -23,6 +23,11 @@ final class User extends Model
         return $query;
     }
 
+    private function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
     #[Scope]
     private function verified(Builder $query): Builder
     {
@@ -57,6 +62,11 @@ final class User extends Model
         return $query;
     }
 
+    protected function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
     #[Scope]
     protected function verified(Builder $query): Builder
     {

tests/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector/Fixture/fixture.php.inc

@@ -23,6 +23,11 @@ class User extends Model
         return $query;
     }
 
+    public function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
     #[Scope]
     public function verified(Builder $query): Builder
     {
@@ -64,6 +69,11 @@ class User extends Model
         return $query;
     }
 
+    protected function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
     #[Scope]
     protected function verified(Builder $query): Builder
     {

tests/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector/Fixture/skip_scope_method_not_scope.php.inc

@@ -0,0 +1,36 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class SkipScopeMethodsNotScope extends Model
+{
+    public function scope(): string
+    {
+        return 'foo';
+    }
+
+    public function scopedItems(): HasMany
+    {
+        return $this->hasMany(ScopedItem::class);
+    }
+
+    public function scopeQuery(): Builder
+    {
+        return $this->where('foo', true);
+    }
+
+    public function scopeString(Builder $builder): string
+    {
+        return 'foo';
+    }
+
+    public function scopeVoid(): void {}
+
+    public function scopeNoReturn(Builder $builder) {}
+}
+
+?>