Fix #172: Replace custom path-handling with Symfony Filesystem

phpstan.neon.dist

@@ -35,14 +35,17 @@ parameters:
         -
             message: '#Call to function assert\(\) with false and string will always evaluate to false.#'
             paths:
+                - src/Internal/Path/Value/Path.php
                 - src/Internal/Translation/Service/Translator.php
                 - src/Internal/Translation/Value/TranslationParameters.php
         -
             message: '#Cannot access offset .(application|name). on mixed.#'
             path: src/Internal/Precondition/Service/ComposerIsAvailable.php
         -
             message: '#In method ".*", caught "Throwable" must be rethrown.*#'
-            path: src/Internal/Translation/Service/Translator.php
+            paths:
+                - src/Internal/Path/Value/Path.php
+                - src/Internal/Translation/Service/Translator.php
         -
             message: '#Method .*SymfonyTranslatorProxy::trans\(\) has parameter \$parameters with no value type specified in iterable type array.*#'
             path: src/Internal/Translation/Service/SymfonyTranslatorProxy.php

rector.php

@@ -2,7 +2,6 @@
 
 use Rector\Caching\ValueObject\Storage\MemoryCacheStorage;
 use Rector\CodeQuality\Rector\ClassMethod\LocallyCalledStaticMethodToNonStaticRector;
-use Rector\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector;
 use Rector\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector;
 use Rector\CodingStyle\Rector\ClassMethod\NewlineBeforeNewAssignSetRector;
 use Rector\CodingStyle\Rector\ClassMethod\UnSpreadOperatorRector;
@@ -13,6 +12,7 @@
 use Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnTagRector;
 use Rector\Php80\Rector\FunctionLike\MixedTypeRector;
 use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector;
+use Rector\Php81\Rector\Property\ReadOnlyPropertyRector;
 use Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector;
 use Rector\Set\ValueObject\LevelSetList;
 use Rector\Set\ValueObject\SetList;
@@ -39,6 +39,7 @@
 
     $rectorConfig->skip([
         CatchExceptionNameMatchingTypeRector::class => [__DIR__],
+        ReadOnlyPropertyRector::class => [__DIR__ . '/src/Internal/Path/Value/Path.php'],
         EncapsedStringsToSprintfRector::class => [__DIR__],
         FinalizeClassesWithoutChildrenRector::class => [__DIR__], // This is duplicative of PHPCS sniff SlevomatCodingStandard.Classes.RequireAbstractOrFinal.
         LocallyCalledStaticMethodToNonStaticRector::class => [__DIR__],
@@ -48,7 +49,6 @@
         NullToStrictStringFuncCallArgRector::class => [__DIR__ . '/tests/PHPUnit/TestUtils/AssertTrait.php'],
         RemoveUselessParamTagRector::class => [__DIR__], // This one has a bug: https://github.com/rectorphp/rector-src/pull/4480
         RemoveUselessReturnTagRector::class => [__DIR__], // This one has a bug: https://github.com/rectorphp/rector-src/pull/4482
-        SimplifyIfReturnBoolRector::class => [__DIR__ . '/src/Internal/Path/Value/WindowsPath.php'],
         UnSpreadOperatorRector::class => [__DIR__],
     ]);
 };

src/Internal/Path/Factory/PathFactory.php

@@ -4,9 +4,7 @@
 
 use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface;
 use PhpTuf\ComposerStager\API\Path\Value\PathInterface;
-use PhpTuf\ComposerStager\Internal\Host\Service\Host;
-use PhpTuf\ComposerStager\Internal\Path\Value\UnixLikePath;
-use PhpTuf\ComposerStager\Internal\Path\Value\WindowsPath;
+use PhpTuf\ComposerStager\Internal\Path\Value\Path;
 
 /**
  * @package Path
@@ -19,10 +17,6 @@ final class PathFactory implements PathFactoryInterface
 {
     public static function create(string $path, ?PathInterface $basePath = null): PathInterface
     {
-        if (Host::isWindows()) {
-            return new WindowsPath($path, $basePath); // @codeCoverageIgnore
-        }
-
-        return new UnixLikePath($path, $basePath);
+        return new Path($path, $basePath);
     }
 }

src/Internal/Path/Value/AbstractPath.php → src/Internal/Path/Value/Path.php

@@ -3,17 +3,17 @@
 namespace PhpTuf\ComposerStager\Internal\Path\Value;
 
 use PhpTuf\ComposerStager\API\Path\Value\PathInterface;
+use Symfony\Component\Filesystem\Path as SymfonyPath;
+use Throwable;
 
 /**
  * @package Path
  *
  * @internal Don't depend directly on this class. It may be changed or removed at any time without notice.
  */
-abstract class AbstractPath implements PathInterface
+final class Path implements PathInterface
 {
-    protected string $basePath;
-
-    abstract protected function doAbsolute(string $basePath): string;
+    private string $basePath;
 
     /**
      * @param string $path
@@ -41,61 +41,16 @@ public function absolute(): string
         return $this->doAbsolute($this->basePath);
     }
 
-    public function relative(PathInterface $basePath): string
+    public function isAbsolute(): bool
     {
-        $basePathAbsolute = $basePath->absolute();
-
-        return $this->doAbsolute($basePathAbsolute);
+        return SymfonyPath::isAbsolute($this->path);
     }
 
-    // Once support for Symfony 4 is dropped, some of this logic could possibly be
-    // eliminated in favor of the new path manipulation utilities in Symfony 5.4:
-    // https://symfony.com/doc/5.4/components/filesystem.html#path-manipulation-utilities
-    protected function normalize(string $absolutePath, string $prefix = ''): string
+    public function relative(PathInterface $basePath): string
     {
-        // If the absolute path begins with a directory separator, append it to
-        // the prefix, or it will be lost below when exploding the string. (A
-        // trailing directory separator SHOULD BE lost.)
-        if (str_starts_with($absolutePath, DIRECTORY_SEPARATOR)) {
-            $prefix .= DIRECTORY_SEPARATOR;
-        }
-
-        // Strip the given prefix.
-        $absolutePath = substr($absolutePath, strlen($prefix));
-
-        // Normalize directory separators and explode around them.
-        $absolutePath = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $absolutePath);
-        $parts = explode(DIRECTORY_SEPARATOR, $absolutePath);
-
-        $normalized = [];
-
-        foreach ($parts as $part) {
-            // A zero-length part comes from (meaningless) double slashes. Skip it.
-            if ($part === '') {
-                continue;
-            }
-
-            // A single dot has no effect. Skip it.
-            if ($part === '.') {
-                continue;
-            }
-
-            // Two dots goes "up" a directory. Pop one off the current normalized array.
-            if ($part === '..') {
-                array_pop($normalized);
-
-                continue;
-            }
-
-            // Otherwise, add the part to the current normalized array.
-            $normalized[] = $part;
-        }
-
-        // Replace directory separators.
-        $normalized = implode(DIRECTORY_SEPARATOR, $normalized);
+        $basePathAbsolute = $basePath->absolute();
 
-        // Replace the prefix and return.
-        return $prefix . $normalized;
+        return $this->doAbsolute($basePathAbsolute);
     }
 
     /**
@@ -113,4 +68,36 @@ private function getcwd(): string
         // static analysis and move on.
         return (string) getcwd();
     }
+
+    private function doAbsolute(string $basePath): string
+    {
+        try {
+            $absolute = SymfonyPath::makeAbsolute($this->path, $basePath);
+        } catch (Throwable) {
+            // SymfonyPath throws an exception if the base path isn't absolute. That
+            // shouldn't be possible here because, in order to get to this point in the
+            // code, the base path must necessarily have been set to an absolute path in
+            // the constructor--either explicitly or via `getcwd()`. Nevertheless, as a
+            // matter of defensive programming, use an assertion and return the normalized
+            // path, whatever it is, in case of the "impossible" in production.
+            //
+            // @todo It's probably better to throw an InvalidArgumentException, but that will
+            //   have a widespread effect on error-handling, so come back to it in a follow-up.
+            assert(false, sprintf('Base paths must be absolute. Got %s.', $basePath));
+
+            /** @noinspection PhpUnreachableStatementInspection */
+            return $this->normalize($this->path);
+        }
+
+        return $this->normalize($absolute);
+    }
+
+    private function normalize(string $absolutePath): string
+    {
+        return str_replace(
+            ['//', '/'], // Some Windows paths end up with double slashes after the drive name.
+            ['/', DIRECTORY_SEPARATOR], // SymfonyPath always uses forward slashes. Use the OS-specific separator.
+            SymfonyPath::canonicalize($absolutePath),
+        );
+    }
 }

tests/PHPUnit/Path/Factory/PathFactoryUnitTest.php

@@ -3,10 +3,8 @@
 namespace PhpTuf\ComposerStager\Tests\Path\Factory;
 
 use PhpTuf\ComposerStager\API\Path\Value\PathInterface;
-use PhpTuf\ComposerStager\Internal\Host\Service\Host;
 use PhpTuf\ComposerStager\Internal\Path\Factory\PathFactory;
-use PhpTuf\ComposerStager\Internal\Path\Value\UnixLikePath;
-use PhpTuf\ComposerStager\Internal\Path\Value\WindowsPath;
+use PhpTuf\ComposerStager\Internal\Path\Value\Path;
 use PhpTuf\ComposerStager\Tests\Path\Value\TestPath;
 use PhpTuf\ComposerStager\Tests\TestCase;
 
@@ -33,27 +31,12 @@ public function testBasicFunctionality(
 
     public function providerBasicFunctionality(): array
     {
-        // It's difficult to meaningfully test this class because it's a static
-        // factory, but it has an external dependency on a PHP constant that cannot
-        // be mocked. The tests themselves must therefore be conditioned on the
-        // external environment (which is obviously "cheating").
-        if (Host::isWindows()) {
-            return [
-                [
-                    'string' => 'test.txt',
-                    'baseDir' => new TestPath(),
-                    'expected' => new WindowsPath('test.txt'),
-                    'expectedWithBaseDir' => new WindowsPath('test.txt', new TestPath()),
-                ],
-            ];
-        }
-
         return [
             [
                 'string' => 'test.txt',
                 'baseDir' => new TestPath(),
-                'expected' => new UnixLikePath('test.txt'),
-                'expectedWithBaseDir' => new UnixLikePath('test.txt', new TestPath()),
+                'expected' => new Path('test.txt'),
+                'expectedWithBaseDir' => new Path('test.txt', new TestPath()),
             ],
         ];
     }