From 8e0deaa2b49d7244d1c3be9208d45dea3e435f52 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sun, 3 May 2020 16:16:10 +0200 Subject: [PATCH 1/3] handle Symplify 7.3 deprecations in CS --- config/services.yaml | 1 + ecs.yaml | 42 --------- .../src/NodeTypeResolver.php | 24 +++-- .../FunctionMethodAndClassNodeVisitorTest.php | 2 +- phpstan.neon | 52 +++++++++- .../NewlineBeforeNewAssignSetRector.php | 25 +++-- .../src/Rector/If_/IfToSpaceshipRector.php | 94 +++++++++++++------ .../Rector/List_/ListSwapArrayOrderRector.php | 3 +- src/Console/Command/SetsCommand.php | 14 +-- .../Class_/AddInterfaceByTraitRector.php | 16 ++-- .../Property/InjectAnnotationClassRector.php | 30 ++++-- .../SwapClassMethodArgumentsRector.php | 30 +++--- .../Dumper/AttributeFilteringNodeDumper.php} | 67 +++---------- src/Testing/Finder/RectorsFinder.php | 48 +++++++--- .../PHPUnit/AbstractNodeVisitorTestCase.php | 24 +++-- .../PHPUnit/AbstractRectorTestCase.php | 28 +++--- src/ValueObject/PhpVersionFeature.php | 5 + .../AbstractServiceAwareRuleTestCase.php | 35 ------- .../SeeAnnotationToTestRuleTest.php | 2 +- 19 files changed, 278 insertions(+), 264 deletions(-) rename src/{PhpParser/BetterNodeDumper.php => Testing/Dumper/AttributeFilteringNodeDumper.php} (66%) delete mode 100644 utils/phpstan-extensions/tests/AbstractServiceAwareRuleTestCase.php diff --git a/config/services.yaml b/config/services.yaml index d8bff01edd9..01f7466e5c4 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -8,6 +8,7 @@ services: exclude: - '../src/Rector/**/*Rector.php' - '../src/Testing/PHPUnit/*' + - '../src/Testing/Dumper/*' - '../src/RectorDefinition/*' - '../src/Exception/*' - '../src/DependencyInjection/CompilerPass/*' diff --git a/ecs.yaml b/ecs.yaml index 1166dae7c29..d43d9af25b8 100644 --- a/ecs.yaml +++ b/ecs.yaml @@ -1,10 +1,4 @@ services: - Symplify\CodingStandard\Sniffs\CleanCode\ClassCognitiveComplexitySniff: - max_class_cognitive_complexity: 50 - - Symplify\CodingStandard\Sniffs\CleanCode\CognitiveComplexitySniff: - max_cognitive_complexity: 9 - Symplify\CodingStandard\Fixer\Order\MethodOrderByTypeFixer: method_order_by_type: Rector\Contract\Rector\PhpRectorInterface: @@ -12,15 +6,6 @@ services: - 'getNodeTypes' - 'refactor' - Symplify\CodingStandard\Fixer\Naming\PropertyNameMatchingTypeFixer: - extra_skipped_classes: - - 'PhpParser\PrettyPrinter\Standard' - - '?string' # bug probably - - Symplify\CodingStandard\Sniffs\Naming\ClassNameSuffixByParentSniff: - extra_parent_types_to_suffixes: - - 'Rector' - parameters: paths: - "bin" @@ -88,33 +73,6 @@ parameters: - 'src/Rector/AbstractRector.php' - 'packages/post-rector/src/Rector/AbstractPostRector.php' - Symplify\CodingStandard\Sniffs\CleanCode\ClassCognitiveComplexitySniff: - # complex case - - 'packages/better-php-doc-parser/src/PhpDocInfo/PhpDocInfo.php' - # node printing - - 'utils/documentation-generator/src/Command/DumpNodesCommand.php' - # 3rd party code - - 'rules/php70/src/EregToPcreTransformer.php' - - Symplify\CodingStandard\Sniffs\CleanCode\CognitiveComplexitySniff: - # todo - - "packages/better-php-doc-parser/src/Printer/WhitespaceDetector.php" - - "packages/node-type-resolver/src/NodeTypeResolver.php" - - "packages/better-php-doc-parser/src/Printer/OriginalSpacingRestorer.php" - # @todo split to multiple rectors - - "rules/php-spec-to-phpunit/src/Rector/MethodCall/PhpSpecPromisesToPHPUnitAssertRector.php" - - - 'packages/better-php-doc-parser/src/PhpDocNode/**/*TagValueNode.php' - - "rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php" - - # per node logic - - 'utils/documentation-generator/src/Command/DumpNodesCommand.php' - # copied 3rd party logic - - 'rules/php70/src/EregToPcreTransformer.php' - # dev - - 'packages/type-declaration/src/Rector/FunctionLike/*TypeDeclarationRector.php' - - 'rules/php70/src/Rector/If_/IfToSpaceshipRector.php' - PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis\AssignmentInConditionSniff.FoundInWhileCondition: null SlevomatCodingStandard\Sniffs\TypeHints\TypeHintDeclarationSniff.MissingParameterTypeHint: diff --git a/packages/node-type-resolver/src/NodeTypeResolver.php b/packages/node-type-resolver/src/NodeTypeResolver.php index e84e3c724e0..bbc6094c429 100644 --- a/packages/node-type-resolver/src/NodeTypeResolver.php +++ b/packages/node-type-resolver/src/NodeTypeResolver.php @@ -284,14 +284,9 @@ private function isMatchingUnionType(Type $requiredType, Type $resolvedType): bo private function resolveFirstType(Node $node): Type { - foreach ($this->nodeTypeResolvers as $nodeTypeResolver) { - foreach ($nodeTypeResolver->getNodeClasses() as $nodeClass) { - if (! is_a($node, $nodeClass)) { - continue; - } - - return $nodeTypeResolver->resolve($node); - } + $type = $this->resolveByNodeTypeResolvers($node); + if ($type !== null) { + return $type; } /** @var Scope|null $nodeScope */ @@ -346,4 +341,17 @@ private function isAnonymousClass(Node $node): bool return $className === null || Strings::contains($className, 'AnonymousClass'); } + + private function resolveByNodeTypeResolvers(Node $node): ?Type + { + foreach ($this->nodeTypeResolvers as $nodeClass => $nodeTypeResolver) { + if (! is_a($node, $nodeClass)) { + continue; + } + + return $nodeTypeResolver->resolve($node); + } + + return null; + } } diff --git a/packages/node-type-resolver/tests/NodeVisitor/FunctionMethodAndClassNodeVisitor/FunctionMethodAndClassNodeVisitorTest.php b/packages/node-type-resolver/tests/NodeVisitor/FunctionMethodAndClassNodeVisitor/FunctionMethodAndClassNodeVisitorTest.php index 94596aa6d47..ee64c2dda10 100644 --- a/packages/node-type-resolver/tests/NodeVisitor/FunctionMethodAndClassNodeVisitor/FunctionMethodAndClassNodeVisitorTest.php +++ b/packages/node-type-resolver/tests/NodeVisitor/FunctionMethodAndClassNodeVisitor/FunctionMethodAndClassNodeVisitorTest.php @@ -44,7 +44,7 @@ public function provideData(): Iterator /** * @param Node[] $nodes */ - protected function visitNodes(array $nodes): void + protected function traverseNodes(array $nodes): void { $nodeTraverser = new NodeTraverser(); $nodeTraverser->addVisitor(new NameResolver()); diff --git a/phpstan.neon b/phpstan.neon index c98a76e222d..7d568a29483 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,9 +4,18 @@ includes: - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon - vendor/slam/phpstan-extensions/conf/slam-rules.neon + # see https://github.com/symplify/coding-standard + - vendor/symplify/coding-standard/config/symplify-rules.neon + parameters: level: max + # see https://github.com/symplify/coding-standard + symplify: + max_cognitive_complexity: 9 # default: 8 + parent_classes: + - Rector + # to allow installing with various phsptan versions without reporting old errors here reportUnmatchedIgnoredErrors: false @@ -27,10 +36,8 @@ parameters: - 'packages/doctrine-annotation-generated/src/ConstantPreservingDocParser.php' - 'packages/doctrine-annotation-generated/src/ConstantPreservingAnnotationReader.php' - - ci/check_services_in_yaml_configs.php - "*/Expected/*" # complex printer - - "utils/phpstan/generate-paths.php" - '*tests/Rector/MethodCall/RenameMethodRector/**/SomeClass.php' # tests files - '*tests/*/Fixture/*' @@ -38,8 +45,6 @@ parameters: - '*tests/Source/*' # part of composer - '*/tests/Rector/Psr4/MultipleClassFileToPsr4ClassesRector/Expected/Just*ExceptionWithoutNamespace.php' - # stubs - - 'stubs/*' ignoreErrors: # false positive @@ -257,3 +262,42 @@ parameters: message: "#^Class \"Rector\\\\PSR4\\\\Rector\\\\Namespace_\\\\NormalizeNamespaceByPSR4ComposerAutoloadRector\" is missing @see annotation with test case class reference$#" count: 1 path: rules/psr4/src/Rector/Namespace_/NormalizeNamespaceByPSR4ComposerAutoloadRector.php + + - '#Method Rector\\Core\\Testing\\Finder\\RectorsFinder\:\:findInDirectories\(\) should return array but returns array#' + + - + message: "#^Class cognitive complexity for \"PhpDocInfo\" is 53, keep it under 50$#" + count: 1 + path: packages/better-php-doc-parser/src/PhpDocInfo/PhpDocInfo.php + + - + message: "#^Cognitive complexity for \"Rector\\\\BetterPhpDocParser\\\\Printer\\\\WhitespaceDetector\\:\\:detectOldWhitespaces\\(\\)\" is 18, keep it under 9$#" + count: 1 + path: packages/better-php-doc-parser/src/Printer/WhitespaceDetector.php + + - + message: "#^Parameter \\#1 \\$input of function array_splice expects array, array\\\\|null given\\.$#" + count: 1 + path: rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php + + - + message: "#^Cognitive complexity for \"Rector\\\\PhpSpecToPHPUnit\\\\Rector\\\\MethodCall\\\\PhpSpecPromisesToPHPUnitAssertRector\\:\\:refactor\\(\\)\" is 13, keep it under 9$#" + count: 1 + path: rules/php-spec-to-phpunit/src/Rector/MethodCall/PhpSpecPromisesToPHPUnitAssertRector.php + + - + message: "#^Class cognitive complexity for \"EregToPcreTransformer\" is 77, keep it under 50$#" + count: 1 + path: rules/php70/src/EregToPcreTransformer.php + + - + message: "#^Cognitive complexity for \"Rector\\\\Php70\\\\EregToPcreTransformer\\:\\:(.*?)\" is (.*?), keep it under 9$#" + path: rules/php70/src/EregToPcreTransformer.php + + - + message: "#^Cognitive complexity for \"Rector\\\\Core\\\\Testing\\\\Dumper\\\\AttributeFilteringNodeDumper\\:\\:dumpSubNodes\\(\\)\" is 16, keep it under 9$#" + count: 1 + path: src/Testing/Dumper/AttributeFilteringNodeDumper.php + + + - '#Use explicit return value over magic &reference#' diff --git a/rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php b/rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php index 3025af5bd16..1706ffa4855 100644 --- a/rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php +++ b/rules/coding-style/src/Rector/ClassMethod/NewlineBeforeNewAssignSetRector.php @@ -82,16 +82,10 @@ public function refactor(Node $node): ?Node $hasChanged = false; - if ($node->stmts === null) { - return null; - } - - foreach ($node->stmts as $key => $stmt) { + foreach ((array) $node->stmts as $key => $stmt) { $currentStmtVariableName = null; - if ($stmt instanceof Expression) { - $stmt = $stmt->expr; - } + $stmt = $this->unwrapExpression($stmt); if ($stmt instanceof Assign || $stmt instanceof MethodCall) { if ($this->shouldSkipLeftVariable($stmt)) { @@ -111,11 +105,7 @@ public function refactor(Node $node): ?Node $this->previousStmtVariableName = $currentStmtVariableName; } - if (! $hasChanged) { - return null; - } - - return $node; + return $hasChanged ? $node : null; } private function reset(): void @@ -180,4 +170,13 @@ private function isPreceededByEmptyLine(Node $node, int $key): bool return abs($currentNode->getLine() - $previousNode->getLine()) >= 2; } + + private function unwrapExpression(Node $node): Node + { + if ($node instanceof Expression) { + return $node->expr; + } + + return $node; + } } diff --git a/rules/php70/src/Rector/If_/IfToSpaceshipRector.php b/rules/php70/src/Rector/If_/IfToSpaceshipRector.php index 9fe9ed594e8..9494021ba08 100644 --- a/rules/php70/src/Rector/If_/IfToSpaceshipRector.php +++ b/rules/php70/src/Rector/If_/IfToSpaceshipRector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Expr\BinaryOp\Smaller; use PhpParser\Node\Expr\BinaryOp\Spaceship; use PhpParser\Node\Expr\Ternary; +use PhpParser\Node\Stmt\Else_; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; use Rector\Core\Rector\AbstractRector; @@ -24,6 +25,7 @@ /** * @see https://wiki.php.net/rfc/combined-comparison-operator * @see https://3v4l.org/LPbA0 + * * @see \Rector\Php70\Tests\Rector\If_\IfToSpaceshipRector\IfToSpaceshipRectorTest */ final class IfToSpaceshipRector extends AbstractRector @@ -53,6 +55,11 @@ final class IfToSpaceshipRector extends AbstractRector */ private $secondValue; + /** + * @var Node|null + */ + private $nextNode; + public function getDefinition(): RectorDefinition { return new RectorDefinition('Changes if/else to spaceship <=> where useful', [ @@ -105,38 +112,13 @@ public function refactor(Node $node): ?Node return null; } - $this->reset(); - - if ($node->cond instanceof Equal || $node->cond instanceof Identical) { - if ($node->stmts[0] instanceof Return_) { - if ($node->stmts[0]->expr === null) { - return null; - } - - $this->onEqual = $this->getValue($node->stmts[0]->expr); - } - } else { + if (! $node->cond instanceof Equal && ! $node->cond instanceof Identical) { return null; } - if ($node->else !== null) { - if (count($node->else->stmts) !== 1) { - return null; - } + $this->reset(); - if ($node->else->stmts[0] instanceof Return_) { - /** @var Return_ $returnNode */ - $returnNode = $node->else->stmts[0]; - if ($returnNode->expr instanceof Ternary) { - $this->processTernary($returnNode->expr); - } - } - } else { - $nextNode = $node->getAttribute(AttributeKey::NEXT_NODE); - if ($nextNode instanceof Return_ && $nextNode->expr instanceof Ternary) { - $this->processTernary($nextNode->expr); - } - } + $this->matchOnEqualFirstValueAndSecondValue($node); if ($this->firstValue === null || $this->secondValue === null) { return null; @@ -146,13 +128,13 @@ public function refactor(Node $node): ?Node return null; } - // is spaceship retun values? + // is spaceship return values? if ([$this->onGreater, $this->onEqual, $this->onSmaller] !== [-1, 0, 1]) { return null; } - if (isset($nextNode)) { - $this->removeNode($nextNode); + if ($this->nextNode !== null) { + $this->removeNode($this->nextNode); } // spaceship ready! @@ -211,4 +193,54 @@ private function areVariablesEqual(BinaryOp $binaryOp, ?Expr $firstValue, ?Expr $secondValue ); } + + private function matchOnEqualFirstValueAndSecondValue(If_ $if): void + { + $this->matchOnEqual($if); + + if ($if->else !== null) { + $this->processElse($if->else); + } else { + $this->nextNode = $if->getAttribute(AttributeKey::NEXT_NODE); + if ($this->nextNode instanceof Return_ && $this->nextNode->expr instanceof Ternary) { + /** @var Ternary $ternary */ + $ternary = $this->nextNode->expr; + $this->processTernary($ternary); + } + } + } + + private function matchOnEqual(If_ $if): void + { + if (count($if->stmts) !== 1) { + return; + } + + $onlyIfStmt = $if->stmts[0]; + + if ($onlyIfStmt instanceof Return_) { + if ($onlyIfStmt->expr === null) { + return; + } + + $this->onEqual = $this->getValue($onlyIfStmt->expr); + } + } + + private function processElse(Else_ $else): void + { + if (count($else->stmts) !== 1) { + return; + } + + if (! $else->stmts[0] instanceof Return_) { + return; + } + + /** @var Return_ $returnNode */ + $returnNode = $else->stmts[0]; + if ($returnNode->expr instanceof Ternary) { + $this->processTernary($returnNode->expr); + } + } } diff --git a/rules/php70/src/Rector/List_/ListSwapArrayOrderRector.php b/rules/php70/src/Rector/List_/ListSwapArrayOrderRector.php index 51ba71b1cdf..609c6e32d82 100644 --- a/rules/php70/src/Rector/List_/ListSwapArrayOrderRector.php +++ b/rules/php70/src/Rector/List_/ListSwapArrayOrderRector.php @@ -12,6 +12,7 @@ use Rector\Core\Rector\AbstractRector; use Rector\Core\RectorDefinition\CodeSample; use Rector\Core\RectorDefinition\RectorDefinition; +use Rector\Core\ValueObject\PhpVersionFeature; /** * @source http://php.net/manual/en/migration70.incompatible.php#migration70.incompatible.variable-handling.list @@ -40,7 +41,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $this->isAtLeastPhpVersion('7.0')) { + if (! $this->isAtLeastPhpVersion(PhpVersionFeature::LIST_SWAP_ORDER)) { return null; } diff --git a/src/Console/Command/SetsCommand.php b/src/Console/Command/SetsCommand.php index 83e1707a520..e361b58e4b2 100644 --- a/src/Console/Command/SetsCommand.php +++ b/src/Console/Command/SetsCommand.php @@ -44,13 +44,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int { $sets = $this->setProvider->provide(); - if ($input->getArgument('name')) { - $sets = $this->filterSetsByName($input, $sets); + $name = (string) $input->getArgument('name'); + if ($name) { + $sets = $this->filterSetsByName($name, $sets); } - $this->symfonyStyle->title(sprintf('%d available sets:', count($sets))); $this->symfonyStyle->listing($sets); + if ($name === '') { + $this->symfonyStyle->note('Tip: filter sets by name, e.g. "vendor/bin/rector sets symfony"'); + } + return ShellCode::SUCCESS; } @@ -58,10 +62,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int * @param string[] $sets * @return string[] */ - private function filterSetsByName(InputInterface $input, array $sets): array + private function filterSetsByName(string $name, array $sets): array { - $name = (string) $input->getArgument('name'); - return array_filter($sets, function (string $set) use ($name): bool { return (bool) Strings::match($set, sprintf('#%s#', $name)); }); diff --git a/src/Rector/Class_/AddInterfaceByTraitRector.php b/src/Rector/Class_/AddInterfaceByTraitRector.php index cb0e7398082..655a6ec58f4 100644 --- a/src/Rector/Class_/AddInterfaceByTraitRector.php +++ b/src/Rector/Class_/AddInterfaceByTraitRector.php @@ -86,17 +86,17 @@ public function refactor(Node $node): ?Node $implementedInterfaceNames = $this->classManipulator->getImplementedInterfaceNames($node); foreach (array_keys($usedTraitNames) as $traitName) { - foreach ($this->interfaceByTrait as $seekedTraitName => $interfaceName) { - if ($traitName !== $seekedTraitName) { - continue; - } + if (! isset($this->interfaceByTrait[$traitName])) { + continue; + } - if (in_array($interfaceName, $implementedInterfaceNames, true)) { - continue; - } + $interfaceNameToAdd = $this->interfaceByTrait[$traitName]; - $node->implements[] = new FullyQualified($interfaceName); + if (in_array($interfaceNameToAdd, $implementedInterfaceNames, true)) { + continue; } + + $node->implements[] = new FullyQualified($interfaceNameToAdd); } return $node; diff --git a/src/Rector/Property/InjectAnnotationClassRector.php b/src/Rector/Property/InjectAnnotationClassRector.php index 0caca82debf..d47dd86db50 100644 --- a/src/Rector/Property/InjectAnnotationClassRector.php +++ b/src/Rector/Property/InjectAnnotationClassRector.php @@ -229,10 +229,12 @@ private function resolveJMSDIInjectType(Node $node, JMSInjectTagValueNode $jmsIn $serviceName = $jmsInjectTagValueNode->getServiceName(); if ($serviceName) { + // 1. service class if (class_exists($serviceName)) { return new ObjectType($serviceName); } + // 2. service name if ($serviceMap->hasService($serviceName)) { $serviceType = $serviceMap->getServiceType($serviceName); if ($serviceType !== null) { @@ -241,6 +243,7 @@ private function resolveJMSDIInjectType(Node $node, JMSInjectTagValueNode $jmsIn } } + // 3. service is in @var annotation /** @var PhpDocInfo $phpDocInfo */ $phpDocInfo = $node->getAttribute(AttributeKey::PHP_DOC_INFO); @@ -250,17 +253,24 @@ private function resolveJMSDIInjectType(Node $node, JMSInjectTagValueNode $jmsIn } // the @var is missing and service name was not found → report it - if ($serviceName) { - /** @var SmartFileInfo $fileInfo */ - $fileInfo = $node->getAttribute(AttributeKey::FILE_INFO); - - $this->errorAndDiffCollector->addErrorWithRectorClassMessageAndFileInfo( - self::class, - sprintf('Service "%s" was not found in DI Container of your Symfony App.', $serviceName), - $fileInfo - ); - } + $this->reportServiceNotFound($serviceName, $node); return new MixedType(); } + + private function reportServiceNotFound(?string $serviceName, Node $node): void + { + if ($serviceName !== null) { + return; + } + + /** @var SmartFileInfo $fileInfo */ + $fileInfo = $node->getAttribute(AttributeKey::FILE_INFO); + + $this->errorAndDiffCollector->addErrorWithRectorClassMessageAndFileInfo( + self::class, + sprintf('Service "%s" was not found in DI Container of your Symfony App.', $serviceName), + $fileInfo + ); + } } diff --git a/src/Rector/StaticCall/SwapClassMethodArgumentsRector.php b/src/Rector/StaticCall/SwapClassMethodArgumentsRector.php index 66a46e7db26..ac12fbebcd4 100644 --- a/src/Rector/StaticCall/SwapClassMethodArgumentsRector.php +++ b/src/Rector/StaticCall/SwapClassMethodArgumentsRector.php @@ -84,17 +84,7 @@ public function refactor(Node $node): ?Node continue; } - foreach ($methodNameAndNewArgumentPositions as $methodName => $newArgumentPositions) { - if (! $this->isMethodStaticCallOrClassMethodName($node, $methodName)) { - continue; - } - - if ($node instanceof ClassMethod) { - $this->swapParameters($node, $newArgumentPositions); - } else { - $this->swapArguments($node, $newArgumentPositions); - } - } + $this->refactorArgumentPositions($methodNameAndNewArgumentPositions, $node); } return $node; @@ -139,6 +129,24 @@ private function swapParameters(ClassMethod $classMethod, array $newParameterPos } } + /** + * @param StaticCall|MethodCall|ClassMethod $node + */ + private function refactorArgumentPositions(array $methodNameAndNewArgumentPositions, Node $node): void + { + foreach ($methodNameAndNewArgumentPositions as $methodName => $newArgumentPositions) { + if (! $this->isMethodStaticCallOrClassMethodName($node, $methodName)) { + continue; + } + + if ($node instanceof ClassMethod) { + $this->swapParameters($node, $newArgumentPositions); + } else { + $this->swapArguments($node, $newArgumentPositions); + } + } + } + /** * @param MethodCall|StaticCall $node * @param int[] $newArgumentPositions diff --git a/src/PhpParser/BetterNodeDumper.php b/src/Testing/Dumper/AttributeFilteringNodeDumper.php similarity index 66% rename from src/PhpParser/BetterNodeDumper.php rename to src/Testing/Dumper/AttributeFilteringNodeDumper.php index 62925284b90..d0a1e71e1f9 100644 --- a/src/PhpParser/BetterNodeDumper.php +++ b/src/Testing/Dumper/AttributeFilteringNodeDumper.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Rector\Core\PhpParser; +namespace Rector\Core\Testing\Dumper; use InvalidArgumentException; use PhpParser\Comment; @@ -13,18 +13,8 @@ use PhpParser\Node\Stmt\UseUse; use PhpParser\NodeDumper; -final class BetterNodeDumper extends NodeDumper +final class AttributeFilteringNodeDumper extends NodeDumper { - /** - * @var bool - */ - private $dumpComments = false; - - /** - * @var bool - */ - private $dumpPositions = false; - /** * @var int[] */ @@ -38,66 +28,44 @@ final class BetterNodeDumper extends NodeDumper /** * @var string[] */ - private $filterAttributes = []; + private $relevantAttributes = []; /** - * Constructs a NodeDumper. - * - * Supported options: - * * bool dumpComments: Whether comments should be dumped. - * * bool dumpPositions: Whether line/offset information should be dumped. To dump offset - * information, the code needs to be passed to dump(). - * - * @param array $options Options (see description) + * @param string[] $relevantAttributes */ - public function __construct(array $options = []) + public function __construct(array $relevantAttributes) { - $this->dumpComments = ! empty($options['dumpComments']); - $this->dumpPositions = ! empty($options['dumpPositions']); - parent::__construct($options); + $this->relevantAttributes = $relevantAttributes; } public function dump($node, ?string $code = null): string { $this->nodeIds = 0; $this->printedNodes = []; - return parent::dump($node, $code); - } - /** - * @param string[] $filterAttributes - */ - public function setFilterAttributes(array $filterAttributes): void - { - $this->filterAttributes = $filterAttributes; + return parent::dump($node, $code); } protected function dumpRecursive($node) { if ($node instanceof Node) { - $r = $this->dumpNode($node); + return $this->dumpNode($node); } elseif (is_array($node)) { - $r = $this->dumpArray($node); + return $this->dumpArray($node); } elseif ($node instanceof Comment) { return $node->getReformattedText(); - } else { - throw new InvalidArgumentException('Can only dump nodes and arrays.'); } - return $r; + throw new InvalidArgumentException('Can only dump nodes and arrays.'); } /** * @return mixed[] */ - private function getFilteredAttributes(Node $node) : array + private function getFilteredAttributes(Node $node): array { $attributes = $node->getAttributes(); - if ($this->filterAttributes !== []) { - $attributes = array_intersect_key($attributes, array_flip($this->filterAttributes)); - } - - return $attributes; + return array_intersect_key($attributes, array_flip($this->relevantAttributes)); } private function dumpSubNodes(Node $node): string @@ -134,7 +102,6 @@ private function dumpSubNodes(Node $node): string } } return $r; - } private function dumpNode(Node $node): string @@ -144,20 +111,14 @@ private function dumpNode(Node $node): string if (isset($this->printedNodes[$splObjectHash])) { return $node->getType() . ' #' . $this->printedNodes[$splObjectHash]; } + $this->printedNodes[$splObjectHash] = $this->nodeIds++; $r = $node->getType() . ' #' . $this->printedNodes[$splObjectHash]; - if ($this->dumpPositions ) { - $r .= $this->dumpPosition($node); - } $r .= '('; $r .= $this->dumpSubNodes($node); - $comments = $node->getComments(); - if ($this->dumpComments && $comments) { - $r .= "\n comments: " . str_replace("\n", "\n ", $this->dumpRecursive($comments)); - } $attributes = $this->getFilteredAttributes($node); if ($attributes !== []) { $r .= "\n attributes: " . str_replace("\n", "\n ", $this->dumpArray($attributes)); @@ -177,7 +138,7 @@ private function dumpArray(array $node): string if ($value === null) { $r .= 'null'; - } elseif (!$value) { + } elseif (! $value) { $r .= 'false'; } elseif ($value === true) { $r .= 'true'; diff --git a/src/Testing/Finder/RectorsFinder.php b/src/Testing/Finder/RectorsFinder.php index 8fbfde8f31e..a2fce9b3871 100644 --- a/src/Testing/Finder/RectorsFinder.php +++ b/src/Testing/Finder/RectorsFinder.php @@ -48,17 +48,10 @@ public function findInDirectory(string $directory): array */ public function findInDirectories(array $directories): array { - $robotLoader = new RobotLoader(); - foreach ($directories as $directory) { - $robotLoader->addDirectory($directory); - } - - $robotLoader->setTempDirectory(sys_get_temp_dir() . '/_rector_finder'); - $robotLoader->acceptFiles = ['*Rector.php']; - $robotLoader->rebuild(); + $foundClasses = $this->findClassesInDirectoriesByName($directories, '*Rector.php'); $rectors = []; - foreach (array_keys($robotLoader->getIndexedClasses()) as $class) { + foreach ($foundClasses as $class) { // special case, because robot loader is case insensitive if ($class === ExceptionCorrector::class) { continue; @@ -86,16 +79,43 @@ public function findInDirectories(array $directories): array $rectors[] = $rector; } + return $this->sortObjectsByShortClassName($rectors); + } + + /** + * @param string[] $directories + * @return string[] + */ + private function findClassesInDirectoriesByName(array $directories, string $name): array + { + $robotLoader = new RobotLoader(); + foreach ($directories as $directory) { + $robotLoader->addDirectory($directory); + } + + $robotLoader->setTempDirectory(sys_get_temp_dir() . '/_rector_finder'); + $robotLoader->acceptFiles = [$name]; + $robotLoader->rebuild(); + + return array_keys($robotLoader->getIndexedClasses()); + } + + /** + * @param object[] $objects + * @return object[] + */ + private function sortObjectsByShortClassName(array $objects): array + { usort( - $rectors, - function (RectorInterface $firstRector, RectorInterface $secondRector): int { - $firstRectorShortClass = Strings::after(get_class($firstRector), '\\', -1); - $secondRectorShortClass = Strings::after(get_class($secondRector), '\\', -1); + $objects, + function (object $firstObject, object $secondObject): int { + $firstRectorShortClass = Strings::after(get_class($firstObject), '\\', -1); + $secondRectorShortClass = Strings::after(get_class($secondObject), '\\', -1); return $firstRectorShortClass <=> $secondRectorShortClass; } ); - return $rectors; + return $objects; } } diff --git a/src/Testing/PHPUnit/AbstractNodeVisitorTestCase.php b/src/Testing/PHPUnit/AbstractNodeVisitorTestCase.php index 39ee4da9499..ff287c34954 100644 --- a/src/Testing/PHPUnit/AbstractNodeVisitorTestCase.php +++ b/src/Testing/PHPUnit/AbstractNodeVisitorTestCase.php @@ -7,10 +7,9 @@ use Iterator; use Nette\Utils\FileSystem; use PhpParser\Node; -use PhpParser\NodeDumper; use Rector\Core\HttpKernel\RectorKernel; -use Rector\Core\PhpParser\BetterNodeDumper; use Rector\Core\PhpParser\Parser\Parser; +use Rector\Core\Testing\Dumper\AttributeFilteringNodeDumper; use Rector\Core\Testing\StaticFixtureProvider; use Symplify\PackageBuilder\Tests\AbstractKernelTestCase; use Symplify\SmartFileSystem\SmartFileInfo; @@ -23,9 +22,9 @@ abstract class AbstractNodeVisitorTestCase extends AbstractKernelTestCase { /** - * @var NodeDumper + * @var AttributeFilteringNodeDumper */ - protected $nodeDumper; + protected $attributeFilteringNodeDumper; /** * @var Parser @@ -39,19 +38,18 @@ abstract class AbstractNodeVisitorTestCase extends AbstractKernelTestCase protected function setUp(): void { - parent::setUp(); $this->bootKernelWithConfigs(RectorKernel::class, []); + $this->fixtureSplitter = new FixtureSplitter($this->getTempPath()); $this->parser = static::$container->get(Parser::class); - $this->nodeDumper = new BetterNodeDumper(); - $this->nodeDumper->setFilterAttributes($this->getRelevantAttributes()); + $this->attributeFilteringNodeDumper = new AttributeFilteringNodeDumper($this->getRelevantAttributes()); } /** * @param Node[] $nodes */ - abstract protected function visitNodes(array $nodes): void; + abstract protected function traverseNodes(array $nodes): void; /** * @return string[] @@ -73,19 +71,19 @@ protected function doTestFile(string $file): void $smartFileInfo, true ); - $smartFileInfo2 = new SmartFileInfo($expectedNodesFile); $originalFileInfo = new SmartFileInfo($originalFile); - $nodes = $this->parser->parseFileInfo($originalFileInfo); + $expectedNodesFileInfo = new SmartFileInfo($expectedNodesFile); - $this->visitNodes($nodes); + $nodes = $this->parser->parseFileInfo($originalFileInfo); + $this->traverseNodes($nodes); - $dumpedNodes = $this->nodeDumper->dump($nodes); + $dumpedNodes = $this->attributeFilteringNodeDumper->dump($nodes); if (getenv('UPDATE_FIXTURE')) { FileSystem::write($file, FileSystem::read($originalFile) . "-----\n" . $dumpedNodes); } else { - $this->assertSame(trim($smartFileInfo2->getContents()), $dumpedNodes, 'Caused by ' . $file); + $this->assertSame(trim($expectedNodesFileInfo->getContents()), $dumpedNodes, 'Caused by ' . $file); } } diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 4fc6dc120e4..9a527597159 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -68,17 +68,10 @@ protected function setUp(): void $enabledRectorsProvider = static::$container->get(EnabledRectorsProvider::class); $enabledRectorsProvider->reset(); } else { - // repare contains with all rectors + // prepare container with all rectors // cache only rector tests - defined in phpunit.xml if (defined('RECTOR_REPOSITORY')) { - if (self::$allRectorContainer === null) { - $this->createContainerWithAllRectors(); - - self::$allRectorContainer = self::$container; - } else { - // load from cache - self::$container = self::$allRectorContainer; - } + $this->createRectorRepositoryContainer(); } else { // 3rd party $configFileTempPath = $this->getConfigFor3rdPartyTest(); @@ -204,10 +197,6 @@ private function createContainerWithAllRectors(): void private function getConfigFor3rdPartyTest(): string { - if ($this->provideConfig() !== '') { - return $this->provideConfig(); - } - $rectorClassWithConfiguration = $this->getCurrentTestRectorClassesWithConfiguration(); $yamlContent = Yaml::dump([ 'services' => $rectorClassWithConfiguration, @@ -265,4 +254,17 @@ private function createCausedByFixtureMessage(string $fixtureFile): string { return (new SmartFileInfo($fixtureFile))->getRelativeFilePathFromCwd(); } + + private function createRectorRepositoryContainer(): void + { + if (self::$allRectorContainer === null) { + $this->createContainerWithAllRectors(); + + self::$allRectorContainer = self::$container; + return; + } + + // load from cache + self::$container = self::$allRectorContainer; + } } diff --git a/src/ValueObject/PhpVersionFeature.php b/src/ValueObject/PhpVersionFeature.php index b4d0cb41de4..66bfefda373 100644 --- a/src/ValueObject/PhpVersionFeature.php +++ b/src/ValueObject/PhpVersionFeature.php @@ -36,6 +36,11 @@ final class PhpVersionFeature */ public const NULL_COALESCE = '7.0'; + /** + * @var string + */ + public const LIST_SWAP_ORDER = '7.0'; + /** * @var string */ diff --git a/utils/phpstan-extensions/tests/AbstractServiceAwareRuleTestCase.php b/utils/phpstan-extensions/tests/AbstractServiceAwareRuleTestCase.php deleted file mode 100644 index c3b6515e314..00000000000 --- a/utils/phpstan-extensions/tests/AbstractServiceAwareRuleTestCase.php +++ /dev/null @@ -1,35 +0,0 @@ -createContainer([$config]); - return $container->getByType($ruleClass); - } - - /** - * @param string[] $configs - */ - private function createContainer(array $configs): Container - { - $containerFactory = new ContainerFactory(getcwd()); - // random for tests cache invalidation in case the container changes - $tempDirectory = sys_get_temp_dir() . '/_phpstan_rector/id_' . random_int(0, 1000); - - return $containerFactory->create($tempDirectory, $configs, [], []); - } -} diff --git a/utils/phpstan-extensions/tests/Rule/SeeAnnotationToTestRule/SeeAnnotationToTestRuleTest.php b/utils/phpstan-extensions/tests/Rule/SeeAnnotationToTestRule/SeeAnnotationToTestRuleTest.php index 009bacb972a..9a8832f1e61 100644 --- a/utils/phpstan-extensions/tests/Rule/SeeAnnotationToTestRule/SeeAnnotationToTestRuleTest.php +++ b/utils/phpstan-extensions/tests/Rule/SeeAnnotationToTestRule/SeeAnnotationToTestRuleTest.php @@ -7,9 +7,9 @@ use Iterator; use PHPStan\Rules\Rule; use Rector\PHPStanExtensions\Rule\SeeAnnotationToTestRule; -use Rector\PHPStanExtensions\Tests\AbstractServiceAwareRuleTestCase; use Rector\PHPStanExtensions\Tests\Rule\SeeAnnotationToTestRule\Fixture\ClassMissingDocBlockRector; use Rector\PHPStanExtensions\Tests\Rule\SeeAnnotationToTestRule\Fixture\ClassMissingSeeAnnotationRector; +use Symplify\PHPStanExtensions\Testing\AbstractServiceAwareRuleTestCase; final class SeeAnnotationToTestRuleTest extends AbstractServiceAwareRuleTestCase { From 1b235fc6c1e91ccfa3281f5fca6f0346bf1fc0d3 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sun, 3 May 2020 21:17:42 +0200 Subject: [PATCH 2/3] use unique error class name --- .../src/Application/ErrorAndDiffCollector.php | 12 ++++++------ .../src/Output/ConsoleOutputFormatter.php | 4 ++-- .../Application/{Error.php => RectorError.php} | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) rename src/ValueObject/Application/{Error.php => RectorError.php} (97%) diff --git a/packages/changes-reporting/src/Application/ErrorAndDiffCollector.php b/packages/changes-reporting/src/Application/ErrorAndDiffCollector.php index ea91117ccb6..83e6d876eb2 100644 --- a/packages/changes-reporting/src/Application/ErrorAndDiffCollector.php +++ b/packages/changes-reporting/src/Application/ErrorAndDiffCollector.php @@ -11,7 +11,7 @@ use Rector\ConsoleDiffer\DifferAndFormatter; use Rector\Core\Application\FileSystem\RemovedAndAddedFilesCollector; use Rector\Core\Error\ExceptionCorrector; -use Rector\Core\ValueObject\Application\Error; +use Rector\Core\ValueObject\Application\RectorError; use Rector\Core\ValueObject\Reporting\FileDiff; use Rector\PostRector\Collector\NodesToRemoveCollector; use Symplify\SmartFileSystem\SmartFileInfo; @@ -20,7 +20,7 @@ final class ErrorAndDiffCollector { /** - * @var Error[] + * @var RectorError[] */ private $errors = []; @@ -69,7 +69,7 @@ public function __construct( } /** - * @return Error[] + * @return RectorError[] */ public function getErrors(): array { @@ -141,7 +141,7 @@ public function addAutoloadError(AnalysedCodeException $analysedCodeException, S { $message = $this->exceptionCorrector->getAutoloadExceptionMessageAndAddLocation($analysedCodeException); - $this->errors[] = new Error($fileInfo, $message); + $this->errors[] = new RectorError($fileInfo, $message); } public function addErrorWithRectorClassMessageAndFileInfo( @@ -149,7 +149,7 @@ public function addErrorWithRectorClassMessageAndFileInfo( string $message, SmartFileInfo $smartFileInfo ): void { - $this->errors[] = new Error($smartFileInfo, $message, null, $rectorClass); + $this->errors[] = new RectorError($smartFileInfo, $message, null, $rectorClass); } public function addThrowableWithFileInfo(Throwable $throwable, SmartFileInfo $fileInfo): void @@ -158,7 +158,7 @@ public function addThrowableWithFileInfo(Throwable $throwable, SmartFileInfo $fi if ($rectorClass) { $this->addErrorWithRectorClassMessageAndFileInfo($rectorClass, $throwable->getMessage(), $fileInfo); } else { - $this->errors[] = new Error($fileInfo, $throwable->getMessage(), $throwable->getCode()); + $this->errors[] = new RectorError($fileInfo, $throwable->getMessage(), $throwable->getCode()); } } } diff --git a/packages/changes-reporting/src/Output/ConsoleOutputFormatter.php b/packages/changes-reporting/src/Output/ConsoleOutputFormatter.php index eb15aa23908..4ddcca11faa 100644 --- a/packages/changes-reporting/src/Output/ConsoleOutputFormatter.php +++ b/packages/changes-reporting/src/Output/ConsoleOutputFormatter.php @@ -10,7 +10,7 @@ use Rector\Core\Configuration\Configuration; use Rector\Core\Configuration\Option; use Rector\Core\PhpParser\Printer\BetterStandardPrinter; -use Rector\Core\ValueObject\Application\Error; +use Rector\Core\ValueObject\Application\RectorError; use Rector\Core\ValueObject\Reporting\FileDiff; use Rector\NodeTypeResolver\Node\AttributeKey; use Symfony\Component\Console\Style\SymfonyStyle; @@ -122,7 +122,7 @@ private function reportFileDiffs(array $fileDiffs): void } /** - * @param Error[] $errors + * @param RectorError[] $errors */ private function reportErrors(array $errors): void { diff --git a/src/ValueObject/Application/Error.php b/src/ValueObject/Application/RectorError.php similarity index 97% rename from src/ValueObject/Application/Error.php rename to src/ValueObject/Application/RectorError.php index 6c0ebf28550..b20c307c4fa 100644 --- a/src/ValueObject/Application/Error.php +++ b/src/ValueObject/Application/RectorError.php @@ -6,7 +6,7 @@ use Symplify\SmartFileSystem\SmartFileInfo; -final class Error +final class RectorError { /** * @var int|null From e9f3945838537df03c32ebe194b09e13f3fb3d50 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sun, 3 May 2020 21:21:07 +0200 Subject: [PATCH 3/3] composer: bump symplify deps --- composer.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.json b/composer.json index 1ad4ae0e70f..a49ad1915e4 100644 --- a/composer.json +++ b/composer.json @@ -31,10 +31,10 @@ "symfony/dependency-injection": "^4.4.6|^5.0.6", "symfony/finder": "^4.4.6|^5.0.6", "symfony/process": "^4.4.6|^5.0.6", - "symplify/auto-bind-parameter": "^7.2.4", - "symplify/autowire-array-parameter": "^7.2.4", - "symplify/package-builder": "^7.2.4", - "symplify/set-config-resolver": "^7.2.4", + "symplify/auto-bind-parameter": "^7.3.5", + "symplify/autowire-array-parameter": "^7.3.5", + "symplify/package-builder": "^7.3.5", + "symplify/set-config-resolver": "^7.3.5", "tracy/tracy": "^2.7" }, "require-dev": { @@ -42,10 +42,10 @@ "ocramius/package-versions": "^1.4|^1.5", "phpunit/phpunit": "^8.4|^9.0", "slam/phpstan-extensions": "^5.0", - "symplify/changelog-linker": "^7.2.4", - "symplify/easy-coding-standard": "^7.2.4", - "symplify/monorepo-builder": "^7.2.4", - "symplify/phpstan-extensions": "^7.2.4", + "symplify/changelog-linker": "^7.3.5", + "symplify/easy-coding-standard": "^7.3.5", + "symplify/monorepo-builder": "^7.3.5", + "symplify/phpstan-extensions": "^7.3.5", "thecodingmachine/phpstan-strict-rules": "^0.12" }, "replace": {