fix(translator): constructor, private-method and abstract redeclaration override rules (#56) --skip-tests

* fix(translator): enforce final and abstract parent constructor rules

checkParentMethodCanBeOverridden() returned immediately for
__construct, so overriding a FINAL parent constructor was accepted
(Zend: "Cannot override final method A::__construct()") and an
ABSTRACT parent constructor's signature was never validated (Zend
checks it exactly like an interface constructor).

Zend's constructor rules (zend_do_inheritance):

  - a concrete parent constructor imposes no signature contract: the
    child may change parameters and even narrow visibility — this
    exemption is kept;
  - a private parent constructor may be redeclared freely, but FINAL
    still wins: `final private function __construct()` cannot be
    overridden (constructors are the one place PHP allows final
    private);
  - an abstract parent constructor's signature is a real contract.

Keep walking the parent chain for constructors, skipping only the
private-override error and the concrete-signature validation; final
checks (userland and built-in parents) and abstract-constructor
validation now run.

* fix(translator): allow redeclaring a parent's private method

checkParentMethodCanBeOverridden() fataled with "Cannot override
private method" when a child declared a method whose nearest parent
declaration is PRIVATE. Zend inherits no private methods: a child may
redeclare one with any signature, visibility or staticness, and FINAL
is ignored on non-constructor private methods (declaring one only
raises "Private methods cannot be final..."). Only the final private
CONSTRUCTOR remains protected, which the constructor path already
enforces.

Dispatch stays correct after removing the fatal:

  - canDevirtualize() (Parser/MethodCallTrait) devirtualizes any call
    whose resolved method is private to the DECLARING class's body.
    That is exactly PHP's private-scope binding (zend_std_get_method
    prefers the calling scope's private copy), verified against the
    manual's Bar/Foo::testPrivate example;
  - method resolution walks from the receiver's static class, so code
    in the child binds the child's redeclaration;
  - a call on a receiver statically typed as the declaring class from
    OUTSIDE its scope is rejected by getNativeMethod()'s accessibility
    check, and dynamically typed receivers go through Zend dispatch;
  - Native (C++) classes give private methods no virtual slot
    (isNativeVirtualMethod() excludes PRIVATE), so no C++ override can
    reroute a parent's internal private call.

The two tests asserting the old fatal encoded rejects-valid programs
(both run fine under Zend 8.4, printing the parent's private result);
they now assert successful compilation.

* fix(translator): validate abstract method redeclarations against the parent chain

An abstract method declared by a class was never checked against its
parent: turning a concrete inherited method abstract compiled (Zend:
"Cannot make non abstract method A::f() abstract in class B"), and an
abstract redeclaration of an inherited abstract contract skipped the
signature check entirely. Both now run through
checkParentMethodCanBeOverridden with a childIsAbstract mode, covering
userland and built-in parents.

Trait-originated abstract requirements are exempt: Zend lets an
inherited concrete method satisfy them, so only abstract methods the
class itself declares participate.

---------

Co-authored-by: tianfenghan <rango@swoole.com>
master
Alessio Giacobbe 8 hours ago committed by GitHub
parent a6cd38ae01
commit 26bfbdda4e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      phpunit/code/abstract_redeclare_concrete.php
  2. 5
      phpunit/code/abstract_redeclare_incompatible.php
  3. 4
      phpunit/code/abstract_redeclare_internal.php
  4. 15
      phpunit/code/abstract_redeclare_valid.php
  5. 12
      phpunit/code/ctor_override_abstract_incompatible.php
  6. 12
      phpunit/code/ctor_override_final.php
  7. 12
      phpunit/code/ctor_override_final_private.php
  8. 39
      phpunit/code/ctor_override_valid.php
  9. 53
      phpunit/code/private_redeclare_valid.php
  10. 39
      phpunit/src/AbstractRedeclarationTest.php
  11. 16
      phpunit/src/ClassTest.php
  12. 41
      phpunit/src/ConstructorOverrideTest.php
  13. 27
      phpunit/src/PrivateMethodRedeclareTest.php
  14. 81
      src/Translator.php

@ -0,0 +1,5 @@
<?php
class A { public function f(): int { return 1; } }
abstract class B extends A { abstract public function f(): int; }
function main() {}

@ -0,0 +1,5 @@
<?php
abstract class A { abstract public function f(): int; }
abstract class B extends A { abstract public function f(): string; }
function main() {}

@ -0,0 +1,4 @@
<?php
abstract class B extends ArrayObject { abstract public function count(): int; }
function main() {}

@ -0,0 +1,15 @@
<?php
// Redeclaring an inherited abstract contract compatibly is legal, a parent's
// private method is not inherited, and a trait's abstract requirement may be
// satisfied by an inherited concrete method.
abstract class A { abstract public function f(): int; }
abstract class B extends A { abstract public function f(): int; }
class C { private function g(): int { return 1; } }
abstract class D extends C { abstract public function g(): int; }
trait RequiresF { abstract public function h(): int; }
class E { public function h(): int { return 1; } }
class F extends E { use RequiresF; }
function main() {}

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

@ -0,0 +1,12 @@
<?php
class A
{
final public function __construct() {}
}
class B extends A
{
public function __construct() {}
}
function main() {}

@ -0,0 +1,12 @@
<?php
class A
{
final private function __construct() {}
}
class B extends A
{
public function __construct() {}
}
function main() {}

@ -0,0 +1,39 @@
<?php
class A
{
public function __construct(int $x) {}
}
// A concrete parent constructor imposes no signature contract: the child may
// change the parameters and even narrow visibility.
class B extends A
{
private function __construct(string $y, array $z)
{
parent::__construct(1);
}
}
class C
{
private function __construct(int $x) {}
}
// A private parent constructor may be redeclared freely.
class D extends C
{
public function __construct(string $y) {}
}
abstract class E
{
abstract public function __construct(int $x);
}
// Adding optional trailing parameters satisfies an abstract constructor.
class F extends E
{
public function __construct(int $x, string $y = '') {}
}
function main() {}

@ -0,0 +1,53 @@
<?php
class A
{
private function helper(): string
{
return 'A';
}
public function run(): string
{
// Binds A::helper() even on a B instance (private-scope binding).
return $this->helper();
}
}
class B extends A
{
// Any signature is allowed: private methods are not inherited.
public function helper(int $n = 0): int
{
return $n;
}
}
class C
{
final private function locked(): void {}
}
// Zend ignores FINAL on non-constructor private methods (declaring one only
// warns), so a child may still redeclare it.
class D extends C
{
public function locked(): void {}
}
class E
{
private static function make(): int
{
return 1;
}
}
class F extends E
{
public static function make(): string
{
return 'f';
}
}
function main() {}

@ -0,0 +1,39 @@
<?php
/**
* An abstract method declared by the class itself participates in the parent
* checks: it cannot turn a concrete inherited method abstract, and it must
* stay compatible with an inherited abstract contract. Trait requirements are
* exempt (an inherited concrete method satisfies them).
*/
class AbstractRedeclarationTest extends BaseTest
{
public function testConcreteParentMethodCannotBeMadeAbstract(): void
{
$this->exec(
'Cannot make non abstract method `A::f()` abstract in class `B`',
'abstract_redeclare_concrete.php'
);
}
public function testAbstractRedeclarationMustStayCompatible(): void
{
$this->exec(
'Declaration of `B::f()` must be compatible with `A::f()`',
'abstract_redeclare_incompatible.php'
);
}
public function testConcreteBuiltinMethodCannotBeMadeAbstract(): void
{
$this->exec(
'Cannot make non abstract method `ArrayObject::count()` abstract in class `B`',
'abstract_redeclare_internal.php'
);
}
public function testValidAbstractRedeclarations(): void
{
$this->compile('abstract_redeclare_valid.php');
}
}

@ -730,9 +730,12 @@ class ClassTest extends \BaseTest
$this->exec('abstract class `AbstractBase` cannot be instantiated', 'abstract-class-new.php');
}
public function testOverridePrivateMethod()
public function testPrivateMethodMayBeRedeclared()
{
$this->exec('Cannot override private method `Base::doWork()`', 'override-private-method.php');
// Private methods are not inherited: Zend lets a child redeclare one
// with any signature. Private calls bind to the declaring class's
// copy, so each class keeps its own implementation.
$this->compile('override-private-method.php');
}
public function testPromotedAsymmetricPropertyRequiresType(): void
@ -776,12 +779,11 @@ class ClassTest extends \BaseTest
$this->exec('Cannot access private method `BaseSecret::secret()`', 'trait-parent-method-private.php');
}
public function testComposedTraitMethodCannotShadowPrivateParentMethod()
public function testComposedTraitMethodMayShadowPrivateParentMethod()
{
$this->exec(
'Cannot override private method `PrivateMethodParent::execute()`',
'trait-method-shadows-private.php'
);
// A trait-composed method redeclaring a parent's PRIVATE method is
// valid in Zend, like any other private redeclaration.
$this->compile('trait-method-shadows-private.php');
}
public function testSelfCanBePartOfUnionType()

@ -0,0 +1,41 @@
<?php
use TypePhp\Exception\TestError;
/**
* Zend exempts constructors from LSP checks against a CONCRETE parent
* constructor, but still forbids overriding a FINAL parent constructor (even a
* final private one), and validates the signature against an ABSTRACT parent
* constructor exactly like an interface constructor.
*/
class ConstructorOverrideTest extends BaseTest
{
public function testConcreteAndPrivateParentConstructorsAreExempt(): void
{
$this->compile('ctor_override_valid.php');
}
public function testFinalParentConstructorCannotBeOverridden(): void
{
$this->exec(
'Cannot override final method `A::__construct()`',
'ctor_override_final.php',
);
}
public function testFinalPrivateParentConstructorCannotBeOverridden(): void
{
$this->exec(
'Cannot override final method `A::__construct()`',
'ctor_override_final_private.php',
);
}
public function testAbstractParentConstructorSignatureIsEnforced(): void
{
$this->exec(
'Declaration of `B::__construct()` must be compatible with `A::__construct()`',
'ctor_override_abstract_incompatible.php',
);
}
}

@ -0,0 +1,27 @@
<?php
use TypePhp\Exception\TestError;
/**
* Private methods are not inherited in Zend: a child class may redeclare one
* with any signature, visibility or staticness, and FINAL is ignored on
* non-constructor private methods. Generated code stays correct because
* private calls devirtualize to the declaring class's body — PHP's own
* private-scope binding — and Native classes give private methods no virtual
* slot.
*/
class PrivateMethodRedeclareTest extends BaseTest
{
public function testPrivateMethodsMayBeRedeclaredFreely(): void
{
$this->compile('private_redeclare_valid.php');
}
public function testFinalPrivateConstructorIsStillProtectedFromOverride(): void
{
$this->exec(
'Cannot override final method `A::__construct()`',
'ctor_override_final_private.php',
);
}
}

@ -4579,13 +4579,22 @@ CODE;
/**
* Check whether a parent method can be overridden: private methods cannot be
* overridden, and the signature must be compatible.
* overridden, and the signature must be compatible. With $childIsAbstract,
* the declaration additionally may not turn a concrete inherited method
* abstract (Zend: "Cannot make non abstract method ... abstract").
*/
protected function checkParentMethodCanBeOverridden(Node\Stmt\ClassMethod $v, string $name): void
{
if ($name === '__construct') {
return;
}
protected function checkParentMethodCanBeOverridden(
Node\Stmt\ClassMethod $v,
string $name,
bool $childIsAbstract = false
): void {
// Zend exempts constructors from the LSP checks against a CONCRETE
// parent constructor (subclasses may freely change the construction
// signature and even narrow its visibility). A FINAL parent
// constructor still cannot be overridden — even a final private one —
// and an ABSTRACT parent constructor imposes a real signature
// contract, exactly like an interface constructor.
$isConstructor = strtolower($name) === '__construct';
$classDef = $this->classDef;
$childFuncDef = $this->methodDef->functionDef;
@ -4597,24 +4606,42 @@ CODE;
// The parent class is a built-in class
if ($classDef->inheritedFromInternalClass) {
$modifiers = Reflection::getClassMethodModifiers($extends, $name);
if ($modifiers & \ReflectionMethod::IS_PRIVATE) {
goto _error;
if (!$isConstructor && ($modifiers & \ReflectionMethod::IS_PRIVATE)) {
// Private methods are not inherited: a child may redeclare
// one with any signature. Zend ignores FINAL on private
// methods outside constructors (declaring one only raises
// a warning), so no final check applies here either.
break;
}
if ($modifiers & \ReflectionMethod::IS_FINAL) {
goto _final_error;
}
$this->validateInternalMethodOverrideSignature($v, $name, $this->methodDef, $extends);
if ($childIsAbstract && $modifiers !== null && !($modifiers & \ReflectionMethod::IS_ABSTRACT)) {
$this->fatalError($v,
"Cannot make non abstract method `{$extends}::{$name}()` abstract in class `{$this->getFullClassName()}`");
}
// Concrete parent constructors are exempt from signature
// compatibility, while abstract constructors still define a
// contract. Ordinary internal methods always use the full
// Reflection-based override validation added on master.
if (!$isConstructor
|| ($modifiers !== null && ($modifiers & \ReflectionMethod::IS_ABSTRACT))
) {
$this->validateInternalMethodOverrideSignature($v, $name, $this->methodDef, $extends);
}
break;
}
$classDef = $this->getClass($extends);
if ($classDef->hasMethod($name)) {
$methodDef = $classDef->getMethod($name);
if ($methodDef->flags & Modifiers::PRIVATE) {
_error:
$message = 'Cannot override private method `' . $extends . '::' . $name . '()`';
$this->fatalGeneratedMethodAttributeIfAny($v, $message, $extends, $name);
$this->fatalError($v,
$message);
if (!$isConstructor && ($methodDef->flags & Modifiers::PRIVATE)) {
// See the internal-parent branch above: a private method
// may be redeclared freely. Generated code stays correct
// because private calls are devirtualized to the declaring
// class's body (canDevirtualize()), matching PHP's
// private-scope binding, and Native classes never give
// private methods a virtual slot (isNativeVirtualMethod()).
break;
}
if ($methodDef->flags & Modifiers::FINAL) {
_final_error:
@ -4633,10 +4660,18 @@ CODE;
$this->fatalError($v,
$message);
}
$this->validateMethodOverrideSignature($v, $name, $this->methodDef, $methodDef, $extends);
if ($childIsAbstract) {
$this->fatalError($v,
"Cannot make non abstract method `{$extends}::{$name}()` abstract in class `{$this->getFullClassName()}`");
}
if (!$isConstructor) {
$this->validateMethodOverrideSignature($v, $name, $this->methodDef, $methodDef, $extends);
}
break;
}
if ($classDef->hasAbstractMethod($name) && isset($classDef->abstractMethodDefs[strtolower($name)])) {
// An abstract parent constructor is validated like an
// interface constructor: its signature is a contract.
$this->validateMethodOverrideSignature($v, $name, $this->methodDef, $classDef->getAbstractMethod($name), $extends);
break;
}
@ -6062,6 +6097,20 @@ CODE;
// only run in the implementation phase.
$this->checkParentMethodCanBeOverridden($v, $name);
$methodCodes[$name] = $this->parseFunction($v);
} elseif ($this->classDef->trait === null
&& !is_string($v->getAttribute(self::TRAIT_ORIGIN_ATTRIBUTE))
&& $this->classDef->hasAbstractMethod($name)
&& isset($this->classDef->abstractMethodDefs[strtolower($name)])
) {
// An abstract method the class itself declares participates in
// the parent checks: it cannot turn a concrete inherited method
// abstract, and it must stay compatible with an inherited
// abstract contract. Abstract requirements arriving from traits
// are exempt — Zend lets an inherited concrete method satisfy
// them.
$this->methodDef = $this->classDef->getAbstractMethod($name);
$this->methodDef->node = $v;
$this->checkParentMethodCanBeOverridden($v, $name, childIsAbstract: true);
}
$this->resetMethod();

Loading…
Cancel
Save