Fix Scope detection false positives

src/NodeAnalyzer/ScopeAnalyzer.php

@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RectorLaravel\NodeAnalyzer;
+
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Stmt\ClassMethod;
+use PHPStan\Type\ObjectType;
+use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\NodeTypeResolver\NodeTypeResolver;
+use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
+
+final readonly class ScopeAnalyzer
+{
+    private const string SCOPE_ATTRIBUTE = 'Illuminate\Database\Eloquent\Attributes\Scope';
+
+    private const string ELOQUENT_BUILDER = 'Illuminate\Database\Eloquent\Builder';
+
+    public function __construct(
+        private NodeNameResolver $nodeNameResolver,
+        private NodeTypeResolver $nodeTypeResolver,
+        private PhpAttributeAnalyzer $phpAttributeAnalyzer,
+    ) {}
+
+    /**
+     * Checks for the "scope" + uppercase char naming convention, a Builder-typed
+     * first parameter, and a void/Builder/untyped return.
+     */
+    public function isNamedScope(ClassMethod $classMethod): bool
+    {
+        $name = $this->nodeNameResolver->getName($classMethod);
+
+        if ($name === null || ! str_starts_with($name, 'scope') || strlen($name) <= 5 || ! ctype_upper($name[5])) {
+            return false;
+        }
+
+        return $this->hasBuilderFirstParameter($classMethod) && $this->hasScopeReturnType($classMethod);
+    }
+
+    /**
+     * Named scope OR #[Scope] attribute.
+     */
+    public function isScopeMethod(ClassMethod $classMethod): bool
+    {
+        return $this->isNamedScope($classMethod)
+            || $this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, self::SCOPE_ATTRIBUTE);
+    }
+
+    private function hasBuilderFirstParameter(ClassMethod $classMethod): bool
+    {
+        if ($classMethod->params === []) {
+            return false;
+        }
+
+        $firstParam = $classMethod->params[0];
+
+        if ($firstParam->type === null) {
+            return true;
+        }
+
+        return $this->nodeTypeResolver->isObjectType($firstParam->type, new ObjectType(self::ELOQUENT_BUILDER));
+    }
+
+    private function hasScopeReturnType(ClassMethod $classMethod): bool
+    {
+        if ($classMethod->returnType === null) {
+            return true;
+        }
+
+        if ($classMethod->returnType instanceof Identifier && $classMethod->returnType->toString() === 'void') {
+            return true;
+        }
+
+        return $this->nodeTypeResolver->isObjectType($classMethod->returnType, new ObjectType(self::ELOQUENT_BUILDER));
+    }
+}

src/Rector/ClassMethod/MakeModelAttributesAndScopesProtectedRector.php

@@ -9,10 +9,10 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\ObjectType;
-use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
 use Rector\PHPStan\ScopeFetcher;
 use Rector\Privatization\NodeManipulator\VisibilityManipulator;
 use RectorLaravel\AbstractRector;
+use RectorLaravel\NodeAnalyzer\ScopeAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -23,7 +23,7 @@ class MakeModelAttributesAndScopesProtectedRector extends AbstractRector
 {
     public function __construct(
         private readonly VisibilityManipulator $visibilityManipulator,
-        private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer
+        private readonly ScopeAnalyzer $scopeAnalyzer,
     ) {}
 
     public function getRuleDefinition(): RuleDefinition
@@ -136,12 +136,6 @@ private function isAttributeMethod(ClassMethod $classMethod): bool
 
     private function isScopeMethod(ClassMethod $classMethod): bool
     {
-        $name = $this->getName($classMethod);
-
-        if ((bool) preg_match('/^scope.+$/', $name)) {
-            return true;
-        }
-
-        return $this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Illuminate\Database\Eloquent\Attributes\Scope');
+        return $this->scopeAnalyzer->isScopeMethod($classMethod);
     }
 }

src/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector.php

@@ -15,6 +15,7 @@
 use PHPStan\Type\ObjectType;
 use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
 use RectorLaravel\AbstractRector;
+use RectorLaravel\NodeAnalyzer\ScopeAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -28,6 +29,7 @@ final class ScopeNamedClassMethodToScopeAttributedClassMethodRector extends Abst
     public function __construct(
         private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer,
         private readonly ReflectionProvider $reflectionProvider,
+        private readonly ScopeAnalyzer $scopeAnalyzer,
     ) {}
 
     public function getRuleDefinition(): RuleDefinition
@@ -87,13 +89,12 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            $name = $this->getName($classMethod);
-            // make sure it starts with scope and the next character is upper case
-            if (! str_starts_with($name, 'scope') || ! ctype_upper(substr($name, 5, 1))) {
+            if (! $this->scopeAnalyzer->isNamedScope($classMethod)) {
                 continue;
             }
 
-            $newName = lcfirst(str_replace('scope', '', $name));
+            $name = $this->getName($classMethod);
+            $newName = lcfirst(substr($name, 5));
 
             if ($classReflection->hasMethod($newName)) {
                 continue;

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

@@ -0,0 +1,29 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+
+class SkipNonScopeSignatures extends Model
+{
+    public function scopeString(Builder $query): string
+    {
+        return 'foo';
+    }
+
+    public function scopeParam(string $param): Builder
+    {
+        return $this->where('foo', true);
+    }
+
+    public function scopeVoid(): void {}
+
+    public function scopeItem(): BelongsTo
+    {
+        return $this->belongsTo(ScopeItem::class);
+    }
+}
+
+?>

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

@@ -0,0 +1,16 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class SkipRelationMethods extends Model
+{
+    public function scopeTypes(): HasMany
+    {
+        return $this->hasMany(ScopeType::class);
+    }
+}
+
+?>

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

@@ -6,7 +6,7 @@ use Illuminate\Database\Eloquent\Model;
 
 class SomeClass extends Model
 {
-    public function scopeSomeMethod()
+    public function scopeSomeMethod($query)
     {
 
     }
@@ -23,7 +23,7 @@ use Illuminate\Database\Eloquent\Model;
 class SomeClass extends Model
 {
     #[\Illuminate\Database\Eloquent\Attributes\Scope]
-    protected function someMethod()
+    protected function someMethod($query)
     {
 
     }

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/non_duplicate_nodes.php.inc

@@ -7,7 +7,7 @@ use Illuminate\Database\Eloquent\Model;
 class NonDuplicateAttributeNodes extends Model
 {
     #[\Illuminate\Database\Eloquent\Attributes\Scope]
-    protected function scopeSomeMethod()
+    protected function scopeSomeMethod($query)
     {
 
     }

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/non_method_duplicate.php.inc

@@ -6,7 +6,7 @@ use Illuminate\Database\Eloquent\Model;
 
 class NonDuplicateMethod extends Model
 {
-    public function scopeSomeMethod()
+    public function scopeSomeMethod($query)
     {
 
     }

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/scope_with_extra_parameters.php.inc

@@ -0,0 +1,34 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+class ScopeWithExtraParameters extends Model
+{
+    public function scopeOfType(Builder $query, string $type)
+    {
+        return $query->where('type', $type);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+class ScopeWithExtraParameters extends Model
+{
+    #[\Illuminate\Database\Eloquent\Attributes\Scope]
+    protected function ofType(Builder $query, string $type)
+    {
+        return $query->where('type', $type);
+    }
+}
+
+?>

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/skip_non_builder_first_param.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+
+class SkipNonBuilderFirstParam extends Model
+{
+    public function scopeCategory(string $category): void
+    {
+    }
+}
+
+?>

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/skip_non_scope_return_type.php.inc

@@ -0,0 +1,16 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+class SkipNonScopeReturnType extends Model
+{
+    public function scopeString(Builder $query): string
+    {
+        return 'foo';
+    }
+}
+
+?>

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/skip_relation_methods.php.inc

@@ -0,0 +1,16 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+
+class SkipRelationMethods extends Model
+{
+    public function scopeTypes(): HasMany
+    {
+        return $this->hasMany(ScopeType::class);
+    }
+}
+
+?>

tests/Rector/ClassMethod/ScopeNamedClassMethodToScopeAttributedClassMethodRector/Fixture/typed_builder_parameter.php.inc

@@ -0,0 +1,34 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+class TypedBuilderParameter extends Model
+{
+    public function scopeActive(Builder $query)
+    {
+        return $query->where('active', 1);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\ScopeNamedClassMethodToScopeAttributedClassMethodRector\Fixture;
+
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
+
+class TypedBuilderParameter extends Model
+{
+    #[\Illuminate\Database\Eloquent\Attributes\Scope]
+    protected function active(Builder $query)
+    {
+        return $query->where('active', 1);
+    }
+}
+
+?>