Skip to content

[12.x] add generics to QueryBuilder’s column related methods #55663

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

Merged

Conversation

taka-oyama
Copy link
Contributor

This is another PR to add and improve the PhpDoc by adding generics to types to the Query/Builder.

This will result in better static type checking and better code completion when using IDEs.

@@ -278,7 +278,7 @@ public function __construct(
/**
* Set the columns to be selected.
*
* @param array|mixed $columns
* @param array<mixed>|mixed $columns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not primarily to be read by humans but by tools and for them mixed and your proposed alternative make a huge difference. If you want to make it more bite-sized, you can always resort to @phpstan-type or the like

@cosmastech
Copy link
Contributor

Just as a note, I got some pushback when suggesting a change with phpstan-type documents. #54783 (comment)

@shaedrich
Copy link
Contributor

@cosmastech Thanks for the hint 👍🏻 I had a very vague memory about the PR and that phpstan-type was somehow topic in a discussion at the back of my head but I couldn't remember which PR it was, so I wasn't sure how the discussion went. That clarified it 👍🏻

@taka-oyama
Copy link
Contributor Author

taka-oyama commented May 7, 2025

@cosmastech Thanks for the heads up.

I totally understand Taylor about him getting PhpDoc fatigue (I'm feeling it too...).

On one hand, it's great when IDE's can utilize it, but on the other hand, it's hard to read and even harder to validate 😅.

So, I think I'll simplify this PR and tone it down a bit to reduce the burden.

Btw, I checked @phpstan-type on PhpStorm and it works perfectly (unwraps the alias) but not so great when using VSCode (just shows the alias name) so I'll revert that back.

@@ -278,7 +278,7 @@ public function __construct(
/**
* Set the columns to be selected.
*
* @param array|mixed $columns
* @param mixed $columns
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

array|mixed can be simplified to mixed

@taylorotwell taylorotwell merged commit ac6efaa into laravel:12.x May 7, 2025
58 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.

4 participants