From eb444b98befb7ba78a126d905a989783b8cd43e5 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Mon, 31 Aug 2026 12:38:21 +0200 Subject: [PATCH] fix(translator): let a trailing child variadic absorb parent parameters 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. --- .../override_parent_variadic_child_extra.php | 12 +++++ .../override_parent_variadic_child_not.php | 12 +++++ .../code/override_variadic_absorbs_params.php | 26 ++++++++++ phpunit/code/override_variadic_bad_type.php | 12 +++++ .../code/override_variadic_byref_mismatch.php | 12 +++++ phpunit/src/InheritanceErrorTest.php | 6 ++- phpunit/src/MethodOverrideVariadicTest.php | 48 +++++++++++++++++++ src/Translator.php | 34 +++++++++---- 8 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 phpunit/code/override_parent_variadic_child_extra.php create mode 100644 phpunit/code/override_parent_variadic_child_not.php create mode 100644 phpunit/code/override_variadic_absorbs_params.php create mode 100644 phpunit/code/override_variadic_bad_type.php create mode 100644 phpunit/code/override_variadic_byref_mismatch.php create mode 100644 phpunit/src/MethodOverrideVariadicTest.php diff --git a/phpunit/code/override_parent_variadic_child_extra.php b/phpunit/code/override_parent_variadic_child_extra.php new file mode 100644 index 00000000..2c6d7d46 --- /dev/null +++ b/phpunit/code/override_parent_variadic_child_extra.php @@ -0,0 +1,12 @@ +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() diff --git a/phpunit/src/MethodOverrideVariadicTest.php b/phpunit/src/MethodOverrideVariadicTest.php new file mode 100644 index 00000000..2773bfd4 --- /dev/null +++ b/phpunit/src/MethodOverrideVariadicTest.php @@ -0,0 +1,48 @@ +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'); + } +} diff --git a/src/Translator.php b/src/Translator.php index be6aa21d..d6e5c7a1 100644 --- a/src/Translator.php +++ b/src/Translator.php @@ -4684,12 +4684,33 @@ protected function validateMethodOverrideSignature( $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); } - // Compare each parent-declared parameter position. - foreach ($parentFuncDef->argInfoList as $i => $parentArg) { - if (!isset($childFuncDef->argInfoList[$i])) { - $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); + // A variadic parent accepts unbounded arguments, so Zend requires the + // override to be variadic as well. + $parentVariadic = $parentFuncDef->hasVariadicArg(); + $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) { $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); } @@ -4699,9 +4720,6 @@ protected function validateMethodOverrideSignature( if ($childArg->byRef !== $parentArg->byRef) { $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.