On Tue, Nov 9, 2021 at 2:11 AM Jeremy Mikola <jmik...@gmail.com> wrote:
> > https://github.com/php/php-src/commit/b302bfabe7fadd07b022ee30aacf7f41ab1ae0fa > recently added automatic implementation of Stringable ( > https://wiki.php.net/rfc/stringable) for internal classes. This introduced > fatal errors for each existing __toString() in ext-mongodb: > > ``` > Declaration of MongoDB\BSON\BinaryInterface::__toString() must be > compatible with Stringable::__toString(): string in Unknown on line 0 > Declaration of MongoDB\BSON\Binary::__toString() must be compatible with > Stringable::__toString(): string in Unknown on line 0 > ... > ``` > > Our arginfos for these methods have historically never reported a return > type, going back to when it was originally written for PHP 5.x. For > example: > > ``` > ZEND_BEGIN_ARG_INFO_EX(ai_BinaryInterface_void, 0, 0, 0) > ZEND_END_ARG_INFO() > > static zend_function_entry php_phongo_binary_interface_me[] = { > ZEND_ABSTRACT_ME(BinaryInterface, __toString, ai_BinaryInterface_void) > // ... > ``` > > I noted that _ZendTestClass (referenced in the original commit's tests) > uses ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX to declare a string return > type ( > > https://github.com/php/php-src/blob/eda9d7ac22c70f75a44bf33139acf8c812d69944/ext/zend_test/test_arginfo.h#L74 > ). > > While that's a trivial change we can make in ext-mongodb, I wonder if this > may have been an unanticipated BC break for third-party extensions. I > imagine ext-mongodb is not the only extension with older arginfo > declarations predating the introduction of type reporting in later PHP > versions. > Thanks for pointing this out! In https://github.com/php/php-src/commit/a551b083073ea08f8fc53b0e1a6380b6de26cc6b I've added a hack to add the string return type if it is missing and thus make the signature compatible with the interface. That should address the immediate compatibility issue. In https://github.com/php/php-src/commit/86379b6710f972e0d4a11c89ce28d5768d9824d3 I have added a warning if this happens. The warning is only for master (i.e. PHP 8.2) to make sure that extensions add the type eventually, and we can drop this workaround. Regards, Nikita
