Skip to content

Warn on implicit bool to int conversion when using as array offset #10466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvorisek opened this issue Jan 28, 2023 · 10 comments
Closed

Warn on implicit bool to int conversion when using as array offset #10466

mvorisek opened this issue Jan 28, 2023 · 10 comments

Comments

@mvorisek
Copy link
Contributor

Description

https://3v4l.org/TrG4u

using bool as array offset can be a mistake, also (string) false = ''

@hormus
Copy link

hormus commented Jan 28, 2023

  1. The manual states that the index can be string or integer
  2. Since php 8.1.0 coercive false to string conversion produces E_DEPRECATED while in php 9 it is promoted to type error Because from an unknown source it's good code to filter/sanitize the data, even the index

https://wiki.php.net/rfc/deprecate-boolean-string-coercion

@mvorisek
Copy link
Contributor Author

Since php 8.1.0 coercive false to string conversion produces E_DEPRECATED while in php 9 it is promoted to type error Because from an unknown source it's good code to filter/sanitize the data, even the index

https://3v4l.org/1c57W no warning even on master

@hormus
Copy link

hormus commented Jan 28, 2023

the mistake is to fix that behavior since it will have to generate Type error on php 9 ex. echo false; rather sanitize and filter your inputs since $arr[null] = 'empty'; is the index with empty and valid string.
Example number 6 says boolean is converted to 0 if false or 1 if true, it's not error from php but the code can be improved by a user function to conform to values ​​of type int or string.
https://www.php.net/manual/en/language.types.array.php

@Girgias
Copy link
Member

Girgias commented Jan 29, 2023

  1. The manual states that the index can be string or integer

    1. Since php 8.1.0 coercive false to string conversion produces E_DEPRECATED while in php 9 it is promoted to type error Because from an unknown source it's good code to filter/sanitize the data, even the index

https://wiki.php.net/rfc/deprecate-boolean-string-coercion

This RFC has not been voted nor implemented.

Frankly, the bool to int conversion here is at least logical as it is standard behaviour and follow the type juggling semantics of the function context. The bigger issue is the implicit null to string conversion, which creates an empty string.

I've got those type juggling things in my rather, but this doesn't deserve its own issue IMHO.

@iluuu1994
Copy link
Member

iluuu1994 commented Mar 23, 2023

A warning for anything but string/int seems reasonable. Luckily we already disallow some problematic types (array and object in particular). The ones that are still allowed are:

  • null
  • bool
  • resource (sadly)

Here's a quick PoC. master...iluuu1994:php-src:array-offset-coercion-warning

@Girgias
Copy link
Member

Girgias commented Mar 23, 2023

Might want to tie it in with #7173 as that also tackles the same part to a degree

@github-actions
Copy link

There has not been any recent activity in this feature request. It will automatically be closed in 14 days if no further action is taken. Please see https://github.com/probot/stale#is-closing-stale-issues-really-a-good-idea to understand why we auto-close stale feature requests.

@github-actions github-actions bot added the Stale label Jun 22, 2023
@mvorisek
Copy link
Contributor Author

This issue is not stale. https://wiki.php.net/rfc/deprecate-boolean-string-coercion should be voted soon.

@iluuu1994
Copy link
Member

@mvorisek We are not planning on voting on this RFC.

@github-actions github-actions bot removed the Stale label Jun 23, 2023
@github-actions
Copy link

There has not been any recent activity in this feature request. It will automatically be closed in 14 days if no further action is taken. Please see https://github.com/probot/stale#is-closing-stale-issues-really-a-good-idea to understand why we auto-close stale feature requests.

@github-actions github-actions bot added Stale and removed Stale labels Sep 22, 2023
@TimWolla TimWolla added the Stale label Oct 5, 2023
@TimWolla TimWolla closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants