Use ClassMetadata[Info] class in type hints instead of the ClassMetadata interface #6744

Closed
opened 2026-01-22 15:37:56 +01:00 by admin · 3 comments
Owner

Originally created by @mpdude on GitHub (Jun 7, 2021).

This was sparked by #8402:

At least \Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata and \Doctrine\ORM\Mapping\ClassMetadataFactory::validateRuntimeMetadata (with the changes of #8402) use \Doctrine\Persistence\Mapping\ClassMetadata (an interface) as DocBlock type-hints for parameters, but access \Doctrine\ORM\Mapping\ClassMetadataInfo::$discriminatorMap through it.

This causes PHPStan and Psalm to complain, since Interfaces cannot have properties.

When code like $parent->discriminatorMap is supposed to be kept as-is (that is, no corresponding method added in the ClassMetadata interface), the type hints need to be changed. This might cascade through a chain of called methods, though.

Originally created by @mpdude on GitHub (Jun 7, 2021). This was sparked by #8402: At least `\Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata` and `\Doctrine\ORM\Mapping\ClassMetadataFactory::validateRuntimeMetadata` (with the changes of #8402) use `\Doctrine\Persistence\Mapping\ClassMetadata` (an interface) as DocBlock type-hints for parameters, but access `\Doctrine\ORM\Mapping\ClassMetadataInfo::$discriminatorMap` through it. This causes PHPStan and Psalm to complain, since `Interfaces cannot have properties`. When code like `$parent->discriminatorMap` is supposed to be kept as-is (that is, no corresponding method added in the `ClassMetadata` interface), the type hints need to be changed. This might cascade through a chain of called methods, though.
admin closed this issue 2026-01-22 15:37:56 +01:00
Author
Owner

@greg0ire commented on GitHub (Jun 14, 2021):

I don't think changing the type declarations is an option for doLoadMetadata: it's forbidden to narrow them down since that would break the LSP.
Instead we could throw if anything else than an ORM ClassMetadata is passed.

For validateRuntimeMetadata() it's another story since there is no method override. We can change the type declaration 👍
After doing that, we should regenerate PHPStan and Psalm baselines in case they contain something about this.

@greg0ire commented on GitHub (Jun 14, 2021): I don't think changing the type declarations is an option for `doLoadMetadata`: it's forbidden to narrow them down since that would break the LSP. Instead we could throw if anything else than an ORM `ClassMetadata` is passed. For `validateRuntimeMetadata()` it's another story since there is no method override. We can change the type declaration 👍 After doing that, we should regenerate PHPStan and Psalm baselines in case they contain something about this.
Author
Owner

@mpdude commented on GitHub (Jun 8, 2023):

Found this old issue. Don't really recall what I was talking about, might have been resolved since then.

@mpdude commented on GitHub (Jun 8, 2023): Found this old issue. Don't really recall what I was talking about, might have been resolved since then.
Author
Owner

@greg0ire commented on GitHub (Jun 8, 2023):

I think the @extends annotation introduced in 98d77043d8 (diff-3e426fe47b58d2d4b970b1b7a0cbf5999af950e4f84e48dce4bee5061b57ed31) does fix it.

@greg0ire commented on GitHub (Jun 8, 2023): I think the `@extends` annotation introduced in https://github.com/doctrine/orm/commit/98d77043d81e79502e1dd99bd4da16bf34aaecbf#diff-3e426fe47b58d2d4b970b1b7a0cbf5999af950e4f84e48dce4bee5061b57ed31 does fix it.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6744