Utility: Review Package

- Fix issue where `Package::getPackagePath()` doesn't use `File::realpath()` to return the canonical path - Make `Package::isInstalled()` public - Fix issue where package versions like `dev-f0c162d8d462...@f0c162d` may be returned
salient-labs · Jan 26, 2025 · 624a6d9 · 624a6d9
1 parent 8ee5ded
commit 624a6d9
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 35 deletions.
diff --git a/src/Toolkit/Utility/Package.php b/src/Toolkit/Utility/Package.php
@@ -130,12 +130,16 @@ public static function getPackagePath(string $package): ?string
             return null;
         }
 
-        return self::filterData(
+        $path = self::filterData(
             Installed::getInstallPath($package),
             Installed::class,
             'getInstallPath',
             $package,
         );
+
+        return $path !== null
+            ? File::realpath($path)
+            : null;
     }
 
     /**
@@ -189,7 +193,7 @@ public static function getNamespacePath(string $namespace): ?string
         // Sort prefixes from longest to shortest
         uksort(
             $prefixes,
-            fn(string $p1, string $p2): int => strlen($p2) <=> strlen($p1)
+            fn($p1, $p2) => strlen($p2) <=> strlen($p1)
         );
 
         foreach ($prefixes as $prefix => $dirs) {
@@ -198,18 +202,15 @@ public static function getNamespacePath(string $namespace): ?string
             }
 
             foreach ((array) $dirs as $dir) {
-                if (!is_dir($dir)) {
-                    // @codeCoverageIgnoreStart
-                    continue;
-                    // @codeCoverageIgnoreEnd
-                }
-                $dir = File::realpath($dir);
-                $subdir = strtr(substr($namespace, strlen($prefix)), '\\', '/');
-                $path = Arr::implode('/', [$dir, $subdir], '');
-                if (is_dir($path)) {
-                    return $path;
+                if (is_dir($dir)) {
+                    $dir = File::realpath($dir);
+                    $subdir = strtr(substr($namespace, strlen($prefix)), '\\', '/');
+                    $path = Arr::implode('/', [$dir, $subdir], '');
+                    if (is_dir($path)) {
+                        return $path;
+                    }
+                    $fallback ??= $path;
                 }
-                $fallback ??= $path;
             }
         }
 
@@ -229,16 +230,20 @@ private static function getRootPackageValue(string $key)
 
         if (!array_key_exists($key, $values)) {
             // @codeCoverageIgnoreStart
-            throw new ShouldNotHappenException(
-                sprintf('Value not found in root package: %s', $key)
-            );
+            throw new ShouldNotHappenException(sprintf(
+                'Value not found in root package: %s',
+                $key,
+            ));
             // @codeCoverageIgnoreEnd
         }
 
         return $values[$key];
     }
 
-    private static function isInstalled(string $package): bool
+    /**
+     * Check if a package is installed
+     */
+    public static function isInstalled(string $package): bool
     {
         return self::filterData(
             Installed::isInstalled($package),
@@ -282,13 +287,11 @@ private static function getRegisteredLoaders(): array
      */
     private static function filterData($data, string $class, string $method, ...$args)
     {
-        if (!class_exists(Event::class) || !Event::isLoaded()) {
-            // @codeCoverageIgnoreStart
-            return $data;
-            // @codeCoverageIgnoreEnd
+        if (class_exists(Event::class) && Event::isLoaded()) {
+            $event = new PackageDataReceivedEvent($data, $class, $method, ...$args);
+            $data = Event::getInstance()->dispatch($event)->getData();
         }
-        $event = new PackageDataReceivedEvent($data, $class, $method, ...$args);
-        return Event::getInstance()->dispatch($event)->getData();
+        return $data;
     }
 
     /**
@@ -299,14 +302,13 @@ private static function formatVersion(
         ?bool $withRef,
         Closure $refCallback
     ): string {
-        if ($withRef !== false && Regex::match('/(?:^dev-|-dev$)/', $version)) {
+        if ($withRef !== false && Regex::match('/(?:^dev-|-dev$)/D', $version)) {
             $ref = $refCallback();
-            if ($ref !== null) {
+            if ($ref !== null && !Str::startsWith($version, ['dev-' . $ref, $ref])) {
                 return $version . "@$ref";
             }
             return $version;
         }
-
         if ($withRef) {
             $ref = $refCallback();
             if ($ref !== null) {

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -1,9 +1,4 @@
 <?php declare(strict_types=1);
 
-use PHPStan\Testing\PHPStanTestCase;
-
-PHPStanTestCase::getContainer();
-
-require_once __DIR__ . '/fixtures/Toolkit/PHPStan/Core/Rules/TypesAssignedByHasMutatorRuleFailures.php';
 require_once __DIR__ . '/fixtures/Toolkit/Utility/Debug/GetCallerFile1.php';
 require_once __DIR__ . '/fixtures/Toolkit/Utility/Debug/GetCallerFile2.php';
diff --git a/tests/unit/Toolkit/Utility/PackageTest.php b/tests/unit/Toolkit/Utility/PackageTest.php
@@ -59,6 +59,17 @@ final class PackageTest extends TestCase
         'dev' => false,
     ];
 
+    private const DEV_REF_PACKAGE = [
+        'name' => 'acme/sync',
+        'pretty_version' => 'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+        'version' => 'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+        'reference' => '87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+        'type' => 'library',
+        'install_path' => __DIR__ . '/../../',
+        'aliases' => [],
+        'dev' => true,
+    ];
+
     /** @var array<string,mixed>|null */
     private static ?array $RootPackage = null;
     private static int $ListenerId;
@@ -184,6 +195,27 @@ public static function versionProvider(): array
                 false,
                 true,
             ],
+            [
+                'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+                self::DEV_REF_PACKAGE,
+            ],
+            [
+                'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+                self::DEV_REF_PACKAGE,
+                false,
+            ],
+            [
+                'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+                self::DEV_REF_PACKAGE,
+                true,
+                true,
+            ],
+            [
+                'dev-87effbba67b4cf767ff832f6f77bbca3f0dea67d',
+                self::DEV_REF_PACKAGE,
+                false,
+                true,
+            ],
         ];
     }
 
@@ -229,10 +261,9 @@ public function testGetPackageVersion(): void
 
     public function testGetPackagePath(): void
     {
-        // `InstalledVersions::getInstallPath()` may return a non-existent
-        // location in phpstan.phar, so don't perform any filesystem tests here
-        $this->assertNotNull($dir = InstalledVersions::getInstallPath('phpunit/phpunit'));
-        $this->assertSame($dir, Package::getPackagePath('phpunit/phpunit'));
+        $this->assertNotNull($dir1 = InstalledVersions::getInstallPath('phpunit/phpunit'));
+        $this->assertNotNull($dir2 = Package::getPackagePath('phpunit/phpunit'));
+        $this->assertTrue(File::same($dir1, $dir2));
         $this->assertNull(Package::getPackagePath('composer/composer'));
     }