Refactor private constant detection from PHPStan + process to clear p…

…hp-parser analysis (#44) * add timeout in seconds * [config] Move from privatize-constants * refactor to php-parser approach without PHPStan, to make direct and more robuts
rectorphp · Sep 5, 2024 · 354eb9d · 354eb9d
1 parent 3f36b2d
commit 354eb9d
Show file tree

Hide file tree

Showing 25 changed files with 429 additions and 556 deletions.
diff --git a/README.md b/README.md
@@ -108,14 +108,14 @@ PHPStan can report unused private class constants, but it skips all the public o
 Do you have lots of class constants, all of them public but want to narrow scope to privates?
 
 ```bash
-vendor/bin/swiss-knife privatize-constants src
+vendor/bin/swiss-knife privatize-constants src test
 ```
 
 This command will:
 
-* make all constants private
-* runs PHPStan to find out, which of them are used
-* restores only the used constants back to `public`
+* find all class constant usages
+* scans classes and constants
+* makes those constant used locally `private`
 
 That way all the constants not used outside will be made `private` safely.
 

diff --git a/composer.json b/composer.json
@@ -7,12 +7,11 @@
     ],
     "require": {
         "php": ">=8.2",
-        "illuminate/container": "^11.0",
+        "illuminate/container": "^11.20",
         "nette/robot-loader": "^4.0",
         "nette/utils": "^4.0",
         "nikic/php-parser": "^4.19",
         "symfony/console": "^6.4",
-        "symfony/process": "^6.4",
         "symfony/finder": "^6.4",
         "webmozart/assert": "^1.11"
     },

diff --git a/config/privatize-constants-phpstan-ruleset.neon b/config/privatize-constants-phpstan-ruleset.neon
diff --git a/phpstan.neon b/phpstan.neon
@@ -16,5 +16,5 @@ parameters:
         constant: 99
 
     ignoreErrors:
-        # known class-string type
-        - '#Method Rector\\SwissKnife\\Helpers\\ClassNameResolver::resolveFromFileContents\(\) should return class-string\|null but returns string#'
+        # unrelated
+        - '#Parameter \#1 \$className of class Rector\\SwissKnife\\ValueObject\\ClassConstant constructor expects class-string, string given#'
diff --git a/src/Command/NamespaceToPSR4Command.php b/src/Command/NamespaceToPSR4Command.php
@@ -43,7 +43,10 @@ protected function configure(): void
         );
     }
 
-    protected function execute(InputInterface $input, OutputInterface $output)
+    /**
+     * @return self::*
+     */
+    protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $path = (string) $input->getArgument('path');
         $namespaceRoot = (string) $input->getOption('namespace-root');

diff --git a/src/Command/PrivatizeConstantsCommand.php b/src/Command/PrivatizeConstantsCommand.php
@@ -6,32 +6,26 @@
 
 use Nette\Utils\FileSystem;
 use Nette\Utils\Strings;
+use PhpParser\NodeTraverser;
+use Rector\SwissKnife\Contract\ClassConstantFetchInterface;
 use Rector\SwissKnife\Finder\PhpFilesFinder;
-use Rector\SwissKnife\Helpers\ClassNameResolver;
-use Rector\SwissKnife\PHPStan\ClassConstantResultAnalyser;
-use Rector\SwissKnife\Resolver\StaticClassConstResolver;
-use Rector\SwissKnife\ValueObject\ClassConstMatch;
+use Rector\SwissKnife\PhpParser\CachedPhpParser;
+use Rector\SwissKnife\PhpParser\NodeVisitor\FindClassConstFetchNodeVisitor;
+use Rector\SwissKnife\PhpParser\NodeVisitor\FindNonPrivateClassConstNodeVisitor;
+use Rector\SwissKnife\ValueObject\ClassConstantFetch\CurrentClassConstantFetch;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\Finder\SplFileInfo;
-use Symfony\Component\Process\Process;
 
 final class PrivatizeConstantsCommand extends Command
 {
-    /**
-     * @var string
-     * @see https://regex101.com/r/wkHZwX/1
-     */
-    private const PUBLIC_CONST_REGEX = '#(    |\t)(public )?const #ms';
-
     public function __construct(
         private readonly SymfonyStyle $symfonyStyle,
-        private readonly ClassConstantResultAnalyser $classConstantResultAnalyser,
-        private readonly StaticClassConstResolver $staticClassConstResolver,
+        private readonly CachedPhpParser $cachedPhpParser
     ) {
         parent::__construct();
     }
@@ -71,170 +65,101 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             return self::SUCCESS;
         }
 
-        $this->privatizeClassConstants($phpFileInfos);
-
-        // special case of self::NAME, that should be protected - their children too
-        $staticClassConstMatches = $this->staticClassConstResolver->resolve($phpFileInfos);
-
-        $phpstanResult = $this->runPHPStanAnalyse($sources);
-
-        $publicAndProtectedClassConstants = $this->classConstantResultAnalyser->analyseResult($phpstanResult);
-        if ($publicAndProtectedClassConstants->isEmpty()) {
-            $this->symfonyStyle->success('No class constant visibility to change');
-            return self::SUCCESS;
-        }
-
-        // make public first, to avoid override to protected
-        foreach ($publicAndProtectedClassConstants->getPublicClassConstMatches() as $publicClassConstMatch) {
-            $this->replacePrivateConstWith($publicClassConstMatch, 'public const');
-        }
-
-        foreach ($publicAndProtectedClassConstants->getProtectedClassConstMatches() as $publicClassConstMatch) {
-            $this->replacePrivateConstWith($publicClassConstMatch, 'protected const');
-        }
-
-        $this->replaceClassAndChildWithProtected($phpFileInfos, $staticClassConstMatches);
-
-        if ($publicAndProtectedClassConstants->getPublicCount() !== 0) {
-            $this->symfonyStyle->success(
-                sprintf('%d constant made public', $publicAndProtectedClassConstants->getPublicCount())
-            );
-        }
+        $this->symfonyStyle->note('1. Finding class const fetches...');
+        $classConstantFetches = $this->findClassConstantFetches($phpFileInfos);
 
-        if ($publicAndProtectedClassConstants->getProtectedCount() !== 0) {
-            $this->symfonyStyle->success(
-                sprintf('%d constant made protected', $publicAndProtectedClassConstants->getProtectedCount())
-            );
-        }
+        $this->symfonyStyle->newLine(2);
+        $this->symfonyStyle->success(sprintf('Found %d class constant fetches', count($classConstantFetches)));
 
-        if ($staticClassConstMatches !== []) {
-            $this->symfonyStyle->success(
-                \sprintf('%d constants made protected for static access', count($staticClassConstMatches))
-            );
+        // go file by file and deal with public + protected constants
+        foreach ($phpFileInfos as $phpFileInfo) {
+            $this->processFileInfo($phpFileInfo, $classConstantFetches);
         }
 
         return self::SUCCESS;
     }
 
     /**
      * @param SplFileInfo[] $phpFileInfos
+     * @return ClassConstantFetchInterface[]
      */
-    private function privatizeClassConstants(array $phpFileInfos): void
+    private function findClassConstantFetches(array $phpFileInfos): array
     {
-        $this->symfonyStyle->note(sprintf('Found %d PHP files, turning constants to private', count($phpFileInfos)));
+        $nodeTraverser = new NodeTraverser();
 
-        $privatizedFileCount = 0;
+        $findClassConstFetchNodeVisitor = new FindClassConstFetchNodeVisitor();
+        $nodeTraverser->addVisitor($findClassConstFetchNodeVisitor);
 
+        $progressBar = $this->symfonyStyle->createProgressBar(count($phpFileInfos));
         foreach ($phpFileInfos as $phpFileInfo) {
-            $originalFileContent = $phpFileInfo->getContents();
-
-            $fileContent = $this->makeClassConstantsPrivate($originalFileContent);
-            if ($originalFileContent === $fileContent) {
-                continue;
-            }
-
-            FileSystem::write($phpFileInfo->getRealPath(), $fileContent);
-            ++$privatizedFileCount;
+            $this->parseAndTraverseFile($phpFileInfo, $nodeTraverser);
+            $progressBar->advance();
         }
 
-        $this->symfonyStyle->success(sprintf('Constants in %d files turned to private', $privatizedFileCount));
+        $progressBar->finish();
+
+        return $findClassConstFetchNodeVisitor->getClassConstantFetches();
     }
 
-    private function makeClassConstantsPrivate(string $fileContents): string
+    private function parseAndTraverseFile(SplFileInfo $phpFileInfo, NodeTraverser $nodeTraverser): void
     {
-        return Strings::replace($fileContents, self::PUBLIC_CONST_REGEX, '$1private const ');
+        $fileStmts = $this->cachedPhpParser->parseFile($phpFileInfo->getRealPath());
+        $nodeTraverser->traverse($fileStmts);
     }
 
     /**
-     * @param string[] $paths
-     * @return array<string, mixed>
+     * @param ClassConstantFetchInterface[] $classConstantFetches
      */
-    private function runPHPStanAnalyse(array $paths): array
-    {
-        $this->symfonyStyle->note('Running PHPStan to spot false-private class constants');
-
-        $phpStanAnalyseProcess = new Process([
-            'vendor/bin/phpstan',
-            'analyse',
-            ...$paths,
-            '--configuration',
-            __DIR__ . '/../../config/privatize-constants-phpstan-ruleset.neon',
-            '--error-format',
-            'json',
-        ]);
-        $phpStanAnalyseProcess->run();
-
-        $this->symfonyStyle->success('PHPStan analysis finished');
-
-        // process output message
-        sleep(1);
-
-        $this->symfonyStyle->newLine();
-
-        $resultOutput = $phpStanAnalyseProcess->getOutput() ?: $phpStanAnalyseProcess->getErrorOutput();
-        return json_decode($resultOutput, true);
-    }
-
-    private function replacePrivateConstWith(ClassConstMatch $publicClassConstMatch, string $replaceString): void
+    private function processFileInfo(SplFileInfo $phpFileInfo, array $classConstantFetches): void
     {
-        $classFileContents = FileSystem::read($publicClassConstMatch->getClassFileName());
-
-        // replace "private const NAME" with "private const NAME"
-        $classFileContents = str_replace(
-            'private const ' . $publicClassConstMatch->getConstantName(),
-            $replaceString . ' ' . $publicClassConstMatch->getConstantName(),
-            $classFileContents
-        );
+        $nodeTraverser = new NodeTraverser();
+        $findNonPrivateClassConstNodeVisitor = new FindNonPrivateClassConstNodeVisitor();
+        $nodeTraverser->addVisitor($findNonPrivateClassConstNodeVisitor);
 
-        FileSystem::write($publicClassConstMatch->getClassFileName(), $classFileContents);
+        $this->parseAndTraverseFile($phpFileInfo, $nodeTraverser);
 
-        // @todo handle case when "AppBundle\Rpc\BEItem\BeItemPackage::ITEM_TYPE_NAME_PACKAGE" constant is in parent class
-        $parentClassConstMatch = $publicClassConstMatch->getParentClassConstMatch();
-        if (! $parentClassConstMatch instanceof ClassConstMatch) {
-            return;
-        }
-
-        $this->replacePrivateConstWith($parentClassConstMatch, $replaceString);
-    }
-
-    /**
-     * @param SplFileInfo[] $phpFileInfos
-     * @param ClassConstMatch[] $staticClassConstsMatches
-     */
-    private function replaceClassAndChildWithProtected(array $phpFileInfos, array $staticClassConstsMatches): void
-    {
-        if ($staticClassConstsMatches === []) {
+        // nothing found
+        if ($findNonPrivateClassConstNodeVisitor->getClassConstants() === []) {
             return;
         }
 
-        foreach ($phpFileInfos as $phpFileInfo) {
-            $fullyQualifiedClassName = ClassNameResolver::resolveFromFileContents($phpFileInfo->getContents());
-
-            if ($fullyQualifiedClassName === null) {
-                // no class to process
-                continue;
-            }
+        foreach ($findNonPrivateClassConstNodeVisitor->getClassConstants() as $classConstant) {
+            $isPublic = false;
+            foreach ($classConstantFetches as $classConstantFetch) {
+                if (! $classConstantFetch->isClassConstantMatch($classConstant)) {
+                    continue;
+                }
 
-            foreach ($staticClassConstsMatches as $staticClassConstMatch) {
-                // update current and all hcildren
-                if (! is_a($fullyQualifiedClassName, $staticClassConstMatch->getClassName(), true)) {
+                if ($classConstantFetch instanceof CurrentClassConstantFetch) {
                     continue;
                 }
 
-                $classFileContents = \str_replace(
-                    'private const ' . $staticClassConstMatch->getConstantName(),
-                    'protected const ' . $staticClassConstMatch->getConstantName(),
-                    $phpFileInfo->getContents()
+                // used externally, make public
+                $isPublic = true;
+            }
+
+            if ($isPublic) {
+                $changedFileContents = Strings::replace(
+                    $phpFileInfo->getContents(),
+                    '#(public\s+)?const\s+' . $classConstant->getConstantName() . '#',
+                    'public const ' . $classConstant->getConstantName()
                 );
 
-                $this->symfonyStyle->warning(sprintf(
-                    'The "%s" constant in "%s" made protected to allow static access. Consider refactoring to better design',
-                    $staticClassConstMatch->getConstantName(),
-                    $staticClassConstMatch->getClassName(),
-                ));
+                FileSystem::write($phpFileInfo->getRealPath(), $changedFileContents);
 
-                FileSystem::write($phpFileInfo->getRealPath(), $classFileContents);
+                $this->symfonyStyle->note(sprintf('Constant %s changed to public', $classConstant->getConstantName()));
+                continue;
             }
+
+            // make private
+            $changedFileContents = Strings::replace(
+                $phpFileInfo->getContents(),
+                '#(public\s+)?const\s+' . $classConstant->getConstantName() . '#',
+                'private const ' . $classConstant->getConstantName()
+            );
+            FileSystem::write($phpFileInfo->getRealPath(), $changedFileContents);
+
+            $this->symfonyStyle->note(sprintf('Constant %s changed to private', $classConstant->getConstantName()));
         }
     }
 }
diff --git a/src/Contract/ClassConstantFetchInterface.php b/src/Contract/ClassConstantFetchInterface.php
@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\SwissKnife\Contract;
+
+use Rector\SwissKnife\ValueObject\ClassConstant;
+
+interface ClassConstantFetchInterface
+{
+    public function getClassName(): string;
+
+    public function getConstantName(): string;
+
+    public function isClassConstantMatch(ClassConstant $classConstant): bool;
+}
diff --git a/src/Exception/NotImplementedYetException.php b/src/Exception/NotImplementedYetException.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\SwissKnife\Exception;
+
+use Exception;
+
+final class NotImplementedYetException extends Exception
+{
+}
diff --git a/src/Exception/ShouldNotHappenException.php b/src/Exception/ShouldNotHappenException.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\SwissKnife\Exception;
+
+use Exception;
+
+final class ShouldNotHappenException extends Exception
+{
+}