[RFC] [Vote] Deprecate functions with overloaded signatures

php.internals

Máté Kocsis

3 years ago
Hi Everyone, As previously announced on the list, I have just started the vote about the "Deprecate functions with overloaded signatures". Link to the RFC: https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures Link to the discussion thread: https://externals.io/message/120146 The vote is open until 2023-07-10 16:00:00 UTC. Regard, Máté

Marco Pivetta

3 years ago
Hey Máté, Only voted "NO" for `ReflectionProperty#setValue()`, mostly because any kind of pre-existing subtype will be kinda broken, but it isn't a major issue. It's fine if libraries drop support for PHP 8.2 before PHP 9.0 is out. Marco Pivetta https://mastodon.social/@ocramius https://ocramius.github.io/ On Mon, 26 Jun 2023 at 15:37, Máté Kocsis <kocsismate90@gmail.com> wrote:

Ben Ramsey

3 years ago
> On Jun 26, 2023, at 08:36, Máté Kocsis <kocsismate90@gmail.com> wrote: > > Hi Everyone, > > As previously announced on the list, I have just started the vote about the > "Deprecate functions with overloaded signatures". > > Link to the RFC: > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures > Link to the discussion thread: https://externals.io/message/120146 > > The vote is open until 2023-07-10 16:00:00 UTC. > > Regard, > Máté
Clarifying my “no” votes… I voted “no” on `array_keys()` because I do not see these as two different function signatures. To me, the single signature should look like this: function array_keys(array $array, ?mixed $filter_value = null, bool $strict = false): array {} I voted “no” on `IntlCalendar::set()` because it seems to me that `setDate()` and `setDateTime()` could share the same signature if `$hour`, `$minute`, and `$second` all default to zero, like this: public function setDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} In the same way, with `IntlGregorianCalendar::__construct()`, `createFromDate()` and `createFromDateTime()` could be combined as: public static function createFromDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} Cheers, Ben

Nikita Popov

3 years ago
On Mon, Jun 26, 2023, at 20:22, Ben Ramsey wrote:
> > On Jun 26, 2023, at 08:36, Máté Kocsis <kocsismate90@gmail.com> wrote: > > > > Hi Everyone, > > > > As previously announced on the list, I have just started the vote about the > > "Deprecate functions with overloaded signatures". > > > > Link to the RFC: > > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures > > Link to the discussion thread: https://externals.io/message/120146 > > > > The vote is open until 2023-07-10 16:00:00 UTC. > > > > Regard, > > Máté > > > Clarifying my “no” votes… > > I voted “no” on `array_keys()` because I do not see these as two different function signatures. To me, the single signature should look like this: > > function array_keys(array $array, ?mixed $filter_value = null, bool $strict = false): array {} > > I voted “no” on `IntlCalendar::set()` because it seems to me that `setDate()` and `setDateTime()` could share the same signature if `$hour`, `$minute`, and `$second` all default to zero, like this: > > public function setDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} > > In the same way, with `IntlGregorianCalendar::__construct()`, `createFromDate()` and `createFromDateTime()` could be combined as: > > public static function createFromDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {}
So commonality here is that these are all arity overloads. I think there's three different kinds of overloads in involved in the RFC: 1. static/non-static overloads. Support for this was dropped in PHP 8, but had to be retained internally just for that one usage in FII, so getting rid of that seems quite high value. 2. type/meaning overloads, where certain parameters change meaning entirely across overloads. These are incompatible with named parameters and result in very unclear function signatures. They only become intelligible once split into separate signatures, which is not something PHP supports. Removing these is also fairly high value. 3. arity overloads, where behavior depends on number of parameters, or certain parameter counts are forbidden, but the actual meaning of the parameters does not change. Equivalent to a func_num_args() check in userland code. I think these arity overloads are pretty harmless. The function signature is meaningful and compatible with named arguments. My overall inclination here is to vote No on all the deprecations that involve arity overloads and vote Yes on the remainder. Possibly I'm missing some kind of complication that the arity overloads are causing? Regards, Nikita

Theodore Brown

3 years ago
On Mon, June 26, 2023, at 14:43 Nikita Popov wrote: > On Mon, Jun 26, 2023, at 20:22, Ben Ramsey wrote: > > > > I voted “no” on `array_keys()` because I do not see these as two different function signatures. To me, the single signature should look like this: > > > > function array_keys(array $array, ?mixed $filter_value = null, bool $strict = false): array {} > > > > I voted “no” on `IntlCalendar::set()` because it seems to me that `setDate()` and `setDateTime()` could share the same signature if `$hour`, `$minute`, and `$second` all default to zero, like this: > > > > public function setDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} > > > > In the same way, with `IntlGregorianCalendar::__construct()`, `createFromDate()` and `createFromDateTime()` could be combined as: > > > > public static function createFromDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} > > So commonality here is that these are all arity overloads. > > I think there's three different kinds of overloads in involved in the RFC: > > 1. static/non-static overloads. Support for this was dropped in PHP 8, but had to be retained internally just for that one usage in FII, so getting rid of that seems quite high value. > > 2. type/meaning overloads, where certain parameters change meaning entirely across overloads. These are incompatible with named parameters and result in very unclear function signatures. They only become intelligible once split into separate signatures, which is not something PHP supports. Removing these is also fairly high value. > > 3. arity overloads, where behavior depends on number of parameters, or certain parameter counts are forbidden, but the actual meaning of the parameters does not change. Equivalent to a func_num_args() check in userland code. I think these arity overloads are pretty harmless. The function signature is meaningful and compatible with named arguments. > > My overall inclination here is to vote No on all the deprecations that involve arity overloads and vote Yes on the remainder. > > Possibly I'm missing some kind of complication that the arity overloads are causing? Currently the following code returns only the array keys that loosely equal null [1]: array_keys($array, null) If the function is changed to a single signature with $filter_value defaulting to null (with an argument count check to preserve the above functionality), its behavior becomes a lot more surprising/unexpected. If a parameter defaults to null, as a user I don't expect the function to work differently when I explicitly pass the default value. To prevent confusion, the function would at least still need to be documented with two separate signatures in the PHP manual, which goes against one of the RFC goals of automatically updating signatures based on stubs. So from my perspective there is value in deprecating arity overloads: it avoids confusing behavior changes and the need to manually keep the overloaded signatures documented and in sync in the manual. Regards, Theodore [1]: https://3v4l.org/Sld4S

Rowan Collins

3 years ago
On 27/06/2023 02:25, Theodore Brown wrote:
> Currently the following code returns only the array keys that loosely equal null [1]: > > array_keys($array, null) > > If the function is changed to a single signature with $filter_value defaulting to null (with an argument count check to preserve the above functionality), its behavior becomes a lot more surprising/unexpected. If a parameter defaults to null, as a user I don't expect the function to work differently when I explicitly pass the default value.
An alternative solution to this situation is to introduce a new dummy value for such parameters, similar to how JavaScript uses the special "Symbol" type to have keys that can't collide with any userland value. In this context, this could be achieved with a single-value enum, i.e.: enum ArrayKeys { case IncludeAll; } function array_keys(array $array, mixed $filter_value = ArrayKeys::IncludeAll, bool $strict = false): array {} That way, the optional parameter has a real default value, distinct from any value the user might wish to filter by, including null, and full compatibility is maintained with existing code. I don't know whether this is a direction we want to go, but thought I'd throw it out there. Regards,
-- Rowan Tommins [IMSoP]

Theodore Brown

3 years ago
On Tue, June 27, 2023, 6:33 Rowan Tommins wrote: > On 27/06/2023 02:25, Theodore Brown wrote: > > Currently the following code returns only the array keys that loosely equal null: > > > > array_keys($array, null) > > > > If the function is changed to a single signature with $filter_value defaulting to null (with an argument count check to preserve the above functionality), its behavior becomes a lot more surprising/unexpected. If a parameter defaults to null, as a user I don't expect the function to work differently when I explicitly pass the default value. > > > An alternative solution to this situation is to introduce a new dummy > value for such parameters, similar to how JavaScript uses the special > "Symbol" type to have keys that can't collide with any userland value. > In this context, this could be achieved with a single-value enum, i.e.: > > enum ArrayKeys { case IncludeAll; } > > function array_keys(array $array, mixed $filter_value = > ArrayKeys::IncludeAll, bool $strict = false): array {} > > That way, the optional parameter has a real default value, distinct from > any value the user might wish to filter by, including null, and full > compatibility is maintained with existing code. > > I don't know whether this is a direction we want to go, but thought I'd > throw it out there. That's an interesting idea, but I'm not sure it would make the behavior any less confusing. Are single-value enums used like this anywhere else in PHP? As a user my expectation when seeing `ArrayKeys::IncludeAll` as the default is that there must be other `ArrayKeys` enum values for additional options. Note that currently there is a draft RFC to support objects in array keys. [1] If that feature is accepted, would it not be allowed to filter array keys by the `ArrayKeys::IncludeAll` enum value? That would also be surprising/unexpected. Based on the current vote tally, `array_keys` looks like it will be the only remaining function in PHP 9 with an overloaded signature, which would make it all the more strange. [1]: https://wiki.php.net/rfc/object_keys_in_arrays

Theodore Brown

3 years ago
On Thu June 29, 2023 at 11:51 AM Theodore Brown wrote: > On Tue, June 27, 2023, 6:33 Rowan Tommins wrote: > > On 27/06/2023 02:25, Theodore Brown wrote: > > > Currently the following code returns only the array keys that loosely equal null: > > > > > > array_keys($array, null) > > > > > > If the function is changed to a single signature with $filter_value defaulting to null (with an argument count check to preserve the above functionality), its behavior becomes a lot more surprising/unexpected. If a parameter defaults to null, as a user I don't expect the function to work differently when I explicitly pass the default value. > > > > > > An alternative solution to this situation is to introduce a new dummy > > value for such parameters, similar to how JavaScript uses the special > > "Symbol" type to have keys that can't collide with any userland value. > > In this context, this could be achieved with a single-value enum, i.e.: > > > > enum ArrayKeys { case IncludeAll; } > > > > function array_keys(array $array, mixed $filter_value = > > ArrayKeys::IncludeAll, bool $strict = false): array {} > > > > That way, the optional parameter has a real default value, distinct from > > any value the user might wish to filter by, including null, and full > > compatibility is maintained with existing code. > > ...Note that currently there is a draft RFC to support objects in array keys. [1] If that feature is accepted, would it not be allowed to filter array keys by the `ArrayKeys::IncludeAll` enum value? That would also be surprising/unexpected. I must have been really tired when I wrote the above paragraph, because I forgot that the `filter_value` parameter matches against the value, not the key (so the Object Keys in Arrays RFC has nothing to do with it). But this makes using an enum as the default `filter_value` even more problematic, since enums can already be used as array values and this would prevent filtering an array to return keys for this value. I.e. the following code would stop working: https://3v4l.org/5Br1a So I still believe the overloaded array_keys signature should be deprecated for consistency and to avoid confusing behavior. Best regards, Theodore [1]: https://wiki.php.net/rfc/object_keys_in_arrays

A.L.E.C

3 years ago
On 26.06.2023 20:22, Ben Ramsey wrote:
> I voted “no” on `IntlCalendar::set()` because it seems to me that `setDate()` and `setDateTime()` could share the same signature if `$hour`, `$minute`, and `$second` all default to zero, like this: > > public function setDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {} > > In the same way, with `IntlGregorianCalendar::__construct()`, `createFromDate()` and `createFromDateTime()` could be combined as: > > public static function createFromDate(int $year, int $month, int $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {}
I also voted No on IntlCalendar and IntlGregorianCalendar changes. The proposed solution is not good enough. IntlGregorianCalendar has already a static fromDateTime() method inherited from IntlCalendar, so a new method is not needed, imo. Also, setTime() method already exists (not the best name), so addition of setDate() and setDateTime() will make it worse.
-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net] ---------------------------------------------------- PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

Máté Kocsis

3 years ago
Hi Everyone, The votes have ended with the following results: - array_keys: rejected (13 vs. 1 vs. 15 votes) - DatePeriod::__construct(): accepted, to be removed in PHP 9.0 (23 vs. 0 vs. 0 votes) - dba_fetch(): accepted, to be removed in PHP 9.0 (24 vs. 0 votes) - FFI::cast(), FFI::new(), and FFI::type(): accepted, to be removed in PHP 9.0 (19 vs. 0 votes) - get_class() and get_parent_class(): accepted, to be removed in PHP 9.0 (22 vs. 0 vs. 0 votes) - IntlCalendar::set(): accepted, to be removed in PHP 9.0 (16 vs. 3 vs. 4 votes) - IntlGregorianCalendar::__construct(): accepted, to be removed in PHP 9.0 (16 vs. 3 vs. 4 votes) - ldap_connect(): accepted, to be removed in PHP 9.0 (21 vs. 0 votes) - ldap_exop(): accepted, to be removed in PHP 9.0 (19 vs. 0 votes) - pg_fetch_result(), pg_field_prtlen(), and pg_field_is_null(): accepted, to be removed in PHP 9.0 (19 vs. 1 vs. 0 votes) - Phar::setStub(): accepted, to be removed in PHP 9.0 (20 vs. 0 votes) - ReflectionMethod::__construct(): accepted, to be removed in PHP 9.0 (18 vs. 1 vs. 0 votes) - ReflectionProperty::setValue(): accepted, to be removed in PHP 9.0 (16 vs. 5 votes) - session_set_save_handler(): accepted, to be removed in PHP 9.0 (17 vs. 4 votes vs. 1 votes) - stream_context_set_option(): accepted, to be removed in PHP 9.0 (18 vs. 5 votes vs. 1 votes) - Policy about new functions: accepted (24 vs. 0 votes) Thanks to everyone for their participation! Regards, Máté