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

[9.x] Allow for int value parameters to whereMonth() and whereDay() #43668

Merged

Conversation

lupinitylabs
Copy link
Contributor

@lupinitylabs lupinitylabs commented Aug 11, 2022

When trying to pass an integer value to whereMonth() or whereDay(), e.g.

User::whereMonth('updated_at', '=', 4)->get();

PHPstan will report Parameter #3 $value of method Illuminate\Database\Eloquent\Builder<App\Models\User>::whereMonth() expects DateTimeInterface|string|null, int given.

At the same time,

User::whereYear('updated_at', '=', 2022)->get();

will accept integer values.

This PR suggests to make the type definitions for whereYear(), whereMonth() and whereDay() more consistent by adding the int type to the @param docblock for $value.

I am swapping out the usage of str_pad() with sprintf() because the latter will accept mixed arguments, whereas for str_pad(), we would have to convert int values to strings first (and it's more compact, too).

@lupinitylabs lupinitylabs changed the title Allow for int value parameters for whereMonth() and whereDay() [9.x] Allow for int value parameters for whereMonth() and whereDay() Aug 11, 2022
@lupinitylabs
Copy link
Contributor Author

lupinitylabs commented Aug 11, 2022

Looks like the failing test is unrelated...?

@lupinitylabs lupinitylabs changed the title [9.x] Allow for int value parameters for whereMonth() and whereDay() [9.x] Allow for int value parameters to whereMonth() and whereDay() Aug 11, 2022
@taylorotwell taylorotwell merged commit c951ba7 into laravel:9.x Aug 15, 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.

None yet

2 participants