Skip to content

Commit

Permalink
Fix parsing of ambiguous classes to avoid ignoring real classes when …
Browse files Browse the repository at this point in the history
…test ones get scanned first (#13)

Refs composer/composer#12140
  • Loading branch information
Seldaek authored Oct 3, 2024
1 parent 465b1b0 commit 98bbf67
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
5 changes: 5 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 29 additions & 2 deletions src/ClassMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

namespace Composer\ClassMapGenerator;

use Composer\Pcre\Preg;

/**
* @author Jordi Boggiano <j.boggiano@seld.be>
*/
Expand Down Expand Up @@ -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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ClassMapGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
37 changes: 29 additions & 8 deletions tests/ClassMapGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 98bbf67

Please sign in to comment.