isRelation checks only if a method exists but not if a relation is returned

[denny-bee]

Laravel Version

10

PHP Version

8.2

Database Driver & Version

MySql

Description

HasAttributes.php has the method isRelation.

This method only checks if there is no mutator with the same name and if there is a method with this name.

But if you have some helper methods which are not accessors ("getXyzAttribute" or "xyz(): Attribute"), it will still say it is an relation, which is not correct.

In my oppinion there should be a check for the return type like it is used in "hasAttributeMutator":

$returnType = (new ReflectionMethod($this, $method))->getReturnType();

return static::$attributeMutatorCache[get_class($this)][$key] =
                    $returnType instanceof ReflectionNamedType &&
                    $returnType->getName() === Attribute::class;

If there are some helper methods which are not relations but return the same type, they could be defined in a property and excluded by this check to support third party packages and custom code.

"Illuminate\Database\Eloquent\Relations\Relation" could be the type instead of Attribute::class in this case.

But with is_a instead of a basic check to include classes which extend Relation::class.

Or is there any other reason why this was not implemented this way?

Steps To Reproduce

1. Create an model
2. Add a helper method e.g. pizza() this model
3. isRelation('pizza') on a object of this model would result in true, but it is not an relation

[crynobone]

Can you create a failing test PR for this use case?

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[driesvints]

I tried to solve this by resolving the method and doing a type check here but unfortunately that broke a few tests: #51593

If you could send in a PR that doesn't breaks existing tests we could maybe consider it. Thanks!

[staudenmeir]

This topic is quite complex, unfortunately. Resolving the method can have unintended consequences, e.g. when doing something like $user->isRelation('delete').

We can use the approach from the model:show command that checks the method's actual code before resolving it:

framework/src/Illuminate/Database/Console/ShowModelCommand.php

Line 191 in 8fde0ea

protected function getRelations($model)

This is much safer. The only downside I know is that it doesn't detect third-party relations.

[driesvints]

@staudenmeir good call