Changes necessary for compatibility with ORM 3 #373
Conversation
| @@ -35,7 +35,7 @@ public function getIdentifier(): array; | |||
| * | |||
| * @return ReflectionClass<T> | |||
| */ | |||
| public function getReflectionClass(): ReflectionClass; | |||
| public function getReflectionClass(); | |||
There was a problem hiding this comment.
In ORM 3, the return type is ReflectionClass|null
| @@ -71,7 +71,7 @@ public function getGlobalBasename(): string | |||
| * | |||
| * @throws MappingException | |||
| */ | |||
| public function getElement(string $className): ClassMetadata | |||
| public function getElement(string $className) | |||
There was a problem hiding this comment.
When running the ORM 3 test suite, line 95 causes lots of issues:
Doctrine\Persistence\Mapping\Driver\FileDriver::getElement(): Return value must be of type Doctrine\Persistence\Mapping\ClassMetadata, SimpleXMLElement returned
There was a problem hiding this comment.
to me, this looks like invalid tests
There was a problem hiding this comment.
hmm, actually, it might be that the new types don't match the actual usages. The XmlDriver of the orm returns array<string, SimpleXMLElement> in loadMappingFile, which is why getElement also returns a SimpleXMLElement.
My guess is that the types added for this method are totally wrong, as it looks like each FileDriver child class is free to choose the format of the data they want to cache per class anyway (as the implementation of the public API loadMetadataForClass is left to the child classes).
And this method should probably have been defined as protected instead.
| @@ -139,7 +139,7 @@ public function getAllClassNames(): array | |||
| * @return ClassMetadata[] | |||
| * @psalm-return array<class-string, ClassMetadata<object>> | |||
| */ | |||
| abstract protected function loadMappingFile(string $file): array; | |||
| abstract protected function loadMappingFile(string $file); | |||
There was a problem hiding this comment.
The type declaration was simply not updated in ORM 3
| @@ -139,7 +139,7 @@ public function getAllClassNames(): array | |||
| * @return ClassMetadata[] | |||
| * @psalm-return array<class-string, ClassMetadata<object>> | |||
| */ | |||
| abstract protected function loadMappingFile(string $file): array; | |||
| abstract protected function loadMappingFile(string $file); | |||
There was a problem hiding this comment.
does the ORM need to return something else than an array here ?
There was a problem hiding this comment.
No, it's just that the return type declaration was not added in ORM 3 (it was forgotten)
|
Closing now that compatibility seems doable: doctrine/orm#11533 |
No description provided.