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:

https://github.com/psalm/psalm-plugin-laravel/blob/723221a6c28676717adc53f02136790de1c4a161/tests/acceptance/EloquentRelationTypes.feature#L335-L348

[caugner]

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

[caugner]

According to a local git bisect, 71a93c472c398fe62bf0d3c0d6ad1e281320f87f 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:

https://github.com/psalm/psalm-plugin-laravel/blob/723221a6c28676717adc53f02136790de1c4a161/src/Handlers/Eloquent/RelationsMethodHandler.php#L60-L61

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

https://github.com/psalm/psalm-plugin-laravel/blob/723221a6c28676717adc53f02136790de1c4a161/src/Handlers/Eloquent/RelationsMethodHandler.php#L88-L98

[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. 🤔