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); + } + } }