From 00a3fb3a177f750d185f21fad59e0cc36055fa5b Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Fri, 14 Feb 2020 16:15:29 +0100 Subject: [PATCH] [SOLID] Add ChangeReadOnlyPropertyWithDefaultValueToConstantRector --- config/set/solid/solid.yaml | 1 + docs/AllRectorsOverview.md | 32 +++- ...opertyWithDefaultValueToConstantRector.php | 176 ++++++++++++++++++ ...tyWithDefaultValueToConstantRectorTest.php | 30 +++ .../Fixture/fixture.php.inc | 47 +++++ .../Fixture/skip_multiple_properties.php.inc | 14 ++ .../Fixture/skip_overriden.php.inc | 23 +++ .../Node/Manipulator/PropertyManipulator.php | 12 +- 8 files changed, 333 insertions(+), 2 deletions(-) create mode 100644 rules/solid/src/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector.php create mode 100644 rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/ChangeReadOnlyPropertyWithDefaultValueToConstantRectorTest.php create mode 100644 rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/fixture.php.inc create mode 100644 rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/skip_multiple_properties.php.inc create mode 100644 rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/skip_overriden.php.inc diff --git a/config/set/solid/solid.yaml b/config/set/solid/solid.yaml index 47f6ab025e5..3408396ab57 100644 --- a/config/set/solid/solid.yaml +++ b/config/set/solid/solid.yaml @@ -3,3 +3,4 @@ services: Rector\SOLID\Rector\ClassConst\PrivatizeLocalClassConstantRector: null Rector\SOLID\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector: null + Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector: null diff --git a/docs/AllRectorsOverview.md b/docs/AllRectorsOverview.md index 1d1b8459562..3a4cd468f7a 100644 --- a/docs/AllRectorsOverview.md +++ b/docs/AllRectorsOverview.md @@ -1,4 +1,4 @@ -# All 450 Rectors Overview +# All 451 Rectors Overview - [Projects](#projects) - [General](#general) @@ -7662,6 +7662,36 @@ Change nested ifs to early return
+### `ChangeReadOnlyPropertyWithDefaultValueToConstantRector` + +- class: `Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector` + +Change property with read only status with default value to constant + +```diff + class SomeClass + { + /** + * @var string[] + */ +- private $magicMethods = [ ++ private const MAGIC_METHODS = [ + '__toString', + '__wakeup', + ]; + + public function run() + { +- foreach ($this->magicMethods as $magicMethod) { ++ foreach (self::MAGIC_METHODS as $magicMethod) { + echo $magicMethod; + } + } + } +``` + +
+ ### `FinalizeClassesWithoutChildrenRector` - class: `Rector\SOLID\Rector\Class_\FinalizeClassesWithoutChildrenRector` diff --git a/rules/solid/src/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector.php b/rules/solid/src/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector.php new file mode 100644 index 00000000000..04bcd905604 --- /dev/null +++ b/rules/solid/src/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector.php @@ -0,0 +1,176 @@ +propertyManipulator = $propertyManipulator; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Change property with read only status with default value to constant', [ + new CodeSample( + <<<'PHP' +class SomeClass +{ + /** + * @var string[] + */ + private $magicMethods = [ + '__toString', + '__wakeup', + ]; + + public function run() + { + foreach ($this->magicMethods as $magicMethod) { + echo $magicMethod; + } + } +} +PHP +, + <<<'PHP' +class SomeClass +{ + /** + * @var string[] + */ + private const MAGIC_METHODS = [ + '__toString', + '__wakeup', + ]; + + public function run() + { + foreach (self::MAGIC_METHODS as $magicMethod) { + echo $magicMethod; + } + } +} +PHP + + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Property::class]; + } + + /** + * @param Property $node + */ + public function refactor(Node $node): ?Node + { + if (count($node->props) !== 1) { + return null; + } + + /** @var PropertyProperty $onlyProperty */ + $onlyProperty = $node->props[0]; + + // we need default value + if ($onlyProperty->default === null) { + return null; + } + + if (! $node->isPrivate()) { + return null; + } + + // is property read only? + if (! $this->propertyManipulator->isReadyOnlyProperty($onlyProperty)) { + return null; + } + + $this->replacePropertyFetchWithClassConstFetch($node, $onlyProperty); + + return $this->createClassConst($node, $onlyProperty); + } + + private function createClassConst(Property $property, PropertyProperty $propertyProperty): ClassConst + { + $constantName = $this->createConstantNameFromProperty($propertyProperty); + + /** @var Expr $defaultValue */ + $defaultValue = $propertyProperty->default; + $constant = new ConstConst($constantName, $defaultValue); + + $classConst = new ClassConst([$constant]); + $classConst->flags = $property->flags; + $classConst->setAttribute(AttributeKey::PHP_DOC_INFO, $property->getAttribute(AttributeKey::PHP_DOC_INFO)); + + return $classConst; + } + + private function createConstantNameFromProperty(PropertyProperty $propertyProperty): string + { + $propertyName = $this->getName($propertyProperty); + $constantName = RectorStrings::camelCaseToUnderscore($propertyName); + + return strtoupper($constantName); + } + + private function replacePropertyFetchWithClassConstFetch(Node $node, PropertyProperty $propertyProperty): void + { + $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); + if ($classNode === null) { + throw new ShouldNotHappenException(); + } + + $propertyName = $this->getName($propertyProperty); + $constantName = $this->createConstantNameFromProperty($propertyProperty); + + $this->traverseNodesWithCallable($classNode, function (Node $node) use ($propertyName, $constantName) { + if (! $node instanceof PropertyFetch) { + return null; + } + + if (! $this->isName($node->var, 'this')) { + return null; + } + + if (! $this->isName($node->name, $propertyName)) { + return null; + } + + // replace with constant fetch + return new ClassConstFetch(new Name('self'), $constantName); + }); + } +} diff --git a/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/ChangeReadOnlyPropertyWithDefaultValueToConstantRectorTest.php b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/ChangeReadOnlyPropertyWithDefaultValueToConstantRectorTest.php new file mode 100644 index 00000000000..d3ba1b6d0fc --- /dev/null +++ b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/ChangeReadOnlyPropertyWithDefaultValueToConstantRectorTest.php @@ -0,0 +1,30 @@ +doTestFile($file); + } + + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + protected function getRectorClass(): string + { + return ChangeReadOnlyPropertyWithDefaultValueToConstantRector::class; + } +} diff --git a/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/fixture.php.inc b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/fixture.php.inc new file mode 100644 index 00000000000..5cb8273e41c --- /dev/null +++ b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/fixture.php.inc @@ -0,0 +1,47 @@ +magicMethods as $magicMethod) { + echo $magicMethod; + } + } +} + +?> +----- + diff --git a/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/skip_multiple_properties.php.inc b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/skip_multiple_properties.php.inc new file mode 100644 index 00000000000..3fc2e2ce1eb --- /dev/null +++ b/rules/solid/tests/Rector/Property/ChangeReadOnlyPropertyWithDefaultValueToConstantRector/Fixture/skip_multiple_properties.php.inc @@ -0,0 +1,14 @@ +magicMethods as $magicMethod) { + echo $magicMethod; + } + + $this->magicMethods = 123; + } +} diff --git a/src/PhpParser/Node/Manipulator/PropertyManipulator.php b/src/PhpParser/Node/Manipulator/PropertyManipulator.php index d03bbfc5caa..f36964c14e5 100644 --- a/src/PhpParser/Node/Manipulator/PropertyManipulator.php +++ b/src/PhpParser/Node/Manipulator/PropertyManipulator.php @@ -107,10 +107,20 @@ final class PropertyManipulator return $propertyFetches; } + public function isReadyOnlyProperty(PropertyProperty $propertyProperty): bool + { + foreach ($this->getAllPropertyFetch($propertyProperty) as $propertyFetch) { + if (! $this->isReadContext($propertyFetch)) { + return false; + } + } + + return true; + } + public function isPropertyUsedInReadContext(PropertyProperty $propertyProperty): bool { $property = $this->getProperty($propertyProperty); - if ($this->isDoctrineProperty($property)) { return true; }