Skip to content

PHPORM-492 Fix PHPDoc return and param types for raw() methods - #3526

Merged
GromNaN merged 5 commits into
mongodb:5.8from
GromNaN:PHPORM-492
Jun 17, 2026
Merged

PHPORM-492 Fix PHPDoc return and param types for raw() methods#3526
GromNaN merged 5 commits into
mongodb:5.8from
GromNaN:PHPORM-492

Conversation

@GromNaN

@GromNaN GromNaN commented Jun 12, 2026

Copy link
Copy Markdown
Member

Fixes https://jira.mongodb.org/browse/PHPORM-492

Summary

The @param and @return PHPDoc types on raw() had several issues.

@param — Closure argument type:
Both Query\Builder::raw() and Eloquent\Builder::raw() declared the Closure as Closure():T (no arguments), but the Closure actually receives a \MongoDB\Collection instance at runtime.

@return — null case:
Both builders declared returning Collection (either Illuminate\Support\Collection or Illuminate\Database\Eloquent\Collection) when $value is null. The method actually returns the raw \MongoDB\Collection object.

@return — Expression case (Eloquent\Builder only):
Expression in Eloquent\Builder resolves to MongoDB\Builder\Expression (the local import), but the method delegates to Query\Builder::raw() which returns an Illuminate\Contracts\Database\Query\Expression.

@return — Closure case (Eloquent\Builder only):
The return type was declared as T (the Closure's return type), but Eloquent\Builder::raw() post-processes the result:

  • If T is a CursorInterface, it is always hydrated into Collection<int, TModel> — the cursor is never returned.
  • If the result is a document (object/array with _id or id), it is converted to a TModel instance.

A nested conditional type fixes the CursorInterface case:

T is CursorInterface ? Collection<int, TModel> : T|TModel|Collection<int, TModel>

PHPStan type tests are added in tests/PHPStan/BuilderRawTypes.php to validate all cases.

GromNaN added 3 commits June 12, 2026 20:23
The @PARAM type for the Closure argument was missing the \MongoDB\Collection
parameter that is passed at runtime. The @return type for the null case
incorrectly referenced Illuminate Collection classes instead of \MongoDB\Collection.
For Eloquent\Builder::raw(), the Closure case was also missing TModel and
Collection<int, TModel> as possible return types, which caused static analysis
tools to report false positives when the result was used as a model.
@GromNaN
GromNaN requested a review from paulinevos June 15, 2026 16:41
@GromNaN
GromNaN marked this pull request as ready for review June 15, 2026 16:42
@GromNaN
GromNaN requested a review from a team as a code owner June 15, 2026 16:42
Copilot AI review requested due to automatic review settings June 15, 2026 16:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates PHPDoc for raw() methods in the MongoDB Laravel query and Eloquent builders to better align with runtime behavior and improve static analysis (PHPStan/Psalm) accuracy.

Changes:

  • Adjust raw() PHPDoc so Closure parameters reflect that a \MongoDB\Collection is passed at runtime.
  • Correct raw() PHPDoc return types for the null case to indicate \MongoDB\Collection is returned.
  • Add PHPStan type-level tests to validate raw() param/return typing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/PHPStan/BuilderRawTypes.php Adds PHPStan assertType() checks for Query\Builder::raw() and Eloquent\Builder::raw() behaviors.
src/Query/Builder.php Updates raw() PHPDoc Closure parameter type and null return type to \MongoDB\Collection.
src/Eloquent/Builder.php Updates raw() PHPDoc Closure parameter type and expands Closure return types; also touches the null return type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Eloquent/Builder.php
Comment on lines +226 to 229
* @param (Closure(\MongoDB\Collection):T)|Expression|null $value
*
* @return ($value is Closure ? T : ($value is null ? Collection : Expression))
* @return ($value is Closure ? T|TModel|Collection<int, TModel> : ($value is null ? \MongoDB\Collection : Expression))
*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What this guy said I guess 😅

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch! Both points fixed:

  • Expression return type now uses \Illuminate\Contracts\Database\Query\Expression (the interface actually returned by $this->query->raw()).
  • For the CursorInterface case, I added a nested conditional: T is CursorInterface ? Collection<int, TModel> : T|TModel|Collection<int, TModel> — PHPStan now correctly infers Collection<int, TModel> and no longer includes CursorInterface in the union.

Comment on lines +45 to +48
assertType(
'Illuminate\Database\Eloquent\Collection<int, MongoDB\Laravel\Tests\Models\User>|MongoDB\Driver\CursorInterface|MongoDB\Laravel\Tests\Models\User',
$builder->raw(fn (MongoDBCollection $c) => $c->find([])),
);
…ion tests

The @return PHPDoc for the non-Closure/non-null case referenced MongoDB\Builder\Expression
(the local import) but the method delegates to Query\Builder::raw() which wraps the value
in a new Illuminate\Database\Query\Expression.
…r::raw() return type

$this->query is declared as Illuminate\Database\Query\Builder, whose raw() returns
the Illuminate\Contracts\Database\Query\Expression interface, not the concrete class.
@GromNaN
GromNaN merged commit e8b6842 into mongodb:5.8 Jun 17, 2026
36 checks passed
@GromNaN
GromNaN deleted the PHPORM-492 branch June 17, 2026 11:37
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