#53635 closed task (blessed) (fixed)
PHP 8.1: various compatibility fixes for WordPress 5.9
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | php81 has-patch has-unit-tests |
Focuses: | coding-standards | Cc: |
Description
Previously:
- #50913 (5.6)
PHP 8.1 release is planned for November 25, 2021. WordPress 5.9 is currently planned for December 2021 and should aim to support PHP 8.1 as much as possible.
This ticket can be used as an "epic", allowing a variety of small patches each fixing a specific failure to be added to and committed against this ticket.
For patches addressing all instances of failures related to one specific PHP 8.1 change across the codebase, separate tickets should still be opened.
For an example of issues/patches with separate tickets, see:
- #52825 PHP 8.1: MySQLi error reporting value changes
- #53299 PHP 8.1: Update
is_serialized
function to accept Enums - #53465 PHP 8.1.: the default value of the flags parameter for htmlentities() et all needs to be explicitly set
When opening a separate ticket, please tag it with the php81
keyword, so these can be easily found using this report: https://core.trac.wordpress.org/query?keywords=~php81
Attachments (10)
Change History (180)
@
4 years ago
Fix a "SomeClass::jsonSerialize() should be compatible with JsonSerializable::jsonSerialize(): mixed" error
#2
@
4 years ago
Some context for the patch I just uploaded:
This relates to the Return types for internal methods RFC in PHP 8.1 and in particular, the change made in PHP PR #7051, which adds a mixed
return type to the JsonSerializable::jsonSerialize()
interface method.
WP only contains one (test) class which implements the JsonSerializable
interface and this patch fixes the issue for that class.
Basically, as of PHP 8.1, the jsonSerialize()
method in classes which implement the JsonSerializable
interface are expected to have a return type declared. The return type should be mixed
or a more specific type.
This complies with the Liskov principle of covariance, which allows the return type of a child overloaded method to be more specific than that of the parent.
The problem with this is that:
- The
mixed
return type was only introduced in PHP 8.0 and therefore cannot be used as WP has a minimum PHP version of 5.6. - Return types in general were only introduced in PHP 7.0 and therefore cannot be used as WP has a minimum PHP version of 5.6.
Luckily an attribute has been added to silence the deprecation warning.
While attributes are a PHP 8.0+ feature, due to the choice of the #[]
syntax, in PHP < 8.0, attributes will just be ignored and treated as comments, so there is no drawback to using the attribute.
So to fix this, there is basically a choice between two options:
- Either decouple from the interface. I'd strongly recommend against this as the interface is being implemented for a reason.
- Or use the attribute, which is what this patch does.
For more details, see the discussion in the upstream PR: https://github.com/php/php-src/pull/7051
@
4 years ago
Fix various "Deprecated: Return type of WP_Hook::[METHODNAME]() should either be compatible with ...., or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice"
@
4 years ago
Fix various "Deprecated: Return type of WP_Theme::[METHODNAME]($offset) should either be compatible with ArrayAccess::[METHODNAME](): type, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice"
#3
@
4 years ago
Two more patches for the same type of errors as before. The same explanation as outlined in comment #2 applies to these patches as well.
#5
@
4 years ago
Notes from live working session:
- Check the docs generator to determine if there any issues from the placement of the attribute (which is above the method/function and below the existing DocBlock)
This check needs action before the docs are generated for 5.9.
#6
@
4 years ago
Linking #53858 for reference - "PHP 8.1: syntax error due to new 'readonly' property"
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
@
4 years ago
PHP 8.1/Tests: fix deprecation notice - another one for the "return type will change" series
#13
@
4 years ago
FYI: some info about some of the runtime dependencies of WordPress (incomplete)
Sodium Compat
I've opened a PR to fix various PHP 8.1 deprecation warnings about the return type change, as well as a PR to turn on CI testing against PHP 8.1.
With the open PHP 8.1 PR merged, there would still be one issue remaining, but how to fix that should be decided by the maintainer in this case.
Random Compat
I've opened a PR to turn on CI testing against PHP 8.1.
There are two test failures in their tests, but how to fix these should be decided by the maintainer in this case.
PHPMailer
Testing against PHP 8.1 has been turned on over a month ago.
I've pulled two fixes and both have been merged:
As things stand, the PHPMailer tests pass against PHP 8.1.
No release containing the fixes is available yet.
Requests
Testing against PHP 8.1 has been turned on the other week.
I've pulled three fixes and these have been merged:
- Input validation fix
- Incorrect default value passed to PHP native functions
- Return type deprecation notices
As things stand, the Requests tests pass against PHP 8.1.
No release containing the fixes is available yet.
GetID3
CI (linting) has been running against PHP 8.1 since December 2020 🎉
The other side of the coin is that there are no tests, so, aside from there not being any parse errors, the actual PHP 8.1 compatibility status of the repo is anyone's guess.
SimplePie
An issue has been open since last year about getting the tests running on PHP 8.0, let alone PHP 8.1. So far there has been no response from the maintainers...
This ticket was mentioned in PR #1556 on WordPress/wordpress-develop by jrfnl.
4 years ago
#14
- Keywords has-patch has-unit-tests added
Fixes strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
notices on PHP 8.1.
Impact: 38 of the pre-existing tests are affected by this issue.
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
This ticket was mentioned in PR #1558 on WordPress/wordpress-develop by jrfnl.
4 years ago
#15
Fixes parse_str(): Passing null to parameter #1 ($string) of type string is deprecated
notices on PHP 8.1.
Impact: 311 of the pre-existing tests are affected by this issue.
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
#16
@
4 years ago
- Keywords has-patch has-unit-tests removed
Linking #53897 for reference: PHP 8.1: strftime() is deprecated
#18
@
4 years ago
Sodium Compat has just released a PHP 8.1 compatible version. Ticket #53907 has been opened to handle the upgrade.
#19
@
4 years ago
FYI: I've been preparing various patches for known PHP 8.1 test failures - mostly "passing null to non-nullable" errors - and adding dedicated, targeted tests for these.
Unfortunately, though not unexpected, the functions affected often don't have any tests yet or the tests which are available, need a lot of work.
So slow going, but we've still got a few months time and it would be good to significantly improve the tests along with making these fixes.
Some WIP PRs are already open on GH (not yet linked to this ticket) and more are forthcoming.
#20
@
4 years ago
@SergeyBiryukov When you've got some time, would be great if the 53635-tests-countable.patch
patch could get committed. Nothing exiting, just the last of the WP Core "return type does not match" patches.
#22
@
4 years ago
FYI: Based on a quick scan, I've found one PHP 8.1 issue in SimplePie and I have submitted a fix this to the repo. Their test suite is far from complete though (similar to WP's own test suite), so there may be more issues lurking under the surface.
#23
@
4 years ago
The patch I've just uploaded - 53635-05-null-to-non-nullable-in-testcode.patch
- is a test code only patch and ready for commit.
Fixes two tests which were erroring out.
Commit details:
PHP 8.1: Tests_Functions_DoEnclose: fix bug in test
As per the PHP manual:
If the component parameter is omitted, an associative array is returned.
If the component parameter is specified, parse_url() returns a string (or an int, in the case of PHP_URL_PORT) instead of an array. If the requested component doesn't exist within the given URL, null will be returned.
Ref: https://www.php.net/manual/en/function.parse-url.php#refsect1-function.parse-url-returnvalues
In this case, parse_url()
is called with the PHP_URL_PATH
as $component
, but the returned value is subsequently checked against false
.
In other words, this condition would previously always result true
and would lead to null
, potentially, being passed to the PHP native pathinfo()
function which expects a string.
On PHP 8.1, this will result in a test failure with a pathinfo(): Passing null to parameter #1 ($path) of type string is deprecated
error.
This ticket was mentioned in PR #1570 on WordPress/wordpress-develop by jrfnl.
4 years ago
#24
- Keywords has-patch added
No input validation was done. Covered by existing Tests_Functions::test_validate_file()
test (well, the null
and string part is).
Error fixed: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated
QUESTION: should we add tests for other scalar input types ?
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
This ticket was mentioned in PR #1572 on WordPress/wordpress-develop by jrfnl.
4 years ago
#25
- Keywords has-unit-tests added
### Tests_Admin_includesFile: add two tests for the download_url() function
The first test is "girl-scouting" to make sure that the code up to the point where the error is expected is tested.
The second test exposed a PHP 8.1 basename(): Passing null to parameter #1 ($path) of type string is deprecated
error due to the call to parse_url()
returning null
when the component requested does not exist in the passed URL.
### PHP 8.1: download_url(): prevent "passing null to non-nullable" notice [1]
Includes removing duplicate function calls.
I've verified that neither the $url
, nor the $url_filename
are changed between when they are first received/defined and when they are re-used, so there is no need to repeat the function calls.
### Tests_Admin_includesFile: add yet another test for the download_url() function
The output of the call to parse_url()
stored in the $url_path
variable is used in more places in the function logic.
This test exposes a second PHP 8.1 deprecation notice, this time for substr(): Passing null to parameter #1 ($string) of type string is deprecated
.
### PHP 8.1: download_url(): prevent "passing null to non-nullable" notice [2]
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
This ticket was mentioned in PR #1576 on WordPress/wordpress-develop by jrfnl.
4 years ago
#27
### Tests_Cron: add test for wp_schedule_event()
### PHP 8.1: wp_schedule_event(): fix deprecation notice
The following deprecation notice shows at the top of a test run on PHP 8.1-beta2:
Deprecated: Automatic conversion of false to array is deprecated in path/to/src/wp-includes/cron.php on line 305
In wp_schedule_event()
, the cron info array is retrieved via a call to _get_cron_array()
, but as the documentation for that function (correctly) states, the return type of that function is array|false
, where false
is returned for a virgin site, where no cron jobs have been scheduled yet.
However, no type check is done on the return value and the wp_schedule_event()
function just blindly continues by assigning a value to a subkey of the $crons
"array".
Fixed by adding validation for the returned value from _get_cron_array()
and initializing an empty array if false
was returned.
Ref: https://developer.wordpress.org/reference/functions/_get_cron_array/
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1577 on WordPress/wordpress-develop by jrfnl.
4 years ago
#28
Fixes WP::parse_request(): Passing null to parameter #1 ($string) of type string is deprecated
notices on PHP 8.1.
Impact: 611 of the pre-existing tests are affected by this issue.
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
#29
@
4 years ago
- Owner set to hellofromTonya
- Status changed from new to accepted
Assigning myself as owner for code reviews and pair programming with @jrf.
This ticket was mentioned in PR #1582 on WordPress/wordpress-develop by jrfnl.
4 years ago
#31
More fixes for null
after parse_url()
, but in files for which unit tests can't easily be added.
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
This ticket was mentioned in PR #1582 on WordPress/wordpress-develop by jrfnl.
4 years ago
#32
More fixes for null
after parse_url()
, but in files for which unit tests can't easily be added.
Note: trac ticket link below not added yet on purpose while this ticket is still WIP.
Trac ticket:
4 years ago
#33
As discussed in the live stream session:
- Rebased on current
master
- Fixed up the double arrow alignment
- Wrote up the details of the issue in the "fix" commit message.
SergeyBiryukov commented on PR #1582:
3 years ago
#50
Why did you removed the extra parentheses in the condition in
ms_cookie_constants()
? They were there to prevent issues with operator precedence.
I think they are redundant here, as &&
always takes precedence over ||
, unless I'm missing something.
In my understanding, they are necessary in cases like this:
if ( a && ( b || c ) )
but don't make a difference in cases like this:
if ( a || ( b && c ) )
If that is incorrect, happy to add them back :)
3 years ago
#51
@SergeyBiryukov That is correct, but most people do not know the ins and outs of operator precedence in as much detail, so the parentheses clarify that for the next person reading the code. I'd say: leave it for now, but know that I did put them in for a reason.
SergeyBiryukov commented on PR #1582:
3 years ago
#52
Ah, that makes perfect sense. I appreciate the explanation and will keep it in mind from now on.
This ticket was mentioned in PR #1599 on WordPress/wordpress-develop by jrfnl.
3 years ago
#53
### Tests: add dedicated test for _set_cron_array()
Add a full set of tests for the _set_cron_array()
function.
### PHP 8.1: add input validation to the _set_cron_array() function
The _set_cron_array()
function expects a cron array as the first parameter, but will often be passed the - potentially updated - output of a call to _get_cron_array()
.
The _get_cron_array()
function may, however, return false
, in which case a "Deprecated: Automatic conversion of false to array is deprecated" warning will be thrown on PHP 8.1.
Fixed now by adding input validation to the _set_cron_array()
function.
Ref: https://developer.wordpress.org/reference/functions/_get_cron_array/
Trac ticket: https://core.trac.wordpress.org/ticket/53635
/cc @pbearne I did see your test for _set_cron_array()
in #1595, but I felt this function needed a more extensive set of tests, especially as it is prone to be hit by the PHP 8.1 deprecation notice. Please review when you have a chance.
This ticket was mentioned in PR #1599 on WordPress/wordpress-develop by jrfnl.
3 years ago
#54
### Tests: add dedicated test for _set_cron_array()
Add a full set of tests for the _set_cron_array()
function.
### PHP 8.1: add input validation to the _set_cron_array() function
The _set_cron_array()
function expects a cron array as the first parameter, but will often be passed the - potentially updated - output of a call to _get_cron_array()
.
The _get_cron_array()
function may, however, return false
, in which case a "Deprecated: Automatic conversion of false to array is deprecated" warning will be thrown on PHP 8.1.
Fixed now by adding input validation to the _set_cron_array()
function.
Ref: https://developer.wordpress.org/reference/functions/_get_cron_array/
Trac ticket: https://core.trac.wordpress.org/ticket/53635
/cc @pbearne I did see your test for _set_cron_array()
in #1595, but I felt this function needed a more extensive set of tests, especially as it is prone to be hit by the PHP 8.1 deprecation notice. Please review when you have a chance.
This ticket was mentioned in Slack in #hosting-community by crixu. View the logs.
3 years ago
#57
@
3 years ago
FYI: PHPMailer has just released a new version with preliminary PHP 8.1 support. Ticket #53953 has been opened to handle the upgrade.
#58
follow-up:
↓ 59
@
3 years ago
@SergeyBiryukov I'd advocate for a different fix for [51633]: silencing the deprecation notice (for now).
The actual auto_detect_line_endings
functionality has not been removed from PHP (yet) and will still work until PHP 9.0 and while the old MacOS line ending of \r
on its own, is nowadays a rarity to encounter, it may still exist in translation files which haven't been updated for a long time.
Also see this similar discussion elsewhere about this.
#59
in reply to:
↑ 58
;
follow-up:
↓ 64
@
3 years ago
Replying to jrf:
The actual
auto_detect_line_endings
functionality has not been removed from PHP (yet) and will still work until PHP 9.0 and while the old MacOS line ending of\r
on its own, is nowadays a rarity to encounter, it may still exist in translation files which haven't been updated for a long time.
Thanks for these details! If the functionality still exists, I agree with silencing the notice for now :)
3 years ago
#60
Oh, before I forget: as this is a "private" function (prefixed with a _
and annotated as such in the docblock). it _could_ be considered to add an array
type declaration instead, which has been supported since PHP 5.0.
- For the function signature, this would not be a BC-break as this function cannot be overloaded.
- For the function functionality, it would be as passing it something which is not an array will now result in a (catchable) fatal error/
TypeError
and any such instances in Core and plugins/themes would have to be fixed.
With that second reason in mind, I'm proposing the current solution, but I wanted to mention that the type declaration option had been considered.
#61
@
3 years ago
PR 1599 is ready for commit
.
Update:
There's one tiny nitpick of a full-stop at the end of an error message (noted in the PR) that the committer could fix. Resolved.
/cc @SergeyBiryukov
This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.
3 years ago
@
3 years ago
Pomo/ PHP 8.1: improve the fix for auto_detect_line_endings
... as per the inline comment.
#64
in reply to:
↑ 59
@
3 years ago
Replying to SergeyBiryukov:
Thanks for these details! If the functionality still exists, I agree with silencing the notice for now :)
@SergeyBiryukov Patch 53635-06-improve-auto_detect_line_endings-fix.patch
uploaded to do just that (with an extensive inline comment explaining it).
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
#67
@
3 years ago
I've just uploaded another patch - 53635-07-incorrect-default-value-for-http_build_query.patch
- which is ready for commit.
Full patch description:
PHP 8.1: WP_Sitemaps_Provider::get_sitemap_url(): fix deprecation notice
The WP_Sitemaps_Provider::get_sitemap_url()
method calls the PHP native http_build_query()
function, the second parameter of which is the _optional_ $numeric_prefix
parameter which expects a string
.
A parameter being optional, however, does not automatically make it nullable.
As of PHP 8.1, passing null
to a non-nullable PHP native function will generate a deprecation notice.
In this case, this function call yielded a http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated
notice.
Changing the null
to an empty string fixes this without BC-break.
This change is already covered by tests as 14 of the existing tests (targetting the sitemap functions) failed on these function calls when running the tests on PHP 8.1.
Refs:
#68
@
3 years ago
FYI: I've pulled fixes for all the known PHP 8.1 issues in SimplePie GetID3 (based on the WP test suite).
https://github.com/JamesHeinrich/getID3/pulls/jrfnl
I suspect there will be (plenty) more issues to fix in SimplePie GetID3, but as there are no tests for the package itself and the tests in WP are limited, these will be hard to pinpoint.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
This ticket was mentioned in PR #1625 on WordPress/wordpress-develop by jrfnl.
3 years ago
#71
The Requests_Cookie
class expects valid - non-null
- attributes to be passed, either as an array or as a Requests_Utility_CaseInsensitiveDictionary
object.
However, the WP_Http_Cookie::get_attributes()
explicitly sets the expires
, path
and domain
index keys in an array with values which _may_ be null
. This will cause strtotime(): Passing null to parameter #1 ($datetime) of type string is deprecated
-like errors when the attributes are passed to the Requests_Cookie
class.
Note: a null
value for path
would generate a similar deprecation notice, but for the preg_match()
function.
Fixed by using array_filter()
on the attributes to explicitly filter out null
values before passing the attributes to Requests_Cookie
.
Note: I'm chosing to explicitly only filter null
values. Using array_filter()
without a callback would filter out all "empty" values, but that may also remove values which are explicitly set to false
or 0
, which may be valid values.
Fixes two errors in the external-http
group in the WordPress Core test suite:
1) Tests_HTTP_Functions::test_get_response_cookies_with_wp_http_cookie_object strtotime(): Passing null to parameter #1 ($datetime) of type string is deprecated /var/www/src/wp-includes/Requests/Cookie.php:268 /var/www/src/wp-includes/Requests/Cookie.php:237 /var/www/src/wp-includes/Requests/Cookie.php:90 /var/www/src/wp-includes/class-http.php:460 /var/www/src/wp-includes/class-http.php:349 /var/www/src/wp-includes/class-http.php:624 /var/www/src/wp-includes/http.php:162 /var/www/tests/phpunit/tests/http/functions.php:156 2) Tests_HTTP_Functions::test_get_cookie_host_only strtotime(): Passing null to parameter #1 ($datetime) of type string is deprecated /var/www/src/wp-includes/Requests/Cookie.php:268 /var/www/src/wp-includes/Requests/Cookie.php:237 /var/www/src/wp-includes/Requests/Cookie.php:90 /var/www/src/wp-includes/class-http.php:460 /var/www/tests/phpunit/tests/http/functions.php:235
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1628 on WordPress/wordpress-develop by jrfnl.
3 years ago
#72
### Tests: add a dedicated test file for the wpdb::_real_escape() method
### PHP 8.1 | wpdb::_real_escape(): fix "passing null to non-nullable" deprecation notices
The PHP native mysqli_real_escape_string()
function expects to be passed a string as the second parameter and this is not a nullable parameter.
Passing null
to it will result in a mysqli_real_escape_string(): Passing null to parameter #2 ($string) of type string is deprecated
notice on PHP 8.1.
Previously, an input type check was put in place to prevent fatal errors on PHP 8.0 when an array, object or resource was passed. Changeset [48980].
A null
value was explicitly excluded from that check, even though a null
value being passed would only ever result in an empty string anyway.
This commit changes the previous input type check to also bow out early for null
values and to automatically return an empty string for those.
Refs:
- https://www.php.net/manual/en/mysqli.real-escape-string.php
- https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg
Trac ticket: https://core.trac.wordpress.org/ticket/53635
@
3 years ago
PHP 8.1 | Tests: fix bug in Tests_Admin_IncludesPlugin::test_get_plugin_files_folder() The Tests_Admin_IncludesPlugin::_create_plugin()
expects the first parameter to be a text string to be written to a plugin file using fwrite()
. Passing null causes a fwrite(): Passing null to parameter #2 ($data) of type string is deprecated
notice. Ref: https://www.php.net/manual/en/function.fwrite
#73
@
3 years ago
Patch 53635-08-null-to-non-nullable-in-testcode.patch
which I just uploaded is ready for commit. Simple test only fix.
GitHub PRs 1625 and 1628 which I opened earlier, are both ready for review.
This ticket was mentioned in PR #1631 on WordPress/wordpress-develop by jrfnl.
3 years ago
#74
👉🏻 This PR will be easiest to review by examining the individual commits in the order they are lined up in.
This commit splits the tests in the tests/phpunit/tests/compat.php
file up into individual test classes for each function being tested.
The tests have been reviewed for typical test issues and those issues have been fixed in each test class.
Fixes are along the lines of:
- Adding
@covers
tags. - Adding visibility modifiers to all methods.
- Adding function availability test.
- Where relevant, fixing the assertion parameter order.
- Where relevant, reworking a test to use a data provider.
- Where relevant, renaming data provider methods to have a more obvious link to the test it applies to.
- Making data providers more readable by adding keys within the data sets and often also updating them to named data sets.
- Making the actual test code more readable by using descriptive variables and multi-line function calls.
- Adding the
$message
parameter to all assertions when a test method contains more than one assertion. - Adding/removing data sets in data providers.
- Renaming fixture classes to more unique names.
One of the tests was causing errors in PHP 8.1. A fix for this is included in a separate commit.
### PHP 8.1 | Compat/_mb_substr(): fix passing null to non-nullable deprecation
On PHP 8.1, the _mb_substr()
function would throw a preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated
notice when passed null
.
To maintain the same behaviour as before, let's just bow out early when $str
is passed as null
as the outcome will, in that case, only ever be an empty string.
Note: this does mean that the _mb_substr()
function now has a subtle difference in behaviour compared to the PHP native mb_substr()
function as the latter *will* throw the deprecation notice.
The existing tests for the compat function already cover this issue.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
Trac ticket: https://core.trac.wordpress.org/ticket/53363
This ticket was mentioned in PR #1632 on WordPress/wordpress-develop by jrfnl.
3 years ago
#75
### Tests_Functions_Anonymization: add a few more invalid IP test cases
... and add extra @covers
tag for the two functions which are testing the wp_privacy_anonymize_ip()
function.
(At class level, the wp_privacy_anonymize_data()
is set as covered).
### PHP 8.1 | wp_privacy_anonymize_ip(): fix null to non-nullable deprecation
The wp_privacy_anonymize_ip()
function expects a string for the $ip_addr
parameter, but did not do any input validation.
One of the pre-existing test cases, passed null
to the function, leading to a substr_count(): Passing null to parameter #1 ($haystack) of type string is deprecated
notice on PHP 8.1.
Fixed now by doing a cursory check on the variable at the start of the function and bowing out early for a number of cases (null
, false
, 0
, ''
) which would all result in the same 0.0.0.0
output anyway.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1633 on WordPress/wordpress-develop by jrfnl.
3 years ago
#76
The WP_Comment_Query::get_comment_ids()
method is supposed to handle null
as a search query, but was throwing a strlen(): Passing null to parameter #1 ($string) of type string is deprecated
notice on PHP 8.1.
Discovered via and already covered via the pre-existing Tests_Comment_Query::test_search_null_should_be_ignored()
test method.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1633 on WordPress/wordpress-develop by jrfnl.
3 years ago
#77
The WP_Comment_Query::get_comment_ids()
method is supposed to handle null
as a search query, but was throwing a strlen(): Passing null to parameter #1 ($string) of type string is deprecated
notice on PHP 8.1.
Discovered via and already covered via the pre-existing Tests_Comment_Query::test_search_null_should_be_ignored()
test method.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1635 on WordPress/wordpress-develop by jrfnl.
3 years ago
#78
The img_caption_shortcode()
method expects a string for the $content
parameter and is expected to return a string as well.
However, the default value for the $content
parameter was set to null
and previously, if the $content
parameter was not passed, the function would return null
.
This to me seems more like a bug in the function declaration signature, than anything else, so I'm suggesting to update the default parameter value for $content
to an empty string.
This _does_ result in a minor/inconsequential BC break, in that the function will now return an empty string instead of null
for that situation, but that seems _more correct_ than before and complies with the specification of the function in the docblock.
Tests which specifically tested for null
being returned have been updated to expect an empty string.
Fixes a few notices in the existing test suite:
strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1636 on WordPress/wordpress-develop by jrfnl.
3 years ago
#79
The term_exists()
function expects a string or an integer for the $term
parameter, but did not do any input validation.
One of the pre-existing test cases, passed null
to the function, leading to a trim(): Passing null to parameter #1 ($string) of type string is deprecated
notice on PHP 8.1.
Fixed now by doing a cursory check on the variable at the start of the function and bowing out early in case the $term
is null
.
Side-note: this function should in all reality be refactored/rewritten to two or three different functions, as the number of different return types for this function introduces a high risk of failure into any code using this function.
The issue was discovered via and is already covered by the Tests_TermExists::test_term_exists_unknown()
test method.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1637 on WordPress/wordpress-develop by jrfnl.
3 years ago
#80
The get_option()
function will return false
for a widget which was not previously registered.
In PHP 8.1, this will lead to an Automatic conversion of false to array is deprecated
notice.
This fixes the test callback to prevent this notice.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1638 on WordPress/wordpress-develop by jrfnl.
3 years ago
#81
Okay, so this one takes a little explaining.
Basically, the whole assertSameIgnoreEOL()
assertion was fundamentally flawed. The assertion contends that it checks that the expected and actual values are of the same type and value, but the reality was very different.
- The function uses
map_deep()
to potentially handle all sorts of inputs. map_deep()
handles arrays and objects with special casing, but will call the callback on everything else without further distinction.- The callback used passes the expected/actual value on to the
str_replace()
function to remove potential new line differences. - And the
str_replace()
function will - with a non-array input for the$subject
- always return a string. - The output of these calls to
map_deep()
will therefore have "normalized" _all properties_ in objects, _all values_ in arrays and _all non-object, non-array values_ to strings. - And a call to
assertSame()
will therefore NEVER do a proper type check as the type of all input has already, unintentionally, been "normalized" to string.
Aside from this clear flaw in the design of the assertion, PHP 8.1 now exposes a further issue as a null
value for an object property, an array value or a plain value, will now yield a str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated
notice.
To fix both these issues, the fix in this PR ensures that the call to str_replace()
will now only be made if the input is a text string.
All other values passed to the callback are left in their original type.
This ensures that a proper value AND type comparison can be done as well as prevents the PHP 8.1 deprecation notices.
Includes adjusting the method documentation for both this method and the assertEqualsIgnoreEOL()
alias method to document that the $expected
and $actual
parameters can be of any type.
Ref:
- https://developer.wordpress.org/reference/functions/map_deep/
- https://www.php.net/manual/en/function.str-replace.php
👉🏻 Note: I'm fully aware that this change _may_ cause existing tests using these assertions to fail. (all the more reason to run this PR through GH) If any tests _would_ fail those tests should be investigated and fixed. Those failures should not be a reason for not accepting this patch.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
Trac ticket: https://core.trac.wordpress.org/ticket/53363
This ticket was mentioned in PR #1640 on WordPress/wordpress-develop by jrfnl.
3 years ago
#82
### Tests_Option_Option::test_bad_option_names(): rework to data provider
- Rework the test method to use a data provider with named data sets instead of a
foreach()
. - Add method visibility modifiers.
- Add the
$message
parameter for each assertion.
### PHP 8.1: (get|add|update|delete)_option: fix null to non-nullable deprecations
In all four of the get_option()
, add_option()
, update_option()
and delete_option()
functions, the $option
parameter is passed to the PHP native trim()
function without prior validation.
In PHP 8.1, this could lead to a trim(): Passing null to parameter #1 ($string) of type string is deprecated
for each of these methods.
trim()
expects a text string and is only useful when _passed_ a text string as no other variable type can contain whitespace.
Fixed now by verifying that the $option
is a string before processing it with trim()
.
This issue is already covered by the existing Tests_Option_Option::test_bad_option_names()
test.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1641 on WordPress/wordpress-develop by jrfnl.
3 years ago
#83
In the WP_Meta_Query::get_sql_for_clause()
, the 'value'
index from a meta query array is passed to the PHP native trim()
function without prior validation.
In PHP 8.1, this could lead to a trim(): Passing null to parameter #1 ($string) of type string is deprecated
notice.
trim()
expects a text string and is only useful when _passed_ a text string as no other variable type can contain whitespace.
Fixed now by verifying that the _value_ is a string before processing it with trim()
.
This issue is already covered by the existing Tests_Meta_Query::test_null_value_sql()
and the Tests_Meta_Query::test_convert_null_value_to_empty_string(0
tests.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
hellofromtonya commented on PR #1599:
3 years ago
#85
Committed in changeset https://core.trac.wordpress.org/changeset/51695.
This ticket was mentioned in Slack in #core-php by hellofromtonya. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.
3 years ago
This ticket was mentioned in PR #1657 on WordPress/wordpress-develop by jrfnl.
3 years ago
#88
... which could be passing null
onto a context which doesn't allow for null
due to the output of wp_parse_url()
with the $component
parameter potentially being null
.
This patch was discussed in detail during the Sept 3rd livestream.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1658 on WordPress/wordpress-develop by jrfnl.
3 years ago
#89
Potential fix for the below issue, but the fix _may_ have unforeseen side-effects. Needs second opinion from someone with detailed domain knowledge of the XMLRPC functionality.
Tests_XMLRPC_mw_newPost::test_post_thumbnail strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated path/to/src/wp-includes/class-wp-xmlrpc-server.php:5681 path/to/src/wp-includes/src/wp-includes/class-wp-xmlrpc-server.php:5606 path/to/tests/phpunit/tests/xmlrpc/mw/newPost.php:147
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in Slack in #core by jrf. View the logs.
3 years ago
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
hellofromtonya commented on PR #1632:
3 years ago
#93
Committed with changesets https://core.trac.wordpress.org/changeset/51792 and https://core.trac.wordpress.org/changeset/51793.
hellofromtonya commented on PR #1636:
3 years ago
#95
Committed with changeset https://core.trac.wordpress.org/changeset/51796.
hellofromtonya commented on PR #1641:
3 years ago
#97
Committed via changeset https://core.trac.wordpress.org/changeset/51797.
hellofromtonya commented on PR #1628:
3 years ago
#99
Committed in changesets https://core.trac.wordpress.org/changeset/51798 and https://core.trac.wordpress.org/changeset/51799.
hellofromtonya commented on PR #1625:
3 years ago
#101
Discussion for this patch is here https://www.youtube.com/watch?v=BQbaDBDCOIA&t=952s.
hellofromtonya commented on PR #1625:
3 years ago
#103
Committed in changeset https://core.trac.wordpress.org/changeset/51801.
hellofromtonya commented on PR #1625:
3 years ago
#104
Discussion for this patch is here https://www.youtube.com/watch?v=BQbaDBDCOIA&t=952s.
hellofromtonya commented on PR #1633:
3 years ago
#106
Committed via changeset https://core.trac.wordpress.org/changeset/51806
hellofromtonya commented on PR #1635:
3 years ago
#107
Asked for review by Core Media Maintainers.
I took at look at the top dozen or so results for plugins that use
img_caption_shortcode
, and I didn't find any examples that would throw an error with this change. The only reference to null at all used loose comparisons, so it would still be fine. (And that didn't compare against$content
, anyway.) Overall, it looks pretty safe.
hellofromtonya commented on PR #1635:
3 years ago
#108
Feedback from @azaozz on slack #core-media channel:
So thinking that it's for the better to change the return type from
null
to empty string when there's an error with the shortcode. Might impact something but it should be very minimal.$content
is expected to always be a string anyway
hellofromtonya commented on PR #1635:
3 years ago
#110
Committed with changeset https://core.trac.wordpress.org/changeset/51816.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
hellofromtonya commented on PR #1640:
3 years ago
#114
Committed with changesets https://core.trac.wordpress.org/changeset/51817 and https://core.trac.wordpress.org/changeset/51818.
hellofromtonya commented on PR #1657:
3 years ago
#116
Committed in changeset https://core.trac.wordpress.org/changeset/51829.
hellofromtonya commented on PR #1637:
3 years ago
#118
Committed in changeset https://core.trac.wordpress.org/changeset/51830.
hellofromtonya commented on PR #1638:
3 years ago
#120
Committed via changeset https://core.trac.wordpress.org/changeset/51831.
#122
@
3 years ago
GetID3 has just released a new version which includes the PHP 8.1 fixes I pulled previously.
I've opened a separate ticket for the update: https://core.trac.wordpress.org/ticket/54162
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
hellofromtonya commented on PR #1631:
3 years ago
#125
Committed with changesets [51852-51853].
https://core.trac.wordpress.org/changeset/51852
https://core.trac.wordpress.org/changeset/51853
This ticket was mentioned in PR #1695 on WordPress/wordpress-develop by jrfnl.
3 years ago
#126
This adds an expectation for a "passing null to non-nullable" deprecation notice to select tests where the deprecation is generated by one of the functions in the wp-includes/formatting.php
file, either via a filter hook callback or by a direct call.
Instead of haphazardly fixing these issues exposed by the tests, a more structural and all-encompassing solution for input validation should be architected and implemented as otherwise, we'll keep running into similar issues time and again with each new PHP version.
To discourage people from "fixing" these issues now anyway, this commit "hides" nearly all of these issues from the test runs.
Once a more structural solution has been designed, these tests and the underlying functions causing the deprecation notices should be revisited and the structural solution put in place.
Includes a few minor other tweaks to select tests:
- Removing a stray
return
(twice) from assertion statements. - Removing calls to
ob_*()
functions in favour of letting PHPUnit manage the output catching.
This prevents warnings along the lines of
Test code or tested code did not (only) close its own output buffers
.
A follow up Trac ticket will be opened to address further uses of ob_*()
functions in the test suite as all of those are liable to cause these same type of warnings.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
3 years ago
#127
Actually, there is already a ticket open to review the use of output buffering/output expectation setting in test - will follow up on the other issues in that ticket: https://core.trac.wordpress.org/ticket/53118
3 years ago
#128
Actually, there is already a ticket open to review the use of output buffering/output expectation setting in test - will follow up on the other issues in that ticket: https://core.trac.wordpress.org/ticket/53118
hellofromtonya commented on PR #1695:
3 years ago
#131
Committed via changeset https://core.trac.wordpress.org/changeset/51968.
This ticket was mentioned in PR #1798 on WordPress/wordpress-develop by anomiex.
3 years ago
#132
This fixes the "Deprecated: Return type of Requests_Cookie_Jar::offsetExists()
should be compatible with ArrayAccess::offsetExists(): bool
" and similar warnings on PHP 8.1.
PHP native interfaces now have declared return types and methods in classes implementing these interfaces need to either have the return type declared (in a covariant compatible manner with the PHP native interface method declaration), or need to silence the deprecation warning using the #[ReturnTypeWillChange]
attribute.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1799 on WordPress/wordpress-develop by anomiex.
3 years ago
#133
This fixes the "Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::offsetExists()
should be compatible with ArrayAccess::offsetExists(): bool
" and similar warnings on PHP 8.1.
PHP native interfaces now have declared return types and methods in classes implementing these interfaces need to either have the return type declared (in a covariant compatible manner with the PHP native interface method declaration), or need to silence the deprecation warning using the #[ReturnTypeWillChange]
attribute.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1800 on WordPress/wordpress-develop by anomiex.
3 years ago
#134
This fixes the "Deprecated: http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated" warning on PHP 8.1.
PHP has started requiring that built-in method arguments that are not explicitly declared as nullable may no longer be passed null. The correct value for the $numeric_prefix
parameter to http_build_query()
is an empty string.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
This ticket was mentioned in PR #1801 on WordPress/wordpress-develop by anomiex.
3 years ago
#135
This fixes a "Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated" warning on PHP 8.1.
PHP has started requiring that built-in method arguments that are not explicitly declared as nullable may no longer be passed null. In this case it seems to make more sense to avoid calling WordPress's own email_exists()
function with null, as that method is declared as taking only strings and a null email is never going to exist anyway.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
#136
@
3 years ago
Hi! I just pushed four PRs to the GitHub mirror. Per the instructions I'm leaving a comment pointing that out as the bot's comments above apparently don't trigger notifications to people watching the ticket.
hellofromtonya commented on PR #1798:
3 years ago
#137
Hello @anomiex and thank you for your contribution. The fixes in this PR are for an external library, i.e. Requests. Fixes for this library should be made in its repo https://github.com/wordpress/requests and not in Core itself. Once Requests releases a new version, then Core will be updated.
hellofromtonya commented on PR #1799:
3 years ago
#138
Hello @anomiex and thank you for your contribution. The fixes in this PR are for an external library, i.e. Requests. Fixes for this library should be made in its repo https://github.com/wordpress/requests and not in Core itself. Once Requests releases a new version, then Core will be updated.
3 years ago
#139
Hmm. Hopefully you get the library updated sooner rather than later, as these errors are blocking our own work on PHP 8.1 compatibility.
3 years ago
#140
FYI, you are in fact running into the deprecation this is fixing in your existing tests. See for example https://github.com/WordPress/wordpress-develop/runs/4083354600?check_suite_focus=true#step:20:403
And a test that would try to pass an empty string as an email address won't actually reach this code, as that gets rejected as "invalid email address" during parameter validation. I can still add such a test if you insist, but it wouldn't actually test the code being changed here.
3 years ago
#141
Hmm. Hopefully you get the library updated sooner rather than later, as these errors are blocking our own work on PHP 8.1 compatibility.
The project was recently moved into the WordPress organization, so the Core group with access (not me!) can cut a new version when needed. At worst, I would expect it in time for the 5.9 Beta 1 (Nov 16 ETA).
3 years ago
#142
I looked at that failing test before and my assessment at the time was that this is an issue with the way the test is set up, not with the actual code.
self::$user
is not being correctly mocked in the wpSetUpBeforeClass()
method in the WP_Test_REST_Users_Controller
class which leads to the null
for the email address.
This is not a situation which should ever occur in "real life" and should therefore not be solved in the src
code, but by improving the test.
3 years ago
#143
You may want to check again. The problem isn't whether the user has an email address set, it's whether the incoming REST request specifies a value for the 'email' parameter (i.e. to update the email).
You should be able to prove that to yourself easily enough: Add something like
{{{php
if ( null === $requestemail? ) return new WP_Error( 'email_is_null', 'Email is null', array( 'status' => '500' ) );
}}}
just before the email_exists()
call, then try to make a REST request to update the description of an existing user that does have an email address.
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
hellofromtonya commented on PR #1800:
3 years ago
#145
Committed in changeset https://core.trac.wordpress.org/changeset/52019.
#146
@
3 years ago
In 52019:
Code Modernization: Pass correct default value to http_build_query()
in get_core_checksums()
and wp_version_check()
.
The get_core_checksums()
and wp_version_check()
functions call the PHP native http_build_query()
function, the second parameter of which is the optional $numeric_prefix
parameter which expects a non-nullable string
.
A parameter being optional, however, does not automatically make it nullable.
As of PHP 8.1, passing null
to a non-nullable PHP native function will generate a deprecation notice.
In this case, this function call yielded a http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated
notice.
Changing the null
to an empty string fixes this without a backward compatibility break.
References:
- PHP Manual: http_build_query()
- PHP RFC: Deprecate passing null to non-nullable arguments of internal functions
Follow-up to [18697], [25540].
Props bjorsch, kraftbj, hellofromTonya, jrf.
See #54229.
This ticket was mentioned in Slack in #core-php by dingo_d. View the logs.
3 years ago
#151
@
3 years ago
As part of my work on the plugin Web stories, we upgraded our tests to run under PHP 8.1. There were a couple of notice errors we saw in our logs.
These errors are
PHP Deprecated: rtrim(): Passing null to parameter #1 ($string) of type string is deprecated in /tmp/wordpress/wp-includes/formatting.php on line 2753
PHP Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in /tmp/wordpress/wp-includes/class-wp-user.php on line 211
These will need to be fixed for full PHP 8.1 support.
This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.
3 years ago
#153
@
3 years ago
@spacedmonkey Deprecations are not errors and the behaviour of PHP has not (yet) changed for these things. The code as-is is compatible with PHP 8.1.
The typical "passing null to non-nullable" deprecations are generally all related to (the lack of) input validation/defensive coding. This is something which has been plaguing us for years now with PHP increasingly getting stricter.
So rather than making yet more hap-snap ad-hoc fixes, a discussion has been started about solving these type of issues in a more structural way by introducing input validation in a consistent manner in Core.
This discussion has only just started and has not reached a conclusion yet, but as these are deprecations (not errors), we have time until PHP 9.0 to have these conversations and solve this properly.
I'd be very happy for you to join these conversations.
Please also see my comment about this on Slack: https://wordpress.slack.com/archives/C02RQBWTW/p1637194383021500?thread_ts=1637182618.499600&cid=C02RQBWTW
@
3 years ago
Avoid notices in untrailingslashit() due to the default value of script translations path being null instead of an empty string. Also the doc blocks don't document null as an accepted value for the path.
This ticket was mentioned in PR #2053 on WordPress/wordpress-develop by noisysocks.
3 years ago
#155
As of PHP 8.1, explode() does not permit null as its second argument. This results in warnings being spat out on every page because of a usage of this in wp-includes/block-supports/layout.php
.
Trac ticket: https://core.trac.wordpress.org/ticket/53635
#157
follow-up:
↓ 158
@
3 years ago
@isabel_brison Where are the tests to accompany [52380] ? There is no safeguard in place now to ensure that this patch does not break something else.
#158
in reply to:
↑ 157
;
follow-up:
↓ 159
@
3 years ago
Replying to jrf:
@isabel_brison Where are the tests to accompany [52380] ? There is no safeguard in place now to ensure that this patch does not break something else.
It's a new function added in WP 5.9. Unfortunately there aren't unit tests for the function though there may be some E2E tests in Gutenberg that cover the behaviour. @youknowriad would know for sure. I wanted to fix this as it's a very prominent warning when visiting any page on a WP 5.9 site running PHP 8.1 🙂
#159
in reply to:
↑ 158
@
3 years ago
Replying to noisysocks:
It's a new function added in WP 5.9. Unfortunately there aren't unit tests for the function though there may be some E2E tests in Gutenberg that cover the behaviour. @youknowriad would know for sure. I wanted to fix this as it's a very prominent warning when visiting any page on a WP 5.9 site running PHP 8.1 🙂
The fact that there apparently are no tests, is all the more reason why tests should be added.
Keep in mind that deprecation warnings are just that: deprecations. The behaviour of PHP is unchanged at this point, it is just a gentle warning that the behaviour of PHP will change in PHP 9.0.
On a normal WP install, deprecation warnings will never show. It is only when someone turns WP_DEBUG
on or fiddles with PHP error_reporting
settings that these will show. So these warnings being displayed is nothing to be concerned about, they are part of a bigger discussion on input validation in Core.
In contrast to what PHP is doing, your change, however, is a behavioural change, so should be covered by tests.
#160
@
3 years ago
Indeed, it seems we have unit tests for most block supports but not the layout, we'd definitely benefit from a new one there. These features are all covered in e2e tests but for these frontend rendering functions, some additional unit tests would be good.
As for the changes itself, I'd like to encourage everyone to ideally make all the Gutenberg specific changes (block supports, ...) in the plugin as well if possible. The plugin is supposed to support two WP versions meaning that folks on previous versions won't benefit from these changes and it would create inconsistencies and some headaches for us doing the back porting for these features. (For instance this might get inadvertently reverted in the next block supports backports)
#161
@
3 years ago
@youknowriad the fix has been merged in Gutenberg too: https://github.com/WordPress/gutenberg/pull/37392
The main reason it was done separately in core and not as part of a backport was so @noisysocks could walk me through the core commit process :)
#164
@
3 years ago
- Resolution set to fixed
- Status changed from accepted to closed
- Summary changed from PHP 8.1: various compatibility fixes to PHP 8.1: various compatibility fixes for WordPress 5.9
With 5.9 RC1 tomorrow, closing this ticket. Ongoing compatibility fixes will continue in #54730 epic ticket during the 6.0 cycle.
Thank you everyone for your contributions!!
If a regression or blocker is identified during the 5.9 RC cycle, this can be referenced and reopened if necessary with dev-reviewed
.
3 years ago
#165
I see that you've closed https://core.trac.wordpress.org/ticket/53635 without addressing the issue this PR fixes. What's the way forward now?
hellofromtonya commented on PR #1801:
3 years ago
#166
I see that you've closed https://core.trac.wordpress.org/ticket/53635 without addressing the issue this PR fixes. What's the way forward now?
@anomiex Trac ticket https://core.trac.wordpress.org/ticket/54730 is now open for continuing PHP 8.x compatibility fixes during the 6.0 cycle. I updated the description in this PR to point at the new ticket.
3 years ago
#167
Looks like #2133 will fix this too, by making the called functions no longer object to being passed null. If that gets merged, this can be closed.
hellofromtonya commented on PR #1801:
3 years ago
#168
Looks like https://github.com/WordPress/wordpress-develop/pull/2133 will fix this too, by making the called functions no longer object to being passed null. If that gets merged, this can be closed.
Once #2133 gets merged, then yes, it will resolve this PR's deprecation notice without further changes. Plus #2133 resolves the issue at the lower level, i.e. where the deprecation notice is triggered. I think overall this PR can be closed in favor of #2133.
Note: There's an improvement opportunity, though out of scope for the purpose of this PR:
- Guard
email_exists()
to invokeget_user_by()
only when a string type is passed toemail_exists()
, i.e. providing a teeny tiny micro-optimization when a non-string is passed. (This tweak deserves a separate PR and tests.)
SergeyBiryukov commented on PR #1658:
2 years ago
#169
Thanks for the PR! I believe this was superseded by #3187.
In 51396: