From d075120c22a64f0e67917db3b19c3be6ff673f83 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 29 Jan 2022 23:48:00 +0700 Subject: [PATCH 1/5] Add support for removal readonly property on RemoveUnusedPromotedPropertyRector --- .../Fixture/some_property_readonly.php.inc | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc new file mode 100644 index 00000000000..6941c22e10c --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc @@ -0,0 +1,29 @@ + +----- + From d1875fc87e97559540fe414fddb89ee597b4f31a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 29 Jan 2022 23:54:12 +0700 Subject: [PATCH 2/5] add fixture for skip public readonly --- .../skip_public_some_property_readonly.php.inc | 14 ++++++++++++++ ...p.inc => some_private_roperty_readonly.php.inc} | 4 ++-- .../RemoveUnusedPromotedPropertyRector.php | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc rename rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/{some_property_readonly.php.inc => some_private_roperty_readonly.php.inc} (85%) diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc new file mode 100644 index 00000000000..cda5133c19e --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc @@ -0,0 +1,14 @@ + \ No newline at end of file diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_private_roperty_readonly.php.inc similarity index 85% rename from rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc rename to rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_private_roperty_readonly.php.inc index 6941c22e10c..68c76c55e97 100644 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_property_readonly.php.inc +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/some_private_roperty_readonly.php.inc @@ -4,7 +4,7 @@ namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyR use stdClass; -class SomePropertyReadonly +class SomePrivatePropertyReadonly { public function __construct(private readonly stdClass $someUnusedDependency) { @@ -19,7 +19,7 @@ namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyR use stdClass; -class SomePropertyReadonly +class SomePrivatePropertyReadonly { public function __construct() { diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php index 538d6c8c13d..1fbc330fad4 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php @@ -24,7 +24,7 @@ final class RemoveUnusedPromotedPropertyRector extends AbstractRector implements { public function __construct( private readonly PropertyFetchFinder $propertyFetchFinder, - private readonly PropertyManipulator $propertyManipulator, + private readonly PropertyManipulator $propertyManipulator ) { } From 322439bb2025bc8c7d9ae068c96591dfb910c289 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 30 Jan 2022 00:18:50 +0700 Subject: [PATCH 3/5] implemented :tada --- .../RemoveUnusedPromotedPropertyRector.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php index 1fbc330fad4..480c4d25e08 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php @@ -13,6 +13,8 @@ use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\MethodName; use Rector\Core\ValueObject\PhpVersionFeature; +use Rector\Core\ValueObject\Visibility; +use Rector\Privatization\NodeManipulator\VisibilityManipulator; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -24,7 +26,8 @@ final class RemoveUnusedPromotedPropertyRector extends AbstractRector implements { public function __construct( private readonly PropertyFetchFinder $propertyFetchFinder, - private readonly PropertyManipulator $propertyManipulator + private readonly PropertyManipulator $propertyManipulator, + private readonly VisibilityManipulator $visibilityManipulator ) { } @@ -94,7 +97,13 @@ public function refactor(Node $node): ?Node foreach ($node->getParams() as $param) { // only private local scope; removing public property might be dangerous if ($param->flags !== Class_::MODIFIER_PRIVATE) { - continue; + if (! $this->visibilityManipulator->isReadonly($param)) { + continue; + } + + if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) { + continue; + } } if ($this->propertyManipulator->isPropertyUsedInReadContext($param)) { From 7d24fc6817ed070ad8e283ca26d07516ad9dc9c7 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 30 Jan 2022 00:20:02 +0700 Subject: [PATCH 4/5] final touch: eol --- .../Fixture/skip_public_some_property_readonly.php.inc | 2 -- 1 file changed, 2 deletions(-) diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc index cda5133c19e..cc17bc37988 100644 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector/Fixture/skip_public_some_property_readonly.php.inc @@ -10,5 +10,3 @@ class SkipPublicSomePropertyReadonly { } } - -?> \ No newline at end of file From 3424e5c78f456dff77b1f11e5c22b03352739a43 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 30 Jan 2022 00:21:41 +0700 Subject: [PATCH 5/5] final touch: utilize VisibilityManipulator service is working --- .../ClassMethod/RemoveUnusedPromotedPropertyRector.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php index 480c4d25e08..014c88cd8ae 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPromotedPropertyRector.php @@ -96,14 +96,8 @@ public function refactor(Node $node): ?Node foreach ($node->getParams() as $param) { // only private local scope; removing public property might be dangerous - if ($param->flags !== Class_::MODIFIER_PRIVATE) { - if (! $this->visibilityManipulator->isReadonly($param)) { - continue; - } - - if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) { - continue; - } + if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) { + continue; } if ($this->propertyManipulator->isPropertyUsedInReadContext($param)) {