rfc:renamed_parameters
Differences
This shows you the differences between two versions of the page.
Next revision | Previous revisionNext revisionBoth sides next revision | ||
rfc:renamed_parameters [2020/07/24 13:51] – created carnage | rfc:renamed_parameters [2020/07/26 13:43] – carnage | ||
---|---|---|---|
Line 1: | Line 1: | ||
====== PHP RFC: Renamed Parameters ====== | ====== PHP RFC: Renamed Parameters ====== | ||
- | * Version: 0.1 | + | * Version: 0.2 |
- | * Date: 2020-07-24 | + | * Date: 2020-07-24 |
* Author: Chris Riley, t.carnage@gmail.com | * Author: Chris Riley, t.carnage@gmail.com | ||
- | * Status: | + | * Status: |
* First Published at: http:// | * First Published at: http:// | ||
Line 21: | Line 21: | ||
class RegistrationHandler implements Handler { | class RegistrationHandler implements Handler { | ||
- | public function handle($registraionCommand); | + | public function handle($registrationCommand); |
} | } | ||
Line 45: | Line 45: | ||
===== Proposal ===== | ===== Proposal ===== | ||
- | === Option 1 === | + | My proposal to resolve these two issues is to add the ability to rename parameters, while also making named parameters explicitly opt in, with a new syntax as follows. |
- | My proposal to resolve these two issues is to add the ability to rename parameters with a new syntax as follows. | + | |
<code php> | <code php> | ||
Line 59: | Line 58: | ||
This allows both the above problems to be resolved, by renaming the internal parameter and keeping the external signature the same. | This allows both the above problems to be resolved, by renaming the internal parameter and keeping the external signature the same. | ||
- | === Option 2 === | ||
- | 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: | + | An additional shortcut syntax would be implemented: |
<code php> | <code php> | ||
Line 84: | Line 82: | ||
</ | </ | ||
- | There are pros and cons to this second | + | There are pros and cons to this 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 could use the rename syntax on it's parameter to prevent errors |
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. | 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. | ||
Line 112: | Line 110: | ||
</ | </ | ||
- | While this would be technically possible | + | While this could be done with the existing named parameters implementation |
Line 140: | Line 138: | ||
===== Proposed PHP Version(s) ===== | ===== Proposed PHP Version(s) ===== | ||
- | PHP 8.0 | + | **PHP 8.0** |
+ | |||
+ | This would be a breaking change to the existing named parameters implementation if implemented in **8.1**, so if accepted must be delivered for **8.0**. There have been concerns that this is a big change very close to the feature freeze cut off date for 8.0 so if accepted we may need to stage the implementation, | ||
+ | |||
+ | At the moment, I propose that we accept or reject the RFC as a whole and leave it to the release managers to decide the best implementation strategy, however it may be appropriate to put this to a vote as well and I invite specific feedback on this issue. | ||
===== RFC Impact ===== | ===== RFC Impact ===== | ||
Line 158: | Line 160: | ||
None | None | ||
- | ===== Open Issues | + | ===== Objections |
- | Make sure there are no open issues | + | |
+ | === Unable to support PHP 7.x and named parameters in one library release === | ||
+ | |||
+ | One of the comments to come up was that having an explicit new syntax to enable named parameters means that library authors won't be able to both support both 7.x versions and also enable named parameters. This would mean there would be an additional delay before users could benefit from the new syntax. This would arguably have a larger impact on users with this feature than others such as scalar types as they would be unable to use the feature in their code when calling libraries which have not yet dropped support for PHP 7.x | ||
+ | |||
+ | While this is certainly a factor worth serious consideration, | ||
+ | |||
+ | === Very close to feature freeze === | ||
+ | |||
+ | A point that has been brought up by a few people is that we are close to the feature cut off date for PHP 8.0 | ||
+ | |||
+ | This is another strong point, however as things stand we are shipping a feature which breaks the object model in PHP in a way that can be avoided by a different implementation, | ||
+ | |||
+ | === LSP breakage was documented and accepted in original RFC === | ||
+ | |||
+ | It has been brought up that the problems I outlined in the introduction were brought up in the original RFC and yet it still passed an acceptance | ||
+ | |||
+ | While I don't claim to speak for anyone who actually voted or their reasons for voting; from my perspective the implications for LSP did not jump out very well. In fact when I read the proposal myself my main objection came from the inclusion of parameter names within the BC breaking surface of a method or function signature, it was only after taking the time to write up this RFC that the other implications of the current implementation became fully apparent to me. If those who were able to vote were fully aware an accepting of the issues I've brought up, a no vote on this proposal will confirm that we they happy to accept the drawbacks of the current implementation, | ||
===== Unaffected PHP Functionality ===== | ===== Unaffected PHP Functionality ===== | ||
Line 170: | Line 189: | ||
===== Proposed Voting Choices ===== | ===== Proposed Voting Choices ===== | ||
- | Proposed two voting options: | ||
- | 1. Implement parameter renaming | + | Straight yes/no with 2/3 majority required. |
- | 2. Make parameter names opt in | + | |
===== Patches and Tests ===== | ===== Patches and Tests ===== | ||
Line 184: | Line 201: | ||
===== References ===== | ===== References ===== | ||
- | This proposal is very similar to argument labels in swift: https:// | + | This proposal is similar to argument labels in swift: https:// |
+ | |||
+ | ===== Change log ===== | ||
- | ===== Rejected Features ===== | + | 26/07/20: V0.2 |
- | Keep this updated with features that were discussed on the mail lists. | + | - Dropped option 1 (rename without explicit opt in) due to concerns over feature freeze timing, |
+ | - Added proposed staging strategy for implementation to allay concerns over feature freeze timing | ||
+ | - Documented objections & rebuttals to RFC |
rfc/renamed_parameters.txt · Last modified: 2020/08/09 17:23 by imsop