Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support Relation->orderBy() #191

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions src/Handlers/Eloquent/RelationsMethodHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
use Illuminate\Database\Query\Builder as QueryBuilder;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Identifier;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Internal\MethodIdentifier;
use Psalm\LaravelPlugin\Util\ProxyMethodReturnTypeProvider;
use Psalm\Plugin\EventHandler\Event\MethodReturnTypeProviderEvent;
use Psalm\Plugin\EventHandler\MethodReturnTypeProviderInterface;
use Psalm\Type;
use Psalm\Type\Union;
use function strtolower;

final class RelationsMethodHandler implements MethodReturnTypeProviderInterface
{
Expand Down Expand Up @@ -57,14 +59,55 @@ public static function getMethodReturnType(MethodReturnTypeProviderEvent $event)

// If this method name is on the builder object, proxy it over there

if ($source->getCodebase()->methods->methodExists(new MethodIdentifier(Builder::class, $method_name_lowercase)) ||
$source->getCodebase()->methods->methodExists(new MethodIdentifier(QueryBuilder::class, $method_name_lowercase))
) {
if ($method_name_lowercase === '__call') {
$stmt = $event->getStmt();

if (!($stmt instanceof MethodCall)) {
return null;
}

$name = $stmt->name;

if (!($name instanceof Identifier)) {
return null;
}

$codebase = $source->getCodebase();
/** @psalm-var lowercase-string $called_method_name_lowercase */
$called_method_name_lowercase = $name->toLowerString();
$type = null;

foreach ([Builder::class, QueryBuilder::class] as $class) {
$method_id = new MethodIdentifier($class, $called_method_name_lowercase);
if ($codebase->methods->methodExists($method_id)) {
$self_class = null;
$type = $codebase->methods->getMethodReturnType(
$method_id,
$self_class,
null,
$event->getCallArgs()
);
}
}

$template_type_parameters = $event->getTemplateTypeParameters();
if (!$template_type_parameters) {
return null;
}

if ($type instanceof Union) {
if ($type->hasType('static')) {
return new Union([
new Type\Atomic\TGenericObject(
$event->getFqClasslikeName(),
$template_type_parameters
),
]);
}

return $type;
}

$fake_method_call = new MethodCall(
new Variable('builder'),
$method_name_lowercase,
Expand Down
12 changes: 9 additions & 3 deletions tests/acceptance/EloquentRelationTypes.feature
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ Feature: Eloquent Relation types
When I run Psalm
Then I see no errors

@skip
Scenario: Relationships return themselves when the underlying method returns a builder
Given I have the following code
"""
Expand All @@ -332,17 +331,24 @@ Feature: Eloquent Relation types
When I run Psalm
Then I see no errors

@skip
Scenario: Relationships return themselves when the proxied method is a query builder method
Given I have the following code
"""
/**
* @param HasOne<Phone> $relationship
* @psalm-return HasOne<Phone>
*/
function test(HasOne $relationship): HasOne {
function test_hasOne(HasOne $relationship): HasOne {
return $relationship->orderBy('id', 'ASC');
}

/**
* @param HasMany<Phone> $relationship
* @psalm-return HasMany<Phone>
*/
function test_hasMany(HasMany $relationship): HasMany {
return $relationship->orderBy('id', 'DESC');
}
"""
When I run Psalm
Then I see no errors
Expand Down