From 0c69b8b357eb2c1edcafdeda248fb6e321fb9f34 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Wed, 2 Sep 2026 02:06:19 +0200 Subject: [PATCH] fix(translator): allow overrides to add a by-ref return (#53) --skip-tests * test(phpunit): anchor TypePhp autoloading to the current checkout A git worktree shares vendor/ with the primary checkout via a symlink, and Composer's generated autoloader resolves the TypePhp\ prefix relative to the realpath of vendor/. The suite then silently loads and tests the OTHER checkout's src/ tree. Prepend an autoloader anchored to this checkout so the tests always exercise the sources they ship with; in a standalone checkout this is a no-op. * fix(translator): allow overrides to add a by-ref return Zend's inheritance check treats return-by-reference as covariant (zend_do_perform_implementation_check): an error is raised only when the parent returns by reference and the child does not. The child adding `&` is a strictly stronger guarantee and is accepted: class A { public function f(): array {} } class B extends A { public function &f(): array {} } // OK in Zend validateMethodOverrideSignature compared returnsByRef with exact equality, rejecting this valid program. Make the check one-directional; dropping a parent's by-ref return remains fatal. --- phpunit/bootstrap.php | 15 ++++++++++++ phpunit/code/override_byref_return_added.php | 19 +++++++++++++++ .../code/override_byref_return_dropped.php | 19 +++++++++++++++ phpunit/src/MethodOverrideByRefReturnTest.php | 24 +++++++++++++++++++ src/Translator.php | 5 +++- 5 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 phpunit/code/override_byref_return_added.php create mode 100644 phpunit/code/override_byref_return_dropped.php create mode 100644 phpunit/src/MethodOverrideByRefReturnTest.php diff --git a/phpunit/bootstrap.php b/phpunit/bootstrap.php index 1eeae725..8506cc64 100644 --- a/phpunit/bootstrap.php +++ b/phpunit/bootstrap.php @@ -5,6 +5,21 @@ use TypePhp\CompilerTest; use TypePhp\Exception\TestError; require __DIR__ . '/../bin/bootstrap.php'; + +// The vendor directory may be shared between checkouts (e.g. a git worktree +// with a symlinked vendor/). Composer's autoloader resolves TypePhp\ against +// the checkout that owns vendor/, which would silently test another tree's +// sources. Prepend a loader anchored to THIS checkout so the test suite always +// exercises the code it ships with. +spl_autoload_register(static function (string $class): void { + if (str_starts_with($class, 'TypePhp\\')) { + $path = dirname(__DIR__) . '/src/' . str_replace('\\', '/', substr($class, strlen('TypePhp\\'))) . '.php'; + if (is_file($path)) { + require $path; + } + } +}, true, true); + require_once __DIR__ . '/../src/polyfills.php'; require __DIR__ . '/../src/gen_stub.php'; diff --git a/phpunit/code/override_byref_return_added.php b/phpunit/code/override_byref_return_added.php new file mode 100644 index 00000000..dc0c5883 --- /dev/null +++ b/phpunit/code/override_byref_return_added.php @@ -0,0 +1,19 @@ +compile('override_byref_return_added.php'); + } + + public function testOverrideCannotDropByRefReturn(): void + { + $this->exec( + 'Declaration of `B::f()` must be compatible with `A::f()`', + 'override_byref_return_dropped.php', + ); + } +} diff --git a/src/Translator.php b/src/Translator.php index be6aa21d..fbadc576 100644 --- a/src/Translator.php +++ b/src/Translator.php @@ -4674,7 +4674,10 @@ CODE; )) { $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); } - if ($childFuncDef->returnsByRef !== $parentFuncDef->returnsByRef) { + // Zend treats by-ref returns as covariant: an override may add `&` + // (callers expecting a value still work), but it must not drop one + // promised by the parent contract. + if ($parentFuncDef->returnsByRef && !$childFuncDef->returnsByRef) { $this->fatalMethodOverrideIncompatible($v, $className, $methodName, $parentClass); }