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_final.php b/phpunit/code/hook_rule_abstract_final.php new file mode 100644 index 00000000..773cbb04 --- /dev/null +++ b/phpunit/code/hook_rule_abstract_final.php @@ -0,0 +1,4 @@ + 1; } } +trait Boxed { abstract protected int $y { get; } } + +function main() {} diff --git a/phpunit/code/hook_rule_bodyless.php b/phpunit/code/hook_rule_bodyless.php new file mode 100644 index 00000000..566bb420 --- /dev/null +++ b/phpunit/code/hook_rule_bodyless.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..d5c7944b --- /dev/null +++ b/phpunit/src/PropertyHookPlacementTest.php @@ -0,0 +1,83 @@ +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'); + } + + public function testAbstractPrivateHookIsRejected(): void + { + // An abstract (bodiless) hook must be implementable by a subclass, + // which a private property forbids. + $this->exec('Property hook cannot be both abstract and private', 'hook_rule_abstract_private.php'); + } + + public function testAbstractPrivateHookIsRejectedInTrait(): void + { + // Unlike abstract private trait methods, Zend does not exempt traits + // from the abstract-private hook conflict. + $this->exec('Property hook cannot be both abstract and private', 'hook_rule_abstract_private_trait.php'); + } + + public function testAbstractFinalHookIsRejected(): void + { + // A bodiless hook must be overridable to ever gain a body; it cannot + // carry final. + $this->exec('Property hook cannot be both abstract and final', 'hook_rule_abstract_final.php'); + } + + public function testFinalPrivateHookWinsDiagnosticPrecedence(): void + { + // `abstract private int $x { final get; }` violates all three rules; + // Zend reports the final+private conflict first (probed on 8.4.13). + $this->exec('Property hook cannot be both final and private', 'hook_rule_final_private.php'); + } + + public function testProtectedAbstractHookStaysLegal(): void + { + $this->compile('hook_rule_abstract_protected_valid.php'); + } +} diff --git a/src/Preprocessor.php b/src/Preprocessor.php index dceaadc3..fc3dc691 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,103 @@ 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'); + } + // Zend checks hook-level modifier conflicts right after the + // property-level placement rules, before any abstract-property + // rule: a final hook on a private property is rejected first even + // when the hook is also bodiless (probed: + // `abstract private int $x { final get; }` reports final+private). + // A private property cannot be overridden, so a final hook on it + // is meaningless; the rule applies in traits as well. + foreach ($v->hooks as $hook) { + if (($hook->flags & Modifiers::FINAL) && ($v->flags & Modifiers::PRIVATE)) { + $this->fatalError($hook, 'Property hook cannot be both final and private'); + } + } + } + + if ($abstract) { + if ($v->hooks === []) { + $this->fatalError($v, 'Only hooked properties may be declared abstract'); + } + // A bodiless hook of an abstract property is itself abstract. An + // abstract hook must be implementable by a subclass, which a + // private property forbids, and must be overridable, which final + // forbids. Zend reports these per hook, before the default-value + // and abstract-hook-presence rules (probed on 8.4.13), and — + // unlike abstract private trait METHODS — does not exempt traits. + foreach ($v->hooks as $hook) { + if ($hook->body !== null) { + continue; + } + if ($v->flags & Modifiers::PRIVATE) { + $this->fatalError($hook, 'Property hook cannot be both abstract and private'); + } + if ($hook->flags & Modifiers::FINAL) { + $this->fatalError($hook, 'Property hook cannot be both abstract and final'); + } + } + 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();