Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor private constant detection from PHPStan + process to clear php-parser analysis #44

Merged
merged 3 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 1 addition & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
5 changes: 0 additions & 5 deletions config/privatize-constants-phpstan-ruleset.neon

This file was deleted.

4 changes: 2 additions & 2 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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#'
5 changes: 4 additions & 1 deletion src/Command/NamespaceToPSR4Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
205 changes: 65 additions & 140 deletions src/Command/PrivatizeConstantsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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()));
}
}
}
16 changes: 16 additions & 0 deletions src/Contract/ClassConstantFetchInterface.php
Original file line number Diff line number Diff line change
@@ -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;
}
11 changes: 11 additions & 0 deletions src/Exception/NotImplementedYetException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Rector\SwissKnife\Exception;

use Exception;

final class NotImplementedYetException extends Exception
{
}
11 changes: 11 additions & 0 deletions src/Exception/ShouldNotHappenException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Rector\SwissKnife\Exception;

use Exception;

final class ShouldNotHappenException extends Exception
{
}
Loading
Loading