[3.x] Add typed properties #2935
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds typed properties and type hints to the codebase, introducing mixed, callable, and union types that were not available in earlier PHP versions. This is a breaking change that modernizes the type system across the ODM.
Key Changes:
- Added explicit type declarations to class constants (e.g.,
public const int STATE_MANAGED = 1) - Converted PHPDoc type annotations to native typed properties
- Added
mixedtype hints to method parameters and return types where values can be of any type - Introduced union types (e.g.,
string|int|null,array|Expr) for more precise type declarations - Used constructor property promotion to reduce boilerplate code
- Made
ReferencePrimera readonly class with promoted constructor properties - Removed deprecated
getFqcnFromAlias()method fromClassMetadataFactory
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/UnitOfWork.php | Added type declarations to constants and method parameters (mixed types for identifier parameters) |
| src/Types/BinData*.php | Converted annotated properties to typed properties for $binDataType |
| src/SchemaManager.php | Added type declarations to all constants (int and array types) |
| src/Repository/UploadOptions.php | Converted to typed properties with explicit default values |
| src/Repository/GridFSRepository.php | Added mixed type hints for identifier parameters |
| src/Repository/DocumentRepository.php | Used constructor promotion for dependencies and added proper type hints |
| src/Repository/DefaultGridFSRepository.php | Added mixed type hints for identifier parameters |
| src/Query/ReferencePrimer.php | Converted to readonly class with constructor property promotion |
| src/Query/QueryExpressionVisitor.php | Added mixed return type |
| src/Query/Filter/BsonFilter.php | Used constructor promotion and added typed properties |
| src/Query/Expr.php | Added union types for parameters, made DocumentManager readonly |
| src/Query/CriteriaMerger.php | Added array type to variadic parameter |
| src/Query/Builder.php | Changed $currentField visibility to private and added type hints throughout |
| src/Mapping/Driver/XmlDriver.php | Added type declarations to constants and constructor parameters |
| src/Mapping/Driver/SimplifiedXmlDriver.php | Added type declarations to constant and constructor parameters |
| src/Mapping/ClassMetadataFactory.php | Removed deprecated method and added type hints to protected methods |
| src/Mapping/ClassMetadata.php | Extensive typed property additions throughout the class |
| src/LockMode.php | Added type declarations to all constants |
| src/LockException.php | Made constructor parameter readonly |
| src/Id/IncrementGenerator.php | Converted to typed properties and added return type |
| src/Events.php | Added string type declarations to all event name constants |
| src/DocumentNotFoundException.php | Added mixed type for identifier parameter |
| src/DocumentManager.php | Added type declarations to constant and method signatures |
| src/Configuration.php | Added int type declarations to all autogeneration constants |
| src/Aggregation/Stage/Operator.php | Added typed property for $expr |
| src/Aggregation/Stage/MatchStage.php | Added typed property and void return type for __clone |
| src/Aggregation/Stage/Group.php | Removed redundant property and constructor (inherited from Operator) |
| src/Aggregation/Stage/Bucket/AbstractOutput.php | Used constructor promotion for $bucket dependency |
| src/Aggregation/Stage/AbstractReplace.php | Added union type hints for constructor and method parameters |
| src/Aggregation/Stage/AbstractBucket.php | Added typed properties with union types |
| src/Aggregation/Stage.php | Used constructor property promotion for $builder |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
IonBazan
force-pushed
the
typed-v3
branch
2 times, most recently
from
f208c3e to
49dbd5b
Compare
| #[DataProvider('dataMethodsAffectedByNoObjectArguments')] | ||
| public function testThrowsExceptionOnNonObjectValues(string $methodName): void | ||
| { | ||
| $this->expectException(InvalidArgumentException::class); | ||
| $this->expectException(TypeError::class); | ||
| $this->dm->$methodName(null); | ||
| } |
There was a problem hiding this comment.
Usually we don't test PHP native behavior of rejecting invalid value. But that's good to have it now to ensure we reject the same values as before.
There was a problem hiding this comment.
Let's leave it here for now, in case we have to relax the types here in the future.
There was a problem hiding this comment.
Unrelated, I wonder if we should keep 2 XML driver classes?
| * | ||
| * @see discriminatorField | ||
| * | ||
| * @var class-string|null |
There was a problem hiding this comment.
You removed the class-string type. I was incorrect, right? We should fix this type in the current 2.x.
There was a problem hiding this comment.
Yes, I think this is wrong. The discriminatorValue holds the value that is used as key in discriminatorMap to get Document FQCN. I thought it can only hold string but it appears that int was a valid use case in #2712.
Let me create a fix for v2 as it's considered a bug.
There was a problem hiding this comment.
Looks like it's a bigger problem, reported in #2936. For now, I will revert this particular change.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Summary
Adds typed properties and missing typehints (especially
mixed,callableand union types which were not available earlier).There are still many places to fix but let's consider this as first round.
Attributes were untouched to avoid conflicts with other changes.