From 79eb121f32eb8da5b78db260b6e7f87616923a37 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 3 Oct 2024 17:57:27 +0200 Subject: [PATCH] Fix parsing of ambiguous classes to avoid ignoring real classes when test ones get scanned first Refs composer/composer#12140 --- phpstan-baseline.neon | 5 +++++ src/ClassMap.php | 31 +++++++++++++++++++++++++-- src/ClassMapGenerator.php | 2 +- tests/ClassMapGeneratorTest.php | 37 ++++++++++++++++++++++++++------- 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1b924ad..6752e97 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -9,3 +9,8 @@ parameters: message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayHasKey\\(\\) with 'A' and array\\\\> will always evaluate to false\\.$#" count: 1 path: tests/ClassMapGeneratorTest.php + + - + message: "#^Offset 'A' does not exist on non\\-empty\\-array\\\\>\\.$#" + count: 4 + path: tests/ClassMapGeneratorTest.php diff --git a/src/ClassMap.php b/src/ClassMap.php index 886eb08..0d06366 100644 --- a/src/ClassMap.php +++ b/src/ClassMap.php @@ -12,6 +12,8 @@ namespace Composer\ClassMapGenerator; +use Composer\Pcre\Preg; + /** * @author Jordi Boggiano */ @@ -71,11 +73,36 @@ public function getPsrViolations(): array * * To get the path the class is being mapped to, call getClassPath * + * By default, paths that contain test(s), fixture(s), example(s) or stub(s) are ignored + * as those are typically not problematic when they're dummy classes in the tests folder. + * If you want to get these back as well you can pass false to $duplicatesFilter. Or + * you can pass your own pattern to exclude if you need to change the default. + * + * @param non-empty-string|false $duplicatesFilter + * * @return array> */ - public function getAmbiguousClasses(): array + public function getAmbiguousClasses($duplicatesFilter = '{/(test|fixture|example|stub)s?/}i'): array { - return $this->ambiguousClasses; + if (false === $duplicatesFilter) { + return $this->ambiguousClasses; + } + + if (true === $duplicatesFilter) { + throw new \InvalidArgumentException('$duplicatesFilter should be false or a string with a valid regex, got true.'); + } + + $ambiguousClasses = []; + foreach ($this->ambiguousClasses as $class => $paths) { + $paths = array_filter($paths, function ($path) use ($duplicatesFilter) { + return !Preg::isMatch($duplicatesFilter, strtr($path, '\\', '/')); + }); + if (\count($paths) > 0) { + $ambiguousClasses[$class] = array_values($paths); + } + } + + return $ambiguousClasses; } /** diff --git a/src/ClassMapGenerator.php b/src/ClassMapGenerator.php index 1907742..8f040e0 100644 --- a/src/ClassMapGenerator.php +++ b/src/ClassMapGenerator.php @@ -191,7 +191,7 @@ public function scanPaths($path, ?string $excluded = null, string $autoloadType foreach ($classes as $class) { if (!$this->classMap->hasClass($class)) { $this->classMap->addClass($class, $filePath); - } elseif ($filePath !== $this->classMap->getClassPath($class) && !Preg::isMatch('{/(test|fixture|example|stub)s?/}i', strtr($this->classMap->getClassPath($class).' '.$filePath, '\\', '/'))) { + } elseif ($filePath !== $this->classMap->getClassPath($class)) { $this->classMap->addAmbiguousClass($class, $filePath); } } diff --git a/tests/ClassMapGeneratorTest.php b/tests/ClassMapGeneratorTest.php index 61821f7..c2bda5c 100644 --- a/tests/ClassMapGeneratorTest.php +++ b/tests/ClassMapGeneratorTest.php @@ -186,9 +186,11 @@ public function testUnambiguousReference(): void { $tempDir = self::getUniqueTmpDirectory(); - file_put_contents($tempDir . '/A.php', "generator->scanPaths($tempDir); + // if we scan src first, then test ambiguous refs will be ignored correctly + $this->generator->scanPaths($tempDir.'/src'); + $this->generator->scanPaths($tempDir.'/ambiguous'); $classMap = $this->generator->getClassMap(); - self::assertCount(0, $classMap->getAmbiguousClasses()); + // but when retrieving without filtering, the ambiguous classes are there + self::assertCount(1, $classMap->getAmbiguousClasses(false)); + self::assertCount(3, $classMap->getAmbiguousClasses(false)['A']); + + // if we scan tests first however, then we always get ambiguous refs as the test one is overriding src + $this->generator = new ClassMapGenerator(['php', 'inc', 'hh']); + $this->generator->scanPaths($tempDir.'/ambiguous'); + $this->generator->scanPaths($tempDir.'/src'); + $classMap = $this->generator->getClassMap(); + + // when retrieving with filtering, only the one from src is seen as ambiguous + self::assertCount(1, $classMap->getAmbiguousClasses()); + self::assertCount(1, $classMap->getAmbiguousClasses()['A']); + self::assertSame($tempDir.'/src/A.php', $classMap->getAmbiguousClasses()['A'][0]); + // when retrieving without filtering, all the ambiguous classes are there + self::assertCount(1, $classMap->getAmbiguousClasses(false)); + self::assertCount(3, $classMap->getAmbiguousClasses(false)['A']); + $fs = new Filesystem(); $fs->remove($tempDir); } @@ -291,7 +312,7 @@ public static function getUniqueTmpDirectory(): string $root = sys_get_temp_dir(); do { - $unique = $root . DIRECTORY_SEPARATOR . uniqid('composer-test-' . random_int(1000, 9000)); + $unique = $root . DIRECTORY_SEPARATOR . uniqid('composer-classmap-' . random_int(1000, 9000)); if (!file_exists($unique) && @mkdir($unique, 0777)) { return (string) realpath($unique);