From 9f26b23e59eb50e4301816f484f819157039cb40 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Mon, 31 Aug 2026 15:54:09 +0200 Subject: [PATCH 1/2] fix(preprocessor): enforce property-hook placement rules for class properties The interface path already validated hook placement; class and trait properties accepted every combination. parseClassPropertyDef now mirrors Zend's compile-time rules (probed on 8.4.13, including the precedence order static -> readonly -> abstract rules): - hooks on a static property ("Cannot declare hooks for static property") - hooks on a readonly property, including properties made readonly by a `readonly class` ("Hooked properties cannot be readonly") - `abstract` on a hook-less property ("Only hooked properties may be declared abstract") - abstract hooked property with a default value ("Cannot specify default value for virtual hooked property A::$x") - abstract hooked property whose hooks all have bodies ("Abstract property A::$x must specify at least one abstract hook") - abstract hooked property in a non-abstract class; traits stay exempt (the consuming class satisfies the hook) and enums are already rejected by the property ban - bodiless hook on a non-abstract property, in classes and traits ("Non-abstract property hook must have a body"); previously the lowering fabricated a concrete backing-store accessor for it --- src/Preprocessor.php | 69 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/Preprocessor.php b/src/Preprocessor.php index 18b99a6e..908b2316 100644 --- a/src/Preprocessor.php +++ b/src/Preprocessor.php @@ -2001,6 +2001,7 @@ protected function propertyTypeDeclToString(NodeAbstract $typeNode): string protected function parseClassPropertyDef(Node\Stmt\Property $v): void { + $this->validateClassPropertyHookPlacement($v); $arrayDef = $this->parseArrayDefinition($v); if ($this->classDef->nativeObject) { if ($v->type === null) { @@ -2051,6 +2052,74 @@ protected function parseClassPropertyDef(Node\Stmt\Property $v): void $this->context = $oriCtx; } + /** + * Mirror Zend's compile-time placement rules for property hooks on class + * (and trait) properties; the interface path enforces its own subset in + * prepareInterfaceProperty(). Check order follows Zend 8.4 precedence: + * static, readonly, then the abstract-property rules. + */ + private function validateClassPropertyHookPlacement(Node\Stmt\Property $v): void + { + $abstract = (bool) ($v->flags & Modifiers::ABSTRACT); + if ($v->hooks === [] && !$abstract) { + return; + } + + $className = $this->classDef->getNamespacedName(false); + $propName = $v->props !== [] ? $this->parseIdentifier($v->props[0]->name) : ''; + if ($v->hooks !== []) { + if ($v->flags & Modifiers::STATIC) { + $this->fatalError($v, 'Cannot declare hooks for static property'); + } + // A readonly class marks every property readonly, exactly like an + // explicit per-property modifier. + if (($v->flags | $this->classDef->flags) & Modifiers::READONLY) { + $this->fatalError($v, 'Hooked properties cannot be readonly'); + } + } + + if ($abstract) { + if ($v->hooks === []) { + $this->fatalError($v, 'Only hooked properties may be declared abstract'); + } + foreach ($v->props as $prop) { + if ($prop->default !== null) { + $this->fatalError( + $v, + "Cannot specify default value for virtual hooked property {$className}::\${$propName}", + ); + } + } + $hasAbstractHook = false; + foreach ($v->hooks as $hook) { + if ($hook->body === null) { + $hasAbstractHook = true; + break; + } + } + if (!$hasAbstractHook) { + $this->fatalError( + $v, + "Abstract property `{$className}::\${$propName}` must specify at least one abstract hook", + ); + } + if (!$this->classDef->trait && !($this->classDef->flags & Modifiers::ABSTRACT)) { + $this->fatalError( + $v, + "Non-abstract class `{$className}` contains abstract hooked property `\${$propName}`", + ); + } + return; + } + + // Without the abstract modifier every declared hook needs a body. + foreach ($v->hooks as $hook) { + if ($hook->body === null) { + $this->fatalError($hook, 'Non-abstract property hook must have a body'); + } + } + } + protected function prepareClassMethod(Node\Stmt\ClassMethod $v, Node\Stmt\Class_|Node\Stmt\Trait_|Node\Stmt\Enum_ $class): void { $this->resetMethod(); From 277fde382676dc85cf584b2d87500554c03c762b Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Tue, 1 Sep 2026 12:12:54 +0200 Subject: [PATCH 2/2] test(preprocessor): cover property-hook placement rules --- .../code/hook_rule_abstract_all_bodies.php | 4 ++ phpunit/code/hook_rule_abstract_no_hooks.php | 4 ++ .../hook_rule_abstract_nonabstract_class.php | 4 ++ phpunit/code/hook_rule_bodyless.php | 4 ++ phpunit/code/hook_rule_readonly.php | 4 ++ phpunit/code/hook_rule_readonly_class.php | 4 ++ phpunit/code/hook_rule_static.php | 4 ++ phpunit/code/hook_rule_valid.php | 4 ++ phpunit/src/PropertyHookPlacementTest.php | 50 +++++++++++++++++++ 9 files changed, 82 insertions(+) create mode 100644 phpunit/code/hook_rule_abstract_all_bodies.php create mode 100644 phpunit/code/hook_rule_abstract_no_hooks.php create mode 100644 phpunit/code/hook_rule_abstract_nonabstract_class.php create mode 100644 phpunit/code/hook_rule_bodyless.php create mode 100644 phpunit/code/hook_rule_readonly.php create mode 100644 phpunit/code/hook_rule_readonly_class.php create mode 100644 phpunit/code/hook_rule_static.php create mode 100644 phpunit/code/hook_rule_valid.php create mode 100644 phpunit/src/PropertyHookPlacementTest.php diff --git a/phpunit/code/hook_rule_abstract_all_bodies.php b/phpunit/code/hook_rule_abstract_all_bodies.php new file mode 100644 index 00000000..3d3d5289 --- /dev/null +++ b/phpunit/code/hook_rule_abstract_all_bodies.php @@ -0,0 +1,4 @@ + 1; } } + +function main() {} diff --git a/phpunit/code/hook_rule_abstract_no_hooks.php b/phpunit/code/hook_rule_abstract_no_hooks.php new file mode 100644 index 00000000..2f4af5b1 --- /dev/null +++ b/phpunit/code/hook_rule_abstract_no_hooks.php @@ -0,0 +1,4 @@ + 1; } } + +function main() {} diff --git a/phpunit/code/hook_rule_readonly_class.php b/phpunit/code/hook_rule_readonly_class.php new file mode 100644 index 00000000..e6b4184d --- /dev/null +++ b/phpunit/code/hook_rule_readonly_class.php @@ -0,0 +1,4 @@ + 1; } } + +function main() {} diff --git a/phpunit/code/hook_rule_static.php b/phpunit/code/hook_rule_static.php new file mode 100644 index 00000000..5b48c210 --- /dev/null +++ b/phpunit/code/hook_rule_static.php @@ -0,0 +1,4 @@ + 1; } } + +function main() {} diff --git a/phpunit/code/hook_rule_valid.php b/phpunit/code/hook_rule_valid.php new file mode 100644 index 00000000..02e0cf52 --- /dev/null +++ b/phpunit/code/hook_rule_valid.php @@ -0,0 +1,4 @@ + $this->b; set { $this->b = $value; } } abstract public string $s { get; } } + +function main() {} diff --git a/phpunit/src/PropertyHookPlacementTest.php b/phpunit/src/PropertyHookPlacementTest.php new file mode 100644 index 00000000..662d2936 --- /dev/null +++ b/phpunit/src/PropertyHookPlacementTest.php @@ -0,0 +1,50 @@ +exec('Cannot declare hooks for static property', 'hook_rule_static.php'); + } + + public function testHooksOnReadonlyPropertyAreRejected(): void + { + $this->exec('Hooked properties cannot be readonly', 'hook_rule_readonly.php'); + } + + public function testHooksInReadonlyClassAreRejected(): void + { + $this->exec('Hooked properties cannot be readonly', 'hook_rule_readonly_class.php'); + } + + public function testAbstractHookedPropertyRequiresAbstractClass(): void + { + $this->exec('Non-abstract class `Box` contains abstract hooked property `$x`', 'hook_rule_abstract_nonabstract_class.php'); + } + + public function testAbstractPropertyNeedsAtLeastOneAbstractHook(): void + { + $this->exec('Abstract property `Box::$x` must specify at least one abstract hook', 'hook_rule_abstract_all_bodies.php'); + } + + public function testOnlyHookedPropertiesMayBeAbstract(): void + { + $this->exec('Only hooked properties may be declared abstract', 'hook_rule_abstract_no_hooks.php'); + } + + public function testNonAbstractHookMustHaveBody(): void + { + $this->exec('Non-abstract property hook must have a body', 'hook_rule_bodyless.php'); + } + + public function testWellFormedHooksStillCompile(): void + { + $this->compile('hook_rule_valid.php'); + } +}