Parameter $column of method orWhere() errors when Illuminate\Database\Query\Expression is given

[ttomdewit]

• Larastan Version: 0.7.0
• --level used: 5
• PHPStan Version: 0.12.75

Description

When on level 5, the orWhere() method on Illuminate\Database\Eloquent\Builder<Illuminate\Database\Eloquent\Model>::orWhere() fails when a Illuminate\Database\Query\Expression is given, even though it's in the DocBlock of aforementioned method.

/**
 * Add an "or where" clause to the query.
 *
 * @param  \Closure|array|string|\Illuminate\Database\Query\Expression  $column
 * @param  mixed  $operator
 * @param  mixed  $value
 * @return $this
 */
public function orWhere($column, $operator = null, $value = null)

Laravel code where the issue was found

$subQuery->orWhere(
    DB::raw(sprintf('lower(%s)', $column)),
    'like',
    '%' . strtolower($searchTerm) . '%'
);

Full error:

{
    "description": "Parameter #1 $column of method Illuminate\\Database\\Eloquent\\Builder<Illuminate\\Database\\Eloquent\\Model>::orWhere() expects array<int|string, mixed>|Closure|string, Illuminate\\Database\\Query\\Expression given.",
    "fingerprint": "3d188e1afc222590ee964c4330945aaa601e2139aaf0454b43c2e3ad64662aef",
    "location": {
        "path": "app/JsonApi/AbstractAdapter.php",
        "lines": {
            "begin": 46
        }
    }
}

Note: it says it expects an array that looks like int|string or mixed, that's another error I think?

[ttomdewit]

Update: please note that when including _ide_helper.php it still errors:

includes:
    - ./vendor/nunomaduro/larastan/extension.neon

parameters:

    paths:
        - app

    level: 5

    scanFiles:
        - _ide_helper.php

[ttomdewit]

Update: please note I'm on Laravel Framework 7.30.4.

[szepeviktor]

Hello @ttomdewit! We use stubs for special purposes.
https://github.com/nunomaduro/larastan/blob/72c7bd2311594a896386b9a7789f709748e9d1cc/stubs/EloquentBuilder.stub#L143-L151
It really lacks that type.
Could you send a PR?

[ttomdewit]

Hello @ttomdewit! We use stubs for special purposes.
https://github.com/nunomaduro/larastan/blob/72c7bd2311594a896386b9a7789f709748e9d1cc/stubs/EloquentBuilder.stub#L143-L151

It really lacks that type.
Could you send a PR?

Yeah I'd be happy to take a look

[canvural]

Hi,

I think our stubs needs updating. Here you can see the accepted types. Just need to add Query\Expression to it.

If you can make a PR with changes and some tests, I can merge it 👍

Edit: Of course @szepeviktor was faster 😄

[szepeviktor]

Thank you!

https://github.com/laravel/framework/blob/9dd38140dc2924daa1a020a3d7a45f9ceff03df3/src/Illuminate/Database/Eloquent/Builder.php#L261-L276

[ttomdewit]

Hi,

I think our stubs needs updating. Here you can see the accepted types. Just need to add Query\Expression to it.

If you can make a PR with changes and some tests, I can merge it

I'll try and PR this addition and add the tests. It's a new codebase for me so might take a second.

[ttomdewit]

@szepeviktor @canvural What about the array type that's included in the stub but not in the original DocBlock? Is that expected?

[szepeviktor]

Just make the parameter type equal to Laravel 8.

[canvural]

@szepeviktor @canvural What about the array type that's included in the stub but not in the original DocBlock? Is that expected?

It is in the original docbloc: https://github.com/laravel/framework/blob/7.x/src/Illuminate/Database/Eloquent/Builder.php#L264

[ttomdewit]

@szepeviktor @canvural What about the array type that's included in the stub but not in the original DocBlock? Is that expected?

It is in the original docbloc: https://github.com/laravel/framework/blob/7.x/src/Illuminate/Database/Eloquent/Builder.php#L264

I must misread it. I thought it accepts either a Closure, a string, an array or a Query expression. Perhaps you're describing what the array should look like. I'll leave it for now and try to PR what @szepeviktor mentioned about copying over the original DocBlock.

[canvural]

@ttomdewit Are you talking about the model-property<TModelClass>|array<model-property<TModelClass> part? If that's the case, please don't remove it, it's needed. It tells Larastan that only the properties of the model can be given as arguments.

[ttomdewit]

@ttomdewit Are you talking about the model-property<TModelClass>|array<model-property<TModelClass> part? If that's the case, please don't remove it, it's needed. It tells Larastan that only the properties of the model can be given as arguments.

I wasn't, no, but that's a good comment. I'll make sure to keep it! I'm referring to the next part:

array<model-property<TModelClass>|int, mixed>

And explicitly the int, mixed part.

[canvural]

And explicitly the int, mixed part.

array<model-property<TModelClass>|int, mixed> this type says it is an array where it's keys can be either model-property<TModelClass> or int, and it's value is mixed type.

[ttomdewit]

And explicitly the int, mixed part.

array<model-property<TModelClass>|int, mixed> this type says it is an array where it's keys can be either model-property<TModelClass> or int, and it's value is mixed type.

That makes perfect sense. Thank you!

For now I've changed it to this and am currently manually testing it:

/**
 * Add an "or where" clause to the query.
 *
 * @param  \Closure|model-property<TModelClass>|array<model-property<TModelClass>|int, mixed>|\Illuminate\Database\Query\Expression  $column
 * @param  mixed  $operator
 * @param  mixed  $value
 * @return $this
 */
public function orWhere($column, $operator = null, $value = null);

[ttomdewit]

@szepeviktor @canvural I'm happy to report that the error is gone when applying the changes mentioned in #784 (comment).

On to the tests now.

[canvural]

Thank you. And I believe Laravel can take Query\Expression in different methods also. Please update them too. I think laravel/framework#34828 added them all to Laravel.

[ttomdewit]

Thank you. And I believe Laravel can take Query\Expression in different methods also. Please update them too. I think laravel/framework#34828 added them all to Laravel.

Allow me to add the test for this particular method, all tests are currently green as expected so I'll need to figure out how to add this particular example.

[ttomdewit]

@szepeviktor @canvural I have absolutely no idea how to test these changes with the way tests are setup in this project. I'll publish the PR and, if you have time, maybe you can point me in the right direction.