rfc:deprecate-bareword-strings

Differences

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

Link to this comparison view

Next revision		Previous revision
rfc:deprecate-bareword-strings [2017/01/28 19:48] – created imsoprfc:deprecate-bareword-strings [2018/03/01 23:25] (current) – RFC was implemented in PHP 7.2 carusogabriel
Line 1: Line 1:
 ====== PHP RFC: Deprecate and Remove Bareword (Unquoted) Strings ====== ====== PHP RFC: Deprecate and Remove Bareword (Unquoted) Strings ======
-  * Version: 0.1 +  * Version: 1.1 
-  * Date: 2017-01-28+  * Date: 2017-03-05
   * Author: Rowan Collins rowan.collins@gmail.com   * Author: Rowan Collins rowan.collins@gmail.com
-  * Status: Draft+  * Status: Implemented (in PHP 7.2)
   * First Published at: http://wiki.php.net/rfc/deprecate-bareword-strings   * First Published at: http://wiki.php.net/rfc/deprecate-bareword-strings
  
 ===== Introduction ===== ===== Introduction =====
  
-When PHP encounters an unquoted token such as ''FROB_ACTIVE'', it tries to resolve it as a built-in or user-defined constant, but if no such constant exists, it treats it as equivalent to the quoted string '''FROB_ACTIVE''', and issues an E_NOTICE message. This behaviour has been around since very early versions of PHP, but is inconsistent with the rest of the language, and can lead to serious bugs. This RFC proposes three things: to raise the level of the message to E_WARNING; to document this behaviour as deprecated; and to remove it in PHP 8.0.+When PHP encounters an unquoted token such as ''FROB_ACTIVE'', it tries to resolve it as a built-in or user-defined constant, but if no such constant exists, it treats it as equivalent to the quoted string '''FROB_ACTIVE''', and issues an E_NOTICE message. This behaviour has been around since very early versions of PHP, but is inconsistent with the rest of the language, and can lead to serious bugs. This RFC proposes three things: to raise the level of the message to E_WARNING; to officially deprecate the fallback; and to remove it in PHP 8.0.
  
 The current behaviour appears to have been added as an attempt to guess the user's intention, and continue gracefully. This is inconsistent with other behaviour in current versions of PHP, which include many features designed to assert the correctness of a program. Most relevantly, referencing an undefined class constant (e.g. ''Foo::FROB_ACTIVE'') or namespaced constant (e.g. ''\Foo\FROB_ACTIVE'') produces a fatal error, as does an unambiguous attempt to reference a global constant, such as ''\FROB_ACTIVE''. The current behaviour appears to have been added as an attempt to guess the user's intention, and continue gracefully. This is inconsistent with other behaviour in current versions of PHP, which include many features designed to assert the correctness of a program. Most relevantly, referencing an undefined class constant (e.g. ''Foo::FROB_ACTIVE'') or namespaced constant (e.g. ''\Foo\FROB_ACTIVE'') produces a fatal error, as does an unambiguous attempt to reference a global constant, such as ''\FROB_ACTIVE''.
  
-This alone would not be sufficient argument to change the behaviour; there are many inconsistencies in PHP, as with any language which has evolved over a period of decades, and we must weigh the cost of changing them with the cost of keeping them.+This alone would not be sufficient argument to change the behaviour; there are many inconsistencies in PHP, as with any language which has evolved over a period of decades, and we must weigh the cost of changing them with the cost of keeping them. However, I believe the value of this feature is sufficiently low, and the problems it causes sufficiently high, that it should be deprecated and removed.
  
 ===== The Problem ===== ===== The Problem =====
  
-The argument for keeping the current behaviour is that some programs might be written to deliberately take advantage of it. For instance, code such as ''$_GET[bar]'' where ''bar'' is taken to be the string key ''bar''. However:+The value of keeping the current behaviour would be for programs written to deliberately take advantage of it. In particular, I have seen sample code of the form ''$_GET[bar]'' where ''bar'' is taken to be the string key ''bar''. However:
  
   * As far as I can see, this has never been documented behaviour, or appeared in any official examples.   * As far as I can see, this has never been documented behaviour, or appeared in any official examples.
Line 28: Line 28:
   * using a built-in constant which is not defined in the current version of PHP, e.g. a ''CURLOPT_'' constant documented on [[http://php.net/curl_setopt]] as added in a later release   * using a built-in constant which is not defined in the current version of PHP, e.g. a ''CURLOPT_'' constant documented on [[http://php.net/curl_setopt]] as added in a later release
   * mis-typing a keyword usable as a value, such as ''true'', ''false'', or ''null''   * mis-typing a keyword usable as a value, such as ''true'', ''false'', or ''null''
-  * mis-typing a keyword used as a standalone statement, such as ''break'', ''continue'', ''return'', or ''yield''+  * mis-typing a keyword usable as a standalone statement, such as ''break'', ''continue'', ''return'', or ''yield''
  
 Here are just a few examples of how allowing the program to continue with a substituted value in each of these cases can lead to serious unintended logic. Here are just a few examples of how allowing the program to continue with a substituted value in each of these cases can lead to serious unintended logic.
Line 42: Line 42:
 </code> </code>
  
-A string on a line of its own is a valid statement, which does nothing. Consequently, the typo ''braek'' is not a syntax error, making a loop continue unexpectedly: +A string on a line of its own is a valid statement, which does nothing. Consequently, the typo ''contniue'' is not a syntax error:
- +
-<code php> +
-$found = false; +
-foreach ( $list as $item ) { +
-   $found = some_test($item); +
-   if ( $found ) { +
-       braek; // this statement issues a notice and does nothing +
-   } +
-} +
-</code> +
- +
-Similar problems can arise with ''continue'':+
  
 <code php> <code php>
Line 65: Line 53:
 } }
 </code> </code>
 +
 +Similar problems can arise with ''break'', ''return'', and ''yield''.
  
 ===== Proposal ===== ===== Proposal =====
Line 71: Line 61:
  
   - In PHP 7.2, raise the severity of the message "Use of undefined constant" from E_NOTICE to E_WARNING   - In PHP 7.2, raise the severity of the message "Use of undefined constant" from E_NOTICE to E_WARNING
-  - Immediately document the fallback from bareword to string as deprecated in the manul +  - Immediately document the fallback from bareword to string as deprecated in the manual 
-  - In PHP 7.2, change the text of the message from ''Use of undefined constant %s - assumed '%s''' to ''Use of undefined constant %s; assumed '%s', but this will change in a future version of PHP''+  - In PHP 7.2, change the text of the message from ''Use of undefined constant %s - assumed '%s''' to ''Use of undefined constant %s; assumed '%s' (this will throw an error in a future version of PHP)''
   - In PHP 8.0, remove the fallback, and replace the ''E_WARNING'' with a thrown ''Error'' with message ''Use of undefined constant %s''   - In PHP 8.0, remove the fallback, and replace the ''E_WARNING'' with a thrown ''Error'' with message ''Use of undefined constant %s''
  
-It might seem surprising to raise an ''E_WARNING'' with text suggesting deprecation, rather than an ''E_DEPRECATED''. However, we need to balance two conflicting aims:+===== E_WARNING vs E_DEPRECATED ===== 
 + 
 +It might seem surprising to raise an ''E_WARNING'' with text suggesting deprecation, rather than an ''E_DEPRECATED''. This was chosen because we need to balance two aims:
  
   * If a user is relying on the fallback to string, we should communicate that this feature is officially deprecated and slated for removal.   * If a user is relying on the fallback to string, we should communicate that this feature is officially deprecated and slated for removal.
-  * If a user was actually intending to reference a constant or keyword, we should increase the chance they will see the message.+  * If a user was actually intending to reference a constant or keyword, we should increase the chance they will see the message. Furthermore, this is necessarily a run-time error - a constant may be defined in some code paths and not others - so some instances may show up only in production.  
 + 
 +To make the message visible, we want to use an error level likely to be enabled both in development and production configurations. Since ''E_DEPRECATED'' is actually //less// likely to be enabled than ''E_NOTICE'', switching to ''E_DEPRECATED'' would effectively "downgrade" the visibility of the message. Our two aims are therefore in direct conflict. 
 + 
 +This RFC takes the position that it is more likely that people will trigger this behaviour by mistake, so the priority is to make such a mistake obvious; thus ''E_WARNING'' is the correct severity. 
 + 
 +The proposed wording is also an attempt to balance these two possibilities. The use of parentheses is to avoid the awkward phrasing "in a future version of PHP in..." which would otherwise appear in the full output:
  
-In my opinion, the latter is more likely, and more configurations will have logging or display of ''E_WARNING'' than either ''E_NOTICE'' or ''E_DEPRECATED'', so that is the most appropriate severity.+<blockquote>Warning: Use of undefined constant FOO - assumed 'FOO' (this will throw an error in a future version of PHP) in foo.php on line 1</blockquote>
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
Line 86: Line 84:
 This change is quite deliberately a change to current behaviour.  This change is quite deliberately a change to current behaviour. 
  
-Browsing [[https://github.com/phplang/php-past/|the archived copies of PHP source code]], we can see that the behaviour has been present since at least PHP 3.0. Early PHP 3 betas apparently [[https://github.com/phplang/php-past/blob/PHP-3.0b4/language-parser.y#L559|treated all unknown barewords as strings]]; however, before the final release, both built-in and user-defined constants were added as a new language feature, and [[https://github.com/phplang/php-past/blob/PHP-3.0/language-parser.y#L553|the first version of the current notice was added]]. It's unclear to me whether the hand-crafted lexer and parser in PHP 2.0 treated bare words as strings or errors; if anyone has it successfully compiled and would like to check, let me know.+Browsing [[https://github.com/phplang/php-past/|the archived copies of PHP source code]] shows that the current behaviour was added late in the development of PHP 3.0. In PHP 2.0, an unquoted string was simply a syntax error, but early PHP 3 betas added a feature to [[https://github.com/phplang/php-past/blob/PHP-3.0b4/language-parser.y#L559|treat all barewords which weren't keywords as strings]]. Before the final release, both built-in and user-defined constants were added as a new language feature, and [[https://github.com/phplang/php-past/blob/PHP-3.0/language-parser.y#L553|the first version of the current notice was added]]. 
  
-However, the old source code also includes the documentation with which it shipped, in which I can find no mention of this behaviour, and no examples which take advantage of it.+The old source code also includes the documentation with which PHP 3 shipped, which seem to have no mention of this behaviour, and no examples which take advantage of it.
  
 As mentioned earlier, it has been //discouraged// in the manual since 2001, so it could be argued that it is //already deprecated//. This RFC takes the conservative view that there should still be a standard period of deprecation before removing it. As mentioned earlier, it has been //discouraged// in the manual since 2001, so it could be argued that it is //already deprecated//. This RFC takes the conservative view that there should still be a standard period of deprecation before removing it.
Line 101: Line 99:
 ===== RFC Impact ===== ===== RFC Impact =====
  
-This change should have no effect on SAPIs, extensions, or OpCache.+This change should have no particular effect on SAPIs, extensions, or OpCache.
  
 +By increasing the robustness of PHP programs, this change would have a minor but positive impact on security.
  
-===== Open Issues ===== 
  
-The exact text of the message to be used could be refined.+===== Open Issues =====
  
 +  * Appropriate locations in the manual to document the deprecation, since it is not clearly documented as a current feature.
  
 ===== Unaffected PHP Functionality ===== ===== Unaffected PHP Functionality =====
  
-  * Unquoted array keys within a double-quoted string will remain valid, e.g. ''"Item bar is $foo[bar]"''+  * Unquoted array keys within a double-quoted string will remain valid, e.g. ''"Item bar is $foo[bar]"''; since this //never// looks up a constant, it does not suffer from the same ambiguities and subtle bugs as the main syntax discussed here. 
 +  * Defining arrays in query strings will continue to be unquoted, e.g. ''test.php?foo[bar]=42''; this has no ambiguity at all, since there is no scope where constants could be defined in order to populate it.
   * Undefined class constants, namespaced constants, and explicit constant references prefixed by ''\'' will continue to throw errors as currently.   * Undefined class constants, namespaced constants, and explicit constant references prefixed by ''\'' will continue to throw errors as currently.
  
Line 119: Line 119:
  
  
-===== Proposed Voting Choices =====+===== Voting =====
  
-A single yes/no vote requiring a 2/3 majority.+Voting opened on 2017-03-08, and will close on 2017-03-22 at 22:00 UTC
  
-In PHP 7.2, replace the "Undefined constant" ''E_NOTICE'' with an ''E_WARNING'' stating that the fallback is deprecated; and in PHP 8.0, remove the fallback and replace the ''E_WARNING'' with an ''Error''.+The vote requires a 2/3 majority to accept the proposal.
  
 +Voting is on the following proposal:
  
-===== Patches and Tests =====+  - In PHP 7.2, raise the severity of the message "Use of undefined constant" from E_NOTICE to E_WARNING, and change the text of the message to ''Use of undefined constant %s; assumed '%s' (this will throw an error in a future version of PHP)'' 
 +  - In PHP 8.0, remove the fallback, and replace the ''E_WARNING'' with a thrown ''Error'' with message ''Use of undefined constant %s''
  
-None yet.+<doodle title="Raise severity of undefined constants to E_WARNING in 7.2, and Error in 8.0?" auth="imsop" voteType="single" closed="true"> 
 +   * Yes 
 +   * No 
 +</doodle>
  
-===== Implementation ===== 
  
-None yet. +===== Implementation =====
- +
-===== References =====+
  
-TODO+  * Pull Request implementing the change itself: https://github.com/php/php-src/pull/2404  
 +  * Merged into master: https://github.com/php/php-src/commit/1b565f1393f82e0ce0c94806cc7f52c6d9c5e87d 
 +  * Pull Request to the Language Spec documenting the new behaviour: https://github.com/php/php-langspec/pull/193
  
 ===== Rejected Features ===== ===== Rejected Features =====
  
 None yet. None yet.
rfc/deprecate-bareword-strings.1485632890.txt.gz · Last modified: 2017/09/22 13:28 (external edit)

Page Tools