From 248128557ef197fc3c145931744a392784ae0b99 Mon Sep 17 00:00:00 2001 From: mscherer Date: Mon, 11 May 2026 15:05:43 +0200 Subject: [PATCH] Fix Generator path regex to tolerate either directory separator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findExistingDtos()` and `findExistingMappers()` both used regex patterns with hardcoded forward slashes: #src/Dto/(.+)\.php$# #Mapper/(.+)Mapper\.php$# RecursiveDirectoryIterator yields paths using the host OS separator, so on Windows where paths come back with backslashes the regex never matched — and the "delete generated DTOs no longer in the spec" pass silently no-op'd, leaving stale files behind on every regenerate. Replacement pattern accepts either separator via a character class `[/\\\\]`. Captured nested paths are normalised to forward-slash form so the same nested entry (e.g. `Sub/Foo` on Linux, `Sub\Foo` on Windows) doesn't land in the result map as two distinct keys depending on host OS. Driving the Windows path on a Linux CI box is awkward; the regression tests exercise the host OS path layout (so the now-DS-tolerant regex still matches the common case) plus the new normalisation step for nested directories. Both tests cover the Dto and Mapper variants symmetrically. phpunit (809/809), phpstan, phpcs all clean. --- src/Generator/Generator.php | 22 +++++--- tests/Generator/GeneratorTest.php | 86 +++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/Generator/Generator.php b/src/Generator/Generator.php index 6f7e20c..5b4b101 100644 --- a/src/Generator/Generator.php +++ b/src/Generator/Generator.php @@ -243,17 +243,25 @@ protected function findExistingDtos(string $path): array $directory = new RecursiveDirectoryIterator($path); $iterator = new RecursiveIteratorIterator($directory); + $suffix = $this->config->get('suffix', 'Dto'); + // Match both `src/Dto/...` (Linux/macOS) and `src\Dto\...` (Windows). + // RecursiveDirectoryIterator yields paths using the host OS separator, + // so the previous forward-slash-only pattern never matched on Windows + // and the "delete DTOs no longer in the spec" pass silently no-op'd, + // leaving stale generated files behind. + $pattern = '#[/\\\\]src[/\\\\]Dto[/\\\\](.+)' . preg_quote($suffix, '#') . '\.php$#'; foreach ($iterator as $fileInfo) { $file = $fileInfo->getPathname(); - $suffix = $this->config->get('suffix', 'Dto'); - if (!preg_match('#src/Dto/(.+)' . preg_quote($suffix, '#') . '\.php$#', $file, $matches)) { + if (!preg_match($pattern, $file, $matches)) { continue; } // Skip mapper files if (str_contains($file, DIRECTORY_SEPARATOR . 'Mapper' . DIRECTORY_SEPARATOR)) { continue; } - $name = $matches[1]; + // Normalize captured nested paths so `Sub/Foo` and `Sub\\Foo` + // don't both land in the result map as separate entries. + $name = strtr($matches[1], '\\', '/'); $files[$name] = $file; } @@ -275,13 +283,15 @@ protected function findExistingMappers(string $path): array $directory = new RecursiveDirectoryIterator($path); $iterator = new RecursiveIteratorIterator($directory); + $suffix = $this->config->get('suffix', 'Dto'); + // Same DS-tolerant pattern as findExistingDtos — see comment there. + $pattern = '#[/\\\\]Mapper[/\\\\](.+)' . preg_quote($suffix, '#') . 'Mapper\.php$#'; foreach ($iterator as $fileInfo) { $file = $fileInfo->getPathname(); - $suffix = $this->config->get('suffix', 'Dto'); - if (!preg_match('#Mapper/(.+)' . preg_quote($suffix, '#') . 'Mapper\.php$#', $file, $matches)) { + if (!preg_match($pattern, $file, $matches)) { continue; } - $name = $matches[1]; + $name = strtr($matches[1], '\\', '/'); $files[$name] = $file; } diff --git a/tests/Generator/GeneratorTest.php b/tests/Generator/GeneratorTest.php index 2c321ff..ffae985 100644 --- a/tests/Generator/GeneratorTest.php +++ b/tests/Generator/GeneratorTest.php @@ -11,6 +11,7 @@ use PhpCollective\Dto\Generator\IoInterface; use PhpCollective\Dto\Generator\RendererInterface; use PHPUnit\Framework\TestCase; +use ReflectionMethod; /** * Tests for Generator class. @@ -55,4 +56,89 @@ public function testPathTraversalInDtoNameThrowsException(): void @rmdir($tempDir); } } + + /** + * Regression: `findExistingDtos()` matched `#src/Dto/(.+)\.php$#` + * which hardcoded forward slashes. On Windows the RecursiveDirectoryIterator + * yields paths with backslashes and the regex never matched — so the + * "delete DTOs no longer in the spec" pass silently no-op'd, leaving stale + * generated files behind. The replacement pattern accepts either + * separator and normalizes captured nested paths to `/` so the same + * nested entry isn't double-counted across OSes. + * + * Exercising the Windows path directly on a Linux CI box is awkward, so + * the now-DS-tolerant regex is driven via a temp-dir round trip on the + * host OS path layout and the test additionally asserts that nested- + * directory entries get normalized to the canonical forward-slash form. + * + * @return void + */ + public function testFindExistingDtosToleratesEitherDirectorySeparator(): void + { + $generator = new Generator( + $this->createMock(Builder::class), + $this->createMock(RendererInterface::class), + $this->createMock(IoInterface::class), + new ArrayConfig([]), + ); + $method = new ReflectionMethod(Generator::class, 'findExistingDtos'); + + $dir = sys_get_temp_dir() . '/dto-find-existing-' . uniqid(); + mkdir($dir . '/src/Dto/Nested', 0777, true); + file_put_contents($dir . '/src/Dto/FooDto.php', 'invoke($generator, $dir . '/src/Dto'); + $keys = array_keys($result); + sort($keys); + $this->assertSame(['Foo', 'Nested/Bar'], $keys); + $this->assertArrayHasKey('Nested/Bar', $result); + } finally { + @unlink($dir . '/src/Dto/FooDto.php'); + @unlink($dir . '/src/Dto/Nested/BarDto.php'); + @rmdir($dir . '/src/Dto/Nested'); + @rmdir($dir . '/src/Dto'); + @rmdir($dir . '/src'); + @rmdir($dir); + } + } + + /** + * Mirror regression for `findExistingMappers()` — same forward-slash + * hardcode in `#Mapper/(.+)Mapper\.php$#`, same fix. + * + * @return void + */ + public function testFindExistingMappersToleratesEitherDirectorySeparator(): void + { + $generator = new Generator( + $this->createMock(Builder::class), + $this->createMock(RendererInterface::class), + $this->createMock(IoInterface::class), + new ArrayConfig([]), + ); + $method = new ReflectionMethod(Generator::class, 'findExistingMappers'); + + $dir = sys_get_temp_dir() . '/dto-find-mappers-' . uniqid(); + mkdir($dir . '/src/Dto/Mapper/Nested', 0777, true); + file_put_contents($dir . '/src/Dto/Mapper/FooDtoMapper.php', 'invoke($generator, $dir . '/src/Dto/Mapper'); + $keys = array_keys($result); + sort($keys); + $this->assertSame(['Foo', 'Nested/Bar'], $keys); + $this->assertArrayHasKey('Nested/Bar', $result); + } finally { + @unlink($dir . '/src/Dto/Mapper/FooDtoMapper.php'); + @unlink($dir . '/src/Dto/Mapper/Nested/BarDtoMapper.php'); + @rmdir($dir . '/src/Dto/Mapper/Nested'); + @rmdir($dir . '/src/Dto/Mapper'); + @rmdir($dir . '/src/Dto'); + @rmdir($dir . '/src'); + @rmdir($dir); + } + } }