fix(translator): let a trailing child variadic absorb parent parameters (#54)

Zend's zend_do_perform_implementation_check does not compare variadic-ness
per position. Its rules are:

  - a variadic parent requires a variadic child (unbounded contract);
  - a trailing child variadic stands in for every remaining parent
    position (decorator pattern), with the variadic's type checked for
    contravariance against each covered parent parameter and by-ref-ness
    matched per position;
  - when the parent is variadic, extra child parameters are validated
    against the parent's variadic slot.

validateMethodOverrideSignature required an exact per-position variadic
match, rejecting valid programs such as parent f(int $a, int $b)
overridden by f(int ...$args). Rework the position loop per the Zend
rules; the required-argument-count and extra-optional-parameter checks
are unchanged.

The pre-existing testVariadicMismatch expectation (untyped f($x)
overridden by f(...$x) must fail) contradicts Zend 8.4, which accepts
it; the test now asserts the program compiles.
master
Alessio Giacobbe 4 days ago committed by GitHub
parent aa1da2eb54
commit c6e6997db8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 12
      phpunit/code/override_parent_variadic_child_extra.php
  2. 12
      phpunit/code/override_parent_variadic_child_not.php
  3. 26
      phpunit/code/override_variadic_absorbs_params.php
  4. 12
      phpunit/code/override_variadic_bad_type.php
  5. 12
      phpunit/code/override_variadic_byref_mismatch.php
  6. 6
      phpunit/src/InheritanceErrorTest.php
  7. 48
      phpunit/src/MethodOverrideVariadicTest.php
  8. 34
      src/Translator.php

@ -0,0 +1,12 @@
<?php
class A
{
public function f(int ...$args): void {}
}
class B extends A
{
public function f(int $a = 1, int ...$rest): void {}
}
function main() {}

@ -0,0 +1,12 @@
<?php
class A
{
public function f(int ...$args): void {}
}
class B extends A
{
public function f(int $a = 0): void {}
}
function main() {}

@ -0,0 +1,26 @@
<?php
class A
{
public function f(int $a, int $b): int
{
return $a + $b;
}
}
class B extends A
{
public function f(int ...$args): int
{
return 0;
}
}
class C extends A
{
public function f(int $a, int ...$rest): int
{
return $a;
}
}
function main() {}

@ -0,0 +1,12 @@
<?php
class A
{
public function f(int $a, string $b): void {}
}
class B extends A
{
public function f(int ...$args): void {}
}
function main() {}

@ -0,0 +1,12 @@
<?php
class A
{
public function f(int &$a): void {}
}
class B extends A
{
public function f(int ...$args): void {}
}
function main() {}

@ -113,9 +113,11 @@ class InheritanceErrorTest extends TestCase
$this->exec('must be compatible', 'inheritance_error_byref.php'); $this->exec('must be compatible', 'inheritance_error_byref.php');
} }
public function testVariadicMismatch() public function testTrailingVariadicMayAbsorbParentParameter()
{ {
$this->exec('must be compatible', 'inheritance_error_variadic.php'); // Zend accepts a trailing child variadic standing in for the remaining
// parent parameter positions (zend_do_perform_implementation_check).
$this->assertCompiles('inheritance_error_variadic.php');
} }
public function testMethodVisibilityMismatch() public function testMethodVisibilityMismatch()

@ -0,0 +1,48 @@
<?php
use TypePhp\Exception\TestError;
/**
* Zend's inheritance check lets a trailing child variadic stand in for every
* remaining parent parameter position (the decorator pattern), provided the
* variadic's type is contravariant-compatible with each covered position and
* by-ref-ness matches. A variadic parent still requires a variadic child, and
* when the parent is variadic every extra child parameter is validated against
* the parent's variadic slot.
*/
class MethodOverrideVariadicTest extends BaseTest
{
public function testTrailingChildVariadicAbsorbsParentParameters(): void
{
$this->compile('override_variadic_absorbs_params.php');
}
public function testChildVariadicTypeMustCoverEveryAbsorbedPosition(): void
{
$this->exec(
'Declaration of `B::f()` must be compatible with `A::f()`',
'override_variadic_bad_type.php',
);
}
public function testChildVariadicMustMatchByRefOfAbsorbedPosition(): void
{
$this->exec(
'Declaration of `B::f()` must be compatible with `A::f()`',
'override_variadic_byref_mismatch.php',
);
}
public function testVariadicParentRequiresVariadicChild(): void
{
$this->exec(
'Declaration of `B::f()` must be compatible with `A::f()`',
'override_parent_variadic_child_not.php',
);
}
public function testExtraChildParametersCheckedAgainstParentVariadic(): void
{
$this->compile('override_parent_variadic_child_extra.php');
}
}

@ -4698,12 +4698,33 @@ CODE;
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
} }
// Compare each parent-declared parameter position. // A variadic parent accepts unbounded arguments, so Zend requires the
foreach ($parentFuncDef->argInfoList as $i => $parentArg) { // override to be variadic as well.
if (!isset($childFuncDef->argInfoList[$i])) { $parentVariadic = $parentFuncDef->hasVariadicArg();
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); $childVariadic = $childFuncDef->hasVariadicArg();
if ($parentVariadic && !$childVariadic) {
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
}
// Compare each parent-declared parameter position. Following Zend's
// inheritance check, a trailing child variadic stands in for every
// remaining parent position (the decorator pattern), and when the
// parent is variadic each extra child parameter is validated against
// the parent's variadic slot.
$positions = count($parentFuncDef->argInfoList);
if ($parentVariadic) {
$positions = max($positions, count($childFuncDef->argInfoList));
}
for ($i = 0; $i < $positions; $i++) {
$parentArg = $parentFuncDef->argInfoList[$i]
?? $parentFuncDef->argInfoList[count($parentFuncDef->argInfoList) - 1];
$childArg = $childFuncDef->argInfoList[$i] ?? null;
if ($childArg === null) {
if (!$childVariadic) {
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
}
$childArg = $childFuncDef->argInfoList[count($childFuncDef->argInfoList) - 1];
} }
$childArg = $childFuncDef->argInfoList[$i];
if ($parentArg->immutable && !$childArg->immutable) { if ($parentArg->immutable && !$childArg->immutable) {
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
} }
@ -4713,9 +4734,6 @@ CODE;
if ($childArg->byRef !== $parentArg->byRef) { if ($childArg->byRef !== $parentArg->byRef) {
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
} }
if ($childArg->variadic !== $parentArg->variadic) {
$this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass);
}
} }
// Any extra child parameters must be optional or variadic. // Any extra child parameters must be optional or variadic.

Loading…
Cancel
Save