Allow null as a return value for association's target#192
Conversation
|
I think it looks a bit like: doctrine/orm#8767 (comment) Now that we are able to add type information through phpdoc and check it with SA tools, there is going to be a phase where we notice all the inconsistencies in how we implement our interfaces. It is still compatible to say the implementation in the ORM still returns something non-nullable, so here the solution to the breakage will be to override the signature to just say that. |
I agree with this, I guess overriding it in the ORM would be enough, I don't think many packages rely on By the way, apparently |
alcaeus
left a comment
There was a problem hiding this comment.
LGTM with static analysis failures addressed: we need to explicitly check for null before using instanceof
|
Thanks @malarzm! |
|
@greg0ire I now have a phpstan error, using orm, because Does https://github.com/doctrine/orm/blob/2.9.x/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php#L3414 can really return |
|
Based on the mapping validation in the same class, it cannot be |
|
I opened doctrine/orm#8907 then |
* 2.3.x: (24 commits) Bump PHPStan and Psalm (doctrine#214) Update `ObjectRepository`, more type inference. (doctrine#213) PHPStan 1.0.1 Reuse workflows from upstream Specify proxy type Undeprecate detach Fix class name Test on PHP 8.1 Deprecate short namespace alias syntax Normalize continuous integration workflow Add explicit `@return` type next to `#[ReturnTypeWillChange]` Describe criteria as array with string keys Allow null as return value for type of field Mark doctrine/annotations dependency as optional Allow null as a return value for association's target (doctrine#192) Add .github, .doctrine-project.json and phpstan-baseline.neon to export ignore Reordered property type resolution to resolve typed properties before untyped ones Merge 2.2.x into 2.3.x (doctrine#191) Don't pass null to strpos() (doctrine#190) Add ReturnTypeWillChange to extended reflection classes ...
* 2.3.x: (24 commits) Bump PHPStan and Psalm (#214) Update `ObjectRepository`, more type inference. (#213) PHPStan 1.0.1 Reuse workflows from upstream Specify proxy type Undeprecate detach Fix class name Test on PHP 8.1 Deprecate short namespace alias syntax Normalize continuous integration workflow Add explicit `@return` type next to `#[ReturnTypeWillChange]` Describe criteria as array with string keys Allow null as return value for type of field Mark doctrine/annotations dependency as optional Allow null as a return value for association's target (#192) Add .github, .doctrine-project.json and phpstan-baseline.neon to export ignore Reordered property type resolution to resolve typed properties before untyped ones Merge 2.2.x into 2.3.x (#191) Don't pass null to strpos() (#190) Add ReturnTypeWillChange to extended reflection classes ...
* 2.3.x: (24 commits) Bump PHPStan and Psalm (doctrine#214) Update `ObjectRepository`, more type inference. (doctrine#213) PHPStan 1.0.1 Reuse workflows from upstream Specify proxy type Undeprecate detach Fix class name Test on PHP 8.1 Deprecate short namespace alias syntax Normalize continuous integration workflow Add explicit `@return` type next to `#[ReturnTypeWillChange]` Describe criteria as array with string keys Allow null as return value for type of field Mark doctrine/annotations dependency as optional Allow null as a return value for association's target (doctrine#192) Add .github, .doctrine-project.json and phpstan-baseline.neon to export ignore Reordered property type resolution to resolve typed properties before untyped ones Merge 2.2.x into 2.3.x (doctrine#191) Don't pass null to strpos() (doctrine#190) Add ReturnTypeWillChange to extended reflection classes ...
* 2.3.x: (24 commits) Bump PHPStan and Psalm (doctrine#214) Update `ObjectRepository`, more type inference. (doctrine#213) PHPStan 1.0.1 Reuse workflows from upstream Specify proxy type Undeprecate detach Fix class name Test on PHP 8.1 Deprecate short namespace alias syntax Normalize continuous integration workflow Add explicit `@return` type next to `#[ReturnTypeWillChange]` Describe criteria as array with string keys Allow null as return value for type of field Mark doctrine/annotations dependency as optional Allow null as a return value for association's target (doctrine#192) Add .github, .doctrine-project.json and phpstan-baseline.neon to export ignore Reordered property type resolution to resolve typed properties before untyped ones Merge 2.2.x into 2.3.x (doctrine#191) Don't pass null to strpos() (doctrine#190) Add ReturnTypeWillChange to extended reflection classes ...
This came up in doctrine/mongodb-odm#2325 (cc @franmomu @alcaeus ). For the background:
targetDocumenttargetDocumentanddiscriminatorMapaltogether to have a reference to any mapped document. Behind the scenes ODM is using FQCN as a discriminator's valueThe change as-is is needed to prevent a break when doctrine/persistence becomes typed. The other solution I see is allowing the method to return an array of FQCNs that would allow ODM to reflect discriminator maps in the method's return value. This would cause a minor inconvenience for ORM I believe, but nothing unseen before (I'm looking at
isSingleValuedAssociationand similar thing for identifiers).