fix: enforce readonly declaration and inheritance rules (#66)

* fix(preprocessor): enforce Zend readonly property declaration rules

The readonly checks previously lived only in the Native-class branch;
ZendVM-backed classes accepted declarations Zend rejects at compile
time. addClassProperty now enforces, for declared and promoted
properties alike (probed against Zend 8.4.13):

- readonly property with a default value ("Readonly property A::$x
  cannot have default value") - a readonly property carries runtime
  initialization state, so a compile-time default is meaningless
- untyped readonly property, including untyped promoted readonly ctor
  params ("Readonly property A::$x must have type")
- static readonly ("Static property A::$x cannot be readonly")
- a `readonly class` applies the same three rules to every property:
  the class-level Modifiers::READONLY flag (already recorded on
  ClassDef->flags for the Translator-side inheritance check) is OR-ed
  into the per-property check

Promoted readonly params keep accepting parameter defaults: the default
belongs to the constructor argument, not the property (Zend-verified).

The inheritance_error_prop_readonly fixture used `readonly int $x = 2`,
which Zend itself rejects with the default-value error before ever
reaching the readonly-mismatch link error; the default is dropped so the
fixture still exercises the inheritance mismatch.

* fix(translator): enforce readonly-class inheritance in both directions

Zend seals readonly-ness across a hierarchy: a non-readonly class cannot
extend a readonly one and vice versa. Both directions compiled silently.

* fix(translator): complete readonly-class contracts for traits, internal parents and attributes

Three readonly-class rules Zend enforces at compile time were still
accepted (all probed on 8.4.13):

- A trait property keeps its own declaration; the consuming class's
  readonly modifier does not upgrade it, so composing a non-readonly
  (or static, which can never be readonly) trait property into a
  readonly class fails: "Readonly class C cannot use trait with a
  non-readonly property T::$value". The check runs in composeTraitAst's
  property pass, which also matches Zend's naming of the directly used
  trait when the property originates in a nested trait. A trait property
  declared readonly composes fine.

- The readonly inheritance check only covered compiled parents; classes
  extending internal ones skipped it entirely, so `readonly class C
  extends ArrayObject {}` compiled. Internal parents now consult host
  reflection (ReflectionClass::isReadOnly), keeping the contract
  two-directional: the host runtime also knows internal readonly classes
  (BcMath\Number, Dom\NamespaceInfo — both final in 8.4, so only the
  readonly-child direction is reachable today).

- #[AllowDynamicProperties] contradicts readonly semantics (every
  property is readonly and declared); Zend rejects the combination:
  "Cannot apply #[AllowDynamicProperties] to readonly class C". The
  pre-existing readonly-class.phpt carried exactly this invalid
  combination and is adjusted to stay a valid positive test.
master
Alessio Giacobbe 1 day ago committed by GitHub
parent c578b1d6b0
commit fafd7f25dc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      phpunit/code/inheritance_error_prop_readonly.php
  2. 5
      phpunit/code/readonly_class_allow_dynamic.php
  3. 5
      phpunit/code/readonly_class_extends.php
  4. 4
      phpunit/code/readonly_class_extends_internal.php
  5. 5
      phpunit/code/readonly_class_extends_rev.php
  6. 5
      phpunit/code/readonly_class_extends_valid.php
  7. 5
      phpunit/code/readonly_class_trait_nonreadonly_prop.php
  8. 5
      phpunit/code/readonly_class_trait_readonly_prop_valid.php
  9. 5
      phpunit/code/readonly_class_trait_static_prop.php
  10. 4
      phpunit/code/readonly_rule_class_static.php
  11. 4
      phpunit/code/readonly_rule_class_untyped.php
  12. 4
      phpunit/code/readonly_rule_default.php
  13. 4
      phpunit/code/readonly_rule_promoted_untyped.php
  14. 4
      phpunit/code/readonly_rule_static.php
  15. 4
      phpunit/code/readonly_rule_untyped.php
  16. 4
      phpunit/code/readonly_rule_valid.php
  17. 40
      phpunit/src/ClassKindInheritanceTest.php
  18. 80
      phpunit/src/ReadonlyDeclarationRulesTest.php
  19. 30
      src/Preprocessor.php
  20. 53
      src/Translator.php
  21. 1
      tests/compiler/class/readonly-class.phpt

@ -6,7 +6,7 @@ class A
class B extends A class B extends A
{ {
public readonly int $x = 2; public readonly int $x;
} }
function main() {} function main() {}

@ -0,0 +1,5 @@
<?php
#[AllowDynamicProperties]
readonly class Cfg { public int $port; public function __construct() { $this->port = 80; } }
function main() {}

@ -0,0 +1,5 @@
<?php
readonly class A {}
class B extends A {}
function main() {}

@ -0,0 +1,4 @@
<?php
readonly class Cfg extends ArrayObject {}
function main() {}

@ -0,0 +1,5 @@
<?php
class A {}
readonly class B extends A {}
function main() {}

@ -0,0 +1,5 @@
<?php
readonly class A { public function __construct(public int $x) {} }
readonly class B extends A {}
function main() {}

@ -0,0 +1,5 @@
<?php
trait Settings { public int $port; }
readonly class Cfg { use Settings; }
function main() {}

@ -0,0 +1,5 @@
<?php
trait Settings { public readonly int $port; }
readonly class Cfg { use Settings; public function __construct() { $this->port = 80; } }
function main() {}

@ -0,0 +1,5 @@
<?php
trait Settings { public static int $port = 80; }
readonly class Cfg { use Settings; }
function main() {}

@ -0,0 +1,4 @@
<?php
readonly class Cfg { public static int $port; }
function main() {}

@ -0,0 +1,4 @@
<?php
readonly class Cfg { public $port; }
function main() {}

@ -0,0 +1,4 @@
<?php
class Cfg { public readonly int $port = 80; }
function main() {}

@ -0,0 +1,4 @@
<?php
class Cfg { public function __construct(public readonly $port) {} }
function main() {}

@ -0,0 +1,4 @@
<?php
class Cfg { public static readonly int $port; }
function main() {}

@ -0,0 +1,4 @@
<?php
class Cfg { public readonly $port; }
function main() {}

@ -0,0 +1,4 @@
<?php
readonly class Cfg { public int $port; public function __construct(public readonly string $host = "a") { $this->port = 80; } }
function main() {}

@ -0,0 +1,40 @@
<?php
/**
* Readonly-ness is part of the inheritance contract in both directions, and
* an interface can only extend other interfaces.
*/
class ClassKindInheritanceTest extends BaseTest
{
public function testNonReadonlyCannotExtendReadonly(): void
{
$this->exec(
'Non-readonly class `B` cannot extend readonly class `A`',
'readonly_class_extends.php'
);
}
public function testReadonlyCannotExtendNonReadonly(): void
{
$this->exec(
'Readonly class `B` cannot extend non-readonly class `A`',
'readonly_class_extends_rev.php'
);
}
public function testReadonlyExtendsReadonlyIsValid(): void
{
$this->compile('readonly_class_extends_valid.php');
}
public function testReadonlyCannotExtendNonReadonlyInternalClass(): void
{
// Internal parents are not in the symbol table; host reflection
// (ReflectionClass::isReadOnly) is authoritative for them.
$this->exec(
'Readonly class `Cfg` cannot extend non-readonly class `ArrayObject`',
'readonly_class_extends_internal.php'
);
}
}

@ -0,0 +1,80 @@
<?php
/**
* Zend readonly property declaration rules for ZendVM-backed classes:
* no defaults, mandatory type, no static readonly, and the readonly
* class modifier applying the same rules to every property.
*/
class ReadonlyDeclarationRulesTest extends BaseTest
{
public function testReadonlyPropertyCannotHaveDefault(): void
{
$this->exec('Readonly property `Cfg::$port` cannot have default value', 'readonly_rule_default.php');
}
public function testReadonlyPropertyMustHaveType(): void
{
$this->exec('Readonly property `Cfg::$port` must have type', 'readonly_rule_untyped.php');
}
public function testStaticPropertyCannotBeReadonly(): void
{
$this->exec('Static property `Cfg::$port` cannot be readonly', 'readonly_rule_static.php');
}
public function testPromotedReadonlyParamMustHaveType(): void
{
$this->exec('Readonly property `Cfg::$port` must have type', 'readonly_rule_promoted_untyped.php');
}
public function testReadonlyClassPropertyMustHaveType(): void
{
$this->exec('Readonly property `Cfg::$port` must have type', 'readonly_rule_class_untyped.php');
}
public function testReadonlyClassCannotDeclareStaticProperty(): void
{
$this->exec('Static property `Cfg::$port` cannot be readonly', 'readonly_rule_class_static.php');
}
public function testWellFormedReadonlyDeclarationsStillCompile(): void
{
// Promoted readonly params may keep a parameter default: it belongs
// to the constructor argument, not to the property.
$this->compile('readonly_rule_valid.php');
}
public function testReadonlyClassCannotUseTraitWithNonReadonlyProperty(): void
{
// A trait property keeps its own declaration; the consuming class's
// readonly modifier does not upgrade it.
$this->exec(
'Readonly class `Cfg` cannot use trait with a non-readonly property `Settings::$port`',
'readonly_class_trait_nonreadonly_prop.php'
);
}
public function testReadonlyClassCannotUseTraitWithStaticProperty(): void
{
// Static properties can never be readonly, so a trait declaring one
// is unusable in a readonly class (Zend reports the same mismatch).
$this->exec(
'Readonly class `Cfg` cannot use trait with a non-readonly property `Settings::$port`',
'readonly_class_trait_static_prop.php'
);
}
public function testReadonlyClassUsingTraitWithReadonlyPropertyIsValid(): void
{
$this->compile('readonly_class_trait_readonly_prop_valid.php');
}
public function testAllowDynamicPropertiesOnReadonlyClassIsRejected(): void
{
// Dynamic properties and readonly semantics are mutually exclusive.
$this->exec(
'Cannot apply #[AllowDynamicProperties] to readonly class `Cfg`',
'readonly_class_allow_dynamic.php'
);
}
}

@ -1209,6 +1209,21 @@ class Preprocessor extends CompilerBase
if (isset($this->symbolDeclInFile[$fullClassNameLower])) { if (isset($this->symbolDeclInFile[$fullClassNameLower])) {
$this->fatalError($class, "Duplicate class `{$fullClassName}`"); $this->fatalError($class, "Duplicate class `{$fullClassName}`");
} }
// Dynamic properties and readonly semantics are mutually exclusive:
// every property of a readonly class is readonly and declared, so
// Zend rejects the attribute at compile time.
if ($class instanceof Node\Stmt\Class_ && ($flags & Modifiers::READONLY)) {
foreach ($class->attrGroups as $group) {
foreach ($group->attrs as $attribute) {
if (strcasecmp($this->getResolvedPhpName($attribute->name), 'AllowDynamicProperties') === 0) {
$this->fatalError(
$attribute,
"Cannot apply #[AllowDynamicProperties] to readonly class `{$fullClassName}`",
);
}
}
}
}
$this->classDef = new ClassDef($this->class, $flags, $this->namespace); $this->classDef = new ClassDef($this->class, $flags, $this->namespace);
$this->classDef->nativeObject = NativeClassAttributeLowering::isNative($class); $this->classDef->nativeObject = NativeClassAttributeLowering::isNative($class);
@ -1674,6 +1689,21 @@ class Preprocessor extends CompilerBase
); );
} }
$flags = $this->parseModifiers($flags); $flags = $this->parseModifiers($flags);
// A `readonly class` marks every property readonly, so the class-level
// flag participates in the same Zend declaration rules as an explicit
// per-property `readonly` modifier.
if (($flags | $this->classDef->flags) & Modifiers::READONLY) {
$className = $this->classDef->getNamespacedName(false);
if ($flags & Modifiers::STATIC) {
$this->fatalError($errorNode, "Static property `{$className}::\${$name}` cannot be readonly");
}
if ($typeNode === null) {
$this->fatalError($errorNode, "Readonly property `{$className}::\${$name}` must have type");
}
if ($defaultNode !== null) {
$this->fatalError($errorNode, "Readonly property `{$className}::\${$name}` cannot have default value");
}
}
$this->validateAsymmetricPropertyDeclaration($name, $flags, $typeNode, $errorNode); $this->validateAsymmetricPropertyDeclaration($name, $flags, $typeNode, $errorNode);
[$type, $class] = $this->resolveTypeDecl($typeNode, self::DECL_TYPE_OF_PROPERTY); [$type, $class] = $this->resolveTypeDecl($typeNode, self::DECL_TYPE_OF_PROPERTY);
$this->assertSupportedNativeObjectTypeNode($typeNode, self::DECL_TYPE_OF_PROPERTY, $errorNode); $this->assertSupportedNativeObjectTypeNode($typeNode, self::DECL_TYPE_OF_PROPERTY, $errorNode);

@ -3368,6 +3368,21 @@ CODE;
} }
continue; continue;
} }
// A trait property keeps its own declaration: the
// consuming class's `readonly` modifier does not
// upgrade it, so Zend refuses to compose a
// non-readonly (or static, which can never be
// readonly) trait property into a readonly class.
// Zend names the directly used trait, even when
// the property originated in a nested trait.
if (($classDef->flags & Modifiers::READONLY)
&& !($traitStmt->flags & Modifiers::READONLY)
) {
$this->fatalError(
$traitStmt,
"Readonly class `{$compositionOwner}` cannot use trait with a non-readonly property `{$traitFullName}::\${$prop->name->toString()}`",
);
}
$traitProperties[$propName] = [$traitStmt, $prop]; $traitProperties[$propName] = [$traitStmt, $prop];
} }
} }
@ -4084,9 +4099,30 @@ CODE;
if ($parent->flags & Modifiers::FINAL) { if ($parent->flags & Modifiers::FINAL) {
$this->fatalError($class, "Class `{$this->class}` cannot extend final class `{$parentClass}`"); $this->fatalError($class, "Class `{$this->class}` cannot extend final class `{$parentClass}`");
} }
// Readonly-ness is part of the inheritance contract in both
// directions (Zend: a readonly class seals its property
// semantics for the whole hierarchy).
$this->assertReadonlyInheritanceContract(
$class,
$parentClass,
(bool) ($parent->flags & Modifiers::READONLY),
);
} else { } else {
$this->fatalError($class, "Class `{$this->class}` inherits from a non-existent class `{$parentClass}`"); $this->fatalError($class, "Class `{$this->class}` inherits from a non-existent class `{$parentClass}`");
} }
} elseif ($this->classDef->extends and $this->classDef->inheritedFromInternalClass and $class instanceof Node\Stmt\Class_) {
// Internal parents are not in the symbol table; the host runtime's
// reflection is authoritative for their readonly-ness (e.g.
// BcMath\Number is an internal readonly class, ArrayObject is not),
// so the contract holds in both directions here as well.
$parentClass = $this->getNamespacedClassName($this->parseIdentifier($class->extends));
if (class_exists($parentClass)) {
$this->assertReadonlyInheritanceContract(
$class,
$parentClass,
(new \ReflectionClass($parentClass))->isReadOnly(),
);
}
} }
if (is_array($this->classDef->implements)) { if (is_array($this->classDef->implements)) {
@ -6190,6 +6226,23 @@ CODE;
} }
} }
/**
* Zend seals readonly-ness across a class hierarchy in both directions: a
* readonly class cannot extend a non-readonly one and vice versa. The
* parent's readonly-ness comes from the symbol table for compiled classes
* and from host reflection for internal ones.
*/
private function assertReadonlyInheritanceContract(NodeAbstract $errorNode, string $parentClass, bool $parentReadonly): void
{
$childReadonly = (bool) ($this->classDef->flags & Modifiers::READONLY);
if ($childReadonly === $parentReadonly) {
return;
}
$this->fatalError($errorNode, $parentReadonly
? "Non-readonly class `{$this->class}` cannot extend readonly class `{$parentClass}`"
: "Readonly class `{$this->class}` cannot extend non-readonly class `{$parentClass}`");
}
private function installComposedTraitDataMembers(Node\Stmt\ClassLike $class): void private function installComposedTraitDataMembers(Node\Stmt\ClassLike $class): void
{ {
foreach ($class->stmts as $stmt) { foreach ($class->stmts as $stmt) {

@ -4,7 +4,6 @@ Readonly Classes (PHP 8.2+)
<?php <?php
// Test basic readonly class // Test basic readonly class
#[\AllowDynamicProperties]
readonly class Point { readonly class Point {
public int $x; public int $x;
public int $y; public int $y;

Loading…
Cancel
Save