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

[mnastalski]

Hey,

Expanding on #380, I would like to add more checks to MakeModelAttributesAndScopesProtectedRector to avoid some methods
that aren't scopes being picked up as ones. Right now if you have a method like scopeItem or scopedItem it's going to get picked up and erroneously have its visibility changed.

I propose adding the following checks:

• first parameter must be of type Illuminate\Database\Eloquent\Builder
• return type must be Illuminate\Database\Eloquent\Builder or void

These are in line with the docs. Additionally, if a method has the #[Scope] attribute then it will be considered a scope unconditionally.

[calebdw]

Not everyone types the param and return types. Are you running into an actual issue or is this a premature optimization?

Right now if you have a method like scopeItem

Why would you have a method like this on a model if it's not a scope?

or scopedItem

I suppose this is valid, we could update the regex to /scope[A-Z]/ so that this is not matched

[mnastalski]

This is an actual issue - I have methods with names similar to the examples I gave and in my case they are relationships.

I would say that scope isn't that rare of a word to assume all methods that start with it are actually laravel scopes (and it doesn't break the framework if you use it as something else), although it's not very strict about it. For example, scopedItem will be a valid ditem() scope

[GeniJaho]

@mnastalski there's not much we can do for the case of scopeItem(), as by Laravel's conventions, it means it should be used as a scope. Doing the checks for the param and return types would limit the effectiveness of this rule.

For the other one though, scopedItem(), the regex proposed by @calebdw should do the trick.

[mnastalski]

@GeniJaho thoughts on making the types optional? I updated the PR with following:

• If name starts with scope... and param type or return type are missing then process as a scope
• If name starts with scope... and param types are specified then only process if param is Builder and return type Builder or void
• Process unconditionally if Scope attribute is used (unchanged)

I can update the regex on top of that but I noticed declaring scope1 or scope_ creates usable scopes as well (scopes('1') and scopes('_') respectively) so not sure what to think about that since it does seem a little silly