Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111179
To: internals@lists.php.net
References: <CABHuAWx16_6rwye6BC4gUctSEdTTV0bq68S=1znNPykwGzXTsQ@mail.gmail.com>
Message-ID: <0806a844-d93f-ab87-48eb-ad8a6be1ee69@gmail.com>
Date: Fri, 24 Jul 2020 18:53:44 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <CABHuAWx16_6rwye6BC4gUctSEdTTV0bq68S=1znNPykwGzXTsQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-GB
Subject: Re: [PHP-DEV] [RFC][Proposal] Renamed parameters
From: rowan.collins@gmail.com (Rowan Tommins)

On Fri, 24 Jul 2020 at 12:12, Chris Riley <t.carnage@gmail.com 
<mailto:t.carnage@gmail.com>> wrote:

    The named parameters RFC has been accepted, despite significant
    objections
    from maintainers of larger OSS projects



Do you have a reference for which "larger OSS projects" you are 
referring to? I don't remember any being named in the previous 
discussion, but I may well have missed it.


    It is likely that the way this will shake out is that some
    maintainers will
    accept the additional overhead of including parameter names in their BC
    guidelines and others will not, this leaves users unsure if they can use
    the new feature without storing up issues in potentially minor/security
    releases of the libraries they use. This is not really an ideal
    situation.




To reiterate a point I mentioned a couple of days ago, library authors 
having to define what constitutes a BC break is definitely not a new 
thing. The most obvious example in PHP is that we don't have any package 
visibility enforced by the language, so it is entirely commonplace for a 
library to have entire classes which are considered implementation 
details and not for direct use.

More subtly, any _behaviour_ change can technically be considered a BC 
break, and nearly all of those changes are completely unenforceable at 
the language level; compatibility is _always_ defined by documentation, 
not just code.

I sympathise with people who don't want their library used with named 
parameters, or want to choose the names more carefully first, but the 
overhead for most projects is adding one line to a README file saying 
so. The PHP manual could equally point out the risks of using the 
feature with third-party code.


    More pressing a point is that the current implementation breaks object
    polymorphism.




Although seemingly separate, this can be seen as an extension of the 
previous point: if something isn't designed to be used with named 
parameters, they should be used with care.

It was also discussed in detail before the vote, and is the subject of 
two entire sections of the RFC: one detailing the selected behaviour 
https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance 
and another detailing the rejected alternatives: 
https://wiki.php.net/rfc/named_params#to_parameter_name_changes_during_inheritance 
as well as linking to a survey of how other languages approach the 
problem: https://externals.io/message/109549#109581



> The first would be to implement it as proposed above, this would allow any
> parameter to be called by name regardless of the intentions of the author
> of the method/function and is closest to the current behaviour.



This would be a useful extension of the feature, and with the right 
syntax (e.g. Benjamin's suggestion of an attribute) could be added in 
8.1, just as scalar types and return types where extended with nullable 
types and void returns in 7.1.


    The second option would be to use this syntax to make named
    parameters in
    userland code explicitly opt in. 




As others have pointed out, this was explicitly discussed in the run-up 
to the original RFC, and anyone who wanted opt-in named parameters could 
have voted No to the version Nikita put to the vote. Assuming all the 
same people vote, it would need 32 people who voted Yes to the current 
proposal to change their minds and back this alternative.



    There are pros and cons to this second approach, on the one hand it
    reduces
    the usefulness of the named parameter syntax by requiring changes to old
    code to enable it (although this could probably be automated fairly
    easily)




The big disadvantage that persuaded me against this approach is that it 
would mean code cannot support both named parameters and PHP 7.x. Since 
named parameters are likely to be most useful in shared libraries, and 
those libraries are likely to support a range of PHP versions, this 
means users would have to wait for:
- the library to drop support for PHP 7
- THEN the library maintainer to add named parameters (likely to be 
alongside other breaking changes)
- THEN the user's application to be compatible with that version of the 
library

While a long lead-time for new features is sometimes necessary, the 
alternative here doesn't seem bad enough to justify this kind of delay.


 > I've never written any code in swift, however having quickly read the
 > documentation I think that my proposal is almost identical to swift
 > argument labels if the first option is chosen.


Swift's argument labels are actually only superficially "named 
parameters", and come from a completely different background, as I 
explained here: https://externals.io/message/110004#110025  They don't 
make a particularly good comparison for PHP.



Regards,
-- 
Rowan Tommins
[IMSoP]