Merge pull request #26 from andreaswolf/task/minor-cleanups

[TASK] Minor code cleanups

Author: sabbelasichon

fractor-xml/composer.json

@@ -15,7 +15,8 @@
         "ext-dom": "*",
         "ext-xml": "*",
         "a9f/fractor": "@dev",
-        "a9f/fractor-extension-installer": "@dev"
+        "a9f/fractor-extension-installer": "@dev",
+        "webmozart/assert": "^1.11"
     },
     "require-dev": {
         "ergebnis/composer-normalize": "^2.42",

fractor-xml/src/DomDocumentIterator.php

@@ -3,6 +3,8 @@
 namespace a9f\FractorXml;
 
 use a9f\FractorXml\Contract\DomNodeVisitor;
+use a9f\FractorXml\Exception\ShouldNotHappenException;
+use Webmozart\Assert\Assert;
 
 final class DomDocumentIterator
 {
@@ -14,6 +16,7 @@ final class DomDocumentIterator
      */
     public function __construct(private readonly iterable $visitors)
     {
+        Assert::allIsInstanceOf($this->visitors, DomNodeVisitor::class);
     }
 
     public function traverseDocument(\DOMDocument $document): void
@@ -38,8 +41,7 @@ private function traverseNode(\DOMNode $node): void
             $result = $visitor->enterNode($node);
 
             if ($node->parentNode === null) {
-                // TODO convert into a custom ShouldNotHappenException
-                throw new \RuntimeException('Node has no parent node');
+                throw new ShouldNotHappenException('Node has no parent node');
             }
 
             if ($result === DomDocumentIterator::REMOVE_NODE) {

fractor-xml/src/Exception/ShouldNotHappenException.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace a9f\FractorXml\Exception;
+
+final class ShouldNotHappenException extends \RuntimeException
+{
+}

fractor-xml/src/XmlFileProcessor.php

@@ -5,6 +5,7 @@
 use a9f\Fractor\Application\Contract\FileProcessor;
 use a9f\Fractor\Application\ValueObject\File;
 use a9f\FractorXml\Contract\XmlFractor;
+use Webmozart\Assert\Assert;
 
 final readonly class XmlFileProcessor implements FileProcessor
 {
@@ -13,6 +14,7 @@
      */
     public function __construct(private iterable $rules)
     {
+        Assert::allIsInstanceOf($this->rules, XmlFractor::class);
     }
 
     public function canHandle(File $file): bool

fractor/config/application.php

@@ -14,6 +14,7 @@
 use Symfony\Component\DependencyInjection\ParameterBag\ContainerBag;
 use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
+use Symfony\Component\Filesystem\Filesystem;
 use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
 use function Symfony\Component\DependencyInjection\Loader\Configurator\tagged_iterator;
 
@@ -88,6 +89,7 @@ static function (ChildDefinition $definition, AsCommand $attribute): void {
     $services->set(Configuration::class)->factory([service(ConfigurationFactory::class), 'create']);
     $services->set(FractorRunner::class)->arg('$processors', tagged_iterator('fractor.file_processor'));
     $services->set(AllowedFileExtensionsResolver::class)->arg('$processors', tagged_iterator('fractor.file_processor'));
+    $services->set(Filesystem::class);
 
     $containerBuilder->registerForAutoconfiguration(FileProcessor::class)->addTag('fractor.file_processor');
 };

fractor/src/Application/FractorRunner.php

@@ -9,6 +9,7 @@
 use a9f\Fractor\Console\Contract\Output;
 use a9f\Fractor\FileSystem\FilesFinder;
 use Nette\Utils\FileSystem;
+use Webmozart\Assert\Assert;
 
 /**
  * Main Fractor class. This takes care of collecting a list of files, iterating over them and calling all registered
@@ -21,6 +22,7 @@
      */
     public function __construct(private FilesFinder $fileFinder, private FilesCollector $fileCollector, private iterable $processors, private Configuration $configuration, private FileWriter $fileWriter)
     {
+        Assert::allIsInstanceOf($this->processors, FileProcessor::class);
     }
 
     public function run(Output $output, bool $dryRun = false): void

fractor/src/Configuration/AllowedFileExtensionsResolver.php

@@ -5,6 +5,7 @@
 namespace a9f\Fractor\Configuration;
 
 use a9f\Fractor\Application\Contract\FileProcessor;
+use Webmozart\Assert\Assert;
 
 final readonly class AllowedFileExtensionsResolver
 {
@@ -13,6 +14,7 @@
      */
     public function __construct(private iterable $processors)
     {
+        Assert::allIsInstanceOf($this->processors, FileProcessor::class);
     }
 
     /**

fractor/src/Configuration/ConfigResolver.php

@@ -8,12 +8,7 @@ final class ConfigResolver
 {
     public static function resolveConfigsFromInput(ArgvInput $input): ?string
     {
-        return self::getConfigFileFromInput($input) ?? getcwd() . '/fractor.php';
-    }
-
-    private static function getConfigFileFromInput(ArgvInput $input): ?string
-    {
-        return self::getOptionValue($input);
+        return self::getOptionValue($input) ?? getcwd() . '/fractor.php';
     }
 
     /**

fractor/src/Configuration/ConfigurationFactory.php

@@ -17,8 +17,8 @@ public function create(): Configuration
     {
         return new Configuration(
             $this->allowedFileExtensionsResolver->resolve(),
-            $this->parameterBag->get(Option::PATHS),
-            $this->parameterBag->get(Option::SKIP),
+            (array)$this->parameterBag->get(Option::PATHS),
+            (array)$this->parameterBag->get(Option::SKIP),
         );
     }
 }

fractor/src/Configuration/Option.php

@@ -10,10 +10,12 @@ final class Option
      * @var string
      */
     public const PATHS = 'paths';
+
     /**
      * @var string
      */
     public const SKIP = 'skip';
+
     /**
      * @var string
      */
@@ -23,6 +25,7 @@ final class Option
      * @var string
      */
     public const CONFIG = 'config';
+
     /**
      * @var string
      */

fractor/src/Configuration/ValueObject/Configuration.php

@@ -15,7 +15,6 @@
      */
     public function __construct(private array $fileExtensions, private array $paths, private array $skip)
     {
-        Assert::notEmpty($this->paths, 'No directories given');
         Assert::allStringNotEmpty($this->paths, 'No directories given');
     }
 

fractor/src/Console/Command/ProcessCommand.php

@@ -11,7 +11,7 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
-#[AsCommand('process', 'Runs Fractor with the given configuration file')]
+#[AsCommand(name: 'process', description: 'Runs Fractor with the given configuration file')]
 final class ProcessCommand extends Command
 {
     public function __construct(private readonly FractorRunner $runner)

fractor/src/DependencyInjection/ContainerContainerBuilder.php

@@ -12,47 +12,48 @@
 class ContainerContainerBuilder
 {
     /**
-     * @param array<int, string> $additionalConfigFiles
+     * @param string[] $additionalConfigFiles
      */
     public function createDependencyInjectionContainer(array $additionalConfigFiles = []): ContainerInterface
     {
         $containerBuilder = new ContainerBuilder();
-        $this->loadFile($containerBuilder, __DIR__ . '/../../config/application.php');
-        $this->importExtensionConfigurations($containerBuilder);
 
         $containerBuilder->addCompilerPass(new AddConsoleCommandPass());
 
-        foreach ($additionalConfigFiles as $additionalConfigFile) {
-            if (!file_exists($additionalConfigFile)) {
+        $configFiles = [
+            __DIR__ . '/../../config/application.php'
+        ];
+        $configFiles = array_merge($configFiles, $additionalConfigFiles);
+        $configFiles = array_merge($configFiles, $this->collectConfigFilesFromExtensions());
+
+        foreach ($configFiles as $configFile) {
+            if (!file_exists($configFile)) {
                 continue;
             }
-            $this->loadFile($containerBuilder, $additionalConfigFile);
+
+            $fileLoader = new PhpFileLoader($containerBuilder, new FileLocator(dirname($configFile)));
+            $fileLoader->load($configFile);
         }
 
         $containerBuilder->compile();
 
         return $containerBuilder;
     }
 
-
-    private function loadFile(ContainerBuilder $containerBuilder, string $pathToFile): void
-    {
-        $fileLoader = new PhpFileLoader($containerBuilder, new FileLocator(dirname($pathToFile)));
-        $fileLoader->load($pathToFile);
-    }
-
-    private function importExtensionConfigurations(ContainerBuilder $containerBuilder): void
+    /**
+     * @return string[]
+     */
+    private function collectConfigFilesFromExtensions(): array
     {
+        $collectedConfigFiles = [];
         if (!class_exists('a9f\\FractorExtensionInstaller\\Generated\\InstalledPackages')) {
-            return;
+            return $collectedConfigFiles;
         }
 
         foreach (InstalledPackages::PACKAGES as $package) {
-            $filePath = $package['path'] . '/config/application.php';
-
-            if (file_exists($filePath)) {
-                $this->loadFile($containerBuilder, $filePath);
-            }
+            $collectedConfigFiles[] = $package['path'] . '/config/application.php';
         }
+
+        return $collectedConfigFiles;
     }
 }

fractor/src/FileSystem/FilesFinder.php

@@ -4,6 +4,7 @@
 
 use a9f\Fractor\Skipper\Skipper\PathSkipper;
 use Symfony\Component\Finder\Finder;
+use Webmozart\Assert\Assert;
 
 final readonly class FilesFinder
 {
@@ -12,12 +13,14 @@ public function __construct(private FilesystemTweaker $filesystemTweaker, privat
     }
 
     /**
-     * @param string[] $source
+     * @param list<non-empty-string> $source
      * @param string[] $suffixes
      * @return string[]
      */
     public function findFiles(array $source, array $suffixes, bool $sortByName = true): array
     {
+        Assert::allStringNotEmpty($source, 'Please provide some paths');
+
         $filesAndDirectories = $this->filesystemTweaker->resolveWithFnmatch($source);
 
         $files = $this->fileAndDirectoryFilter->filterFiles($filesAndDirectories);

fractor/src/Skipper/FileSystem/FilePathHelper.php

@@ -14,7 +14,7 @@ final class FilePathHelper
      * @see https://regex101.com/r/d4F5Fm/1
      * @var string
      */
-    private const SCHEME_PATH_REGEX = '#^([a-z]+)\\:\\/\\/(.+)#';
+    private const SCHEME_PATH_REGEX = '#^([a-z]+)://(.+)#';
 
     /**
      * @see https://regex101.com/r/no28vw/1
@@ -26,12 +26,9 @@ final class FilePathHelper
      * @var string
      */
     private const SCHEME_UNDEFINED = 'undefined';
-    private Filesystem $filesystem;
 
-    public function __construct(
-
-    ) {
-        $this->filesystem = new Filesystem();
+    public function __construct(private readonly Filesystem $filesystem)
+    {
     }
 
     public function relativePath(string $fileRealPath): string

fractor/src/Skipper/Matcher/FileInfoMatcher.php

@@ -34,12 +34,8 @@ public function doesFileInfoMatchPatterns(string $filePath, array $filePatterns)
         return false;
     }
 
-    /**
-     * Supports both relative and absolute $file path. They differ for PHP-CS-Fixer and PHP_CodeSniffer.
-     */
     private function doesFileMatchPattern(string $filePath, string $ignoredPath): bool
     {
-        // in rector.php, the path can be absolute
         if ($filePath === $ignoredPath) {
             return true;
         }

fractor/tests/Skipper/FileSystem/FilePathHelper/FilePathHelperTest.php

@@ -8,14 +8,15 @@
 use Iterator;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
 
 final class FilePathHelperTest extends TestCase
 {
     private FilePathHelper $subject;
 
     protected function setUp(): void
     {
-        $this->subject = new FilePathHelper();
+        $this->subject = new FilePathHelper(new Filesystem());
     }
 
     #[DataProvider('provideData')]