Support hasMany(...)->orderBy()

[caugner]

Describe the bug

The following causes both MixedInferredReturnType and MixedReturnStatement.

    public function books(): HasMany
    {
        return $this->hasMany(Book::class)->orderBy('year');
    }

Same for ->orderByDesc('updated_at') (which doesn't work, while ->latest() does).

Impacted Versions

laravel/framework                     v8.50.0   The Laravel Framework.

barryvdh/laravel-ide-helper           v2.9.1    Laravel IDE Helper, generates correct PHPDocs for all Facade classes, to improve auto-completion.
laravel/framework                     v8.50.0   The Laravel Framework.
psalm/plugin-laravel                  v1.5.0    A Laravel plugin for Psalm
spatie/laravel-ray                    1.23.0    Easily debug Laravel apps
vimeo/psalm                           4.8.1     A static analysis tool for finding errors in PHP applications

Additional context
(None.)

[caugner]

Seems to be a known issue:

psalm-plugin-laravel/tests/acceptance/EloquentRelationTypes.feature

Lines 335 to 348 in 723221a

	@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 {
	return $relationship->orderBy('id', 'ASC');
	}
	"""
	When I run Psalm
	Then I see no errors

[caugner]

And here's the related commit by @mr-feek: 71360de

[caugner]

According to a local git bisect, 71a93c4 is the culprit:

71a93c472c398fe62bf0d3c0d6ad1e281320f87f is the first bad commit
commit 71a93c472c398fe62bf0d3c0d6ad1e281320f87f
Author: Matthew Brown <github@muglug.com>
Date:   Mon Oct 19 20:24:26 2020 -0400

    Support Psalm 4.0

 composer.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

If this is true, the error was introduced upstream in 4.0.0-beta3...4.0.0.

[caugner]

It looks like the MethodReturnTypeProviderInterface is called with different parameters:

Previously, the $method_name_lowercase was __call, but now it's orderby which causes these ->methodExists to both return false:

psalm-plugin-laravel/src/Handlers/Eloquent/RelationsMethodHandler.php

Lines 60 to 61 in 723221a

	if ($source->getCodebase()->methods->methodExists(new MethodIdentifier(Builder::class, $method_name_lowercase)) ||
	$source->getCodebase()->methods->methodExists(new MethodIdentifier(QueryBuilder::class, $method_name_lowercase))

Accordingly, the RelationsMethodHandler returns null (line 98) instead of the Union (line 92 ff.):

psalm-plugin-laravel/src/Handlers/Eloquent/RelationsMethodHandler.php

Lines 88 to 98 in 723221a

	foreach ($type->getAtomicTypes() as $type) {
	if ($type instanceof Type\Atomic\TNamedObject && $type->value === Builder::class) {
	// ta-da. now we return "this" relation instance
	return new Union([
	new Type\Atomic\TGenericObject($event->getFqClasslikeName(), $template_type_parameters),
	]);
	}
	}
	}
	return null;

[caugner]

Here's the current (failing) output:

array:3 [
  "stmt" => PhpParser\Node\Expr\MethodCall^ {#446775
    +var: PhpParser\Node\Expr\Variable^ {#446776
      +name: "relationship"
      #attributes: array:3 [
        "startLine" => 30
        "startFilePos" => 1235
        "endFilePos" => 1247
      ]
    }
    +name: PhpParser\Node\Identifier^ {#446777
      +name: "orderBy"
      #attributes: array:3 [
        "startLine" => 30
        "startFilePos" => 1250
        "endFilePos" => 1256
      ]
    }
    +args: array:2 [
      0 => PhpParser\Node\Arg^ {#446778
        +name: null
        +value: PhpParser\Node\Scalar\String_^ {#446779
          +value: "id"
          #attributes: array:4 [
            "startLine" => 30
            "startFilePos" => 1258
            "endFilePos" => 1261
            "kind" => 1
          ]
        }
        +byRef: false
        +unpack: false
        #attributes: array:3 [
          "startLine" => 30
          "startFilePos" => 1258
          "endFilePos" => 1261
        ]
      }
      1 => PhpParser\Node\Arg^ {#446780
        +name: null
        +value: PhpParser\Node\Scalar\String_^ {#446781
          +value: "ASC"
          #attributes: array:4 [
            "startLine" => 30
            "startFilePos" => 1264
            "endFilePos" => 1268
            "kind" => 1
          ]
        }
        +byRef: false
        +unpack: false
        #attributes: array:3 [
          "startLine" => 30
          "startFilePos" => 1264
          "endFilePos" => 1268
        ]
      }
    ]
    #attributes: array:3 [
      "startLine" => 30
      "startFilePos" => 1235
      "endFilePos" => 1269
    ]
  }
  "method_name_lowercase" => "orderby"
  "called_method_name_lowercase" => null
]

[caugner]

I dumped a bit and found that the Psalm\Internal\Analyzer\Statements\Expression\Call\Method\AtomicMethodCallAnalysisResult actually has two existent_method_ids:

0 => "Illuminate\Database\Eloquent\Relations\HasOne::orderby"
1 => "Illuminate\Database\Eloquent\Relations\HasOne::__call"

So maybe Psalm thinks that orderby actually exists, so our checks fail. 🤔