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

Author: mnastalski

src/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector.php

@@ -5,6 +5,8 @@
 namespace RectorLaravel\Rector\ClassMethod;
 
 use PhpParser\Node;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
@@ -142,12 +144,36 @@ 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;
+        }
+
+        // Skip if method has no parameters
+        if (! isset($classMethod->params[0])) {
+            return false;
+        }
+
+        // Process if return type or first parameter has no type specified
+        if (
+            !$classMethod->returnType instanceof Node
+            || ($classMethod->params[0] instanceof Param && !$classMethod->params[0]->type instanceof Node)
+        ) {
             return true;
         }
 
-        return $this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Illuminate\Database\Eloquent\Attributes\Scope');
+        // Skip if the first parameter is not eloquent builder
+        if (! $this->isObjectType($classMethod->params[0], new ObjectType('Illuminate\Database\Eloquent\Builder'))) {
+            return false;
+        }
+
+        // Process if return type is void or eloquent builder
+        return ($classMethod->returnType instanceof Identifier && $classMethod->returnType->toString() === 'void')
+            || $this->isObjectType($classMethod->returnType, new ObjectType('Illuminate\Database\Eloquent\Builder'));
     }
 }

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

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

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

@@ -23,6 +23,26 @@ class User extends Model
         return $query;
     }
 
+    public function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
+    public function scopeParam($query): void
+    {
+        $query->where('active', false);
+    }
+
+    public function scopeNone($query)
+    {
+        $query->where('active', false);
+    }
+
+    public function scopeReturn(Builder $query)
+    {
+        $query->where('active', false);
+    }
+
     #[Scope]
     public function verified(Builder $query): Builder
     {
@@ -64,6 +84,26 @@ class User extends Model
         return $query;
     }
 
+    protected function scopeInactive(Builder $query): void
+    {
+        $query->where('active', false);
+    }
+
+    protected function scopeParam($query): void
+    {
+        $query->where('active', false);
+    }
+
+    protected function scopeNone($query)
+    {
+        $query->where('active', false);
+    }
+
+    protected function scopeReturn(Builder $query)
+    {
+        $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,45 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class SkipScopeMethodsNotScope extends Model
+{
+    public function scope(): string
+    {
+        return 'foo';
+    }
+
+    public function scopeItem(): BelongsTo
+    {
+        return $this->belongsTo(ScopeItem::class);
+    }
+
+    public function scopedItems(): HasMany
+    {
+        return $this->hasMany(ScopedItem::class);
+    }
+
+    public function scopeQuery(): Builder
+    {
+        return $this->where('foo', true);
+    }
+
+    public function scopeParam(string $param): Builder
+    {
+        return $this->where('foo', true);
+    }
+
+    public function scopeString(Builder $builder): string
+    {
+        return 'foo';
+    }
+
+    public function scopeVoid(): void {}
+}
+
+?>