From d1b47798899d8a4cc2ea409d2082718eeea36619 Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Sat, 15 Feb 2020 00:21:14 +0100 Subject: [PATCH] apply properties to constants rule from SOLID --- .../src/NodeCollector/ParsedNodeCollector.php | 6 +++--- phpstan.neon | 1 + rector-ci.yaml | 3 +++ ...moveAlwaysTrueConditionSetInConstructorRector.php | 8 +++++++- .../FuncCall/ConsistentPregDelimiterRector.php | 4 +++- src/Php/Regex/RegexPatternArgumentManipulator.php | 8 ++++---- src/PhpParser/BetterNodeDumper.php | 7 +------ src/PhpParser/Node/AssignAndBinaryMap.php | 12 ++++++------ .../Node/Manipulator/IdentifierManipulator.php | 6 +++--- .../Node/Manipulator/VisibilityManipulator.php | 6 +++--- src/Rector/Property/InjectAnnotationClassRector.php | 8 ++++---- 11 files changed, 38 insertions(+), 31 deletions(-) diff --git a/packages/node-collector/src/NodeCollector/ParsedNodeCollector.php b/packages/node-collector/src/NodeCollector/ParsedNodeCollector.php index b28cc50ff23..4eadf74749b 100644 --- a/packages/node-collector/src/NodeCollector/ParsedNodeCollector.php +++ b/packages/node-collector/src/NodeCollector/ParsedNodeCollector.php @@ -29,9 +29,9 @@ use Rector\NodeTypeResolver\Node\AttributeKey; final class ParsedNodeCollector { /** - * @var string[] + * @var class-string[] */ - private $collectableNodeTypes = [ + private const COLLECTABLE_NODE_TYPES = [ Class_::class, Interface_::class, ClassConst::class, @@ -163,7 +163,7 @@ final class ParsedNodeCollector public function isCollectableNode(Node $node): bool { - foreach ($this->collectableNodeTypes as $collectableNodeType) { + foreach (self::COLLECTABLE_NODE_TYPES as $collectableNodeType) { if (is_a($node, $collectableNodeType, true)) { return true; } diff --git a/phpstan.neon b/phpstan.neon index 80cb3da6ccc..864d290238c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -239,3 +239,4 @@ parameters: - '#Parameter \#1 \$possibleSubtype of method Rector\\TypeDeclaration\\PhpParserTypeAnalyzer\:\:isSubtypeOf\(\) expects PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\|PhpParser\\Node\\UnionType, PhpParser\\Node given#' - '#Parameter \#2 \$inferredReturnNode of method Rector\\TypeDeclaration\\Rector\\FunctionLike\\ReturnTypeDeclarationRector\:\:addReturnType\(\) expects PhpParser\\Node, PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\|PhpParser\\Node\\UnionType\|null given#' - '#Method Rector\\FileSystemRector\\Rector\\AbstractFileSystemRector\:\:wrapToArg\(\) should return array but returns array#' + - '#Strict comparison using \=\=\= between mixed and null will always evaluate to false#' diff --git a/rector-ci.yaml b/rector-ci.yaml index 57c537c26c2..0dd101b39b2 100644 --- a/rector-ci.yaml +++ b/rector-ci.yaml @@ -1,3 +1,6 @@ +services: + Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector: null + parameters: sets: - 'coding-style' diff --git a/rules/code-quality/src/Rector/If_/RemoveAlwaysTrueConditionSetInConstructorRector.php b/rules/code-quality/src/Rector/If_/RemoveAlwaysTrueConditionSetInConstructorRector.php index 08dbc068784..01368b42a9d 100644 --- a/rules/code-quality/src/Rector/If_/RemoveAlwaysTrueConditionSetInConstructorRector.php +++ b/rules/code-quality/src/Rector/If_/RemoveAlwaysTrueConditionSetInConstructorRector.php @@ -118,6 +118,7 @@ PHP return null; } + $haveNodeChanged = false; foreach ((array) $node->stmts as $key => $stmt) { if ($stmt instanceof Expression) { $stmt = $stmt->expr; @@ -133,11 +134,16 @@ PHP continue; } + $haveNodeChanged = true; // move all nodes one level up array_splice($node->stmts, $key, count($stmt->stmts) - 1, $stmt->stmts); } - return $node; + if ($haveNodeChanged) { + return $node; + } + + return null; } private function isAlwaysTruableNode(Node $node): bool diff --git a/rules/coding-style/src/Rector/FuncCall/ConsistentPregDelimiterRector.php b/rules/coding-style/src/Rector/FuncCall/ConsistentPregDelimiterRector.php index 7a8c2ffb214..ebbec273ea2 100644 --- a/rules/coding-style/src/Rector/FuncCall/ConsistentPregDelimiterRector.php +++ b/rules/coding-style/src/Rector/FuncCall/ConsistentPregDelimiterRector.php @@ -120,10 +120,12 @@ PHP } $this->refactorArgument($node->args[$position]); + + return $node; } } - return $node; + return null; } private function refactorFuncCall(FuncCall $funcCall): FuncCall diff --git a/src/Php/Regex/RegexPatternArgumentManipulator.php b/src/Php/Regex/RegexPatternArgumentManipulator.php index 7126ce3dfc6..4b64ed78d16 100644 --- a/src/Php/Regex/RegexPatternArgumentManipulator.php +++ b/src/Php/Regex/RegexPatternArgumentManipulator.php @@ -25,7 +25,7 @@ final class RegexPatternArgumentManipulator /** * @var int[] */ - private $functionsWithPatternsToArgumentPosition = [ + private const FUNCTIONS_WITH_PATTERNS_TO_ARGUMENT_POSITION = [ 'preg_match' => 0, 'preg_replace_callback_array' => 0, 'preg_replace_callback' => 0, @@ -38,7 +38,7 @@ final class RegexPatternArgumentManipulator /** * @var int[][] */ - private $staticMethodsWithPatternsToArgumentPosition = [ + private const STATIC_METHODS_WITH_PATTERNS_TO_ARGUMENT_POSITION = [ Strings::class => [ 'match' => 1, 'matchAll' => 1, @@ -107,7 +107,7 @@ final class RegexPatternArgumentManipulator */ private function processFuncCall(FuncCall $funcCall): array { - foreach ($this->functionsWithPatternsToArgumentPosition as $functionName => $argumentPosition) { + foreach (self::FUNCTIONS_WITH_PATTERNS_TO_ARGUMENT_POSITION as $functionName => $argumentPosition) { if (! $this->nodeNameResolver->isName($funcCall, $functionName)) { continue; } @@ -127,7 +127,7 @@ final class RegexPatternArgumentManipulator */ private function processStaticCall(StaticCall $staticCall): array { - foreach ($this->staticMethodsWithPatternsToArgumentPosition as $type => $methodNamesToArgumentPosition) { + foreach (self::STATIC_METHODS_WITH_PATTERNS_TO_ARGUMENT_POSITION as $type => $methodNamesToArgumentPosition) { if (! $this->nodeTypeResolver->isObjectType($staticCall->class, $type)) { continue; } diff --git a/src/PhpParser/BetterNodeDumper.php b/src/PhpParser/BetterNodeDumper.php index 7eb3b280bda..d7d2a367b73 100644 --- a/src/PhpParser/BetterNodeDumper.php +++ b/src/PhpParser/BetterNodeDumper.php @@ -35,11 +35,6 @@ final class BetterNodeDumper extends NodeDumper */ private $nodeIds = 0; - /** - * @var bool - */ - private $skipEmpty = true; - /** * @var string[] */ @@ -127,7 +122,7 @@ final class BetterNodeDumper extends NodeDumper foreach ($node->getSubNodeNames() as $key) { $value = $node->{$key}; - if ($this->skipEmpty && ($value === null || $value === [])) { + if (($value === null || $value === [])) { continue; } diff --git a/src/PhpParser/Node/AssignAndBinaryMap.php b/src/PhpParser/Node/AssignAndBinaryMap.php index f053ca91894..56b97814eaf 100644 --- a/src/PhpParser/Node/AssignAndBinaryMap.php +++ b/src/PhpParser/Node/AssignAndBinaryMap.php @@ -45,7 +45,7 @@ final class AssignAndBinaryMap /** * @var string[] */ - private $binaryOpToInverseClasses = [ + private const BINARY_OP_TO_INVERSE_CLASSES = [ Identical::class => NotIdentical::class, NotIdentical::class => Identical::class, Equal::class => NotEqual::class, @@ -57,9 +57,9 @@ final class AssignAndBinaryMap ]; /** - * @var string[] + * @var class-string[] */ - private $assignOpToBinaryOpClasses = [ + private const ASSIGN_OP_TO_BINARY_OP_CLASSES = [ AssignBitwiseOr::class => BitwiseOr::class, AssignBitwiseAnd::class => BitwiseAnd::class, AssignBitwiseXor::class => BitwiseXor::class, @@ -81,7 +81,7 @@ final class AssignAndBinaryMap public function __construct() { - $this->binaryOpToAssignClasses = array_flip($this->assignOpToBinaryOpClasses); + $this->binaryOpToAssignClasses = array_flip(self::ASSIGN_OP_TO_BINARY_OP_CLASSES); } public function getAlternative(Node $node): ?string @@ -89,7 +89,7 @@ final class AssignAndBinaryMap $nodeClass = get_class($node); if ($node instanceof AssignOp) { - return $this->assignOpToBinaryOpClasses[$nodeClass] ?? null; + return self::ASSIGN_OP_TO_BINARY_OP_CLASSES[$nodeClass] ?? null; } if ($node instanceof BinaryOp) { @@ -103,6 +103,6 @@ final class AssignAndBinaryMap { $nodeClass = get_class($binaryOp); - return $this->binaryOpToInverseClasses[$nodeClass] ?? null; + return self::BINARY_OP_TO_INVERSE_CLASSES[$nodeClass] ?? null; } } diff --git a/src/PhpParser/Node/Manipulator/IdentifierManipulator.php b/src/PhpParser/Node/Manipulator/IdentifierManipulator.php index 5197ce1d9f5..350a80655b6 100644 --- a/src/PhpParser/Node/Manipulator/IdentifierManipulator.php +++ b/src/PhpParser/Node/Manipulator/IdentifierManipulator.php @@ -26,7 +26,7 @@ final class IdentifierManipulator /** * @var string[] */ - private $nodeClassesWithIdentifier = [ + private const NODE_CLASSES_WITH_IDENTIFIER = [ ClassConstFetch::class, MethodCall::class, PropertyFetch::class, StaticCall::class, ClassMethod::class, ]; @@ -74,7 +74,7 @@ final class IdentifierManipulator private function ensureNodeHasIdentifier(Node $node): void { - if (in_array(get_class($node), $this->nodeClassesWithIdentifier, true)) { + if (in_array(get_class($node), self::NODE_CLASSES_WITH_IDENTIFIER, true)) { return; } @@ -82,7 +82,7 @@ final class IdentifierManipulator 'Node "%s" does not contain a "$name" property with "%s". Pass only one of "%s".', get_class($node), Identifier::class, - implode('", "', $this->nodeClassesWithIdentifier) + implode('", "', self::NODE_CLASSES_WITH_IDENTIFIER) )); } } diff --git a/src/PhpParser/Node/Manipulator/VisibilityManipulator.php b/src/PhpParser/Node/Manipulator/VisibilityManipulator.php index 3f32de75142..858e88746af 100644 --- a/src/PhpParser/Node/Manipulator/VisibilityManipulator.php +++ b/src/PhpParser/Node/Manipulator/VisibilityManipulator.php @@ -16,7 +16,7 @@ final class VisibilityManipulator /** * @var string[] */ - private $allowedNodeTypes = [ClassMethod::class, Property::class, ClassConst::class, Class_::class]; + private const ALLOWED_NODE_TYPES = [ClassMethod::class, Property::class, ClassConst::class, Class_::class]; /** * @param ClassMethod|Property|ClassConst $node @@ -117,7 +117,7 @@ final class VisibilityManipulator private function ensureIsClassMethodOrProperty(Node $node, string $location): void { - foreach ($this->allowedNodeTypes as $allowedNodeType) { + foreach (self::ALLOWED_NODE_TYPES as $allowedNodeType) { if (is_a($node, $allowedNodeType, true)) { return; } @@ -126,7 +126,7 @@ final class VisibilityManipulator throw new InvalidNodeTypeException(sprintf( '"%s" only accepts "%s" types. "%s" given.', $location, - implode('", "', $this->allowedNodeTypes), + implode('", "', self::ALLOWED_NODE_TYPES), get_class($node) )); } diff --git a/src/Rector/Property/InjectAnnotationClassRector.php b/src/Rector/Property/InjectAnnotationClassRector.php index 840a3e30c3f..63b1c4707ca 100644 --- a/src/Rector/Property/InjectAnnotationClassRector.php +++ b/src/Rector/Property/InjectAnnotationClassRector.php @@ -37,7 +37,7 @@ final class InjectAnnotationClassRector extends AbstractRector /** * @var string[] */ - private $annotationToTagClass = [ + private const ANNOTATION_TO_TAG_CLASS = [ PHPDIInject::class => PHPDIInjectTagValueNode::class, JMSInject::class => JMSInjectTagValueNode::class, ]; @@ -134,7 +134,7 @@ PHP foreach ($this->annotationClasses as $annotationClass) { $this->ensureAnnotationClassIsSupported($annotationClass); - $tagClass = $this->annotationToTagClass[$annotationClass]; + $tagClass = self::ANNOTATION_TO_TAG_CLASS[$annotationClass]; $injectTagValueNode = $phpDocInfo->getByType($tagClass); if ($injectTagValueNode === null) { @@ -155,14 +155,14 @@ PHP private function ensureAnnotationClassIsSupported(string $annotationClass): void { - if (isset($this->annotationToTagClass[$annotationClass])) { + if (isset(self::ANNOTATION_TO_TAG_CLASS[$annotationClass])) { return; } throw new NotImplementedException(sprintf( 'Annotation class "%s" is not implemented yet. Use one of "%s" or add custom tag for it to Rector.', $annotationClass, - implode('", "', array_keys($this->annotationToTagClass)) + implode('", "', array_keys(self::ANNOTATION_TO_TAG_CLASS)) )); }