rector ci: enable solid + fix class const privatization for using parent

This commit is contained in:
TomasVotruba 2020-02-22 16:26:14 +01:00
parent afb9554414
commit eeb43d2b01
12 changed files with 253 additions and 73 deletions

View File

@ -11,40 +11,63 @@ use Symfony\Component\Yaml\Yaml;
require __DIR__ . '/../vendor/autoload.php';
$yamlConfigFileProvider = new YamlConfigFileProvider();
$serviceConfigurationValidator = new ServiceConfigurationValidator();
$serviceExistenceConfigChecker = new ServiceExistenceConfigChecker();
$serviceExistenceConfigChecker->check();
foreach ($yamlConfigFileProvider->provider() as $configFileInfo) {
$yamlContent = Yaml::parseFile($configFileInfo->getRealPath());
if (! isset($yamlContent['services'])) {
continue;
final class ServiceExistenceConfigChecker
{
/**
* @var YamlConfigFileProvider
*/
private $yamlConfigFileProvider;
/**
* @var ServiceConfigurationValidator
*/
private $serviceConfigurationValidator;
public function __construct()
{
$this->yamlConfigFileProvider = new YamlConfigFileProvider();
$this->serviceConfigurationValidator = new ServiceConfigurationValidator();
}
foreach ($yamlContent['services'] as $service => $serviceConfiguration) {
// configuration → skip
if (Strings::startsWith($service, '_')) {
continue;
public function check(): void
{
foreach ($this->yamlConfigFileProvider->provider() as $configFileInfo) {
$yamlContent = Yaml::parseFile($configFileInfo->getRealPath());
if (! isset($yamlContent['services'])) {
continue;
}
foreach ($yamlContent['services'] as $service => $serviceConfiguration) {
// configuration → skip
if (Strings::startsWith($service, '_')) {
continue;
}
// autodiscovery → skip
if (Strings::endsWith($service, '\\')) {
continue;
}
if (! ClassExistenceStaticHelper::doesClassLikeExist($service)) {
throw new ShouldNotHappenException(sprintf(
'Service "%s" from config "%s" was not found. Check if it really exists or is even autoload, please',
$service,
$configFileInfo->getRealPath()
));
}
$this->serviceConfigurationValidator->validate($service, $serviceConfiguration, $configFileInfo);
}
}
// autodiscovery → skip
if (Strings::endsWith($service, '\\')) {
continue;
}
if (! ClassExistenceStaticHelper::doesClassLikeExist($service)) {
throw new ShouldNotHappenException(sprintf(
'Service "%s" from config "%s" was not found. Check if it really exists or is even autoload, please',
$service,
$configFileInfo->getRealPath()
));
}
$serviceConfigurationValidator->validate($service, $serviceConfiguration, $configFileInfo);
echo 'All configs have existing services - good job!' . PHP_EOL;
}
}
class YamlConfigFileProvider
final class YamlConfigFileProvider
{
/**
* @return SplFileInfo[]
@ -59,7 +82,7 @@ class YamlConfigFileProvider
}
}
class ServiceConfigurationValidator
final class ServiceConfigurationValidator
{
public function validate(string $serviceClass, $configuration, SplFileInfo $configFileInfo): void
{
@ -119,5 +142,3 @@ class ServiceConfigurationValidator
return $constructorParameterNames;
}
}
echo 'All configs have existing services - good job!' . PHP_EOL;

View File

@ -21,7 +21,6 @@ services:
parameters:
paths:
- "ci"
- "bin"
- "src"
- "packages"

View File

@ -20,7 +20,7 @@ final class JoinTablePhpDocNodeFactory extends AbstractPhpDocNodeFactory
/**
* @var string
*/
public const INVERSE_JOIN_COLUMNS = 'inverseJoinColumns';
private const INVERSE_JOIN_COLUMNS = 'inverseJoinColumns';
/**
* @var string

View File

@ -0,0 +1,56 @@
<?php
declare(strict_types=1);
namespace Rector\NodeCollector\NodeFinder;
use Rector\SOLID\Analyzer\ClassConstantFetchAnalyzer;
final class ClassConstParsedNodesFinder
{
/**
* @var ClassConstantFetchAnalyzer
*/
private $classConstantFetchAnalyzer;
public function __construct(ClassConstantFetchAnalyzer $classConstantFetchAnalyzer)
{
$this->classConstantFetchAnalyzer = $classConstantFetchAnalyzer;
}
/**
* @return string[]
*/
public function findDirectClassConstantFetches(string $desiredClassName, string $desiredConstantName): array
{
$classConstantFetchByClassAndName = $this->classConstantFetchAnalyzer->provideClassConstantFetchByClassAndName();
return $classConstantFetchByClassAndName[$desiredClassName][$desiredConstantName] ?? [];
}
/**
* @return string[]
*/
public function findIndirectClassConstantFetches(string $desiredClassName, string $desiredConstantName): array
{
$classConstantFetchByClassAndName = $this->classConstantFetchAnalyzer->provideClassConstantFetchByClassAndName();
foreach ($classConstantFetchByClassAndName as $className => $classesByConstantName) {
if (! isset($classesByConstantName[$desiredConstantName])) {
continue;
}
// include child usages
if (! is_a($desiredClassName, $className, true)) {
continue;
}
if ($desiredClassName === $className) {
continue;
}
return $classesByConstantName[$desiredConstantName];
}
return [];
}
}

View File

@ -67,7 +67,6 @@ parameters:
- '#Call to function in_array\(\) with arguments string, (.*?) and true will always evaluate to false#'
# known values
- '#Access to an undefined property PhpParser\\Node\\Expr::\$left#'
- '#Access to an undefined property PhpParser\\Node\\Expr::\$right#'
- '#Access to an undefined property PhpParser\\Node\\Expr\\MethodCall\|PhpParser\\Node\\Stmt\\ClassMethod::\$params#'

View File

@ -1,22 +1,26 @@
services:
Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector: null
Rector\SOLID\Rector\ClassMethod\ChangeReadOnlyVariableWithDefaultValueToConstantRector: null
#services:
# Rector\SOLID\Rector\ClassConst\PrivatizeLocalClassConstantRector: null
# Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector: null
# Rector\SOLID\Rector\ClassMethod\ChangeReadOnlyVariableWithDefaultValueToConstantRector: null
parameters:
sets:
- 'coding-style'
- 'code-quality'
- 'dead-code'
- 'nette-utils-code-quality'
# - 'coding-style'
# - 'code-quality'
# - 'dead-code'
# - 'nette-utils-code-quality'
- 'solid'
paths:
- 'ci'
- 'src'
- 'rules'
- 'packages'
- 'tests'
autoload_paths:
- 'ci/check_services_in_yaml_configs.php'
# needed for FixtureSplitter::SPLIT_LINE public use
- 'ci/check_keep_fixtures.php'
# - 'ci/check_services_in_yaml_configs.php'
exclude_paths:
- '/Fixture/'

View File

@ -8,6 +8,7 @@ use PhpParser\Node\Expr\ClassConstFetch;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\UnionType;
use Rector\Core\Testing\PHPUnit\PHPUnitEnvironment;
use Rector\NodeCollector\NodeCollector\ParsedNodeCollector;
use Rector\NodeNameResolver\NodeNameResolver;
@ -75,7 +76,7 @@ final class ClassConstantFetchAnalyzer
$resolvedClassType = $this->nodeTypeResolver->resolve($classConstFetch->class);
$className = $this->matchClassTypeThatContainsConstant($resolvedClassType, $constantName);
$className = $this->resolveClassTypeThatContainsConstantOrFirstUnioned($resolvedClassType, $constantName);
if ($className === null) {
return;
}
@ -113,4 +114,27 @@ final class ClassConstantFetchAnalyzer
return null;
}
private function resolveClassTypeThatContainsConstantOrFirstUnioned(
Type $resolvedClassType,
string $constantName
): ?string {
$className = $this->matchClassTypeThatContainsConstant($resolvedClassType, $constantName);
if ($className !== null) {
return $className;
}
// we need at least one original user class
if ($resolvedClassType instanceof UnionType) {
foreach ($resolvedClassType->getTypes() as $unionedType) {
if (! $unionedType instanceof ObjectType) {
continue;
}
return $unionedType->getClassName();
}
}
return null;
}
}

View File

@ -10,8 +10,8 @@ use Rector\Core\Rector\AbstractRector;
use Rector\Core\RectorDefinition\CodeSample;
use Rector\Core\RectorDefinition\RectorDefinition;
use Rector\Core\ValueObject\PhpVersionFeature;
use Rector\NodeCollector\NodeFinder\ClassConstParsedNodesFinder;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\SOLID\Analyzer\ClassConstantFetchAnalyzer;
use Rector\SOLID\NodeFinder\ParentClassConstantNodeFinder;
use Rector\SOLID\Reflection\ParentConstantReflectionResolver;
use Rector\SOLID\ValueObject\ConstantVisibility;
@ -24,12 +24,7 @@ final class PrivatizeLocalClassConstantRector extends AbstractRector
/**
* @var string
*/
public const HAS_NEW_ACCESS_LEVEL = 'has_new_access_level';
/**
* @var ClassConstantFetchAnalyzer
*/
private $classConstantFetchAnalyzer;
private const HAS_NEW_ACCESS_LEVEL = 'has_new_access_level';
/**
* @var ParentConstantReflectionResolver
@ -41,14 +36,19 @@ final class PrivatizeLocalClassConstantRector extends AbstractRector
*/
private $parentClassConstantNodeFinder;
/**
* @var ClassConstParsedNodesFinder
*/
private $classConstParsedNodesFinder;
public function __construct(
ClassConstantFetchAnalyzer $classConstantFetchAnalyzer,
ParentConstantReflectionResolver $parentConstantReflectionResolver,
ParentClassConstantNodeFinder $parentClassConstantNodeFinder
ParentClassConstantNodeFinder $parentClassConstantNodeFinder,
ClassConstParsedNodesFinder $classConstParsedNodesFinder
) {
$this->classConstantFetchAnalyzer = $classConstantFetchAnalyzer;
$this->parentConstantReflectionResolver = $parentConstantReflectionResolver;
$this->parentClassConstantNodeFinder = $parentClassConstantNodeFinder;
$this->classConstParsedNodesFinder = $classConstParsedNodesFinder;
}
public function getDefinition(): RectorDefinition
@ -122,9 +122,18 @@ PHP
return $node;
}
$useClasses = $this->findClassConstantFetches($class, $constant);
$directUseClasses = $this->classConstParsedNodesFinder->findDirectClassConstantFetches($class, $constant);
$indirectUseClasses = $this->classConstParsedNodesFinder->findIndirectClassConstantFetches($class, $constant);
return $this->changeConstantVisibility($node, $useClasses, $parentClassConstantVisibility, $class);
$this->changeConstantVisibility(
$node,
$directUseClasses,
$indirectUseClasses,
$parentClassConstantVisibility,
$class
);
return $node;
}
private function shouldSkip(ClassConst $classConst): bool
@ -168,42 +177,41 @@ PHP
);
}
private function findClassConstantFetches(string $className, string $constantName): ?array
{
$classConstantFetchByClassAndName = $this->classConstantFetchAnalyzer->provideClassConstantFetchByClassAndName();
return $classConstantFetchByClassAndName[$className][$constantName] ?? null;
}
/**
* @param string[]|null $useClasses
* @param string[] $directUseClasses
* @param string[] $indirectUseClasses
*/
private function changeConstantVisibility(
ClassConst $classConst,
?array $useClasses,
array $directUseClasses,
array $indirectUseClasses,
?ConstantVisibility $constantVisibility,
string $class
): Node {
): void {
// 1. is actually never used (@todo use in "dead-code" set)
if ($useClasses === null) {
$this->makePrivateOrWeaker($classConst, $constantVisibility);
return $classConst;
if ($directUseClasses === []) {
if ($indirectUseClasses !== [] && $constantVisibility !== null) {
$this->makePrivateOrWeaker($classConst, $constantVisibility);
}
return;
}
// 2. is only local use? → private
if ($useClasses === [$class]) {
$this->makePrivateOrWeaker($classConst, $constantVisibility);
return $classConst;
if ($directUseClasses === [$class]) {
if ($indirectUseClasses === []) {
$this->makePrivateOrWeaker($classConst, $constantVisibility);
}
return;
}
// 3. used by children → protected
if ($this->isUsedByChildrenOnly($useClasses, $class)) {
if ($this->isUsedByChildrenOnly($directUseClasses, $class)) {
$this->makeProtected($classConst);
} else {
$this->makePublic($classConst);
}
return $classConst;
}
private function makePrivateOrWeaker(ClassConst $classConst, ?ConstantVisibility $parentConstantVisibility): void

View File

@ -0,0 +1,22 @@
<?php
namespace Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Fixture;
use Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Source\AbstractInBetweenVariableParentClassUser;
use Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Source\AbstractVariableParentClassUser;
class SkipMultiInheritance extends AbstractInBetweenVariableParentClassUser
{
/**
* @var string
*/
public const SHORT_NAME = '@Id';
}
class TheVariableUse
{
public function run(AbstractVariableParentClassUser $value)
{
return $value::SHORT_NAME;
}
}

View File

@ -0,0 +1,26 @@
<?php
namespace Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Fixture;
use Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Fixture\ParentClass;
class SkipUsedByParentClass extends ParentClassUser
{
/**
* @var string
*/
public const SHORT_NAME = '@Assert\Type';
}
class ParentClassUser
{
}
class TheUse
{
public function run()
{
return ParentClassUser::SHORT_NAME;
}
}

View File

@ -0,0 +1,10 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Source;
abstract class AbstractInBetweenVariableParentClassUser extends AbstractVariableParentClassUser
{
}

View File

@ -0,0 +1,11 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\Tests\Rector\ClassConst\PrivatizeLocalClassConstantRector\Source;
abstract class AbstractVariableParentClassUser
{
}