rfc:renamed_parameters

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Last revision Both sides next revision
rfc:renamed_parameters [2020/07/26 13:43]
carnage
rfc:renamed_parameters [2020/08/09 17:03]
carnage
Line 1: Line 1:
-====== PHP RFC: Renamed Parameters ====== +====== PHP RFC: Named Parameters explicit opt in ====== 
-  * Version: 0.2+  * Version: 0.3
   * Date: 2020-07-24   * Date: 2020-07-24
   * Author: Chris Riley, t.carnage@gmail.com   * Author: Chris Riley, t.carnage@gmail.com
Line 9: Line 9:
 ===== Introduction ===== ===== Introduction =====
  
-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+The current implementation of named parameters breaks object polymorphism
  
-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. +Consider this example
- +
-More pressing a point is that the current implementation breaks object polymorphism. Consider this example (simplified from one of my codebases)+
  
 <code php> <code php>
Line 29: Line 27:
  
 class MessageBus { class MessageBus {
-    //... +    /... */ 
-    public function addHandler(string $message, Handler $handler) { //... } +    public function addHandler(string $message, Handler $handler) { /... */ 
-    public function getHandler(string $messageType): Handler { //... }+    public function getHandler(string $messageType): Handler { /... */ }
     public function dispatch($message)     public function dispatch($message)
     {     {
Line 39: Line 37:
 </code> </code>
  
-This code breaks at run time+This will raise an Error exception at runtime as you have specified an unknown 
 +parameter name
  
-Proposals were made for resolutions to this issue however all of them require trade offs and could potentially break existing code. I offer a new proposal which offers some advantages.+The situation will be further complicated if the method that is being called has a variadic 
 +argument as in this situation no error will be raised, and the unknown parameter will 
 +be silently included into the variadic array 
  
-===== Proposal =====+<code php>
  
-My proposal to resolve these two issues is to add the ability to rename parameterswhile also making named parameters explicitly opt in, with a new syntax as follows.+interface Pager  
 +
 +    public function fetch($page = 0, ...$categories); 
 +}
  
-<code php> +class DbPager implements Pager 
-function callBar(Foo $internalName:externalName) { +
-    $internalName->bar();+    public function fetch($seite = 0, ...$kategorien) 
 +    { 
 +        /* ... */ 
 +    }
 } }
  
-$= new Foo(); +$dbPager = new DbPager(); 
-callBar(externalName$x);+$dbPager->fetch(page1, categories: 2); 
 </code> </code>
  
-This allows both the above problems to be resolvedby renaming the internal parameter and keeping the external signature the same.+In this situation the error passes unnoticed, and you end up retrieving the first 
 +results page with categories 1 and 2 instead of the second page with just category 
 +2.
  
 +Proposals were made for resolutions to this issue however all of them require trade offs and could potentially break existing code. 
  
-An additional shortcut syntax would be implemented: $: to designate named parameter which uses the same name both internally and externallyeg+I offer new proposal which offers some advantages. 
 + 
 +===== Proposal ===== 
 + 
 +The proposal is to make named parameters explicitly opt in using a new syntax as follows.
  
 <code php> <code php>
-function callBar($:externalName) { +function callBar(Foo $:parameterName) { 
-    $externalName->bar();+    $internalName->bar();
 } }
  
 $x = new Foo(); $x = new Foo();
-callBar(externalName: $x);+callBar(parameterName: $x);
 </code> </code>
  
-If a parameter is not opted in, a compile time error is raised:+This will enable us to resolve the polymorphism issue by restricting changes to 
 +parameter names in child classes, without impacting existing userland code which 
 +may rely on renaming parameters already. 
 + 
 +If a parameter has not opted in, a compile time error will be raised:
  
 <code php> <code php>
Line 78: Line 97:
  
 $x = new Foo(); $x = new Foo();
-callBar(externalName: $x); // Error: cannot call parameter $externalName by name.+callBar(externalName: $x); // Error: cannot call function callBar() using parameter $externalName by name.
  
 </code> </code>
  
-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 or the parent class could just not opt into the named parameter syntax and the code would function as expected. +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  
-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. + easily) however using the opt in only named parameter implementation, we gain some flexibility to  
 + tighten up the LSP rules relating to named parameter inheritance. Which allows the bugs/errors from the 
 + introduction section to be discovered and prevented at compile time
  
 <code php> <code php>
 class Foo { class Foo {
-    public function bar($:param) { //... +    public function bar($:param) { /... */ }
-    public function baz($internal:external) { //... }+
 } }
  
 // OK // OK
-class Bar { +class Bar extends Foo 
-    public function bar($renamed:param) { //... +    public function bar($:param) { /... */ }
-    public function baz($renamed:external) { //... }+
 } }
  
 // Compile time error cannot rename named parameter $:param (renamed to $:renamedParam) // Compile time error cannot rename named parameter $:param (renamed to $:renamedParam)
-class Baz { +class Baz extends Foo 
-    public function bar($:renamedParam) { //... }+    public function bar($:renamedParam) { /... */ }
 } }
  
-// Compile time error cannot rename named parameter $:external (renamed to $:renamed+</code> 
-class Baz + 
-    public function baz($internal:renamed) { //... }+While this could be done with the existing named parameters implementation it would break any existing code  
 +which renames a parameter as every parameter would be subject to these rules not just those that had opted  
 +in to allow named parameters. 
 + 
 +In line with the existing RFC on named parameters, named parameter opt ins should follow the same rules as for 
 +calling functions using named parameters. As such: 
 + 
 +<code php> 
 + 
 +// OK 
 +function foo($positional, $:named) { /* ... */ } 
 + 
 +// Error: named parameters must follow positional parameters 
 +function bar($:named, $positional) { /* ... */ } 
 + 
 +</code> 
 + 
 +Extending classes using named parameters will require that the method signature matches with regard to named 
 +parameters 
 + 
 +<code php> 
 + 
 +class Foo 
 +
 +    public function bar($:namedParam{ /* ... */ } 
 +
 + 
 +//OK 
 +class Bar extends Foo 
 +
 +    public function bar($:namedParam, $:anotherNamedParam = "default") { /* ... *
 +
 + 
 +// Error: parameter $namedParam must be declared as a named parameter 
 +class Baz extends Foo 
 +
 + public function bar($namedParam, $:anotherNamedParam) { /* ... */ }
 } }
  
 </code> </code>
  
-While this could be done with the existing named parameters implementation it would break any existing code which renames a parameter as every parameter would be subject to these rules not just those that had opted in to allow named parameters.+To aid users upgrading from previous PHP versions, there will **not** be an error thrown if the base class 
 +is PHP built in class. Instead, a depreciation warning will be raised, and the parameter will be opted in 
 +automatically. It should be assumed by developers that the depreciation will be upgraded to an error in line  
 +with user land code in PHP 9.0, however this shall be subject to a separate RFC at the time which can assess  
 +how appropriate that course of action is.
  
 +====== Reflection ======
  
-===== Alternative solutions =====+A new method isNamedParameter would be added to the ReflectionParameter class, this would return true if  
 +the parameter can be called by name and false otherwise.
  
-==== Attributes ==== +====== PHP standard library ======
-It has been suggested that some of the issues raised in this RFC could be solved by attributes an internal attribute could be +
-created to specify a parameter name alias and allow argument renaming. This would work similarly to option 1, however has the downside of splitting the definition of a parameter over two parts of the code.+
  
-Option 2 could also be achieved using an attribute to either enable or disable named parameters on per function/method basis.+It is proposed that all PHP standard functions and methods are opted in to named parameters. This will require 
 +that some classes have parameter names updated to match the entire hierarchy as identified in the initial 
 + RFC, to which this is proposed enhancement.
  
 +====== Alternative syntax using Attributes ======
  
-==== Opt out named parameters ==== +Many people have expressed a preference towards using attributes to control the behaviour of named parameters, 
-The comparison to swift brings up another alternative implementation whereby a function/method author can explicitly opt out of  +this is not mutually exclusive to this proposal. In fact this proposal could be implemented using an attribute 
-named parameter syntax by using the special argument reference _ we could adopt something similar as a middle ground between option 1 and option 2An example might be+instead of the new $: syntax choiceAttributes may in fact provide a better solution since they could be used 
 +to opt in entire functions, methods or classes in one go eg
  
 <code php> <code php>
-function foo($:bar) { //... }+class Foo { 
 +    public function bar(@@NamedParameter $param) { /... */ } 
 +}
  
-foo(bar: 'test')// Error: cannot call parameter $bar by name+ 
 +class Bar { 
 +    @@NamedParameters 
 +    public function bar($param, $param2, $param3 ... $paramN) { /* ... */ } 
 +
 + 
 +@@NamedParameters 
 +class Baz { 
 +    public function foo($param, $param2, $param3 ... $paramN) { /* ... */ } 
 +    public function bar($param, $param2, $param3 ... $paramN/* ... *
 +    public function baz($param, $param2, $param3 ... $paramN) { /* ... */ } 
 +}
 </code> </code>
  
-This wouldn't allow compile time checks of inheritance issues as doing so would break existing userland code.+All the other examples in this RFC would work in the same way, this would just simplify opting in to named 
 +parameters for userland code. This will be offered as a voting choice against my proposed $: syntax.
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
-None+ 
 +Userland code which extends PHP built in classes and have renamed the arguments will cause a compile time 
 +error. It is expected that this will be a very small quantity of code based on the same assumptions as the  
 +original RFC. 
  
 ===== 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 release managers discretion. My proposed staged approach would be to implement the explicit opt-in ($:) syntax for named parameters as part of **8.0** and to only implement the renaming portion of the RFC for **8.1**  +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**. 
- +
-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.+
  
 +Due to the time constraints I have slimmed down this proposal to focus on only the breaking changes that are required for 
 +PHP **8.0** If accepted, I intend to start a separate RFC to handle the previous renaming behaviour for **8.1** using
 +either the attribute or $: syntax as accepted.
 ===== RFC Impact ===== ===== RFC Impact =====
 ==== To SAPIs ==== ==== To SAPIs ====
Line 160: Line 241:
 None None
  
-===== Objections =====+===== Future Scope =====
  
-=== Unable to support PHP 7.x and named parameters in one library release ===+A future RFC will bring in the ability to specify different names to be used internally to the function/method 
 +separate to the callable external nameThis syntax would look like:
  
-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 parametersThis would mean there would be an additional delay before users could benefit from the new syntaxThis 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+<code php> 
 +function foo($internalName:externalName) { /* ... */ } 
 +</code>
  
-While this is certainly a factor worth serious consideration, it is mitigated by time: over time all actively maintained libraries will upgrade to only supporting PHP 8.x+ and their authors can opt in the methods to use named parameters where appropriate, users will know which can support named parameter calls via the function signature and IDE support for the new syntax. PHP has been around a long time and shows no sign of slowing down. It seems short sighted to implement a feature in a way that priorities short term adoption over long term maintainability+as proposed originally in this RFC
  
-=== Very close to feature freeze ===+===== Proposed Voting Choices =====
  
-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 +Two votes:
  
-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, which is going to be difficult if not impossible to fix down the line should we regret it. If we want to fix it, it needs to be done now. This is also the safer of the implementation options when compared to the current implementation; we will always be able to expand to allow all parameters to be called by name (although at the cost of keeping the redundant $: syntax) we will not be able to go back to opt-in only named params without breaking users code. +Straight yes/no with 2/3 majority required to require opt-in for named parameters
- +
-=== 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 vote by an appreciable margin. +
- +
-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, if not this gives them a chance to reconsider and choose an alternative. +
- +
-===== Unaffected PHP Functionality ===== +
-List existing areas/features of PHP that will not be changed by the RFC. +
- +
-This helps avoid any ambiguity, shows that you have thought deeply about the RFC's impact, and helps reduces mail list noise. +
- +
-===== Future Scope ===== +
-This section details areas where the feature might be improved in future, but that are not currently proposed in this RFC. +
- +
-===== Proposed Voting Choices =====+
  
-Straight yes/no with 2/3 majority required.+Majority vote between the implementation options of $: or using Attributes.
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
Line 204: Line 272:
  
 ===== Change log ===== ===== Change log =====
 +
 +09/07/20: V0.3 
 +- Tightened up scope to focus on opt in and technical challenges
  
 26/07/20: V0.2  26/07/20: V0.2 
Line 209: Line 280:
 - Added proposed staging strategy for implementation to allay concerns over feature freeze timing - Added proposed staging strategy for implementation to allay concerns over feature freeze timing
 - Documented objections & rebuttals to RFC - Documented objections & rebuttals to RFC
 +
rfc/renamed_parameters.txt · Last modified: 2020/08/09 17:23 by imsop