From eeb43d2b014d0a04443c00989961239532116580 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sat, 22 Feb 2020 16:26:14 +0100 Subject: [PATCH 1/3] rector ci: enable solid + fix class const privatization for using parent --- ci/check_services_in_yaml_configs.php | 77 ++++++++++++------- ecs.yaml | 1 - .../Property_/JoinTablePhpDocNodeFactory.php | 2 +- .../ClassConstParsedNodesFinder.php | 56 ++++++++++++++ phpstan.neon | 1 - rector-ci.yaml | 22 +++--- .../Analyzer/ClassConstantFetchAnalyzer.php | 26 ++++++- .../PrivatizeLocalClassConstantRector.php | 70 +++++++++-------- .../Fixture/skip_multi_inheritance.php.inc | 22 ++++++ .../skip_used_by_parents_class.php.inc | 26 +++++++ ...stractInBetweenVariableParentClassUser.php | 10 +++ .../AbstractVariableParentClassUser.php | 11 +++ 12 files changed, 252 insertions(+), 72 deletions(-) create mode 100644 packages/node-collector/src/NodeFinder/ClassConstParsedNodesFinder.php create mode 100644 rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Fixture/skip_multi_inheritance.php.inc create mode 100644 rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Fixture/skip_used_by_parents_class.php.inc create mode 100644 rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Source/AbstractInBetweenVariableParentClassUser.php create mode 100644 rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Source/AbstractVariableParentClassUser.php diff --git a/ci/check_services_in_yaml_configs.php b/ci/check_services_in_yaml_configs.php index 1708d175f119..a4faf45a216b 100644 --- a/ci/check_services_in_yaml_configs.php +++ b/ci/check_services_in_yaml_configs.php @@ -11,40 +11,63 @@ 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; - foreach ($yamlContent['services'] as $service => $serviceConfiguration) { - // configuration → skip - if (Strings::startsWith($service, '_')) { - continue; - } + /** + * @var ServiceConfigurationValidator + */ + private $serviceConfigurationValidator; - // autodiscovery → skip - if (Strings::endsWith($service, '\\')) { - continue; - } + public function __construct() + { + $this->yamlConfigFileProvider = new YamlConfigFileProvider(); + $this->serviceConfigurationValidator = new ServiceConfigurationValidator(); + } - 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() - )); + 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); + } } - $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 @@ public function provider(): array } } -class ServiceConfigurationValidator +final class ServiceConfigurationValidator { public function validate(string $serviceClass, $configuration, SplFileInfo $configFileInfo): void { @@ -119,5 +142,3 @@ private function resolveClassConstructorArgumentNames(string $class): array return $constructorParameterNames; } } - -echo 'All configs have existing services - good job!' . PHP_EOL; diff --git a/ecs.yaml b/ecs.yaml index f2ba937975c9..60b9310b845a 100644 --- a/ecs.yaml +++ b/ecs.yaml @@ -21,7 +21,6 @@ services: parameters: paths: - - "ci" - "bin" - "src" - "packages" diff --git a/packages/better-php-doc-parser/src/PhpDocNodeFactory/Doctrine/Property_/JoinTablePhpDocNodeFactory.php b/packages/better-php-doc-parser/src/PhpDocNodeFactory/Doctrine/Property_/JoinTablePhpDocNodeFactory.php index 462dd1934726..80711ca9b65d 100644 --- a/packages/better-php-doc-parser/src/PhpDocNodeFactory/Doctrine/Property_/JoinTablePhpDocNodeFactory.php +++ b/packages/better-php-doc-parser/src/PhpDocNodeFactory/Doctrine/Property_/JoinTablePhpDocNodeFactory.php @@ -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 diff --git a/packages/node-collector/src/NodeFinder/ClassConstParsedNodesFinder.php b/packages/node-collector/src/NodeFinder/ClassConstParsedNodesFinder.php new file mode 100644 index 000000000000..8abd57a1c6a7 --- /dev/null +++ b/packages/node-collector/src/NodeFinder/ClassConstParsedNodesFinder.php @@ -0,0 +1,56 @@ +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 []; + } +} diff --git a/phpstan.neon b/phpstan.neon index 13fd143d8774..814eb234265a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -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#' diff --git a/rector-ci.yaml b/rector-ci.yaml index f30ca4e7736e..33c3581c11da 100644 --- a/rector-ci.yaml +++ b/rector-ci.yaml @@ -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/' diff --git a/rules/solid/src/Analyzer/ClassConstantFetchAnalyzer.php b/rules/solid/src/Analyzer/ClassConstantFetchAnalyzer.php index 4390dab56d22..d9b722ed2877 100644 --- a/rules/solid/src/Analyzer/ClassConstantFetchAnalyzer.php +++ b/rules/solid/src/Analyzer/ClassConstantFetchAnalyzer.php @@ -8,6 +8,7 @@ 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 @@ private function addClassConstantFetch(ClassConstFetch $classConstFetch): void $resolvedClassType = $this->nodeTypeResolver->resolve($classConstFetch->class); - $className = $this->matchClassTypeThatContainsConstant($resolvedClassType, $constantName); + $className = $this->resolveClassTypeThatContainsConstantOrFirstUnioned($resolvedClassType, $constantName); if ($className === null) { return; } @@ -113,4 +114,27 @@ private function matchClassTypeThatContainsConstant(Type $type, string $constant 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; + } } diff --git a/rules/solid/src/Rector/ClassConst/PrivatizeLocalClassConstantRector.php b/rules/solid/src/Rector/ClassConst/PrivatizeLocalClassConstantRector.php index 8efc87bbc3fe..3ae2f433ba8b 100644 --- a/rules/solid/src/Rector/ClassConst/PrivatizeLocalClassConstantRector.php +++ b/rules/solid/src/Rector/ClassConst/PrivatizeLocalClassConstantRector.php @@ -10,8 +10,8 @@ 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 @@ public function refactor(Node $node): ?Node return $node; } - $useClasses = $this->findClassConstantFetches($class, $constant); + $directUseClasses = $this->classConstParsedNodesFinder->findDirectClassConstantFetches($class, $constant); + $indirectUseClasses = $this->classConstParsedNodesFinder->findIndirectClassConstantFetches($class, $constant); + + $this->changeConstantVisibility( + $node, + $directUseClasses, + $indirectUseClasses, + $parentClassConstantVisibility, + $class + ); - return $this->changeConstantVisibility($node, $useClasses, $parentClassConstantVisibility, $class); + return $node; } private function shouldSkip(ClassConst $classConst): bool @@ -168,42 +177,41 @@ private function findParentClassConstantAndRefactorIfPossible(string $class, str ); } - 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 diff --git a/rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Fixture/skip_multi_inheritance.php.inc b/rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Fixture/skip_multi_inheritance.php.inc new file mode 100644 index 000000000000..e88293416365 --- /dev/null +++ b/rules/solid/tests/Rector/ClassConst/PrivatizeLocalClassConstantRector/Fixture/skip_multi_inheritance.php.inc @@ -0,0 +1,22 @@ + Date: Sun, 23 Feb 2020 16:03:19 +0100 Subject: [PATCH 2/3] add add ShortNameAwareTagInterface --- ci/check_keep_fixtures.php | 9 +++++---- .../PhpDocNode/ShortNameAwareTagInterface.php | 12 ++++++++++++ .../src/PhpDocInfo/PhpDocInfo.php | 6 +++--- .../Doctrine/AbstractDoctrineTagValueNode.php | 3 ++- .../Doctrine/Class_/EntityTagValueNode.php | 10 +++++----- .../Doctrine/Class_/IndexTagValueNode.php | 12 ++++++------ .../Doctrine/Class_/InheritanceTypeTagValueNode.php | 10 +++++----- .../Doctrine/Class_/TableTagValueNode.php | 10 +++++----- .../Class_/UniqueConstraintTagValueNode.php | 12 ++++++------ .../Doctrine/Property_/ColumnTagValueNode.php | 10 +++++----- .../Property_/CustomIdGeneratorTagValueNode.php | 10 +++++----- .../Property_/GeneratedValueTagValueNode.php | 10 +++++----- .../Doctrine/Property_/IdTagValueNode.php | 10 +++++----- .../Doctrine/Property_/JoinColumnTagValueNode.php | 12 ++++++------ .../Doctrine/Property_/JoinTableTagValueNode.php | 10 +++++----- .../Doctrine/Property_/ManyToManyTagValueNode.php | 10 +++++----- .../Doctrine/Property_/ManyToOneTagValueNode.php | 10 +++++----- .../Doctrine/Property_/OneToManyTagValueNode.php | 10 +++++----- .../Doctrine/Property_/OneToOneTagValueNode.php | 10 +++++----- .../PhpDocNode/Symfony/SymfonyRouteTagValueNode.php | 13 +++++++------ rector-ci.yaml | 5 ----- .../Ast/PhpDoc/PhpDocTagNodeFactory.php | 8 +++----- .../RemoveRepositoryFromEntityAnnotationRector.php | 1 + src/Testing/PHPUnit/FixtureSplitter.php | 10 +++------- src/Testing/ValueObject/SplitLine.php | 13 +++++++++++++ 25 files changed, 127 insertions(+), 109 deletions(-) create mode 100644 packages/better-php-doc-parser/src/Contract/PhpDocNode/ShortNameAwareTagInterface.php create mode 100644 src/Testing/ValueObject/SplitLine.php diff --git a/ci/check_keep_fixtures.php b/ci/check_keep_fixtures.php index 5e6dde1b915f..74b750bffb57 100644 --- a/ci/check_keep_fixtures.php +++ b/ci/check_keep_fixtures.php @@ -4,6 +4,7 @@ use Nette\Utils\Strings; use Rector\Core\Testing\PHPUnit\FixtureSplitter; +use Rector\Core\Testing\ValueObject\SplitLine; use Symfony\Component\Finder\Finder; use Symplify\PackageBuilder\Console\ShellCode; use Symplify\PackageBuilder\Console\Style\SymfonyStyleFactory; @@ -29,12 +30,12 @@ /** @var SmartFileInfo $smartFileInfo */ foreach ($smartFileInfos as $smartFileInfo) { - if (! Strings::match($smartFileInfo->getContents(), FixtureSplitter::SPLIT_LINE)) { + if (! Strings::match($smartFileInfo->getContents(), SplitLine::SPLIT_LINE)) { continue; } // original → expected - [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), FixtureSplitter::SPLIT_LINE); + [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), SplitLine::SPLIT_LINE); if ($originalContent !== $expectedContent) { continue; } @@ -48,8 +49,8 @@ $symfonyStyle->warning(sprintf( 'These files have same content before "%s" and after it. Remove the content after "%s"', - FixtureSplitter::SPLIT_LINE, - FixtureSplitter::SPLIT_LINE + SplitLine::SPLIT_LINE, + SplitLine::SPLIT_LINE )); $symfonyStyle->listing($errors); diff --git a/packages/better-php-doc-parser/src/Contract/PhpDocNode/ShortNameAwareTagInterface.php b/packages/better-php-doc-parser/src/Contract/PhpDocNode/ShortNameAwareTagInterface.php new file mode 100644 index 000000000000..cbb5704dd809 --- /dev/null +++ b/packages/better-php-doc-parser/src/Contract/PhpDocNode/ShortNameAwareTagInterface.php @@ -0,0 +1,12 @@ +phpDocNode->children[] = $phpDocChildNode; } - public function addTagValueNodeWithShortName(AbstractTagValueNode $tagValueNode): void + public function addTagValueNodeWithShortName(ShortNameAwareTagInterface $shortNameAwareTag): void { - $spacelessPhpDocTagNode = new SpacelessPhpDocTagNode($tagValueNode::SHORT_NAME, $tagValueNode); + $spacelessPhpDocTagNode = new SpacelessPhpDocTagNode($shortNameAwareTag->getShortName(), $shortNameAwareTag); $this->addPhpDocTagNode($spacelessPhpDocTagNode); } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/AbstractDoctrineTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/AbstractDoctrineTagValueNode.php index 3f4e179b9ebb..08b3bf2d6585 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/AbstractDoctrineTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/AbstractDoctrineTagValueNode.php @@ -5,8 +5,9 @@ namespace Rector\BetterPhpDocParser\PhpDocNode\Doctrine; use Rector\BetterPhpDocParser\Contract\Doctrine\DoctrineTagNodeInterface; +use Rector\BetterPhpDocParser\Contract\PhpDocNode\ShortNameAwareTagInterface; use Rector\BetterPhpDocParser\PhpDocNode\AbstractTagValueNode; -abstract class AbstractDoctrineTagValueNode extends AbstractTagValueNode implements DoctrineTagNodeInterface +abstract class AbstractDoctrineTagValueNode extends AbstractTagValueNode implements DoctrineTagNodeInterface, ShortNameAwareTagInterface { } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/EntityTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/EntityTagValueNode.php index 6388e1cc1067..a04ffe565767 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/EntityTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/EntityTagValueNode.php @@ -8,11 +8,6 @@ final class EntityTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\Entity'; - /** * @var string|null */ @@ -53,4 +48,9 @@ public function removeRepositoryClass(): void { $this->repositoryClass = null; } + + public function getShortName(): string + { + return '@ORM\Entity'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/IndexTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/IndexTagValueNode.php index fab64497dc5b..68257bee50d5 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/IndexTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/IndexTagValueNode.php @@ -6,13 +6,13 @@ final class IndexTagValueNode extends AbstractIndexTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\Index'; - public function getTag(): ?string { - return $this->tag ?: self::SHORT_NAME; + return $this->tag ?: $this->getShortName(); + } + + public function getShortName(): string + { + return '@ORM\Index'; } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/InheritanceTypeTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/InheritanceTypeTagValueNode.php index 50aba48ad885..1f8ec887a800 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/InheritanceTypeTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/InheritanceTypeTagValueNode.php @@ -8,11 +8,6 @@ final class InheritanceTypeTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\InheritanceType'; - /** * @var string|null */ @@ -36,4 +31,9 @@ public function __toString(): string return '(value="' . $this->value . '")'; } + + public function getShortName(): string + { + return '@ORM\InheritanceType'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/TableTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/TableTagValueNode.php index a15dcc3bfd51..90e9aa364c3e 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/TableTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/TableTagValueNode.php @@ -8,11 +8,6 @@ final class TableTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\Table'; - /** * @var string|null */ @@ -152,4 +147,9 @@ public function getName(): ?string { return $this->name; } + + public function getShortName(): string + { + return '@ORM\Table'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/UniqueConstraintTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/UniqueConstraintTagValueNode.php index cfcfd1bec841..7b70a3a90546 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/UniqueConstraintTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Class_/UniqueConstraintTagValueNode.php @@ -6,13 +6,13 @@ final class UniqueConstraintTagValueNode extends AbstractIndexTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\UniqueConstraint'; - public function getTag(): ?string { - return $this->tag ?: self::SHORT_NAME; + return $this->tag ?: $this->getShortName(); + } + + public function getShortName(): string + { + return '@ORM\UniqueConstraint'; } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ColumnTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ColumnTagValueNode.php index a7a026222f8d..fcf9e2310f76 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ColumnTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ColumnTagValueNode.php @@ -9,11 +9,6 @@ final class ColumnTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\Column'; - /** * @var string|null */ @@ -167,4 +162,9 @@ public function isNullable(): ?bool { return $this->nullable; } + + public function getShortName(): string + { + return '@ORM\Column'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/CustomIdGeneratorTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/CustomIdGeneratorTagValueNode.php index 2150aa2ce763..582d5198ad77 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/CustomIdGeneratorTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/CustomIdGeneratorTagValueNode.php @@ -8,11 +8,6 @@ final class CustomIdGeneratorTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\CustomIdGenerator'; - /** * @var string */ @@ -28,4 +23,9 @@ public function __toString(): string { return sprintf('(class="%s")', $this->class); } + + public function getShortName(): string + { + return '@ORM\CustomIdGenerator'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/GeneratedValueTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/GeneratedValueTagValueNode.php index a5f227bb91c1..a115d719eedd 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/GeneratedValueTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/GeneratedValueTagValueNode.php @@ -8,11 +8,6 @@ final class GeneratedValueTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\GeneratedValue'; - /** * @var string */ @@ -27,4 +22,9 @@ public function __toString(): string { return sprintf('(strategy="%s")', $this->strategy); } + + public function getShortName(): string + { + return '@ORM\GeneratedValue'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/IdTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/IdTagValueNode.php index 71bf3844aa9b..7718c4ddbd8c 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/IdTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/IdTagValueNode.php @@ -8,13 +8,13 @@ final class IdTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\Id'; - public function __toString(): string { return ''; } + + public function getShortName(): string + { + return '@ORM\Id'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinColumnTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinColumnTagValueNode.php index 3a920da9cf97..6a8927c59dfb 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinColumnTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinColumnTagValueNode.php @@ -9,11 +9,6 @@ final class JoinColumnTagValueNode extends AbstractDoctrineTagValueNode implements TagAwareNodeInterface { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\JoinColumn'; - /** * @var bool|null */ @@ -126,11 +121,16 @@ public function isNullable(): ?bool public function getTag(): ?string { - return $this->tag ?: self::SHORT_NAME; + return $this->tag ?: $this->getShortName(); } public function getUnique(): ?bool { return $this->unique; } + + public function getShortName(): string + { + return '@ORM\JoinColumn'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinTableTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinTableTagValueNode.php index 47f6e420500b..11c9a0fcebd3 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinTableTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/JoinTableTagValueNode.php @@ -8,11 +8,6 @@ final class JoinTableTagValueNode extends AbstractDoctrineTagValueNode { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\JoinTable'; - /** * @var string */ @@ -111,4 +106,9 @@ public function __toString(): string return $this->printContentItems($contentItems); } + + public function getShortName(): string + { + return '@ORM\JoinTable'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToManyTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToManyTagValueNode.php index 09b036727aac..344319d9240e 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToManyTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToManyTagValueNode.php @@ -11,11 +11,6 @@ final class ManyToManyTagValueNode extends AbstractDoctrineTagValueNode implements ToManyTagNodeInterface, MappedByNodeInterface, InversedByNodeInterface { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\ManyToMany'; - /** * @var string */ @@ -148,4 +143,9 @@ public function changeTargetEntity(string $targetEntity): void { $this->targetEntity = $targetEntity; } + + public function getShortName(): string + { + return '@ORM\ManyToMany'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToOneTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToOneTagValueNode.php index f1a27ae30fb9..a92f0e3ba27d 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToOneTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/ManyToOneTagValueNode.php @@ -10,11 +10,6 @@ final class ManyToOneTagValueNode extends AbstractDoctrineTagValueNode implements ToOneTagNodeInterface, InversedByNodeInterface { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\ManyToOne'; - /** * @var string */ @@ -101,4 +96,9 @@ public function changeTargetEntity(string $targetEntity): void { $this->targetEntity = $targetEntity; } + + public function getShortName(): string + { + return '@ORM\ManyToOne'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToManyTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToManyTagValueNode.php index 1dc67f298127..b811dd670cc8 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToManyTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToManyTagValueNode.php @@ -11,11 +11,6 @@ final class OneToManyTagValueNode extends AbstractDoctrineTagValueNode implements ToManyTagNodeInterface, MappedByNodeInterface, TypeAwareTagValueNodeInterface { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\OneToMany'; - /** * @var string|null */ @@ -124,4 +119,9 @@ public function changeTargetEntity(string $targetEntity): void { $this->targetEntity = $targetEntity; } + + public function getShortName(): string + { + return '@ORM\OneToMany'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToOneTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToOneTagValueNode.php index 2a9df70a4e5b..bfdcb8a31c18 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToOneTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Doctrine/Property_/OneToOneTagValueNode.php @@ -11,11 +11,6 @@ final class OneToOneTagValueNode extends AbstractDoctrineTagValueNode implements ToOneTagNodeInterface, MappedByNodeInterface, InversedByNodeInterface { - /** - * @var string - */ - public const SHORT_NAME = '@ORM\OneToOne'; - /** * @var string */ @@ -138,4 +133,9 @@ public function changeTargetEntity(string $targetEntity): void { $this->targetEntity = $targetEntity; } + + public function getShortName(): string + { + return '@ORM\OneToOne'; + } } diff --git a/packages/better-php-doc-parser/src/PhpDocNode/Symfony/SymfonyRouteTagValueNode.php b/packages/better-php-doc-parser/src/PhpDocNode/Symfony/SymfonyRouteTagValueNode.php index 8d8256403fde..584ca39b3a6e 100644 --- a/packages/better-php-doc-parser/src/PhpDocNode/Symfony/SymfonyRouteTagValueNode.php +++ b/packages/better-php-doc-parser/src/PhpDocNode/Symfony/SymfonyRouteTagValueNode.php @@ -5,16 +5,12 @@ namespace Rector\BetterPhpDocParser\PhpDocNode\Symfony; use Nette\Utils\Strings; +use Rector\BetterPhpDocParser\Contract\PhpDocNode\ShortNameAwareTagInterface; use Rector\BetterPhpDocParser\PhpDocNode\AbstractTagValueNode; use Symfony\Component\Routing\Annotation\Route; -final class SymfonyRouteTagValueNode extends AbstractTagValueNode +final class SymfonyRouteTagValueNode extends AbstractTagValueNode implements ShortNameAwareTagInterface { - /** - * @var string - */ - public const SHORT_NAME = '@Route'; - /** * @var string */ @@ -128,6 +124,11 @@ public function changeMethods(array $methods): void $this->methods = $methods; } + public function getShortName(): string + { + return '@Route'; + } + private function createPath(): string { if ($this->isPathExplicit) { diff --git a/rector-ci.yaml b/rector-ci.yaml index 33c3581c11da..2d1b9d2ed754 100644 --- a/rector-ci.yaml +++ b/rector-ci.yaml @@ -17,11 +17,6 @@ parameters: - 'packages' - 'tests' - autoload_paths: - # needed for FixtureSplitter::SPLIT_LINE public use - - 'ci/check_keep_fixtures.php' -# - 'ci/check_services_in_yaml_configs.php' - exclude_paths: - '/Fixture/' - '/Source/' diff --git a/rules/doctrine/src/PhpDocParser/Ast/PhpDoc/PhpDocTagNodeFactory.php b/rules/doctrine/src/PhpDocParser/Ast/PhpDoc/PhpDocTagNodeFactory.php index 94dbb02e4476..5aca3d2e692e 100644 --- a/rules/doctrine/src/PhpDocParser/Ast/PhpDoc/PhpDocTagNodeFactory.php +++ b/rules/doctrine/src/PhpDocParser/Ast/PhpDoc/PhpDocTagNodeFactory.php @@ -61,14 +61,12 @@ public function createJoinTableTagNode(Property $property): PhpDocTagNode [new JoinColumnTagValueNode(null, 'uuid')] ); - return new SpacelessPhpDocTagNode(JoinTableTagValueNode::SHORT_NAME, $joinTableTagValueNode); + return new SpacelessPhpDocTagNode($joinTableTagValueNode->getShortName(), $joinTableTagValueNode); } - public function createJoinColumnTagNode(bool $isNullable): PhpDocTagNode + public function createJoinColumnTagNode(bool $isNullable): JoinColumnTagValueNode { - $joinColumnTagValueNode = new JoinColumnTagValueNode(null, 'uuid', null, $isNullable); - - return new SpacelessPhpDocTagNode(JoinColumnTagValueNode::SHORT_NAME, $joinColumnTagValueNode); + return new JoinColumnTagValueNode(null, 'uuid', null, $isNullable); } private function createVarTagValueNodeWithType(TypeNode $typeNode): AttributeAwareVarTagValueNode diff --git a/rules/doctrine/src/Rector/Class_/RemoveRepositoryFromEntityAnnotationRector.php b/rules/doctrine/src/Rector/Class_/RemoveRepositoryFromEntityAnnotationRector.php index 1f42d9ba0b44..54b07b923199 100644 --- a/rules/doctrine/src/Rector/Class_/RemoveRepositoryFromEntityAnnotationRector.php +++ b/rules/doctrine/src/Rector/Class_/RemoveRepositoryFromEntityAnnotationRector.php @@ -65,6 +65,7 @@ public function refactor(Node $node): ?Node return null; } + /** @var EntityTagValueNode|null $doctrineEntityTag */ $doctrineEntityTag = $phpDocInfo->getByType(EntityTagValueNode::class); if ($doctrineEntityTag === null) { return null; diff --git a/src/Testing/PHPUnit/FixtureSplitter.php b/src/Testing/PHPUnit/FixtureSplitter.php index 16e06fa4692e..551641da893f 100644 --- a/src/Testing/PHPUnit/FixtureSplitter.php +++ b/src/Testing/PHPUnit/FixtureSplitter.php @@ -6,15 +6,11 @@ use Nette\Utils\FileSystem; use Nette\Utils\Strings; +use Rector\Core\Testing\ValueObject\SplitLine; use Symplify\SmartFileSystem\SmartFileInfo; final class FixtureSplitter { - /** - * @var string - */ - public const SPLIT_LINE = '#-----' . PHP_EOL . '#'; - /** * @var string */ @@ -32,9 +28,9 @@ public function splitContentToOriginalFileAndExpectedFile( SmartFileInfo $smartFileInfo, bool $autoloadTestFixture ): array { - if (Strings::match($smartFileInfo->getContents(), self::SPLIT_LINE)) { + if (Strings::match($smartFileInfo->getContents(), SplitLine::SPLIT_LINE)) { // original → expected - [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), self::SPLIT_LINE); + [$originalContent, $expectedContent] = Strings::split($smartFileInfo->getContents(), SplitLine::SPLIT_LINE); } else { // no changes $originalContent = $smartFileInfo->getContents(); diff --git a/src/Testing/ValueObject/SplitLine.php b/src/Testing/ValueObject/SplitLine.php new file mode 100644 index 000000000000..9b608c2bc40a --- /dev/null +++ b/src/Testing/ValueObject/SplitLine.php @@ -0,0 +1,13 @@ + Date: Sun, 23 Feb 2020 16:34:43 +0100 Subject: [PATCH 3/3] require at least 2 iffs for nested foreach --- rector-ci.yaml | 13 ++++--------- ...arameterFromArrayToArrayCollectionRector.php | 2 +- .../Closure/ClosureToArrowFunctionRector.php | 2 +- ...ngeNestedForeachIfsToEarlyContinueRector.php | 2 +- .../Fixture/skip_single_line.php.inc | 17 +++++++++++++++++ 5 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_single_line.php.inc diff --git a/rector-ci.yaml b/rector-ci.yaml index 2d1b9d2ed754..e26ecd96d932 100644 --- a/rector-ci.yaml +++ b/rector-ci.yaml @@ -1,14 +1,9 @@ -#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: diff --git a/rules/doctrine-code-quality/src/Rector/Class_/ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector.php b/rules/doctrine-code-quality/src/Rector/Class_/ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector.php index 43b8aca832b6..211bc43a173c 100644 --- a/rules/doctrine-code-quality/src/Rector/Class_/ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector.php +++ b/rules/doctrine-code-quality/src/Rector/Class_/ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector.php @@ -23,7 +23,7 @@ * * @see \Rector\DoctrineCodeQuality\Tests\Rector\Class_\ChangeQuerySetParametersMethodParameterFromArrayToArrayCollection\ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRectorTest */ -class ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector extends AbstractRector +final class ChangeQuerySetParametersMethodParameterFromArrayToArrayCollectionRector extends AbstractRector { public function getNodeTypes(): array { diff --git a/rules/php-74/src/Rector/Closure/ClosureToArrowFunctionRector.php b/rules/php-74/src/Rector/Closure/ClosureToArrowFunctionRector.php index 756a489b16ea..466cdea33840 100644 --- a/rules/php-74/src/Rector/Closure/ClosureToArrowFunctionRector.php +++ b/rules/php-74/src/Rector/Closure/ClosureToArrowFunctionRector.php @@ -93,7 +93,7 @@ public function refactor(Node $node): ?Node $arrowFunction->expr = $return->expr; - if ($node->static === true) { + if ($node->static) { $arrowFunction->static = true; } diff --git a/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php b/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php index 315910e50d84..ca2ee51157d2 100644 --- a/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php +++ b/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php @@ -98,7 +98,7 @@ public function getNodeTypes(): array public function refactor(Node $node): ?Node { $nestedIfsWithOnlyNonReturn = $this->ifManipulator->collectNestedIfsWithNonBreaking($node); - if ($nestedIfsWithOnlyNonReturn === []) { + if (count($nestedIfsWithOnlyNonReturn) < 2) { return null; } diff --git a/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_single_line.php.inc b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_single_line.php.inc new file mode 100644 index 000000000000..e81651c122a8 --- /dev/null +++ b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_single_line.php.inc @@ -0,0 +1,17 @@ +