From d1ebdd570e9787ef3b68e58c467b8a0329ae644a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 10 Aug 2026 00:16:18 +0200 Subject: [PATCH] [deprecation] Deprecate PropertyHookRector, as property hooks are a matter of preference --- config/set/php84.php | 3 - phpstan.neon | 1 + .../Fixture/skip_different_variable.php.inc | 20 --- .../Fixture/skip_get_set_magic.php.inc | 26 ---- .../Fixture/skip_getter_and_readonly.php.inc | 18 --- .../skip_method_with_attributes.php.inc | 22 ---- ...ip_mismatching_getter_setter_names.php.inc | 18 --- .../Fixture/skip_multi_stmts_getter.php.inc | 15 --- ...n_final_class_to_avoid_child_break.php.inc | 18 --- .../Fixture/skip_parent_contract.php.inc | 15 --- .../Fixture/skip_readonly_class.php.inc | 18 --- .../skip_readonly_getter_and_setter.php.inc | 18 --- .../skip_readonly_promoted_property.php.inc | 17 --- .../Fixture/some_fixture.php.inc | 36 ------ .../PropertyHookRectorTest.php | 28 ----- .../Source/SomeParentContractInterface.php | 8 -- .../config/configured_rule.php | 13 -- .../Php84/NodeFactory/PropertyHookFactory.php | 43 ------- .../NodeFinder/SetterAndGetterFinder.php | 68 ----------- .../Rector/Class_/PropertyHookRector.php | 114 ++---------------- 20 files changed, 9 insertions(+), 510 deletions(-) delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_different_variable.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_get_set_magic.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_getter_and_readonly.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_method_with_attributes.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_mismatching_getter_setter_names.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_multi_stmts_getter.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_non_final_class_to_avoid_child_break.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_parent_contract.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_class.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_getter_and_setter.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_promoted_property.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/some_fixture.php.inc delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/PropertyHookRectorTest.php delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/Source/SomeParentContractInterface.php delete mode 100644 rules-tests/Php84/Rector/Class_/PropertyHookRector/config/configured_rule.php delete mode 100644 rules/Php84/NodeFactory/PropertyHookFactory.php delete mode 100644 rules/Php84/NodeFinder/SetterAndGetterFinder.php diff --git a/config/set/php84.php b/config/set/php84.php index 2fb2433c360..b0f0d0b2e06 100644 --- a/config/set/php84.php +++ b/config/set/php84.php @@ -22,8 +22,5 @@ ForeachToArrayFindKeyRector::class, ForeachToArrayAllRector::class, ForeachToArrayAnyRector::class, - - // optional - // \Rector\Php84\Rector\Class_\PropertyHookRector::class, ]); }; diff --git a/phpstan.neon b/phpstan.neon index 0741d929a8e..8a1aff66b00 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -275,6 +275,7 @@ parameters: - '#Register "Rector\\Php80\\Rector\\NotIdentical\\MbStrContainsRector" service to "php80\.php" config set#' - '#Register "Rector\\Php85\\Rector\\StmtsAwareInterface\\SequentialAssignmentsToPipeOperatorRector" service to "php85\.php" config set#' - '#Register "Rector\\Php85\\Rector\\Expression\\NestedFuncCallsToPipeOperatorRector" service to "php85\.php" config set#' + - '#Register "Rector\\Php84\\Rector\\Class_\\PropertyHookRector" service to "php84\.php" config set#' # closure detailed - '#Method Rector\\Config\\RectorConfig\:\:singleton\(\) has parameter \$concrete with no signature specified for Closure#' diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_different_variable.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_different_variable.php.inc deleted file mode 100644 index 30cb25da195..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_different_variable.php.inc +++ /dev/null @@ -1,20 +0,0 @@ -surname; - } - - public function setName(string $name): void - { - $this->surname = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_get_set_magic.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_get_set_magic.php.inc deleted file mode 100644 index e5b35b1204e..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_get_set_magic.php.inc +++ /dev/null @@ -1,26 +0,0 @@ -name; - } - - public function setName(string $name): void - { - $this->name = ucfirst($name); - } - - public function __get($name) - { - } - - public function __set($name, $value) - { - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_getter_and_readonly.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_getter_and_readonly.php.inc deleted file mode 100644 index a8dd2371297..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_getter_and_readonly.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -name = $name; - } - - public function getName(): string - { - return $this->name; - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_method_with_attributes.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_method_with_attributes.php.inc deleted file mode 100644 index 4741a2e3eb7..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_method_with_attributes.php.inc +++ /dev/null @@ -1,22 +0,0 @@ -name; - } - - #[Required] - public function setName(string $name): void - { - $this->name = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_mismatching_getter_setter_names.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_mismatching_getter_setter_names.php.inc deleted file mode 100644 index e5bc9900851..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_mismatching_getter_setter_names.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -name; - } - - public function setAnotherName(string $name): void - { - $this->name = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_multi_stmts_getter.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_multi_stmts_getter.php.inc deleted file mode 100644 index 82bf0145be7..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_multi_stmts_getter.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -name; - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_non_final_class_to_avoid_child_break.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_non_final_class_to_avoid_child_break.php.inc deleted file mode 100644 index 651b06fa9e6..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_non_final_class_to_avoid_child_break.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -name; - } - - public function setName(string $name): void - { - $this->name = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_parent_contract.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_parent_contract.php.inc deleted file mode 100644 index 287c336b6af..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_parent_contract.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -name; - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_class.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_class.php.inc deleted file mode 100644 index 4e4c98fe329..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_class.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -name; - } - - public function setName(string $name): void - { - $this->name = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_getter_and_setter.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_getter_and_setter.php.inc deleted file mode 100644 index 2dce01eb3dc..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_getter_and_setter.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -name; - } - - public function setName(string $name): void - { - $this->name = ucfirst($name); - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_promoted_property.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_promoted_property.php.inc deleted file mode 100644 index 0e1c8210636..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/skip_readonly_promoted_property.php.inc +++ /dev/null @@ -1,17 +0,0 @@ -value; - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/some_fixture.php.inc b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/some_fixture.php.inc deleted file mode 100644 index 051d0840fb9..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Fixture/some_fixture.php.inc +++ /dev/null @@ -1,36 +0,0 @@ -name; - } - - public function setName(string $name): void - { - $this->name = ucfirst($name); - } -} - -?> ------ - $this->name; - set(string $name) { - $this->name = ucfirst($name); - } - } -} - -?> diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/PropertyHookRectorTest.php b/rules-tests/Php84/Rector/Class_/PropertyHookRector/PropertyHookRectorTest.php deleted file mode 100644 index 9a3360ed62b..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/PropertyHookRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Source/SomeParentContractInterface.php b/rules-tests/Php84/Rector/Class_/PropertyHookRector/Source/SomeParentContractInterface.php deleted file mode 100644 index ab4920f92cf..00000000000 --- a/rules-tests/Php84/Rector/Class_/PropertyHookRector/Source/SomeParentContractInterface.php +++ /dev/null @@ -1,8 +0,0 @@ -rule(PropertyHookRector::class); - - $rectorConfig->phpVersion(PhpVersion::PHP_84); -}; diff --git a/rules/Php84/NodeFactory/PropertyHookFactory.php b/rules/Php84/NodeFactory/PropertyHookFactory.php deleted file mode 100644 index 634574d6aaf..00000000000 --- a/rules/Php84/NodeFactory/PropertyHookFactory.php +++ /dev/null @@ -1,43 +0,0 @@ -name->toString(); - - if ($methodName === 'get' . ucfirst($propertyName)) { - $methodName = 'get'; - } elseif ($methodName === 'set' . ucfirst($propertyName)) { - $methodName = 'set'; - } else { - return null; - } - - Assert::notNull($classMethod->stmts); - - $soleStmt = $classMethod->stmts[0]; - - // use sole Expr - if (($soleStmt instanceof Expression || $soleStmt instanceof Return_) && $methodName !== 'set') { - $body = $soleStmt->expr; - } else { - $body = [$soleStmt]; - } - - $setterPropertyHook = new PropertyHook($methodName, $body); - $setterPropertyHook->params = $classMethod->params; - - return $setterPropertyHook; - } -} diff --git a/rules/Php84/NodeFinder/SetterAndGetterFinder.php b/rules/Php84/NodeFinder/SetterAndGetterFinder.php deleted file mode 100644 index 2adcc7f15be..00000000000 --- a/rules/Php84/NodeFinder/SetterAndGetterFinder.php +++ /dev/null @@ -1,68 +0,0 @@ -findGetterClassMethod($class, $propertyName); - if ($getterClassMethod instanceof ClassMethod) { - $classMethods[] = $getterClassMethod; - } - - $setterClassMethod = $this->findSetterClassMethod($class, $propertyName); - if ($setterClassMethod instanceof ClassMethod) { - $classMethods[] = $setterClassMethod; - } - - return $classMethods; - } - - public function findGetterClassMethod(Class_ $class, string $propertyName): ?ClassMethod - { - foreach ($class->getMethods() as $classMethod) { - if (! $this->classMethodAndPropertyAnalyzer->hasPropertyFetchReturn($classMethod, $propertyName)) { - continue; - } - - return $classMethod; - } - - return null; - } - - public function findSetterClassMethod(Class_ $class, string $propertyName): ?ClassMethod - { - foreach ($class->getMethods() as $classMethod) { - - if ($classMethod->isMagic()) { - continue; - } - - if (! $this->classMethodAndPropertyAnalyzer->hasOnlyPropertyAssign($classMethod, $propertyName)) { - continue; - } - - return $classMethod; - } - - return null; - } -} diff --git a/rules/Php84/Rector/Class_/PropertyHookRector.php b/rules/Php84/Rector/Class_/PropertyHookRector.php index 5cdd9f103eb..cebaa66cc97 100644 --- a/rules/Php84/Rector/Class_/PropertyHookRector.php +++ b/rules/Php84/Rector/Class_/PropertyHookRector.php @@ -4,34 +4,21 @@ namespace Rector\Php84\Rector\Class_; -use PhpParser\Modifiers; use PhpParser\Node; -use PhpParser\Node\PropertyHook; use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassMethod; -use Rector\Configuration\Parameter\FeatureFlags; -use Rector\Php84\NodeFactory\PropertyHookFactory; -use Rector\Php84\NodeFinder\SetterAndGetterFinder; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; -use Rector\ValueObject\MethodName; use Rector\ValueObject\PhpVersionFeature; -use Rector\VendorLocker\ParentClassMethodTypeOverrideGuard; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\Php84\Rector\Class_\PropertyHookRector\PropertyHookRectorTest + * @deprecated This rule is deprecated, as property hooks are a matter of preference. The rule was never part of any set, there is no real upgrade path from getters/setters, and the hooked property mixes state and behavior in a single place, making the code harder to read. Keep the explicit getter/setter methods instead. */ -final class PropertyHookRector extends AbstractRector implements MinPhpVersionInterface +final class PropertyHookRector extends AbstractRector implements MinPhpVersionInterface, DeprecatedInterface { - public function __construct( - private readonly SetterAndGetterFinder $setterAndGetterFinder, - private readonly PropertyHookFactory $propertyHookFactory, - private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Replace getter/setter with property hook', [ @@ -78,99 +65,14 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if ($node->isReadonly()) { - return null; - } - - // avoid breaking of child class getter/setter method use - if (! $node->isFinal() && FeatureFlags::treatClassesAsFinal($node) === false) { - return null; - } - - if ($this->hasMagicGetSetMethod($node)) { - return null; - } - - // nothing to hook to - if ($node->getProperties() === []) { - return null; - } - - $classMethodsToRemove = []; - - foreach ($node->getProperties() as $property) { - $propertyName = $this->getName($property); - - if ($property->isReadonly()) { - continue; - } - - $candidateClassMethods = $this->setterAndGetterFinder->findGetterAndSetterClassMethods( - $node, - $propertyName - ); - - foreach ($candidateClassMethods as $candidateClassMethod) { - if (count((array) $candidateClassMethod->stmts) !== 1) { - continue; - } - - // skip attributed methods - if ($candidateClassMethod->attrGroups !== []) { - continue; - } - - // avoid parent contract/method override - if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($candidateClassMethod)) { - continue; - } - - $propertyHook = $this->propertyHookFactory->create($candidateClassMethod, $propertyName); - if (! $propertyHook instanceof PropertyHook) { - continue; - } - - if (! $property->isPublic()) { - $property->flags = Modifiers::PUBLIC; - } - - $property->hooks[] = $propertyHook; - $classMethodsToRemove[] = $candidateClassMethod; - } - } - - if ($classMethodsToRemove === []) { - return null; - } - - foreach ($node->stmts as $key => $classStmt) { - if (! $classStmt instanceof ClassMethod) { - continue; - } - - if (! in_array($classStmt, $classMethodsToRemove)) { - continue; - } - - unset($node->stmts[$key]); - } - - return $node; + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as property hooks are a matter of preference. They provide no upgrade value and make the code harder to read', + self::class + )); } public function provideMinPhpVersion(): int { return PhpVersionFeature::PROPERTY_HOOKS; } - - private function hasMagicGetSetMethod(Class_ $class): bool - { - $magicGetMethod = $class->getMethod(MethodName::__GET); - if ($magicGetMethod instanceof ClassMethod) { - return true; - } - - $magicSetMethod = $class->getMethod(MethodName::__SET); - return $magicSetMethod instanceof ClassMethod; - } }