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

[5.8] Fix Database Query Builder dynamicWhere method $parameters type #28734

Merged
merged 1 commit into from
Jun 5, 2019
Merged

[5.8] Fix Database Query Builder dynamicWhere method $parameters type #28734

merged 1 commit into from
Jun 5, 2019

Conversation

antonkomarev
Copy link
Contributor

dynamicWhere method accepts arrays only, because under the hood it calls addDynamic method where $parameters used as array:

$this->where(Str::snake($segment), '=', $parameters[$index], $bool);

It's a possible place where array typehints should be added to methods. But I'm not sure if it wouldn't break some magical stuff.

@taylorotwell taylorotwell merged commit 2290bed into laravel:5.8 Jun 5, 2019
@antonkomarev antonkomarev deleted the fix/query-builder-parameters-parameter-type branch June 5, 2019 13:03
@bolechen
Copy link

bolechen commented Jun 16, 2019

My code like below have static check error when i upgrade to laravel 5.8.23

$shopId = "1";
$user = User::whereShopId($shopId);

the error is:

------ -------------------------------------------------------------------------------------------------------------------
  Line   app/Http/Controllers/Api/v1/UserController.php
 ------ -------------------------------------------------------------------------------------------------------------------
  47     Parameter #1 $parameters of method Illuminate\Database\Query\Builder::dynamicWhere() expects array, string given.

how i change this? thx

@GrahamCampbell
Copy link
Member

The analyser is just incorrect here?

@GrahamCampbell
Copy link
Member

This PR does genuinely correct the type.

@bolechen
Copy link

This PR does genuinely correct the type.

May be is the analyser bug, i will report this to larastan.

szepeviktor added a commit to larastan/larastan that referenced this pull request Jun 23, 2019
fatihdirikman added a commit to fatihdirikman/Larastan that referenced this pull request Jan 7, 2022
luoboton pushed a commit to luoboton/larastan that referenced this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants