[13.x] Modernize Database Query Builder with Native Property Types#59486
[13.x] Modernize Database Query Builder with Native Property Types#59486genius-asif-hub wants to merge 1 commit intolaravel:13.xfrom
Conversation
af27a11 to
f540e94
Compare
|
You have absolutely 0% chance of getting this merged to the 13.x branch. At a minimum it must go to master, and even then the maintainers are unlikely to accept it. |
shaedrich
left a comment
There was a problem hiding this comment.
Adding native types over PHPDoc block ones requires targeting master/14.x
Furthermore, I would advise against changing formatting when this is not part of what the PR is trying to solve
| /** | ||
| * The table joins for the query. | ||
| * | ||
| * @var array|null |
There was a problem hiding this comment.
You can type this array:
| * @var array|null | |
| * @var array<\Illuminate\Database\Query\JoinClause>|null |
| @@ -112,163 +112,195 @@ class Builder implements BuilderContract | |||
| * | |||
| * @var bool|array | |||
There was a problem hiding this comment.
You can narrow this further down:
| * @var bool|array | |
| * @var bool|array<\Illuminate\Contracts\Database\Query\Expression|string>> |
| /** | ||
| * The where constraints for the query. | ||
| * | ||
| * @var array |
There was a problem hiding this comment.
You can type this, too:
| * @var array | |
| * @var array{ | |
| * type: 'Expression'|'Basic'|'JsonBoolean'|'Bitwise'|'NullSafeEquals'|'Null'|'NotNull', | |
| * column: string|\Illuminate\Contracts\Database\Query\Expression, | |
| * operator: string, | |
| * value: mixed, | |
| * boolean: 'and'|'or' | |
| * }[] |
| /** | ||
| * The groupings for the query. | ||
| * | ||
| * @var array|null |
There was a problem hiding this comment.
You may type this:
| * @var array|null | |
| * @var array<\Illuminate\Contracts\Database\Query\Expression|string>|null |
| /** | ||
| * The having constraints for the query. | ||
| * | ||
| * @var array|null |
| /** | ||
| * The orderings for the query. | ||
| * | ||
| * @var array|null |
There was a problem hiding this comment.
This might be typed as well:
| * @var array|null | |
| * @var array{ | |
| * column: \Illuminate\Contracts\Database\Query\Expression|string, | |
| * direction: 'asc'|'desc' | |
| * }[]|null |
| /** | ||
| * The query union statements. | ||
| * | ||
| * @var array|null |
There was a problem hiding this comment.
Also typeable:
| * @var array|null | |
| * @var array{ | |
| * query: \Closure|\Illuminate\Database\Query\Builder|\Illuminate\Database\Eloquent\Builder<*>, | |
| * all: bool | |
| * }[]|null |
| /** | ||
| * The callbacks that should be invoked before the query is executed. | ||
| * | ||
| * @var array |
There was a problem hiding this comment.
Same here:
| * @var array | |
| * @var array<\Closure(\Illuminate\Database\Query\Builder): mixed> |
This PR modernizes the
Illuminate\Database\Query\Builderclass by adding native PHP property type hints to properties that were previously only type-hinted in docblocks.Why this is important:
Changes:
$connection,$grammar,$processor,$bindings,$columns,$wheres,$joins,$limit,$offset, and 15+ other core properties.nullwhere appropriate to prevent "uninitialized property" access errors.IndexHintand others.Verified with
vendor/bin/phpunit tests/Database/- All tests passing.