Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-08-09 Thread Rowan Tommins

On 09/08/2020 18:14, Chris Riley wrote:

Hi all,

RFC link: https://wiki.php.net/rfc/renamed_parameters

I have spent the weekend working on a final revision for this RFC to
address several of the points brought up.



My opinion on this RFC remains unchanged:

- It would have been a reasonable alternative proposal to bring to the 
list 3 months ago [1] when Nikita formally resurrected his RFC
- The problem it highlights would be a suitable reason to vote "No" to 
that proposal


But:

- It is effectively a complete replacement for, not a minor amendment 
of, the previous RFC
- The solution it proposes was discussed on the list at least once but 
didn't receive substantial support [2]
- It doesn't present any evidence that those who voted Yes to the 
previous RFC [3] were missing information, have changed their minds, or 
voted for a proposal they thought was non-optimal, in sufficient numbers 
to reverse a majority of 39.



It is true that the behaviour of variadic parameters wasn't discussed 
very thoroughly, and some concerns with it were raised during voting 
[4]. An RFC amending or removing *that* behaviour would feel like a more 
reasonable amendment, since it doesn't change the core feature that has 
been overwhelmingly accepted.



[1] https://externals.io/message/110004
[2] https://externals.io/message/109549#109558 (I used :$foo in my 
examples rather than $:foo, but it's otherwise an identical proposal I 
think)

[3] https://wiki.php.net/rfc/named_params#vote
[4] https://externals.io/message/110910#110961


Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-08-09 Thread Benas IML
After sending out the email, I found out that I replied to the wrong email,
nevermind... Sorry about that!

Best regards,
Benas

On Sun, Aug 9, 2020, 8:25 PM Benas IML  wrote:

> Hey Chris,
>
> thanks for the RFC but I'd like to remind you that we are already past the
> feature freeze. Moreover, it seems that you don't have an actual
> implementation as well.
>
> Best regards,
> Benas
>
>
> On Sun, Aug 9, 2020, 8:15 PM Chris Riley  wrote:
>
>> Hi all,
>>
>> RFC link: https://wiki.php.net/rfc/renamed_parameters
>>
>> I have spent the weekend working on a final revision for this RFC to
>> address several of the points brought up. Specifically:
>>
>> - Cut down the scope of the RFC to only focus on delivering an opt in to
>> named parameters, with the renaming ability explicitly pushed into future
>> scope as opposed to leaving it to the RM discretion. This will allow a
>> fuller discussion on how best to support renaming going forward, be it
>> using my proposed syntax or using attributes.
>> - Focused in on the technical issues as opposed to subjective arguments
>> for/against.
>> - Added additional example of the polymorphism problem when it comes to
>> the
>> behaviour of variadics, which didn't seem to come up in the original RFC.
>> - Added resolution on dealing with the PHP standard library, as this is an
>> enhancement to the original named parameters RFC, this RFC proposes to opt
>> in the PHP standard library. There are a couple of minor changes to
>> behaviour for classes which extend built ins to make upgrading smoother.
>> - Clarified changes to reflection parameter
>> - Added alternative implementation option using attributes which will be
>> the subject of an additional voting option for those who would prefer to
>> go
>> down this route.
>>
>> I think the RFC now represents a concrete problem and solution suitable
>> for
>> voting on, but I'm going to leave discussion open for a few days in case
>> anyone has any queries on the revisions.
>>
>> Regards,
>> Chris
>>
>>
>> On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:
>>
>> > Hi all,
>> >
>> > The named parameters RFC has been accepted, despite significant
>> objections
>> > from maintainers of larger OSS projects due to the overhead it adds to
>> > maintaining backwards compatibility as it has now made method/function
>> > parameter names part of the API; a change to them would cause a BC break
>> > for any library users who decide to use the new feature.
>> >
>> > 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.
>> >
>> > More pressing a point is that the current implementation breaks object
>> > polymorphism. Consider this example (simplified from one of my
>> codebases)
>> >
>> > interface Handler {
>> > public function handle($message);
>> > }
>> >
>> > class RegistrationHandler implements Handler {
>> > public function handle($registraionCommand);
>> > }
>> >
>> > class ForgottenPasswordHandler implements Handler {
>> > public function handle($forgottenPasswordCommand);
>> > }
>> >
>> > class MessageBus {
>> > //...
>> > public function addHandler(string $message, Handler $handler) {
>> //... }
>> > public function getHandler(string $messageType): Handler { //... }
>> > public function dispatch($message)
>> > {
>> > $this->getHandler(get_class($message))->handle(message:
>> $message);
>> > }
>> > }
>> >
>> > This code breaks at run time.
>> >
>> > Proposals were made for resolutions to this issue however all of them
>> > require trade offs and could potentially break existing code.
>> >
>> > My proposal to resolve these two issues is to add the ability to rename
>> > parameters with a new syntax as follows.
>> >
>> > function callBar(Foo $internalName:externalName) {
>> > $internalName->bar();
>> > }
>> >
>> > $x = new Foo();
>> > callBar(externalName: $x);
>> >
>> > This allows both the above problems to be resolved, by renaming the
>> > internal parameter and keeping the external signature the same.
>> >
>> > I propose that the RFC would have two voting options.
>> >
>> > 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.
>> >
>> > The second option would be to use this syntax to make named parameters
>> in
>> > userland code explicitly opt in. As such an additional shortcut syntax
>> > would be implemented: $: to designate a named parameter. eg
>> >
>> > function callBar($:externalName) {
>> > $externalName->bar();
>> > }
>> >
>> > $x = new Foo();
>> > 

Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-08-09 Thread Benas IML
Hey Chris,

thanks for the RFC but I'd like to remind you that we are already past the
feature freeze. Moreover, it seems that you don't have an actual
implementation as well.

Best regards,
Benas


On Sun, Aug 9, 2020, 8:15 PM Chris Riley  wrote:

> Hi all,
>
> RFC link: https://wiki.php.net/rfc/renamed_parameters
>
> I have spent the weekend working on a final revision for this RFC to
> address several of the points brought up. Specifically:
>
> - Cut down the scope of the RFC to only focus on delivering an opt in to
> named parameters, with the renaming ability explicitly pushed into future
> scope as opposed to leaving it to the RM discretion. This will allow a
> fuller discussion on how best to support renaming going forward, be it
> using my proposed syntax or using attributes.
> - Focused in on the technical issues as opposed to subjective arguments
> for/against.
> - Added additional example of the polymorphism problem when it comes to the
> behaviour of variadics, which didn't seem to come up in the original RFC.
> - Added resolution on dealing with the PHP standard library, as this is an
> enhancement to the original named parameters RFC, this RFC proposes to opt
> in the PHP standard library. There are a couple of minor changes to
> behaviour for classes which extend built ins to make upgrading smoother.
> - Clarified changes to reflection parameter
> - Added alternative implementation option using attributes which will be
> the subject of an additional voting option for those who would prefer to go
> down this route.
>
> I think the RFC now represents a concrete problem and solution suitable for
> voting on, but I'm going to leave discussion open for a few days in case
> anyone has any queries on the revisions.
>
> Regards,
> Chris
>
>
> On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:
>
> > Hi all,
> >
> > The named parameters RFC has been accepted, despite significant
> objections
> > from maintainers of larger OSS projects due to the overhead it adds to
> > maintaining backwards compatibility as it has now made method/function
> > parameter names part of the API; a change to them would cause a BC break
> > for any library users who decide to use the new feature.
> >
> > 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.
> >
> > More pressing a point is that the current implementation breaks object
> > polymorphism. Consider this example (simplified from one of my codebases)
> >
> > interface Handler {
> > public function handle($message);
> > }
> >
> > class RegistrationHandler implements Handler {
> > public function handle($registraionCommand);
> > }
> >
> > class ForgottenPasswordHandler implements Handler {
> > public function handle($forgottenPasswordCommand);
> > }
> >
> > class MessageBus {
> > //...
> > public function addHandler(string $message, Handler $handler) {
> //... }
> > public function getHandler(string $messageType): Handler { //... }
> > public function dispatch($message)
> > {
> > $this->getHandler(get_class($message))->handle(message:
> $message);
> > }
> > }
> >
> > This code breaks at run time.
> >
> > Proposals were made for resolutions to this issue however all of them
> > require trade offs and could potentially break existing code.
> >
> > My proposal to resolve these two issues is to add the ability to rename
> > parameters with a new syntax as follows.
> >
> > function callBar(Foo $internalName:externalName) {
> > $internalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x);
> >
> > This allows both the above problems to be resolved, by renaming the
> > internal parameter and keeping the external signature the same.
> >
> > I propose that the RFC would have two voting options.
> >
> > 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.
> >
> > The second option would be to use this syntax to make named parameters in
> > userland code explicitly opt in. As such an additional shortcut syntax
> > would be implemented: $: to designate a named parameter. eg
> >
> > function callBar($:externalName) {
> > $externalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x);
> >
> > If a parameter is not opted in, a compile time error is raised:
> >
> > function callBar($externalName) {
> > $externalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x); // Error: cannot call parameter $externalName
> > by name.
> >
> > There are pros and cons to this second 

[PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-08-09 Thread Chris Riley
Hi all,

RFC link: https://wiki.php.net/rfc/renamed_parameters

I have spent the weekend working on a final revision for this RFC to
address several of the points brought up. Specifically:

- Cut down the scope of the RFC to only focus on delivering an opt in to
named parameters, with the renaming ability explicitly pushed into future
scope as opposed to leaving it to the RM discretion. This will allow a
fuller discussion on how best to support renaming going forward, be it
using my proposed syntax or using attributes.
- Focused in on the technical issues as opposed to subjective arguments
for/against.
- Added additional example of the polymorphism problem when it comes to the
behaviour of variadics, which didn't seem to come up in the original RFC.
- Added resolution on dealing with the PHP standard library, as this is an
enhancement to the original named parameters RFC, this RFC proposes to opt
in the PHP standard library. There are a couple of minor changes to
behaviour for classes which extend built ins to make upgrading smoother.
- Clarified changes to reflection parameter
- Added alternative implementation option using attributes which will be
the subject of an additional voting option for those who would prefer to go
down this route.

I think the RFC now represents a concrete problem and solution suitable for
voting on, but I'm going to leave discussion open for a few days in case
anyone has any queries on the revisions.

Regards,
Chris


On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:

> Hi all,
>
> The named parameters RFC has been accepted, despite significant objections
> from maintainers of larger OSS projects due to the overhead it adds to
> maintaining backwards compatibility as it has now made method/function
> parameter names part of the API; a change to them would cause a BC break
> for any library users who decide to use the new feature.
>
> 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.
>
> More pressing a point is that the current implementation breaks object
> polymorphism. Consider this example (simplified from one of my codebases)
>
> interface Handler {
> public function handle($message);
> }
>
> class RegistrationHandler implements Handler {
> public function handle($registraionCommand);
> }
>
> class ForgottenPasswordHandler implements Handler {
> public function handle($forgottenPasswordCommand);
> }
>
> class MessageBus {
> //...
> public function addHandler(string $message, Handler $handler) { //... }
> public function getHandler(string $messageType): Handler { //... }
> public function dispatch($message)
> {
> $this->getHandler(get_class($message))->handle(message: $message);
> }
> }
>
> This code breaks at run time.
>
> Proposals were made for resolutions to this issue however all of them
> require trade offs and could potentially break existing code.
>
> My proposal to resolve these two issues is to add the ability to rename
> parameters with a new syntax as follows.
>
> function callBar(Foo $internalName:externalName) {
> $internalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> This allows both the above problems to be resolved, by renaming the
> internal parameter and keeping the external signature the same.
>
> I propose that the RFC would have two voting options.
>
> 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.
>
> The second option would be to use this syntax to make named parameters in
> userland code explicitly opt in. As such an additional shortcut syntax
> would be implemented: $: to designate a named parameter. eg
>
> function callBar($:externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> If a parameter is not opted in, a compile time error is raised:
>
> function callBar($externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x); // Error: cannot call parameter $externalName
> by name.
>
> 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) however it does provide a neater solution to the second problem in
> that, to prevent the runtime errors in the second issue example, every
> child class would need to use the rename syntax on it's parameter to
> prevent errors, whereas if we went down this route, the parent class could
> just not opt 

Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-27 Thread tyson andre
Hi internals,

Continuing on my response in https://externals.io/message/61#92 , and 
considering ways to enhance named arguments without a BC break
(while minimizing the impact on application/library authors that wish for their 
libraries not to be used with named parameters)

I was considering setting up a short straw poll on the wiki for a week with two 
questions, open for a week:

1. Whether voters consider it appropriate to add amendments/enhancements to 
named parameters (in general) in php 8.0 past the feature freeze. 
(Both/Backward Compatible Enhancements Only/No)
   (yes if interested in any of the alternatives proposed in 
https://externals.io/message/61)

   I'd recognize that named parameters are potentially a large change to what 
is considered the public API of a project,
   so I'd think continuing to add enhancements would be worthwhile, but I'd 
like to know where others stand on this.
   (e.g. if any proposals I made should be postponed to 8.1)

   I'd also think that implementing a backwards incompatible change after the 
feature freeze (in terms of impact on code targeting 8.0 alphas at the time of 
the feature freeze)
would be a bad precedent.

2. Interest in adding support for positional-only arguments in 8.0 or 8.1 (3 
options: 8.0, 8.1, or no)

   (e.g. with a symbol such as `/` to indicate that parameters or variadic 
parameters prior to the symbol are positional-only)

   I'd consider positional-only arguments useful in some cases, such as where 
the names would always be confusing,
   (or automatically generated code)

   - `function my_merge(string $firstArg, ...string $otherArgs, /) { }`
 This also provides an easy way for user-defined code to add restrictions 
similar to what `array_merge()` already has.
   - `function my_pow($x, $y, $z = null, /,) {}`
   - `function autoGeneratedCode($arg1, $arg2, /) {}

   Other syntaxes are possible, such as using attributes
   (I would find 5 attributes rather verbose if there were 5 positional-only 
parameters),
   or keywords such as `__END_POSITIONAL_PARAMETERS`.
   Nothing stood out as a good option (e.g. `_`, `...`, `%` seem meaningless, 
`*` would be the opposite of python, `#` can't be used),
   and I've only seen markers for the end of positional-only parameters in 
python after a quick check, so at least some users would find `/` easier to 
learn/remember.



On an unrelated note,

1. A few people had suggested adding a line to a README indicating that named 
parameters aren't supported.
An idea I had was to standardize on a machine-readable file format (e.g. 
".php_analysis_config.json") that IDEs/analyzers may choose to support.
It might have JSON entries such as `"supports_named_parameters": false` to 
indicate that code (e.g. src/main.php) using files in that directory (e.g. 
vendor/a/b/ with vendor/a/b/.php_analysis_config.json)
should not invoke functions/methods in vendor/a/b/ with named parameters,
because there is no guarantee the names will remain the same.
(TOML or ini settings might be more readable, but a complicated format requires 
extra dependencies and ini files won't support arrays if future settings get 
added)

- I can't think of many other settings I'd want there that aren't covered by 
composer.json, editorconfig, or other means.
  Maybe less importantly `"supports_classes_being_extended": bool`
- Alternately, it might be possible to put it in "extra" of composer.json,
  but some projects/libraries don't use composer.json (e.g. a project has both 
vendor/ and third-party/)
- I'm not aware of similar indicators for python for named arguments, so there 
might not be much interest in such a proposal. Then again, I think python had 
named arguments for much longer.

2. Another attribute idea I had was `<>` on a 
class/method,
to make PHP enforce that method overrides other than __construct must
have the same names in the same positions and not lead to errors when valid 
named arguments are passed to subclasses,
but I don't plan to propose that any earlier than 8.1
(e.g. for classes that have calls such as `$this->method(someFlag: true);`)

Thanks,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-26 Thread Rowan Tommins

Hi Chris,


On 26/07/2020 14:52, Chris Riley wrote:

Thanks for the feedback so far. In light of the feedback received both here
and privately, I've made 3 changes to the RFC document



Firstly, a reminder of the guideline in the RFC howto that the link to 
the RFC should be included in replies, which is particularly relevant 
when announcing changes to the text. For others trying to find it, it is 
here: https://wiki.php.net/rfc/renamed_parameters



Secondly, regardless of the merits of your proposal in itself, I think 
an RFC in this position should explicitly state why it is proposing to 
re-visit an accepted feature. I can think of a handful of possible 
reasons, but none seem to apply:


- If new concerns have come to light which are likely to change the 
opinion of those who voted Yes. This is not the case for the concerns in 
your introduction.
- If the RFC passed only by a narrow margin, or a low turnout, and this 
version is expected to gain a larger majority. The RFC passed with a 
ratio of 3:1, with 75 votes cast [1].
- If the RFC discussion was rushed, so that people did not have adequate 
time to understand the proposal and discuss its implications. The RFC 
was informally resurrected at the start of April [2] and formally at the 
start of May [3] and saw plenty of discussion.
- If there is evidence that people voted Yes despite reservations that 
this proposal resolves. No evidence is presented of this, and the only 
message of that sort in the voting thread expressed reservations 
unrelated to this proposal. [4]
- If there is evidence that (a significant number of) people who voted 
Yes have now changed their minds having re-considered the implications. 
No evidence is presented of this.



As Benjamin says, the pragmatic way forward would be to discuss 
enhancements on top of the accepted feature, rather than last-minute 
alternatives to it.



[1] This appears to be the second highest turnout after Scalar Type 
Declarations, according to https://php-rfc-watch.beberlei.de/

[2] https://externals.io/message/109549
[3] https://externals.io/message/110004
[4] https://externals.io/message/110910#110961

Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-26 Thread Benjamin Eberlei
On Sun, Jul 26, 2020 at 3:52 PM Chris Riley  wrote:

> Hi all,
>
> Thanks for the feedback so far. In light of the feedback received both here
> and privately, I've made 3 changes to the RFC document:
>
> 1. The original option 1, allowing renaming parameters but not requiring an
> explicit opt in to enable them to be called by name has been dropped. The
> proposal is now only for explicit opt in parameters with renaming as a
> possibility. The reasoning for this, is that although I included option 1
> as I thought it might be more likely to be accepted; many people pointed
> out that we are very close to the cutoff date for PHP 8.0 and that
> implementing such a change would likely be too big to get done in time.
> Option 1 would be possible to include in PHP 8.1 as it doesn't break BC,
> this means that the proposal can be brought back targeting 8.1 should
> option 2 not be accepted
>

Can you clarify if opt-in would only be the behavior on userland functions,
or if all internal functions would also change to opt-in?

If no, then the inconsistency must be explained.
if yes then the next steps for each internal API to decide on allowing
named params or not needs to be explained

Both approaches lead to a lot of problems down the road that were nicely
circumvented by auto-optin of all functions.

Implementation wise, with external name, you have only one name on the call
site. That means if you decide to rename an argument,
then you cannot support both the old and the new name for one major release
cycle to allow your users to upgrade smoothly.
As such it is my opinion that this is an inferior approach to having an
alias that allows declaring a second name.

Have you given an E_NOTICE/E_WARNING a thought? A user of a library could
then overwrite a library authors "with" to disallow named parameters.

Can you add an example how the MessageHandler problem would look like with
your proposal? Is the external name inherited? Is it forced to inherit? Can
it be overwritten?


> 2. With respect to the feature freeze date, I've added a possible strategy
> to deal with a staged implementation, should the release managers not feel
> comfortable including the full feature at this late stage.
>

Can you update the Proposed Voting Choices section with the two questions
for the staged implementation? or the one question combining them both? The
wording is going to be signifcant for further discussion.


> 3. I have documented the main objections to the RFC on the RFC itself and
> included my rebuttals; should anyone feel I've not represented their point
> fairly let me know and I'll update.
>

Your "objections" section mentions the inheritance / polymorphism part
being buried/hidden in the RFC, but its a section on its own
https://wiki.php.net/rfc/named_params#parameter_name_changes_during_inheritance
and Nikita sent several mails to the list about this topic, asking for
specific feedback about this outstanding issue alone.

The main change in your RFC from explicitly enabled for all userland and
internal functions to an opt-in approach for userland functions was also
discussed in depth.

Similar approaches to your suggestions were also mentioned again in the RFC
prominently under "Alternatives", overruled by the voted upon
implementation that got 76% acceptance.


>
> Regards,
> Chris
>

At this point I would much prefer to discuss amendments within the current
behavior and not against it.

My counter-proposal is still to have two, potentially three attributes:

- @@NameAlias to define a second name/alias for a parameter. Would help
solve both polymorphic name changes and refactoring of names.
- @@PositionalArgumentsOnly to define on a function or class, throwing an
exception if used with named arguments. This only slightly makes the use
case of named arguments slower.

Maybe the reverse with @@NamedArgumentsOnly - however checking for that
would probably slightly make all positional calls slower, which will
probably stay the primary way of calling functions. Trade offs are unclear
here.

I know attributes "feel" wrong here, being a new feature and not been used
for anything yet in the language. But attributes have the benefit of not
introducing new keywords or syntax to the language and are the right tool
to "re-configure" a feature from its primary behavior. Use of attributes
would also keep open future changes (by introducing new attributes or
arguments to the existing ones) leaving our options open instead of locking
them down further.


> On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:
>
> > Hi all,
> >
> > The named parameters RFC has been accepted, despite significant
> objections
> > from maintainers of larger OSS projects due to the overhead it adds to
> > maintaining backwards compatibility as it has now made method/function
> > parameter names part of the API; a change to them would cause a BC break
> > for any library users who decide to use the new feature.
> >
> > It is likely that the way this will 

Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-26 Thread tyson andre
Hi Chris Riley,

I agree with Rowan Tommin's arguments in 
https://externals.io/message/61#79 - I wanted named parameters by 
default.

Miscellaneous comments:

1. https://wiki.php.net/rfc/renamed_parameters should be moved to "In 
Discussion" on https://wiki.php.net/rfc/
2. I think that the RFC title should really mention "Making named parameters 
opt-in",
   since that part of the RFC has the largest impact.
   (e.g. "Renamed parameters and making named parameters opt-in").
   (The RFC URL should not be changed)
3. For your examples, I assume you mean "Class Bar extends Foo {" instead of 
"class Bar {"
4. "Error: cannot call parameter $externalName by name." seems incorrect,
I assume "cannot call callBar() with parameter $externalName by name" or 
something along those lines was intended
5. https://wiki.php.net/rfc/renamed_parameters#attributes still mentions 
"option 1" and "option 2", but those were removed from the current version of 
the proposal, making this confusing
6. How will this RFC expect internal functions such as substr_compare() or 
internal methods such as `ArrayObject::__construct()` included in php-src?

   What about PECLs - will existing function declaration macros be treated as 
opted out of or into named parameters?
7. This is missing some details on how reflection and backtrace generation will 
be affected.
   I assume `ReflectionParameter->getInternalName(): string`, 
`ReflectionParameter->getExternalName(): ?string`, etc. will need to be added.
   getTrace() and getTraceAsString()
8. Renaming parameters offers only a small performance benefit and I don't 
think it would get used very frequently.
It's possible to add `$newName = $oldName; unset($oldName);` (or in most 
cases, to update the method implementation body).
9. Are declarations such as `function test($:publicName, $nonPublicName) {}` an 
error?

   I'd personally prefer 
https://wiki.php.net/rfc/named_params#positional-only_and_named-only_parameters 
to allow API designers to explicitly opt out of named parameters.
10. As Rowin Tommins had said, "maintainers of larger OSS projects" is a broad 
claim and could be clarified
   (e.g. what fraction of maintainers? Were there polls/discussion threads of 
maintainers/owners of OSS projects?)

Since there are strong objections from some maintainers of supporting 
always-enabled named parameters,
I'd think a useful alternative would be to add a positional-only parameter 
syntax instead in 8.0, similar to what Python,
so that maintainers that want to avoid supporting named parameters in their API 
can clearly express that in a release requiring ^8.0.
This is using syntax for 
https://www.python.org/dev/peps/pep-0570/#history-of-positional-only-parameter-semantics-in-python
 for clarity,
but obviously other syntax might suit PHP better.
(mentioned in 
https://wiki.php.net/rfc/named_params#positional-only_and_named-only_parameters)

```
function test(int $x, /, string $y) {}
test(x: 1, y: "test");  // Error: test() does not support being called with 
parameter $x by name
test(1, y: "test");  // allowed
function test_varargs(...$args, /) {}
test_varargs(x: 1);  // Error: test_varargs() does not support being called 
with named variable argument $x in ...$args
test_varargs(...['x' => 1]);  // Error: test_varargs() does not support being 
called with named variable argument $x in ...$args
```

There may be concerns such as whether `/` can be added when overriding,
or in forbidding using `...$newArgs, /` to override `...$originalArgs`
but since parameter renaming was already allowed in the Named Arguments RFC 
this should not be a new issue.

Regards,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-26 Thread Chris Riley
Hi all,

Thanks for the feedback so far. In light of the feedback received both here
and privately, I've made 3 changes to the RFC document:

1. The original option 1, allowing renaming parameters but not requiring an
explicit opt in to enable them to be called by name has been dropped. The
proposal is now only for explicit opt in parameters with renaming as a
possibility. The reasoning for this, is that although I included option 1
as I thought it might be more likely to be accepted; many people pointed
out that we are very close to the cutoff date for PHP 8.0 and that
implementing such a change would likely be too big to get done in time.
Option 1 would be possible to include in PHP 8.1 as it doesn't break BC,
this means that the proposal can be brought back targeting 8.1 should
option 2 not be accepted.

2. With respect to the feature freeze date, I've added a possible strategy
to deal with a staged implementation, should the release managers not feel
comfortable including the full feature at this late stage.

3. I have documented the main objections to the RFC on the RFC itself and
included my rebuttals; should anyone feel I've not represented their point
fairly let me know and I'll update.

Regards,
Chris

On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:

> Hi all,
>
> The named parameters RFC has been accepted, despite significant objections
> from maintainers of larger OSS projects due to the overhead it adds to
> maintaining backwards compatibility as it has now made method/function
> parameter names part of the API; a change to them would cause a BC break
> for any library users who decide to use the new feature.
>
> 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.
>
> More pressing a point is that the current implementation breaks object
> polymorphism. Consider this example (simplified from one of my codebases)
>
> interface Handler {
> public function handle($message);
> }
>
> class RegistrationHandler implements Handler {
> public function handle($registraionCommand);
> }
>
> class ForgottenPasswordHandler implements Handler {
> public function handle($forgottenPasswordCommand);
> }
>
> class MessageBus {
> //...
> public function addHandler(string $message, Handler $handler) { //... }
> public function getHandler(string $messageType): Handler { //... }
> public function dispatch($message)
> {
> $this->getHandler(get_class($message))->handle(message: $message);
> }
> }
>
> This code breaks at run time.
>
> Proposals were made for resolutions to this issue however all of them
> require trade offs and could potentially break existing code.
>
> My proposal to resolve these two issues is to add the ability to rename
> parameters with a new syntax as follows.
>
> function callBar(Foo $internalName:externalName) {
> $internalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> This allows both the above problems to be resolved, by renaming the
> internal parameter and keeping the external signature the same.
>
> I propose that the RFC would have two voting options.
>
> 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.
>
> The second option would be to use this syntax to make named parameters in
> userland code explicitly opt in. As such an additional shortcut syntax
> would be implemented: $: to designate a named parameter. eg
>
> function callBar($:externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> If a parameter is not opted in, a compile time error is raised:
>
> function callBar($externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x); // Error: cannot call parameter $externalName
> by name.
>
> 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) however it does provide a neater solution to the second problem in
> that, to prevent the runtime errors in the second issue example, every
> child class would need to use the rename syntax on it's parameter to
> prevent errors, whereas if we went down this route, the parent class could
> just not opt into the named parameter syntax and the code would function as
> expected.
>
> Another advantage is that with the ability to rename parameters using the
> opt in, we gain some flexibility to tighten up the LSP rules relating to
> named 

Re: [PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-24 Thread Benjamin Eberlei
On Fri, Jul 24, 2020 at 4:00 PM Chris Riley  wrote:

> Hi all,
>
> Following up from this I've created a draft RFC:
> https://wiki.php.net/rfc/renamed_parameters will move to in discussion
> once
> I've ensured everything important has been captured.
>
> Regards,
> Chris
>

You added PHP 8.0 as a propsoed version, but that will not be possible
anymore 2 weeks of discussion + 2 weeks of voting are not possible to fit
in before the feature freeze, which is in 11 days.

>
> On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:
>
> > Hi all,
> >
> > The named parameters RFC has been accepted, despite significant
> objections
> > from maintainers of larger OSS projects due to the overhead it adds to
> > maintaining backwards compatibility as it has now made method/function
> > parameter names part of the API; a change to them would cause a BC break
> > for any library users who decide to use the new feature.
> >
> > 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.
> >
> > More pressing a point is that the current implementation breaks object
> > polymorphism. Consider this example (simplified from one of my codebases)
> >
> > interface Handler {
> > public function handle($message);
> > }
> >
> > class RegistrationHandler implements Handler {
> > public function handle($registraionCommand);
> > }
> >
> > class ForgottenPasswordHandler implements Handler {
> > public function handle($forgottenPasswordCommand);
> > }
> >
> > class MessageBus {
> > //...
> > public function addHandler(string $message, Handler $handler) {
> //... }
> > public function getHandler(string $messageType): Handler { //... }
> > public function dispatch($message)
> > {
> > $this->getHandler(get_class($message))->handle(message:
> $message);
> > }
> > }
> >
> > This code breaks at run time.
> >
> > Proposals were made for resolutions to this issue however all of them
> > require trade offs and could potentially break existing code.
> >
> > My proposal to resolve these two issues is to add the ability to rename
> > parameters with a new syntax as follows.
> >
> > function callBar(Foo $internalName:externalName) {
> > $internalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x);
> >
> > This allows both the above problems to be resolved, by renaming the
> > internal parameter and keeping the external signature the same.
> >
> > I propose that the RFC would have two voting options.
> >
> > 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.
> >
> > The second option would be to use this syntax to make named parameters in
> > userland code explicitly opt in. As such an additional shortcut syntax
> > would be implemented: $: to designate a named parameter. eg
> >
> > function callBar($:externalName) {
> > $externalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x);
> >
> > If a parameter is not opted in, a compile time error is raised:
> >
> > function callBar($externalName) {
> > $externalName->bar();
> > }
> >
> > $x = new Foo();
> > callBar(externalName: $x); // Error: cannot call parameter $externalName
> > by name.
> >
> > 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) however it does provide a neater solution to the second problem
> in
> > that, to prevent the runtime errors in the second issue example, every
> > child class would need to use the rename syntax on it's parameter to
> > prevent errors, whereas if we went down this route, the parent class
> could
> > just not opt into the named parameter syntax and the code would function
> as
> > expected.
> >
> > Another advantage is that with the ability to rename parameters using the
> > opt in, we gain some flexibility to tighten up the LSP rules relating to
> > named parameter inheritance.
> >
> > class Foo {
> > public function bar($:param) { //... }
> > public function baz($internal:external) { //... }
> > }
> >
> > // OK
> > class Bar {
> > public function bar($renamed:param) { //... }
> > public function baz($renamed:external) { //... }
> > }
> >
> > // Compile time error cannot rename named parameter $:param (renamed to
> > $:renamedParam)
> > class Baz {
> > public function bar($:renamedParam) { //... }
> > }
> >
> > // Compile time error cannot rename named parameter 

[PHP-DEV] Re: [RFC][Proposal] Renamed parameters

2020-07-24 Thread Chris Riley
Hi all,

Following up from this I've created a draft RFC:
https://wiki.php.net/rfc/renamed_parameters will move to in discussion once
I've ensured everything important has been captured.

Regards,
Chris

On Fri, 24 Jul 2020 at 12:12, Chris Riley  wrote:

> Hi all,
>
> The named parameters RFC has been accepted, despite significant objections
> from maintainers of larger OSS projects due to the overhead it adds to
> maintaining backwards compatibility as it has now made method/function
> parameter names part of the API; a change to them would cause a BC break
> for any library users who decide to use the new feature.
>
> 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.
>
> More pressing a point is that the current implementation breaks object
> polymorphism. Consider this example (simplified from one of my codebases)
>
> interface Handler {
> public function handle($message);
> }
>
> class RegistrationHandler implements Handler {
> public function handle($registraionCommand);
> }
>
> class ForgottenPasswordHandler implements Handler {
> public function handle($forgottenPasswordCommand);
> }
>
> class MessageBus {
> //...
> public function addHandler(string $message, Handler $handler) { //... }
> public function getHandler(string $messageType): Handler { //... }
> public function dispatch($message)
> {
> $this->getHandler(get_class($message))->handle(message: $message);
> }
> }
>
> This code breaks at run time.
>
> Proposals were made for resolutions to this issue however all of them
> require trade offs and could potentially break existing code.
>
> My proposal to resolve these two issues is to add the ability to rename
> parameters with a new syntax as follows.
>
> function callBar(Foo $internalName:externalName) {
> $internalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> This allows both the above problems to be resolved, by renaming the
> internal parameter and keeping the external signature the same.
>
> I propose that the RFC would have two voting options.
>
> 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.
>
> The second option would be to use this syntax to make named parameters in
> userland code explicitly opt in. As such an additional shortcut syntax
> would be implemented: $: to designate a named parameter. eg
>
> function callBar($:externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x);
>
> If a parameter is not opted in, a compile time error is raised:
>
> function callBar($externalName) {
> $externalName->bar();
> }
>
> $x = new Foo();
> callBar(externalName: $x); // Error: cannot call parameter $externalName
> by name.
>
> 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) however it does provide a neater solution to the second problem in
> that, to prevent the runtime errors in the second issue example, every
> child class would need to use the rename syntax on it's parameter to
> prevent errors, whereas if we went down this route, the parent class could
> just not opt into the named parameter syntax and the code would function as
> expected.
>
> Another advantage is that with the ability to rename parameters using the
> opt in, we gain some flexibility to tighten up the LSP rules relating to
> named parameter inheritance.
>
> class Foo {
> public function bar($:param) { //... }
> public function baz($internal:external) { //... }
> }
>
> // OK
> class Bar {
> public function bar($renamed:param) { //... }
> public function baz($renamed:external) { //... }
> }
>
> // Compile time error cannot rename named parameter $:param (renamed to
> $:renamedParam)
> class Baz {
> public function bar($:renamedParam) { //... }
> }
>
> // Compile time error cannot rename named parameter $:external (renamed to
> $:renamed)
> class Baz {
> public function baz($internal:renamed) { //... }
> }
>
> While this would be technically possible with the first option (no opt in)
> it would break any existing code which renames a parameter as every
> parameter would be subject to these rules.
>
> I don't have Wiki karma so can't post this yet; but I want to get the ball
> rolling on discussion as feature freeze is coming up fast and if we want to
> go for the second option, that must hit before the named parameter