From a6b38ce98f1ad6330b2fef0dbde42f9f4c88dc32 Mon Sep 17 00:00:00 2001 From: Yevhen Sidelnyk Date: Tue, 5 Aug 2025 15:46:13 +0300 Subject: [PATCH 01/12] Optimize: use O(1) item lookup instead of O(M) When checking $includedFiles for an included file, the previous `in_array()` approach, executed in a loop, is very expensive. Basically, it results in performance of O(N * M), where N - number of declared classes, M - number of included classes. The current approach is O(N), since `isset()` check has constant `O(1)` time. --- src/Persistence/Mapping/Driver/ColocatedMappingDriver.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index a7661364..cbb8eb04 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -19,7 +19,6 @@ use function array_unique; use function assert; use function get_declared_classes; -use function in_array; use function is_dir; use function preg_match; use function preg_quote; @@ -158,7 +157,8 @@ public function getAllClassNames(): array } $sourceFilePathNames = $this->pathNameIterator($filesIterator); - $includedFiles = []; + /** @var array $includedFiles */ + $includedFiles = []; foreach ($sourceFilePathNames as $sourceFile) { if (preg_match('(^phar:)i', $sourceFile) === 0) { @@ -179,7 +179,7 @@ public function getAllClassNames(): array require_once $sourceFile; - $includedFiles[] = $sourceFile; + $includedFiles[$sourceFile] = true; } $classes = []; @@ -190,7 +190,7 @@ public function getAllClassNames(): array $sourceFile = $rc->getFileName(); - if (! in_array($sourceFile, $includedFiles, true) || $this->isTransient($className)) { + if (! isset($includedFiles[$sourceFile]) || $this->isTransient($className)) { continue; } From 8b501b9d2935964b790580266e49bec462c2bb74 Mon Sep 17 00:00:00 2001 From: Yevhen Sidelnyk Date: Thu, 7 Aug 2025 18:10:03 +0300 Subject: [PATCH 02/12] Refactor: extract `FilePathNameIterator` This change makes it easy for the depending libraries to use iterable of `SplFileInfo`, adapted to the format of `ColocatedMappingDriver` file paths. For example, one could provide an instance of Symfony Finder, adapted with `FilePathNameIterator` without having to reinvent it from scratch. --- .../Mapping/Driver/ColocatedMappingDriver.php | 16 ++--------- .../Mapping/Driver/FilePathNameIterator.php | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) create mode 100644 src/Persistence/Mapping/Driver/FilePathNameIterator.php diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index cbb8eb04..61952c5f 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -7,7 +7,6 @@ use AppendIterator; use Doctrine\Persistence\Mapping\MappingException; use FilesystemIterator; -use Generator; use Iterator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -156,7 +155,8 @@ public function getAllClassNames(): array $filesIterator->append($iterator); } - $sourceFilePathNames = $this->pathNameIterator($filesIterator); + /** @var iterable $sourceFilePathNames */ + $sourceFilePathNames = new FilePathNameIterator($filesIterator); /** @var array $includedFiles */ $includedFiles = []; @@ -201,16 +201,4 @@ public function getAllClassNames(): array return $classes; } - - /** - * @param iterable $filesIterator - * - * @return Generator - */ - private function pathNameIterator(iterable $filesIterator): Generator - { - foreach ($filesIterator as $file) { - yield $file->getPathname(); - } - } } diff --git a/src/Persistence/Mapping/Driver/FilePathNameIterator.php b/src/Persistence/Mapping/Driver/FilePathNameIterator.php new file mode 100644 index 00000000..03ebb930 --- /dev/null +++ b/src/Persistence/Mapping/Driver/FilePathNameIterator.php @@ -0,0 +1,27 @@ + */ +final class FilePathNameIterator implements IteratorAggregate +{ + public function __construct( + /** @var iterable */ + private readonly iterable $filesIterator, + ) { + } + + /** @return Generator */ + public function getIterator(): Generator + { + foreach ($this->filesIterator as $file) { + yield $file->getPathname(); + } + } +} From df06aca065468c6776a4c8ea6f544d264abeba7c Mon Sep 17 00:00:00 2001 From: Yevhen Sidelnyk Date: Fri, 8 Aug 2025 15:02:46 +0300 Subject: [PATCH 03/12] Refactor: extract `DirectoryFilesIterator` This change makes it easy for the client code to migrate toward the newer version. The migration would be as easy as passing `new FilePathIterator(new DirectoryFilesIterator($paths))` into the constructor instead of the array of `$paths`. --- .../Mapping/Driver/ColocatedMappingDriver.php | 37 ++--------- .../Mapping/Driver/DirectoryFilesIterator.php | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 33 deletions(-) create mode 100644 src/Persistence/Mapping/Driver/DirectoryFilesIterator.php diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index 61952c5f..8e2ee822 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -4,25 +4,15 @@ namespace Doctrine\Persistence\Mapping\Driver; -use AppendIterator; use Doctrine\Persistence\Mapping\MappingException; -use FilesystemIterator; -use Iterator; -use RecursiveDirectoryIterator; -use RecursiveIteratorIterator; use ReflectionClass; -use RegexIterator; -use SplFileInfo; use function array_merge; use function array_unique; use function assert; use function get_declared_classes; -use function is_dir; use function preg_match; -use function preg_quote; use function realpath; -use function sprintf; use function str_contains; use function str_replace; @@ -134,33 +124,14 @@ public function getAllClassNames(): array throw MappingException::pathRequiredForDriver(static::class); } - /** @var AppendIterator> $filesIterator */ - $filesIterator = new AppendIterator(); + $dirFilesIterator = new DirectoryFilesIterator($this->paths, $this->fileExtension); + /** @var iterable $filePathsIterator */ + $filePathsIterator = new FilePathNameIterator($dirFilesIterator); - foreach ($this->paths as $path) { - if (! is_dir($path)) { - throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($path); - } - - /** @var Iterator $iterator */ - $iterator = new RegexIterator( - new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS), - RecursiveIteratorIterator::LEAVES_ONLY, - ), - sprintf('/%s$/', preg_quote($this->fileExtension, '/')), - RegexIterator::MATCH, - ); - - $filesIterator->append($iterator); - } - - /** @var iterable $sourceFilePathNames */ - $sourceFilePathNames = new FilePathNameIterator($filesIterator); /** @var array $includedFiles */ $includedFiles = []; - foreach ($sourceFilePathNames as $sourceFile) { + foreach ($filePathsIterator as $sourceFile) { if (preg_match('(^phar:)i', $sourceFile) === 0) { $sourceFile = realpath($sourceFile); assert($sourceFile !== false); diff --git a/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php b/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php new file mode 100644 index 00000000..7066cffd --- /dev/null +++ b/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php @@ -0,0 +1,61 @@ + */ +final class DirectoryFilesIterator implements IteratorAggregate +{ + public function __construct( + /** @var list */ + private readonly array $paths, + private readonly string $fileExtension = '.php', + ) { + } + + /** + * @return Iterator + * + * @throws MappingException + */ + public function getIterator(): Iterator + { + /** @var AppendIterator> $filesIterator */ + $filesIterator = new AppendIterator(); + + foreach ($this->paths as $path) { + if (! is_dir($path)) { + throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($path); + } + + /** @var Iterator $iterator */ + $iterator = new RegexIterator( + new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::LEAVES_ONLY, + ), + sprintf('/%s$/', preg_quote($this->fileExtension, '/')), + RegexIterator::MATCH, + ); + + $filesIterator->append($iterator); + } + + return $filesIterator; + } +} From a241c8297dfe3e4679ade82f5eacc5f8933963de Mon Sep 17 00:00:00 2001 From: Yevhen Sidelnyk Date: Thu, 31 Jul 2025 15:12:41 +0300 Subject: [PATCH 04/12] Feature: accept `iterable` of file paths This commit adds support for `ColocatedMappingDriver` to accept `iterable` of file paths. Before it was only possible to accept an array of directory paths. Now, one could provide fine-grained iterator of only necessary files to the Driver. Bundles should use Symfony Finder to implement it, since it gives much flexibility to the client code to configure which files should be included and which not. Backward compatibility is achieved with the following approach: If it's an array, then it should be OK to take a look at `$paths[0]` and determine if it is a file or a directory. If it's not an array, we can assume that it's `$filePaths` iterable that's given. --- .../Mapping/Driver/ColocatedMappingDriver.php | 32 +++++++-- src/Persistence/Mapping/MappingException.php | 2 +- .../Mapping/ColocatedMappingDriverTest.php | 66 +++++++++++++++---- 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index 8e2ee822..0dbb2161 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -21,8 +21,11 @@ */ trait ColocatedMappingDriver { + /** @var iterable */ + private iterable $filePaths; + /** - * The paths where to look for mapping files. + * The directory paths where to look for mapping files. * * @var array */ @@ -39,7 +42,7 @@ trait ColocatedMappingDriver protected string $fileExtension = '.php'; /** - * Cache for getAllClassNames(). + * Cache for {@see getAllClassNames()}. * * @var array|null * @phpstan-var list|null @@ -67,7 +70,7 @@ public function getPaths(): array } /** - * Append exclude lookup paths to metadata driver. + * Append exclude lookup paths to a metadata driver. * * @param string[] $paths */ @@ -120,13 +123,17 @@ public function getAllClassNames(): array return $this->classNames; } - if ($this->paths === []) { + if ($this->paths === [] && ! isset($this->filePaths)) { throw MappingException::pathRequiredForDriver(static::class); } $dirFilesIterator = new DirectoryFilesIterator($this->paths, $this->fileExtension); + /** @var iterable $filePathsIterator */ - $filePathsIterator = new FilePathNameIterator($dirFilesIterator); + $filePathsIterator = $this->concatIterables( + $this->filePaths ?? [], + new FilePathNameIterator($dirFilesIterator), + ); /** @var array $includedFiles */ $includedFiles = []; @@ -172,4 +179,19 @@ public function getAllClassNames(): array return $classes; } + + /** + * @param iterable $iterable1 + * @param iterable $iterable2 + * + * @return iterable + * + * @template TKey + * @template T + */ + private function concatIterables(iterable $iterable1, iterable $iterable2): iterable + { + yield from $iterable1; + yield from $iterable2; + } } diff --git a/src/Persistence/Mapping/MappingException.php b/src/Persistence/Mapping/MappingException.php index 7a25fdf2..3037ddfc 100644 --- a/src/Persistence/Mapping/MappingException.php +++ b/src/Persistence/Mapping/MappingException.php @@ -30,7 +30,7 @@ public static function classNotFoundInNamespaces( public static function pathRequiredForDriver(string $driverClassName): self { return new self(sprintf( - 'Specifying the paths to your entities is required when using %s to retrieve all class names.', + 'Specifying source file paths to your entities is required when using %s to retrieve all class names.', $driverClassName, )); } diff --git a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php index 7ddadf7a..9010b68e 100644 --- a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php +++ b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php @@ -12,14 +12,16 @@ use Doctrine\Tests\Persistence\Mapping\_files\colocated\TestClass; use Generator; use PHPUnit\Framework\TestCase; +use Traversable; +use function is_file; use function sort; class ColocatedMappingDriverTest extends TestCase { public function testAddGetPaths(): void { - $driver = $this->createDriver(__DIR__ . '/_files/colocated'); + $driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated'); self::assertSame([ __DIR__ . '/_files/colocated', ], $driver->getPaths()); @@ -35,7 +37,7 @@ public function testAddGetPaths(): void public function testAddGetExcludePaths(): void { - $driver = $this->createDriver(__DIR__ . '/_files/colocated'); + $driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated'); self::assertSame([], $driver->getExcludePaths()); $driver->addExcludePaths(['/test/path1', '/test/path2']); @@ -48,7 +50,7 @@ public function testAddGetExcludePaths(): void public function testGetSetFileExtension(): void { - $driver = $this->createDriver(__DIR__ . '/_files/colocated'); + $driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated'); self::assertSame('.php', $driver->getFileExtension()); $driver->setFileExtension('.php1'); @@ -56,10 +58,10 @@ public function testGetSetFileExtension(): void self::assertSame('.php1', $driver->getFileExtension()); } - /** @dataProvider pathProvider */ - public function testGetAllClassNames(string $path): void + /** @dataProvider directoryPathProvider */ + public function testGetAllClassNamesForDirectory(string $dirPath): void { - $driver = $this->createDriver($path); + $driver = $this->createDirectoryPathDriver($dirPath); $classes = $driver->getAllClassNames(); @@ -67,16 +69,50 @@ public function testGetAllClassNames(string $path): void self::assertSame([Entity::class, EntityFixture::class], $classes); } + public function testGetAllClassNamesForFilePaths(): void + { + $driver = $this->createFilePathsDriver([ + __DIR__ . '/_files/colocated/Entity.php', + __DIR__ . '/_files/colocated/TestClass.php', + ]); + + $classes = $driver->getAllClassNames(); + + self::assertSame([Entity::class], $classes, 'The driver should only return the class names for the provided file path names, excluding transient class names.'); + } + + public function testGetAllClassNamesWorksBothForFilePathsAndRetroactivelyAddedDirectoryPaths(): void + { + $driver = $this->createFilePathsDriver([__DIR__ . '/_files/colocated/Entity.php']); + + $driver->addPaths([__DIR__ . '/_files/colocated/']); + + $classes = $driver->getAllClassNames(); + sort($classes); + + self::assertSame( + [Entity::class, EntityFixture::class], + $classes, + 'The driver should return class names from both the provided file path names and the retroactively added directory paths (these should not be ignored).', + ); + } + /** @return Generator */ - public static function pathProvider(): Generator + public static function directoryPathProvider(): Generator { yield 'straigthforward path' => [__DIR__ . '/_files/colocated']; yield 'winding path' => [__DIR__ . '/../Mapping/_files/colocated']; } - private function createDriver(string $path): MyDriver + private function createDirectoryPathDriver(string $dirPath): MyDriver { - return new MyDriver([$path]); + return new MyDriver([$dirPath]); + } + + /** @param list $filePaths */ + private function createFilePathsDriver(array $filePaths): MyDriver + { + return new MyDriver($filePaths); } } @@ -84,10 +120,16 @@ final class MyDriver implements MappingDriver { use ColocatedMappingDriver; - /** @param non-empty-list $paths One or multiple paths where mapping classes can be found. */ - public function __construct(array $paths) + /** @param iterable $paths One or multiple paths where mapping classes can be found. */ + public function __construct(iterable $paths) { - $this->addPaths($paths); + $isFilePaths = $paths instanceof Traversable || is_file($paths[0]); + + if (! $isFilePaths) { + $this->paths = $paths; + } else { + $this->filePaths = $paths; + } } /** From dbed9f5311a3425dc0f9ea0c273aa98733174841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 12 Aug 2025 19:08:00 +0200 Subject: [PATCH 05/12] Introduce ClassLocator to find classes names for attribute drivers --- composer.json | 3 +- .../Mapping/Driver/ClassLocator.php | 14 ++ src/Persistence/Mapping/Driver/ClassNames.php | 23 +++ .../Mapping/Driver/ColocatedMappingDriver.php | 92 ++------- .../Mapping/Driver/DirectoryFilesIterator.php | 61 ------ .../Mapping/Driver/FileClassLocator.php | 174 ++++++++++++++++++ .../Mapping/Driver/FilePathNameIterator.php | 27 --- src/Persistence/Mapping/MappingException.php | 5 + .../Mapping/ColocatedMappingDriverTest.php | 18 +- .../Mapping/Driver/ClassNamesTest.php | 25 +++ .../Mapping/Driver/FileClassLocatorTest.php | 163 ++++++++++++++++ 11 files changed, 431 insertions(+), 174 deletions(-) create mode 100644 src/Persistence/Mapping/Driver/ClassLocator.php create mode 100644 src/Persistence/Mapping/Driver/ClassNames.php delete mode 100644 src/Persistence/Mapping/Driver/DirectoryFilesIterator.php create mode 100644 src/Persistence/Mapping/Driver/FileClassLocator.php delete mode 100644 src/Persistence/Mapping/Driver/FilePathNameIterator.php create mode 100644 tests/Persistence/Mapping/Driver/ClassNamesTest.php create mode 100644 tests/Persistence/Mapping/Driver/FileClassLocatorTest.php diff --git a/composer.json b/composer.json index c584c2c7..b742a8e3 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,8 @@ "phpstan/phpstan-strict-rules": "^1.1", "doctrine/coding-standard": "^12", "phpunit/phpunit": "^9.6", - "symfony/cache": "^4.4 || ^5.4 || ^6.0 || ^7.0" + "symfony/cache": "^4.4 || ^5.4 || ^6.0 || ^7.0", + "symfony/finder": "^4.4 || ^5.4 || ^6.0 || ^7.0" }, "autoload": { "psr-4": { diff --git a/src/Persistence/Mapping/Driver/ClassLocator.php b/src/Persistence/Mapping/Driver/ClassLocator.php new file mode 100644 index 00000000..7d4a3403 --- /dev/null +++ b/src/Persistence/Mapping/Driver/ClassLocator.php @@ -0,0 +1,14 @@ + */ + public function getClassNames(): array; +} diff --git a/src/Persistence/Mapping/Driver/ClassNames.php b/src/Persistence/Mapping/Driver/ClassNames.php new file mode 100644 index 00000000..3b2c57f3 --- /dev/null +++ b/src/Persistence/Mapping/Driver/ClassNames.php @@ -0,0 +1,23 @@ + $classNames */ + public function __construct( + private array $classNames, + ) { + } + + /** @return list */ + public function getClassNames(): array + { + return $this->classNames; + } +} diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index 0dbb2161..6408f2de 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -5,24 +5,17 @@ namespace Doctrine\Persistence\Mapping\Driver; use Doctrine\Persistence\Mapping\MappingException; -use ReflectionClass; +use function array_filter; use function array_merge; use function array_unique; -use function assert; -use function get_declared_classes; -use function preg_match; -use function realpath; -use function str_contains; -use function str_replace; /** * The ColocatedMappingDriver reads the mapping metadata located near the code. */ trait ColocatedMappingDriver { - /** @var iterable */ - private iterable $filePaths; + private ClassLocator $classLocator; /** * The directory paths where to look for mapping files. @@ -123,75 +116,24 @@ public function getAllClassNames(): array return $this->classNames; } - if ($this->paths === [] && ! isset($this->filePaths)) { - throw MappingException::pathRequiredForDriver(static::class); - } - - $dirFilesIterator = new DirectoryFilesIterator($this->paths, $this->fileExtension); - - /** @var iterable $filePathsIterator */ - $filePathsIterator = $this->concatIterables( - $this->filePaths ?? [], - new FilePathNameIterator($dirFilesIterator), - ); - - /** @var array $includedFiles */ - $includedFiles = []; - - foreach ($filePathsIterator as $sourceFile) { - if (preg_match('(^phar:)i', $sourceFile) === 0) { - $sourceFile = realpath($sourceFile); - assert($sourceFile !== false); - } - - foreach ($this->excludePaths as $excludePath) { - $realExcludePath = realpath($excludePath); - assert($realExcludePath !== false); - $exclude = str_replace('\\', '/', $realExcludePath); - $current = str_replace('\\', '/', $sourceFile); - - if (str_contains($current, $exclude)) { - continue 2; - } - } - - require_once $sourceFile; - - $includedFiles[$sourceFile] = true; - } + if ($this->paths !== []) { + $classNames = FileClassLocator::createFromDirectories($this->paths, $this->excludePaths, $this->fileExtension)->getClassNames(); - $classes = []; - $declared = get_declared_classes(); - - foreach ($declared as $className) { - $rc = new ReflectionClass($className); - - $sourceFile = $rc->getFileName(); - - if (! isset($includedFiles[$sourceFile]) || $this->isTransient($className)) { - continue; + if (isset($this->classLocator)) { + $classNames = array_unique([ + ...$classNames, + ...$this->classLocator->getClassNames(), + ]); } - - $classes[] = $className; + } elseif (isset($this->classLocator)) { + $classNames = $this->classLocator->getClassNames(); + } else { + throw MappingException::pathRequiredForDriver(static::class); } - $this->classNames = $classes; - - return $classes; - } - - /** - * @param iterable $iterable1 - * @param iterable $iterable2 - * - * @return iterable - * - * @template TKey - * @template T - */ - private function concatIterables(iterable $iterable1, iterable $iterable2): iterable - { - yield from $iterable1; - yield from $iterable2; + return $this->classNames = array_filter( + $classNames, + fn (string $className): bool => ! $this->isTransient($className), + ); } } diff --git a/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php b/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php deleted file mode 100644 index 7066cffd..00000000 --- a/src/Persistence/Mapping/Driver/DirectoryFilesIterator.php +++ /dev/null @@ -1,61 +0,0 @@ - */ -final class DirectoryFilesIterator implements IteratorAggregate -{ - public function __construct( - /** @var list */ - private readonly array $paths, - private readonly string $fileExtension = '.php', - ) { - } - - /** - * @return Iterator - * - * @throws MappingException - */ - public function getIterator(): Iterator - { - /** @var AppendIterator> $filesIterator */ - $filesIterator = new AppendIterator(); - - foreach ($this->paths as $path) { - if (! is_dir($path)) { - throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($path); - } - - /** @var Iterator $iterator */ - $iterator = new RegexIterator( - new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS), - RecursiveIteratorIterator::LEAVES_ONLY, - ), - sprintf('/%s$/', preg_quote($this->fileExtension, '/')), - RegexIterator::MATCH, - ); - - $filesIterator->append($iterator); - } - - return $filesIterator; - } -} diff --git a/src/Persistence/Mapping/Driver/FileClassLocator.php b/src/Persistence/Mapping/Driver/FileClassLocator.php new file mode 100644 index 00000000..3027b72b --- /dev/null +++ b/src/Persistence/Mapping/Driver/FileClassLocator.php @@ -0,0 +1,174 @@ + $fileNames An iterable of file names to include. + * + * @throws MappingException if any of the files do not exist. + */ + public function __construct( + private iterable $fileNames, + ) { + } + + /** @return list */ + public function getClassNames(): array + { + $includedFiles = []; + + foreach ($this->fileNames as $fileName) { + if (! is_file($fileName)) { + throw MappingException::fileDoesNotExist($fileName); + } + + // realpath() can return false if the file is in a phar archive + // @phpstan-ignore ternary.shortNotAllowed + $fileName = realpath($fileName) ?: $fileName; + if (isset($includedFiles[$fileName])) { + continue; + } + + $includedFiles[$fileName] = true; + require_once $fileName; + } + + $classes = []; + foreach (get_declared_classes() as $className) { + $fileName = (new ReflectionClass($className))->getFileName(); + + if ($fileName === false || ! array_key_exists($fileName, $includedFiles)) { + continue; + } + + $classes[] = $className; + } + + return $classes; + } + + /** + * Creates a FileClassLocator from an array of directories. + * + * @param list $directories + * @param list $excludedDirectories Directories to exclude from the search. + * @param string $fileExtension The file extension to look for (default is '.php'). + * + * @throws MappingException if any of the directories are not valid. + */ + public static function createFromDirectories( + array $directories, + array $excludedDirectories = [], + string $fileExtension = '.php', + ): self { + $filesIterator = new AppendIterator(); + + foreach ($directories as $directory) { + if (! is_dir($directory)) { + throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($directory); + } + + /** @var Iterator $iterator */ + $iterator = new RegexIterator( + new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($directory, FilesystemIterator::SKIP_DOTS), + RecursiveIteratorIterator::LEAVES_ONLY, + ), + sprintf('/%s$/', preg_quote($fileExtension, '/')), + RegexIterator::MATCH, + ); + + $filesIterator->append($iterator); + } + + if ($excludedDirectories !== []) { + // Get + $excludedDirectories = array_map( + // @phpstan-ignore ternary.shortNotAllowed + static fn (string $dir): string => realpath($dir) ?: $dir, + $excludedDirectories, + ); + + $filesIterator = new CallbackFilterIterator( + $filesIterator, + static function (SplFileInfo $file) use ($excludedDirectories): bool { + if (str_starts_with($file->getPath(), 'phar:')) { + $sourceFile = $file->getPathname(); + } else { + $sourceFile = $file->getRealPath(); + } + + foreach ($excludedDirectories as $excludedDirectory) { + if (str_starts_with($sourceFile, $excludedDirectory)) { + return false; + } + } + + return true; + }, + ); + } + + return self::createFromSplFiles($filesIterator); + } + + /** + * Creates a FileClassLocator from an iterable of SplFileInfo objects. + * + * This method can be used with a Symfony Finder or any other iterable + * + * @param iterable $files + */ + public static function createFromSplFiles(iterable $files): self + { + return new self((static function ($files) { + foreach ($files as $file) { + // @phpstan-ignore function.alreadyNarrowedType, instanceof.alwaysTrue + assert($file instanceof SplFileInfo, new InvalidArgumentException(sprintf('Expected an iterable of SplFileInfo, got %s', get_debug_type($file)))); + + // Skip directories or non-file entries + if (! $file->isFile()) { + continue; + } + + // Files in phar does not have a real path, so we use the pathname + // @phpstan-ignore ternary.shortNotAllowed + yield $file->getRealPath() ?: $file->getPathname(); + } + })($files)); + } +} diff --git a/src/Persistence/Mapping/Driver/FilePathNameIterator.php b/src/Persistence/Mapping/Driver/FilePathNameIterator.php deleted file mode 100644 index 03ebb930..00000000 --- a/src/Persistence/Mapping/Driver/FilePathNameIterator.php +++ /dev/null @@ -1,27 +0,0 @@ - */ -final class FilePathNameIterator implements IteratorAggregate -{ - public function __construct( - /** @var iterable */ - private readonly iterable $filesIterator, - ) { - } - - /** @return Generator */ - public function getIterator(): Generator - { - foreach ($this->filesIterator as $file) { - yield $file->getPathname(); - } - } -} diff --git a/src/Persistence/Mapping/MappingException.php b/src/Persistence/Mapping/MappingException.php index 3037ddfc..212b640d 100644 --- a/src/Persistence/Mapping/MappingException.php +++ b/src/Persistence/Mapping/MappingException.php @@ -67,6 +67,11 @@ public static function invalidMappingFile(string $entityName, string $fileName): )); } + public static function fileDoesNotExist(string $fileName): self + { + return new self(sprintf("File '%s' does not exist", $fileName)); + } + public static function nonExistingClass(string $className): self { return new self(sprintf("Class '%s' does not exist", $className)); diff --git a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php index 9010b68e..07c3da62 100644 --- a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php +++ b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php @@ -5,16 +5,16 @@ namespace Doctrine\Tests\Persistence\Mapping; use Doctrine\Persistence\Mapping\ClassMetadata; +use Doctrine\Persistence\Mapping\Driver\ClassLocator; use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver; +use Doctrine\Persistence\Mapping\Driver\FileClassLocator; use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Doctrine\Tests\Persistence\Mapping\_files\colocated\Entity; use Doctrine\Tests\Persistence\Mapping\_files\colocated\EntityFixture; use Doctrine\Tests\Persistence\Mapping\_files\colocated\TestClass; use Generator; use PHPUnit\Framework\TestCase; -use Traversable; -use function is_file; use function sort; class ColocatedMappingDriverTest extends TestCase @@ -112,7 +112,7 @@ private function createDirectoryPathDriver(string $dirPath): MyDriver /** @param list $filePaths */ private function createFilePathsDriver(array $filePaths): MyDriver { - return new MyDriver($filePaths); + return new MyDriver(new FileClassLocator($filePaths)); } } @@ -120,15 +120,13 @@ final class MyDriver implements MappingDriver { use ColocatedMappingDriver; - /** @param iterable $paths One or multiple paths where mapping classes can be found. */ - public function __construct(iterable $paths) + /** @param string[]|ClassLocator $paths One or multiple paths where mapping classes can be found. */ + public function __construct(array|ClassLocator $paths) { - $isFilePaths = $paths instanceof Traversable || is_file($paths[0]); - - if (! $isFilePaths) { - $this->paths = $paths; + if ($paths instanceof ClassLocator) { + $this->classLocator = $paths; } else { - $this->filePaths = $paths; + $this->paths = $paths; } } diff --git a/tests/Persistence/Mapping/Driver/ClassNamesTest.php b/tests/Persistence/Mapping/Driver/ClassNamesTest.php new file mode 100644 index 00000000..869f0551 --- /dev/null +++ b/tests/Persistence/Mapping/Driver/ClassNamesTest.php @@ -0,0 +1,25 @@ +getClassNames()); + } +} diff --git a/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php new file mode 100644 index 00000000..9f245cb4 --- /dev/null +++ b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php @@ -0,0 +1,163 @@ +getClassNames(); + sort($classes, SORT_STRING); + + self::assertSame([ + Entity::class, + EntityFixture::class, + ], $classes); + } + + public function testFileDoesNotExistException(): void + { + $this->expectException(MappingException::class); + $this->expectExceptionMessage("File '/non/existent/file' does not exist"); + + $locator = new FileClassLocator(['/non/existent/file']); + $locator->getClassNames(); + } + + public function testGetClassNamesWithEmptyIterator(): void + { + $locator = new FileClassLocator(new EmptyIterator()); + self::assertSame([], $locator->getClassNames()); + } + + public function testCreateFromDirectory(): void + { + $locator = FileClassLocator::createFromDirectories([__DIR__ . '/../_files/colocated']); + + $classes = $locator->getClassNames(); + sort($classes, SORT_STRING); + + self::assertSame([ + Entity::class, + EntityFixture::class, + TestClass::class, + ], $classes); + } + + public function testCreateFromDirectoryWithExtension(): void + { + $locator = FileClassLocator::createFromDirectories([__DIR__ . '/../_files/colocated'], [], '.mphp'); + + $classes = $locator->getClassNames(); + sort($classes, SORT_STRING); + + // @phpstan-ignore staticMethod.impossibleType + self::assertSame(['Doctrine\Tests\Persistence\Mapping\_files\colocated\Foo'], $classes); + } + + public function testCreateFromDirectoryWithNonExistentDirectory(): void + { + $this->expectException(MappingException::class); + $this->expectExceptionMessage('File mapping drivers must have a valid directory path, however the given path [/non/existent/directory] seems to be incorrect!'); + + FileClassLocator::createFromDirectories(['/non/existent/directory']); + } + + public function testCreateFromEmptyDirectory(): void + { + $locator = FileClassLocator::createFromDirectories([__DIR__ . '/../_files/Bar']); + + self::assertSame([], $locator->getClassNames()); + } + + public function testCreateFromSplFileIterator(): void + { + $locator = FileClassLocator::createFromSplFiles( + new DirectoryIterator(__DIR__ . '/../_files/colocated'), + ); + + $classes = $locator->getClassNames(); + sort($classes, SORT_STRING); + + self::assertSame([ + Entity::class, + EntityFixture::class, + 'Doctrine\Tests\Persistence\Mapping\_files\colocated\Foo', + TestClass::class, + ], $classes); + } + + public function testCreateFromSymfonyFinder(): void + { + $finder = Finder::create() + ->in(__DIR__ . '/../_files/colocated') + ->name('*.php') + ->notName('Test*'); + + $locator = FileClassLocator::createFromSplFiles($finder); + + $classes = $locator->getClassNames(); + sort($classes, SORT_STRING); + + self::assertSame([ + Entity::class, + EntityFixture::class, + ], $classes); + } + + public function testWithPharFiles(): void + { + if (ini_get('phar.readonly') === '1') { + self::markTestSkipped('creating archive disabled by the php.ini setting phar.readonly'); + } + + // Create a temporary Phar file with a PHP class + $pharFile = dirname(__DIR__) . '/_files/colocated.phar'; + $phar = new Phar($pharFile); + $phar->startBuffering(); + $phar->addFromString('Entity.php', 'addFromString('EntityFixture.php', 'addFromString('Excluded/EntityExcluded.php', 'addFromString('Foo.mphp', 'stopBuffering(); + + $locator = FileClassLocator::createFromDirectories(['phar://' . $pharFile], ['phar://' . $pharFile . '/Excluded']); + + $classes = $locator->getClassNames(); + sort($classes, SORT_STRING); + unlink($pharFile); + + // @phpstan-ignore staticMethod.impossibleType + self::assertSame([ + 'Doctrine\Phar\Entity', + 'Doctrine\Phar\EntityFixture', + ], $classes); + } +} From 539d8eac731d30b30dfb539a8b9b49977cb843d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 13 Aug 2025 18:33:50 +0200 Subject: [PATCH 06/12] Refactor getAllClassNames with early throw --- .../Mapping/Driver/ColocatedMappingDriver.php | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index 6408f2de..35655565 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -15,7 +15,7 @@ */ trait ColocatedMappingDriver { - private ClassLocator $classLocator; + private ClassLocator|null $classLocator = null; /** * The directory paths where to look for mapping files. @@ -116,21 +116,19 @@ public function getAllClassNames(): array return $this->classNames; } - if ($this->paths !== []) { - $classNames = FileClassLocator::createFromDirectories($this->paths, $this->excludePaths, $this->fileExtension)->getClassNames(); - - if (isset($this->classLocator)) { - $classNames = array_unique([ - ...$classNames, - ...$this->classLocator->getClassNames(), - ]); - } - } elseif (isset($this->classLocator)) { - $classNames = $this->classLocator->getClassNames(); - } else { + if ($this->paths === [] && $this->classLocator === null) { throw MappingException::pathRequiredForDriver(static::class); } + $classNames = $this->classLocator?->getClassNames() ?? []; + + if ($this->paths !== []) { + $classNames = array_unique([ + ...FileClassLocator::createFromDirectories($this->paths, $this->excludePaths, $this->fileExtension)->getClassNames(), + ...$classNames, + ]); + } + return $this->classNames = array_filter( $classNames, fn (string $className): bool => ! $this->isTransient($className), From 32063d9a7b27fedc0f9b833b42451c963b3af458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 13 Aug 2025 19:17:19 +0200 Subject: [PATCH 07/12] Fix array-key in class name list --- .../Mapping/Driver/ColocatedMappingDriver.php | 5 +++-- .../Mapping/Driver/FileClassLocator.php | 20 +++++-------------- .../Mapping/ColocatedMappingDriverTest.php | 4 +++- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php index 35655565..745c1861 100644 --- a/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php +++ b/src/Persistence/Mapping/Driver/ColocatedMappingDriver.php @@ -9,6 +9,7 @@ use function array_filter; use function array_merge; use function array_unique; +use function array_values; /** * The ColocatedMappingDriver reads the mapping metadata located near the code. @@ -129,9 +130,9 @@ public function getAllClassNames(): array ]); } - return $this->classNames = array_filter( + return $this->classNames = array_values(array_filter( $classNames, fn (string $className): bool => ! $this->isTransient($className), - ); + )); } } diff --git a/src/Persistence/Mapping/Driver/FileClassLocator.php b/src/Persistence/Mapping/Driver/FileClassLocator.php index 3027b72b..88212299 100644 --- a/src/Persistence/Mapping/Driver/FileClassLocator.php +++ b/src/Persistence/Mapping/Driver/FileClassLocator.php @@ -26,6 +26,7 @@ use function preg_quote; use function realpath; use function sprintf; +use function str_replace; use function str_starts_with; /** @@ -36,11 +37,7 @@ */ final class FileClassLocator implements ClassLocator { - /** - * @param iterable $fileNames An iterable of file names to include. - * - * @throws MappingException if any of the files do not exist. - */ + /** @param iterable $fileNames An iterable of file names to include. */ public function __construct( private iterable $fileNames, ) { @@ -59,9 +56,6 @@ public function getClassNames(): array // realpath() can return false if the file is in a phar archive // @phpstan-ignore ternary.shortNotAllowed $fileName = realpath($fileName) ?: $fileName; - if (isset($includedFiles[$fileName])) { - continue; - } $includedFiles[$fileName] = true; require_once $fileName; @@ -116,21 +110,17 @@ public static function createFromDirectories( } if ($excludedDirectories !== []) { - // Get $excludedDirectories = array_map( // @phpstan-ignore ternary.shortNotAllowed - static fn (string $dir): string => realpath($dir) ?: $dir, + static fn (string $dir): string => str_replace('\\', '/', realpath($dir) ?: $dir), $excludedDirectories, ); $filesIterator = new CallbackFilterIterator( $filesIterator, static function (SplFileInfo $file) use ($excludedDirectories): bool { - if (str_starts_with($file->getPath(), 'phar:')) { - $sourceFile = $file->getPathname(); - } else { - $sourceFile = $file->getRealPath(); - } + // @phpstan-ignore ternary.shortNotAllowed + $sourceFile = str_replace('\\', '/', $file->getRealPath() ?: $file->getPathname()); foreach ($excludedDirectories as $excludedDirectory) { if (str_starts_with($sourceFile, $excludedDirectory)) { diff --git a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php index 07c3da62..89f3156b 100644 --- a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php +++ b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php @@ -72,8 +72,10 @@ public function testGetAllClassNamesForDirectory(string $dirPath): void public function testGetAllClassNamesForFilePaths(): void { $driver = $this->createFilePathsDriver([ - __DIR__ . '/_files/colocated/Entity.php', __DIR__ . '/_files/colocated/TestClass.php', + // This file is after the transient class, to validate that getAllClassNames() + // returns a list without gaps in the indexes + __DIR__ . '/_files/colocated/Entity.php', ]); $classes = $driver->getAllClassNames(); From b2a26ec2e2bb5cfa16a988fd15a48647cf6944a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 13 Aug 2025 20:58:45 +0200 Subject: [PATCH 08/12] Make FileClassLocator accept an iterable of SplFileInfo --- .../Mapping/Driver/FileClassLocator.php | 45 +++++-------------- src/Persistence/Mapping/MappingException.php | 5 --- .../Mapping/ColocatedMappingDriverTest.php | 23 +++++----- .../Mapping/Driver/FileClassLocatorTest.php | 22 +++------ 4 files changed, 29 insertions(+), 66 deletions(-) diff --git a/src/Persistence/Mapping/Driver/FileClassLocator.php b/src/Persistence/Mapping/Driver/FileClassLocator.php index 88212299..0c7c29af 100644 --- a/src/Persistence/Mapping/Driver/FileClassLocator.php +++ b/src/Persistence/Mapping/Driver/FileClassLocator.php @@ -22,7 +22,6 @@ use function get_debug_type; use function get_declared_classes; use function is_dir; -use function is_file; use function preg_quote; use function realpath; use function sprintf; @@ -37,9 +36,9 @@ */ final class FileClassLocator implements ClassLocator { - /** @param iterable $fileNames An iterable of file names to include. */ + /** @param iterable $files An iterable of file names to include. */ public function __construct( - private iterable $fileNames, + private iterable $files, ) { } @@ -48,14 +47,18 @@ public function getClassNames(): array { $includedFiles = []; - foreach ($this->fileNames as $fileName) { - if (! is_file($fileName)) { - throw MappingException::fileDoesNotExist($fileName); + foreach ($this->files as $file) { + // @phpstan-ignore function.alreadyNarrowedType, instanceof.alwaysTrue + assert($file instanceof SplFileInfo, new InvalidArgumentException(sprintf('Expected an iterable of SplFileInfo, got %s', get_debug_type($file)))); + + // Skip non-files + if (! $file->isFile()) { + continue; } // realpath() can return false if the file is in a phar archive // @phpstan-ignore ternary.shortNotAllowed - $fileName = realpath($fileName) ?: $fileName; + $fileName = $file->getRealPath() ?: $file->getPathname(); $includedFiles[$fileName] = true; require_once $fileName; @@ -133,32 +136,6 @@ static function (SplFileInfo $file) use ($excludedDirectories): bool { ); } - return self::createFromSplFiles($filesIterator); - } - - /** - * Creates a FileClassLocator from an iterable of SplFileInfo objects. - * - * This method can be used with a Symfony Finder or any other iterable - * - * @param iterable $files - */ - public static function createFromSplFiles(iterable $files): self - { - return new self((static function ($files) { - foreach ($files as $file) { - // @phpstan-ignore function.alreadyNarrowedType, instanceof.alwaysTrue - assert($file instanceof SplFileInfo, new InvalidArgumentException(sprintf('Expected an iterable of SplFileInfo, got %s', get_debug_type($file)))); - - // Skip directories or non-file entries - if (! $file->isFile()) { - continue; - } - - // Files in phar does not have a real path, so we use the pathname - // @phpstan-ignore ternary.shortNotAllowed - yield $file->getRealPath() ?: $file->getPathname(); - } - })($files)); + return new self($filesIterator); } } diff --git a/src/Persistence/Mapping/MappingException.php b/src/Persistence/Mapping/MappingException.php index 212b640d..3037ddfc 100644 --- a/src/Persistence/Mapping/MappingException.php +++ b/src/Persistence/Mapping/MappingException.php @@ -67,11 +67,6 @@ public static function invalidMappingFile(string $entityName, string $fileName): )); } - public static function fileDoesNotExist(string $fileName): self - { - return new self(sprintf("File '%s' does not exist", $fileName)); - } - public static function nonExistingClass(string $className): self { return new self(sprintf("Class '%s' does not exist", $className)); diff --git a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php index 89f3156b..3dd5b974 100644 --- a/tests/Persistence/Mapping/ColocatedMappingDriverTest.php +++ b/tests/Persistence/Mapping/ColocatedMappingDriverTest.php @@ -6,8 +6,8 @@ use Doctrine\Persistence\Mapping\ClassMetadata; use Doctrine\Persistence\Mapping\Driver\ClassLocator; +use Doctrine\Persistence\Mapping\Driver\ClassNames; use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver; -use Doctrine\Persistence\Mapping\Driver\FileClassLocator; use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Doctrine\Tests\Persistence\Mapping\_files\colocated\Entity; use Doctrine\Tests\Persistence\Mapping\_files\colocated\EntityFixture; @@ -69,13 +69,14 @@ public function testGetAllClassNamesForDirectory(string $dirPath): void self::assertSame([Entity::class, EntityFixture::class], $classes); } - public function testGetAllClassNamesForFilePaths(): void + public function testGetAllClassNamesRemovesTransient(): void { - $driver = $this->createFilePathsDriver([ - __DIR__ . '/_files/colocated/TestClass.php', - // This file is after the transient class, to validate that getAllClassNames() - // returns a list without gaps in the indexes - __DIR__ . '/_files/colocated/Entity.php', + $driver = $this->createClassNamesDriver([ + // This class is transient, so it should not be returned by getAllClassNames() + // placed before the Entity class to validate that the driver returns an + // array without gaps in the indexes + TestClass::class, + Entity::class, ]); $classes = $driver->getAllClassNames(); @@ -85,7 +86,7 @@ public function testGetAllClassNamesForFilePaths(): void public function testGetAllClassNamesWorksBothForFilePathsAndRetroactivelyAddedDirectoryPaths(): void { - $driver = $this->createFilePathsDriver([__DIR__ . '/_files/colocated/Entity.php']); + $driver = $this->createClassNamesDriver([Entity::class]); $driver->addPaths([__DIR__ . '/_files/colocated/']); @@ -111,10 +112,10 @@ private function createDirectoryPathDriver(string $dirPath): MyDriver return new MyDriver([$dirPath]); } - /** @param list $filePaths */ - private function createFilePathsDriver(array $filePaths): MyDriver + /** @param list $classes */ + private function createClassNamesDriver(array $classes): MyDriver { - return new MyDriver(new FileClassLocator($filePaths)); + return new MyDriver(new ClassNames($classes)); } } diff --git a/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php index 9f245cb4..d02dbe76 100644 --- a/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php +++ b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php @@ -13,6 +13,7 @@ use Doctrine\Tests\Persistence\Mapping\_files\colocated\TestClass; use EmptyIterator; use Phar; +use SplFileInfo; use Symfony\Component\Finder\Finder; use function dirname; @@ -27,8 +28,8 @@ final class FileClassLocatorTest extends DoctrineTestCase public function testGetClassNames(): void { $locator = new FileClassLocator([ - __DIR__ . '/../_files/colocated/Entity.php', - __DIR__ . '/../_files/colocated/EntityFixture.php', + new SplFileInfo(__DIR__ . '/../_files/colocated/Entity.php'), + new SplFileInfo(__DIR__ . '/../_files/colocated/EntityFixture.php'), ]); $classes = $locator->getClassNames(); @@ -40,15 +41,6 @@ public function testGetClassNames(): void ], $classes); } - public function testFileDoesNotExistException(): void - { - $this->expectException(MappingException::class); - $this->expectExceptionMessage("File '/non/existent/file' does not exist"); - - $locator = new FileClassLocator(['/non/existent/file']); - $locator->getClassNames(); - } - public function testGetClassNamesWithEmptyIterator(): void { $locator = new FileClassLocator(new EmptyIterator()); @@ -95,11 +87,9 @@ public function testCreateFromEmptyDirectory(): void self::assertSame([], $locator->getClassNames()); } - public function testCreateFromSplFileIterator(): void + public function testCreateFromDirectoryIterator(): void { - $locator = FileClassLocator::createFromSplFiles( - new DirectoryIterator(__DIR__ . '/../_files/colocated'), - ); + $locator = new FileClassLocator(new DirectoryIterator(__DIR__ . '/../_files/colocated')); $classes = $locator->getClassNames(); sort($classes, SORT_STRING); @@ -119,7 +109,7 @@ public function testCreateFromSymfonyFinder(): void ->name('*.php') ->notName('Test*'); - $locator = FileClassLocator::createFromSplFiles($finder); + $locator = new FileClassLocator($finder); $classes = $locator->getClassNames(); sort($classes, SORT_STRING); From 2b0e795eaa43b010d5ce82291145b896bb13d39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 13 Aug 2025 21:04:17 +0200 Subject: [PATCH 09/12] Import class from Foo.mphp file --- phpstan.neon | 1 + tests/Persistence/Mapping/Driver/FileClassLocatorTest.php | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index ec520037..2d2827e9 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -11,6 +11,7 @@ parameters: paths: - src - tests + - tests/Persistence/Mapping/_files/colocated/Foo.mphp excludePaths: - tests/Persistence/Mapping/_files/Doctrine.Tests.Persistence.Mapping.PHPTestEntity.php diff --git a/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php index d02dbe76..abf57a53 100644 --- a/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php +++ b/tests/Persistence/Mapping/Driver/FileClassLocatorTest.php @@ -10,6 +10,7 @@ use Doctrine\Tests\DoctrineTestCase; use Doctrine\Tests\Persistence\Mapping\_files\colocated\Entity; use Doctrine\Tests\Persistence\Mapping\_files\colocated\EntityFixture; +use Doctrine\Tests\Persistence\Mapping\_files\colocated\Foo; use Doctrine\Tests\Persistence\Mapping\_files\colocated\TestClass; use EmptyIterator; use Phar; @@ -68,8 +69,7 @@ public function testCreateFromDirectoryWithExtension(): void $classes = $locator->getClassNames(); sort($classes, SORT_STRING); - // @phpstan-ignore staticMethod.impossibleType - self::assertSame(['Doctrine\Tests\Persistence\Mapping\_files\colocated\Foo'], $classes); + self::assertSame([Foo::class], $classes); } public function testCreateFromDirectoryWithNonExistentDirectory(): void @@ -97,7 +97,7 @@ public function testCreateFromDirectoryIterator(): void self::assertSame([ Entity::class, EntityFixture::class, - 'Doctrine\Tests\Persistence\Mapping\_files\colocated\Foo', + Foo::class, TestClass::class, ], $classes); } From 76acfc039224b62aed598aea573458d545630254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 13 Aug 2025 21:15:37 +0200 Subject: [PATCH 10/12] ternary.shortNotAllowed identifier added in v1.6 https://github.com/phpstan/phpstan-strict-rules/commit/477f53a560823cef4751429d91d62630ce265992 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b742a8e3..971abb76 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,7 @@ "require-dev": { "phpstan/phpstan": "1.12.7", "phpstan/phpstan-phpunit": "^1", - "phpstan/phpstan-strict-rules": "^1.1", + "phpstan/phpstan-strict-rules": "^1.6", "doctrine/coding-standard": "^12", "phpunit/phpunit": "^9.6", "symfony/cache": "^4.4 || ^5.4 || ^6.0 || ^7.0", From f75ac79a5f65dd8872674dd85d092b095d975ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 14 Aug 2025 09:42:17 +0200 Subject: [PATCH 11/12] Fix comment Co-authored-by: Yevhen Sidelnyk <41589422+rela589n@users.noreply.github.com> --- src/Persistence/Mapping/Driver/FileClassLocator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Persistence/Mapping/Driver/FileClassLocator.php b/src/Persistence/Mapping/Driver/FileClassLocator.php index 0c7c29af..8df88f74 100644 --- a/src/Persistence/Mapping/Driver/FileClassLocator.php +++ b/src/Persistence/Mapping/Driver/FileClassLocator.php @@ -36,7 +36,7 @@ */ final class FileClassLocator implements ClassLocator { - /** @param iterable $files An iterable of file names to include. */ + /** @param iterable $files An iterable of files to include. */ public function __construct( private iterable $files, ) { From 49858ad1ea929aa26dd8ac561d808ae317d50459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 14 Aug 2025 14:09:57 +0200 Subject: [PATCH 12/12] Update comment for ternary.shortNotAllowed and realpath --- src/Persistence/Mapping/Driver/FileClassLocator.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Persistence/Mapping/Driver/FileClassLocator.php b/src/Persistence/Mapping/Driver/FileClassLocator.php index 8df88f74..dae77929 100644 --- a/src/Persistence/Mapping/Driver/FileClassLocator.php +++ b/src/Persistence/Mapping/Driver/FileClassLocator.php @@ -56,8 +56,8 @@ public function getClassNames(): array continue; } - // realpath() can return false if the file is in a phar archive - // @phpstan-ignore ternary.shortNotAllowed + // getRealPath() returns false if the file is in a phar archive + // @phpstan-ignore ternary.shortNotAllowed (false is the only falsy value getRealPath() may return) $fileName = $file->getRealPath() ?: $file->getPathname(); $includedFiles[$fileName] = true; @@ -114,7 +114,8 @@ public static function createFromDirectories( if ($excludedDirectories !== []) { $excludedDirectories = array_map( - // @phpstan-ignore ternary.shortNotAllowed + // realpath() returns false if the file is in a phar archive + // @phpstan-ignore ternary.shortNotAllowed (false is the only falsy value realpath() may return) static fn (string $dir): string => str_replace('\\', '/', realpath($dir) ?: $dir), $excludedDirectories, ); @@ -122,7 +123,8 @@ public static function createFromDirectories( $filesIterator = new CallbackFilterIterator( $filesIterator, static function (SplFileInfo $file) use ($excludedDirectories): bool { - // @phpstan-ignore ternary.shortNotAllowed + // getRealPath() returns false if the file is in a phar archive + // @phpstan-ignore ternary.shortNotAllowed (false is the only falsy value getRealPath() may return) $sourceFile = str_replace('\\', '/', $file->getRealPath() ?: $file->getPathname()); foreach ($excludedDirectories as $excludedDirectory) {