Support ClassLocator in StaticPHPDriver #486
Conversation
Use the ColocatedMappingDriver that provides this feature
….Mapping.PHPTestEntityAssert.php that asserts $metadata variable existence
There was a problem hiding this comment.
Pull request overview
This PR refactors StaticPHPDriver to leverage the ClassLocator abstraction and eliminate code duplication by using the ColocatedMappingDriver trait. The changes enable the driver to accept either directory paths or a ClassLocator instance, simplifying its implementation and improving maintainability.
Key Changes:
- Refactored
StaticPHPDriverto useColocatedMappingDrivertrait, removing ~80 lines of duplicated code - Added support for
ClassLocatorparameter inStaticPHPDriverconstructor - Fixed test isolation issue in
StaticPHPDriverTest::testGetAllClassNamesby narrowing the scan path
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Mapping/Driver/StaticPHPDriver.php | Replaced custom implementation with ColocatedMappingDriver trait and added ClassLocator support to constructor |
| src/Mapping/Driver/ColocatedMappingDriver.php | Updated addPaths() to use spread operator syntax for array merging |
| tests/Mapping/StaticPHPDriverTest.php | Refactored tests to use colocated test entity, fixed path scanning issue, and added new test for ClassLocator support |
| tests/Mapping/_files/colocated/Entity.php | Added loadMetadata() method to make this a valid test entity for StaticPHPDriver |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| public function testGetAllClassNames(): void | ||
| { | ||
| $driver = new StaticPHPDriver([__DIR__]); |
There was a problem hiding this comment.
This directory contains Doctrine.Tests.Persistence.Mapping.PHPTestEntityAssert.php that cannot be includes without setting the $metadata variable. Not supported by the StaticPHPDriver.
| * @param string[] $directories | ||
| * @param string[] $excludedDirectories Directories to exclude from the search. |
There was a problem hiding this comment.
We don't use the key of the array when iterating. Relaxing this type si that array<int, string> is accepted, which is the result of array_unique.
| /** | ||
| * {@inheritDoc} | ||
| * | ||
| * @todo Same code exists in ColocatedMappingDriver, should we re-use it |
There was a problem hiding this comment.
This deleted code is duplicate of FileClassLocator.
| * @var array<int, string> | ||
| */ | ||
| private array $paths = []; | ||
| use ColocatedMappingDriver; |
There was a problem hiding this comment.
This introduces more public methods to the class. Maybe it needs to be documented in UPGRADE.md
2 participants
The
ClassLocatorabstraction added in #433 should be used for theStaticPHPDriver.By using the
ColocatedMappingDrivertrait, we remove code duplication.fix
StaticPHPDriverTest::testGetAllClassNamesby only scanning/_files/colocated/. Otherwise it loads theDoctrine.Tests.Persistence.Mapping.PHPTestEntityAssert.phpfile that expects the variable$metadatato be defined. This test passes when all the test suite is run, but not individually.Relax PHPStan type for list of directories in
FileClassLocator::createFromDirectories()as we don't care about the array key.