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

Make types of parameter of join method consistent in the Query Builder #48386

Conversation

melicerte
Copy link
Contributor

There is a consistency problem with the typing of the $second parameter in functions that perform joins.

In the JoinClause class, the "on" function accepts an Expression in the $second parameter:
@param \Illuminate\Contracts\Database\Query\Expression|string|null $second

Which is perfectly normal.

However, in the "join" function of the "Builder" class, the "JoinClause::on" function is called with the $second parameter, which is not typed "Expression".

At runtime, you can pass an expression in $second, which poses no problem and the request is executed correctly.
However, if you use a static analysis tool like phpstan, the compatibility problem appears:

Parameter #4 $second of method Illuminate\Database\Query\Builder::leftJoin() expects string|null,
         Illuminate\Contracts\Database\Query\Expression given.

My suggestion is to add the "Expression" type to the $second parameters in order to guarantee type consistency in the Query Builder at this level.

@taylorotwell taylorotwell merged commit 0429a6f into laravel:10.x Sep 13, 2023
19 checks passed
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.

3 participants