Fix parsing of ambiguous classes to avoid ignoring real classes when test ones get scanned first (#13)

Refs composer/composer#12140

Author: Seldaek

phpstan-baseline.neon

@@ -9,3 +9,8 @@ parameters:
 			message: "#^Call to static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayHasKey\\(\\) with 'A' and array\\<class\\-string, array\\<non\\-empty\\-string\\>\\> will always evaluate to false\\.$#"
 			count: 1
 			path: tests/ClassMapGeneratorTest.php
+
+		-
+			message: "#^Offset 'A' does not exist on non\\-empty\\-array\\<class\\-string, array\\<non\\-empty\\-string\\>\\>\\.$#"
+			count: 4
+			path: tests/ClassMapGeneratorTest.php

src/ClassMap.php

@@ -12,6 +12,8 @@
 
 namespace Composer\ClassMapGenerator;
 
+use Composer\Pcre\Preg;
+
 /**
  * @author Jordi Boggiano <j.boggiano@seld.be>
  */
@@ -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<class-string, array<non-empty-string>>
      */
-    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;
     }
 
     /**

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

tests/ClassMapGeneratorTest.php

@@ -186,9 +186,11 @@ public function testUnambiguousReference(): void
     {
         $tempDir = self::getUniqueTmpDirectory();
 
-        file_put_contents($tempDir . '/A.php', "<?php\nclass A {}");
+        mkdir($tempDir.'/src');
+        mkdir($tempDir.'/ambiguous');
+        file_put_contents($tempDir . '/src/A.php', "<?php\nclass A {}");
         file_put_contents(
-            $tempDir . '/B.php',
+            $tempDir . '/src/B.php',
             "<?php
                 if (true) {
                     interface B {}
@@ -199,17 +201,36 @@ interface B extends Iterator {}
         );
 
         foreach (array('test', 'fixture', 'example') as $keyword) {
-            if (!is_dir($tempDir . '/' . $keyword)) {
-                mkdir($tempDir . '/' . $keyword, 0777, true);
+            if (!is_dir($tempDir . '/ambiguous/' . $keyword)) {
+                mkdir($tempDir . '/ambiguous/' . $keyword, 0777, true);
             }
-            file_put_contents($tempDir . '/' . $keyword . '/A.php', "<?php\nclass A {}");
+            file_put_contents($tempDir . '/ambiguous/' . $keyword . '/A.php', "<?php\nclass A {}");
         }
 
-        $this->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'.DIRECTORY_SEPARATOR.'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);