Fix function argument type issues reported by PHPStan

[SergeyBiryukov]

Background: #50913

Splitting this out from comment:44:ticket:50913. @xknown is going through some ​issues reported by PHPStan, mostly in places where we are passing the incorrect type to WP core or PHP's built-in functions.

Let's use this ticket to include these fixes in core.

Goals are to:

• retain the current behavior (i.e. to avoid breaking changes)
• retain the triggering of notice/warning/error, which are clues and indicators (i.e. rather than silently letting the problem propagate downstream)
• gain PHP 8 compatibility
• ensure code coverage to validate changes

This PR attempts to fix the arguments type passed to a WP core and PHP's built-in functions, which would hopefully help reduce the number of PHP fatals for PHP 8.0.

The full list of reported issues can be seen at

​https://gist.github.com/xknown/77f8cbe233da75080d1e9258a8c94f95

https://core.trac.wordpress.org/ticket/51423

Looked through a few of the changes (by far not all) and would like to observe the following: a lot of these changes bypass underlying errors by doing type casting, instead of handling the errors properly. While this will work, I'm not sure this is the always the right choice.

Looked through a few of the changes (by far not all) and would like to observe the following: a lot of these changes bypass underlying errors by doing type casting, instead of handling the errors properly. While this will work, I'm not sure this is always the right choice.

Thanks for the feedback, and sorry for the late response. I have been busy with some work-related things, and some afk time. I agree that most of the changes are only adding explicit casts, which is probably fine in most cases.

I've been manually reviewing these changes, and while I add some proper error checking in some places, some commits only add explicit casts mostly because the surrounding code don't really handle very well error conditions (one example that comes to my mind is errors happening when running wp_install()). If you point me out a few cases where do you think we can do something different, then please let me know.

[xknown]

I compiled a list of changes that it'd good to include in the 5.6 release to avoid potential PHP 8.0 issues (some of them fix real PHP 8.0 fatals), if we want to leave the unneeded type casts changes out of them, that's fine for me. At this point I think it'd be good to add either these patches here or maybe have an individual PR for each change, so it's easier to be reviewed.

• ​https://github.com/WordPress/wordpress-develop/commit/bbd0a6674ec51a73f43c17527175e46c15005d4a.patch
• ​https://github.com/WordPress/wordpress-develop/commit/2293464146dfba70b1e4b6f25f9dde3343cbc397.patch
• ​https://github.com/WordPress/wordpress-develop/commit/6faab47f0d02c65f0fccbf6e7bd2c222b565fcbe.patch
• ​https://github.com/WordPress/wordpress-develop/commit/174414b2b5a6d2803a49c045fc96a687c46186d3.patch
• ​https://github.com/WordPress/wordpress-develop/commit/4df45db40eb7dee4fe35e1bd55858e84ab250816.patch
• ​https://github.com/WordPress/wordpress-develop/commit/943b24fe5f052e701f6859a0c1901b1a29ab3a46.patch
• ​https://github.com/WordPress/wordpress-develop/commit/b141c7d20796304ab43d25ab014250159c8764fe.patch
• ​https://github.com/WordPress/wordpress-develop/commit/8f4155d9570ad647701122e65dfd30a0b7443b5c.patch
• ​https://github.com/WordPress/wordpress-develop/commit/8abbe61141ac599ed9a39557d4018fc1458197ed.patch
• ​https://github.com/WordPress/wordpress-develop/commit/4610deabb881676a830e2d3b729b437a67b7ed7f.patch
• ​https://github.com/WordPress/wordpress-develop/commit/30e3c0aa8e4a64242cf72b39b77294f1b5eadfb6.patch
• ​https://github.com/WordPress/wordpress-develop/commit/6abe05c0f104323381a7af5a95dc2fd52d3666eb.patch
• ​https://github.com/WordPress/wordpress-develop/commit/514d8c25292d2a75ac0d74b0b561fce2f2dea8d2.patch
• ​https://github.com/WordPress/wordpress-develop/commit/a791daffbd642d9239f69e6d62eba421a3d9298a.patch
• ​https://github.com/WordPress/wordpress-develop/commit/2ecb556ca593f2a2ac8ee96b4f3aa2b221248d14.diff
• ​https://github.com/WordPress/wordpress-develop/commit/fae1cc76011b45cf269025f740b5909d0c02acaf.patch
• ​https://github.com/WordPress/wordpress-develop/commit/2959d5d997de9ade045ac508ea122ddf371c59a7.patch (wp_http_supports invalid parameter)
• ​https://github.com/WordPress/wordpress-develop/commit/08ccb0f64513004872b538df90bf4c0c8ab5edb2.patch
• ​https://github.com/WordPress/wordpress-develop/commit/ab4657c1adbd7b104543a6d0b9423365513eca3d.diff
• ​https://github.com/WordPress/wordpress-develop/commit/b71e78396a8c3fa255fe90e49afed88d19f2fb00.patch
• ​https://github.com/WordPress/wordpress-develop/commit/d12e5090f6adec44c65912b8caf67e980c1b5e41.patch false PHPStan trigger
• ​https://github.com/WordPress/wordpress-develop/commit/fe3ed74e60fce4c5a7a852e4368afe4b968a595f.patch
• ​https://github.com/WordPress/wordpress-develop/commit/19ed05fecd4c466f1c39534dd752bf84e2ffea77.patch

[hellofromTonya]

Replying to xknown:

At this point I think it'd be good to add either these patches here or maybe have an individual PR for each change, so it's easier to be reviewed.

Talking with @xknown and @jrf, I'll help with the task of splitting these commits into separate patches or PRs. These smaller chunks of code will allow for discussion and code review at more granular level. I'll get this task done today.

Updates wp_update_link to handle invalid link ids, it also adds a couple
of unit tests.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

It fixes the following PHP fatal when the fetch_feed function is called.

PHP Fatal error:  Uncaught TypeError: call_user_func_array(): Argument
WP_Feed_Cache::create() cannot be called statically in /var/www/wp/wp-includes/SimplePie/Registry.php:214

Trac ticket: https://core.trac.wordpress.org/ticket/51423

• Make sure we handle properly the cases where core functions return a

WP_Error instance. Otherwise they could produce type errors.

• Update the PHPDoc type hints to take into account the default values.
• Adds explicit casts to str_repeat which expects an integer. This is

not required, but might be good to start fixing them as they might need
to be updated in future PHP versions.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Trac ticket: https://core.trac.wordpress.org/ticket/51423

wp-admin/nav-menus.php: Handle error conditions, and also pass the appropriate types to avoid potential PHP type errors/warnings.

Props @xknown.

Split of larger PR for this ​commit.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split of larger PR for ​commit.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split of larger PR for this commit](​https://github.com/WordPress/wordpress-develop/commit/b71e78396a8c3fa255fe90e49afed88d19f2fb00).

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split of a larger PR in commit ​https://github.com/WordPress/wordpress-develop/commit/ab4657c1adbd7b104543a6d0b9423365513eca3d

​`fopen`

Returns a file pointer resource on success, or FALSE on failure

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split of a larger PR for this commit ​https://github.com/WordPress/wordpress-develop/commit/08ccb0f64513004872b538df90bf4c0c8ab5edb2

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split from larger PR in commit ​https://github.com/WordPress/wordpress-develop/commit/2959d5d997de9ade045ac508ea122ddf371c59a7

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Props @xknown

Split of larger PR for commit ​https://github.com/WordPress/wordpress-develop/commit/fae1cc76011b45cf269025f740b5909d0c02acaf

Update WP_Site_Icon::create_attachment_object() as well as
WP_Site_Icon::insert_attachment() to handle error conditions.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Add WP_Error checks for the get_terms calls to avoid PHP 8.0 type errors.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

list_files might return false, which will produce a type error in PHP 8.0 when used in the

array_map call.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Don't attempt to use wp_list_pluck on a WP_Error object.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Check the potential error conditions which might produce type errors in PHP 8.0.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

• Adds a check to make sure we are iterating on a non null collection.
• Fixes also a bug on a ternary operator that casts the condition to integer (the cast operator takes precedence over the ternary operator), not the page value.

See the example below.

$page = 'a0';
var_dump((int) ( '' == $page ) ? 1 : $page);
---
string(2) "a0"

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Check for error conditions to avoid potential type errors.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Handle error conditions to avoid potential type errors.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Handle error conditions.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Closing in favor of individual PRs. See https://core.trac.wordpress.org/ticket/51423 for more details.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Split from larger PR for commit ​https://github.com/WordPress/wordpress-develop/commit/2ecb556ca593f2a2ac8ee96b4f3aa2b221248d14

Closing in favor of PR ​https://github.com/WordPress/wordpress-develop/pull/735 which includes using false failure check for fopen for consistency, ie instead of is_resource.

There seems to be already a fix for this issue in https://core.trac.wordpress.org/ticket/51629, and https://core.trac.wordpress.org/ticket/29204

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Fixes wp-includes/bookmark.php for array_unique:

"Parameter #1 $input of function array_unique expects array, array|WP_Error given."

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Trac ticket: https://core.trac.wordpress.org/ticket/51423

[jrf]

Hi @xknown and @hellofromTonya, thank you so much for taking this on, breaking the changes up into individual PRs and adding tests. This will definitely make it easier to review and discuss the proposed changes.

At the request of @hellofromTonya I've reviewed ​PR #736 now and left an extensive comment for you to consider: ​https://github.com/WordPress/wordpress-develop/pull/736#pullrequestreview-528734649

The feedback I left there might also help inform how to handle some of the other changes needed.

Hope it helps!

@jrfnl You are correct.

### Changed code's behavior

The code's behavior was changed when WP_Error is returned when 'link_category' taxonomy is not registered. This is not desired as it could be a breaking change with no benefits.

This was bugging me last night, but wasn't well-formed. Thank you for thoroughly and clearly laying out why and next steps. 👏

Updates:

• Commit ​https://github.com/WordPress/wordpress-develop/pull/736/commits/db3e545ae4ad0f83106a1fd14ab4aadb3b8948a8 returns the original behavior by setting link_category property to null when the returned value is not an array. A few notes:

• is_wp_error() fires an action. Using it then also changes the behavior.
• Switched to check for the non-error state, i.e. array where wp_get_object_terms() returns array unless the taxonomy does not exist, then returns WP_Error.

• Commit ​https://github.com/WordPress/wordpress-develop/pull/736/commits/b3eb1d74763adbd8119cc463497622afe0835bde triggers the warning to retain the behavior and be a clue and indication if something went wrong.

### Test Cases

• Commit ​https://github.com/WordPress/wordpress-develop/pull/736/commits/96f915d567e52f39990dc4219a2d414e298fb72a: Took your suggestion to simplifying getting the expected value. Good idea 💡 .

• Commit ​https://github.com/WordPress/wordpress-develop/pull/736/commits/ad73fe019f72fa5539f136fa686baa00d091e3be: Doh on my part.

[hellofromTonya]

Replying to jrf:

At the request of @hellofromTonya I've reviewed ​PR #736 now and left an extensive comment for you to consider: ​https://github.com/WordPress/wordpress-develop/pull/736#pullrequestreview-528734649

The feedback I left there might also help inform how to handle some of the other changes needed.

Hope it helps!

Your thorough and extensive comment gave me an Aha Moment! I can now clearly see both the goal of this ticket, the process, and next steps for all of its patches/PRs.

The goal of this ticket is to match the data types of each function parameter, i.e. as part of the PHP 8 compatibility.

We should seek:

• to retain the current behavior (i.e. to avoid breaking changes)
• to retain the triggering of notice/warning/error, which are clues and indicators (i.e. rather than silently letting the problem propagate downstream)
• to gain PHP 8 compatibility

@hellofromtonya Glad you found the feedback useful.

As a side-note: the chances of this error condition ever being hit in a real life situation are very small as that would involve a plugin/theme explicitly (and potentially maliciously) de-registering the WP native link_category taxonomy and if that would happen, more things are likely to break anyhow.

Having said that, this doesn't negate the validity of this patch. No matter how small the chance of an error condition being hit, the error condition should preferably be handled gracefully, which is now is. So :+1: for me for this patch to be committed.

[hellofromTonya]

Moving this ticket to 5.7 as RC1 is tomorrow. Why? Discussions are ongoing to figure out how to achieve the stated goals. The incremental PRs/patches do change source code. Thus, need more time past RC.

Marking as early to ensure it gets continued attention early enough in the next cycle.

[SergeyBiryukov]

Didn't get a chance to review this before 5.6 RC, but I think if some of these fixes help prevent unnecessary breakage, they might still be a good fit for this release, even if there are some code changes.

Moving back for now, would like to take a closer look for 5.6 RC2.

Closing this PR as the fix is not necessary. This is a false PHPStan trigger.

Why? str_replace returns either string or array for PHP 5, 7, and 8. See check here ​https://3v4l.org/Lfmgp. The original code will work the same in PHP 8.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

• Walker_Comment: ensures avatar size is an integer type when passed to get_avatar()
• Adds happy and unhappy path to validate avatar for different size types

Closes in preference to ​https://github.com/WordPress/wordpress-develop/pull/755 which contains fix to HTML5 method and adds more unhappy and happy test cases.

AtomParser is part of an external library that was added to core's source in 2.3.0. The original library has not been updated ​since 2007.

This PR modifies this original library's source. Do we wish to make this change in core or contribute upstream?

Status: Blocked

The solution for this PR is currently blocked until we figure out the best way to handle wp_get_object_terms. The discussion is happening in ​https://github.com/WordPress/wordpress-develop/pull/736. Once we land on an implementation, I can update this PR.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Function: wp_privacy_generate_personal_data_export_file()

Resolves "Parameter #1 $arr1 of function array_merge expects array, array|bool given."

• Fixes the type match for array_merge to prevent the PHP 8 fatal error
• Adds test coverage
• Refactors tests to use data provider

## TODO:

• [x] Analyze
• [x] Add code coverage
• [x] Update fix
• [ ] Add error for when option is exists but is not an array, ie meaning a 3rd party has changed its expected type.

Close for newer updates in ​https://github.com/WordPress/wordpress-develop/pull/758

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Function: wp_privacy_process_personal_data_export_page()

Resolves "Parameter #1 $arr1 of function array_merge expects array, array|bool given."

Fixes the type match for array_merge to prevent the PHP 8 fatal error

• Adds test coverage
• Retains error behavior when/if '_export_data_raw' is not an array type. Error is passed via wp_send_json_error. Tests added to validate.

[hellofromTonya]

Punting this ticket to 5.7 as today is 5.6 RC2.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[hellofromTonya]

With 5.7 RC1 landing tomorrow, ran out of time for this ticket in 5.7. Punting it to 5.8.

Continuing in 5.8:
Each instance is being reviewed and validated, one-by-one. Effort will continue in 5.8 with incremental guarding and improvements to be committed for each instance.

[hellofromTonya]

Switching ownership to me as I'm actively working on it and want to shepherd it forward throughout 5.8 cycle.

Also removing patch and tests keywords as the patches are yet ready for review or commit.

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Fixes a data type bug as well as correcting the expected data type for the WP_User_Search::$page property.

After pair programming session, decided to split this PR into 2:

• Fixing the page property type mismatch in the constructor done here in PR #1091
• Fixing the other part will require additional investigation to determine what types can be returned and where the mismatch exists at the root

Closing this PR as the new ones take precedence and will contain the latest fixes and progression.

Defer to the ​reorganize branch here and specifically here in the wp-admin/includes/bookmark.php file ​https://github.com/jrfnl/wordpress-develop-official/blob/reaarrange/xknown/fix/51423-wp-admin-includes-bookmark.php/src/wp-admin/includes/bookmark.php

Force push to reset the file back to original state (except for the indentation). Why? To first show the problem exists in PHP 8, but not in the other PHP versions. Doing this step shows the problem in action for crafting the right fix without causing a breaking change.

## Manual Testing BEFORE the Fix

Environment:

• Using Local as the localhost app
• Nginx
• MySQL 8.0.16
• OS: Mac Big Sur
• Browser: Firefox

To make it testable, needed to create an environment where fopen fails to create the file:

• Changed the permissions of the existing wp-config.php to 000

{{{bash

---

}}}

• Skipped file_exists check by commenting the check on lines 58 to 69
• Flushed the realpath cache for the file

{{{php
clearstatcache( ABSPATH . 'wp-config.php' );
}}}

### Test 1: with PHP 7.3.5

Results: Works, but PHP Warning

From the error.log

PHP Warning:  fwrite() expects parameter 1 to be resource, bool given in .../setupconfig/app/public/wp-admin/setup-config.php on line 443

### Test 2: with PHP 8.0

Results: Does not work due to TypeError fatal error

From the error.log

PHP Fatal error:  Uncaught TypeError: fwrite(): Argument #1 ($stream) must be of type resource, bool given in .../setupconfig/app/public/wp-admin/setup-config.php:443
Stack trace:
#0 .../setupconfig/app/public/wp-admin/setup-config.php(443): fwrite(false, '<?php\r\n')
#1 {main}
  thrown in .../setupconfig/app/public/wp-admin/setup-config.php on line 443

## Manual Testing AFTER the Fix

Same environment and test conditions.

### Test 1: with PHP 7.3.5

Results: Works with no breaking change.

• wp-config.php file permissions is changed
• Error message displays as expected
• Error Log:
  • No fwrite PHP Warning
  • PHP Warning for fopen as expected: PHP Warning: fopen(/.../setupconfig/app/public/wp-config.php): Failed to open stream: Permission denied

### Test 2: with PHP 8.0

Results: Works.

• wp-config.php file permissions is changed
• Error message displays as expected
• Error log:
  • No PHP TypeError Fatal Error
  • PHP Warning for fopen as expected: PHP Warning: fopen(/.../setupconfig/app/public/wp-config.php): Failed to open stream: Permission denied

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Fixes the to string handling for the 'limited_email_domains' option in the network settings admin page.

The 3rd argument for ​`str_replace` is expected to be a string or array. It will return a string as the search and replace args are strings.

However, get_option returns mixed data types including string, array, false, or any other "filtered" data type (i.e. passing back from the option filters).

Reference:

• PR #720
• Gist: ​https://gist.github.com/jrfnl/ca6ed0642e839e2ca8711bc2a389c01b

I think the PHPStan flag on this one is a false positive and a fix is not needed here.

Why?

• The code behaves the same regardless if it's PHP 5.6, 7+, or 8.
• The fix is a breaking change as the fatal error will no longer happen.

Why?

• str_replace 3rd parameter (haystack) is documented as only accepting a string or array, but in tests it also handles boolean, int, and float without a notice, warning, or error. See it ​https://3v4l.org/EGTX0. If it receives anything else, a fatal error is throwing in PHP 5.6+. See it in action here ​https://3v4l.org/poqHP.
• Though get_site_option can return a variety of mixed values, the str_replace already handles it consistently.
• If no error occurs, then str_replace will always return a string. Why? Its search and replace params are strings.
• implode will always received a string that is then type casted to an array.

@jrfnl What do you think?

## Testing BEFORE fixes

### PHP 7.3.5

Results:

• Export file downloaded ✅
• 2 PHP Warnings

<img width="567" alt="Screen Shot 2021-03-18 at 4 46 23 PM" src="​https://user-images.githubusercontent.com/7284611/111701969-9624a800-8809-11eb-875d-2f2fa8fe896a.png">

Error log:

[18-Mar-2021 21:46:02 UTC] PHP Warning:  array_merge(): Expected parameter 2 to be an array, string given in .../public/wp-admin/includes/privacy-tools.php on line 397
[18-Mar-2021 21:46:02 UTC] PHP Stack trace:
[18-Mar-2021 21:46:02 UTC] PHP   1. {main}() .../public/wp-admin/admin-ajax.php:0
[18-Mar-2021 21:46:02 UTC] PHP   2. do_action() .../public/wp-admin/admin-ajax.php:187
[18-Mar-2021 21:46:02 UTC] PHP   3. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[18-Mar-2021 21:46:02 UTC] PHP   4. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[18-Mar-2021 21:46:02 UTC] PHP   5. wp_ajax_wp_privacy_export_personal_data() .../public/wp-includes/class-wp-hook.php:292
[18-Mar-2021 21:46:02 UTC] PHP   6. apply_filters() .../public/wp-admin/includes/ajax-actions.php:4911
[18-Mar-2021 21:46:02 UTC] PHP   7. WP_Hook->apply_filters() .../public/wp-includes/plugin.php:212
[18-Mar-2021 21:46:02 UTC] PHP   8. wp_privacy_process_personal_data_export_page() .../public/wp-includes/class-wp-hook.php:292
[18-Mar-2021 21:46:02 UTC] PHP   9. do_action() .../public/wp-admin/includes/privacy-tools.php:844
[18-Mar-2021 21:46:02 UTC] PHP  10. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[18-Mar-2021 21:46:02 UTC] PHP  11. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[18-Mar-2021 21:46:02 UTC] PHP  12. wp_privacy_generate_personal_data_export_file() .../public/wp-includes/class-wp-hook.php:292
[18-Mar-2021 21:46:02 UTC] PHP  13. array_merge() .../public/wp-admin/includes/privacy-tools.php:397
[18-Mar-2021 21:46:02 UTC] PHP Warning:  count(): Parameter must be an array or an object that implements Countable in .../public/wp-admin/includes/privacy-tools.php on line 399
[18-Mar-2021 21:46:02 UTC] PHP Stack trace:
[18-Mar-2021 21:46:02 UTC] PHP   1. {main}() .../public/wp-admin/admin-ajax.php:0
[18-Mar-2021 21:46:02 UTC] PHP   2. do_action() .../public/wp-admin/admin-ajax.php:187
[18-Mar-2021 21:46:02 UTC] PHP   3. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[18-Mar-2021 21:46:02 UTC] PHP   4. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[18-Mar-2021 21:46:02 UTC] PHP   5. wp_ajax_wp_privacy_export_personal_data() .../public/wp-includes/class-wp-hook.php:292
[18-Mar-2021 21:46:02 UTC] PHP   6. apply_filters() .../public/wp-admin/includes/ajax-actions.php:4911
[18-Mar-2021 21:46:02 UTC] PHP   7. WP_Hook->apply_filters() .../public/wp-includes/plugin.php:212
[18-Mar-2021 21:46:02 UTC] PHP   8. wp_privacy_process_personal_data_export_page() .../public/wp-includes/class-wp-hook.php:292
[18-Mar-2021 21:46:02 UTC] PHP   9. do_action() .../public/wp-admin/includes/privacy-tools.php:844
[18-Mar-2021 21:46:02 UTC] PHP  10. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[18-Mar-2021 21:46:02 UTC] PHP  11. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[18-Mar-2021 21:46:02 UTC] PHP  12. wp_privacy_generate_personal_data_export_file() .../public/wp-includes/class-wp-hook.php:292

### PHP 8.0

Results:

• Fatal error
• Export failed

<img width="477" alt="Screen Shot 2021-03-18 at 4 44 47 PM" src="​https://user-images.githubusercontent.com/7284611/111701813-58278400-8809-11eb-9a69-fc555f1ed66f.png">

Error log:

[18-Mar-2021 21:37:33 UTC] PHP Fatal error:  Uncaught TypeError: array_merge(): Argument #2 must be of type array, string given in .../public/wp-admin/includes/privacy-tools.php:397
Stack trace:
#0 .../public/wp-admin/includes/privacy-tools.php(397): array_merge(Array, 'invalid type')
#1 .../app/public/wp-includes/class-wp-hook.php(292): wp_privacy_generate_personal_data_export_file(21)
#2 .../public/wp-includes/class-wp-hook.php(316): WP_Hook->apply_filters(NULL, Array)
#3 .../public/wp-includes/plugin.php(484): WP_Hook->do_action(Array)
#4 .../public/wp-admin/includes/privacy-tools.php(844): do_action('wp_privacy_pers...', 21)
#5 .../public/wp-includes/class-wp-hook.php(292): wp_privacy_process_personal_data_export_page(Array, 3, 'tester2@test.co...', 1, 21, false, 'wordpress-media')
#6 .../public/wp-includes/plugin.php(212): WP_Hook->apply_filters(Array, Array)
#7 .../public/wp-admin/includes/ajax-actions.php(4911): apply_filters('wp_privacy_pers...', Array, 3, 'tester2@test.co...', 1, 21, false, 'wordpress-media')
#8 .../public/wp-includes/class-wp-hook.php(292): wp_ajax_wp_privacy_export_personal_data('')
#9 .../public/wp-includes/class-wp-hook.php(316): WP_Hook->apply_filters('', Array)
#10 .../public/wp-includes/plugin.php(484): WP_Hook->do_action(Array)
#11 .../public/wp-admin/admin-ajax.php(187): do_action('wp_ajax_wp-priv...')
#12 {main}
  thrown in .../public/wp-admin/includes/privacy-tools.php on line 397

### How Tested

Used this code in a must-use to change the meta value to a string value:

{{{php
<?php

add_action( 'wp_privacy_personal_data_export_file', function( $request_id ) {

update_post_meta( $request_id, '_export_data_grouped', 'invalid type' );

} , 5 );
}}}

Tests to reproduce:

1. Create a wp-content/mu-plugins/51423.php and add code above.
2. Add a test user, e.g. username tester2 and password tester2@test.com.
3. Go to Tools > Export Personal Data.
4. Create a request for the user.
5. Attempt to download the export file.

[hellofromTonya]

@SergeyBiryukov ​PR1091 is for ready for commit. @jrf and I have reviewed and tested it.

This one fixes a data type bug which then also fixes the assignment of the WP_User_Search::$page to match its expected data type.

## Testing AFTER fixes

### PHP 7.3.5

Results:

• Export file downloaded ✅
• No PHP Warnings
• 1 PHP Notice

<img width="1088" alt="Screen Shot 2021-03-22 at 2 18 05 PM" src="​https://user-images.githubusercontent.com/7284611/112046002-7cd96f80-8b19-11eb-8a07-0255e63a6cbe.png">

Error log:

[22-Mar-2021 19:16:05 UTC] PHP Notice:  wp_privacy_generate_personal_data_export_file was called <strong>incorrectly</strong>. The <code>'_export_data_grouped'</code> post meta must be an array. Please see <a href="https://wordpress.org/support/article/debugging-in-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 5.8.0.) in .../public/wp-includes/functions.php on line 5313
[22-Mar-2021 19:16:05 UTC] PHP Stack trace:
[22-Mar-2021 19:16:05 UTC] PHP   1. {main}() .../public/wp-admin/admin-ajax.php:0
[22-Mar-2021 19:16:05 UTC] PHP   2. do_action() .../public/wp-admin/admin-ajax.php:187
[22-Mar-2021 19:16:05 UTC] PHP   3. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[22-Mar-2021 19:16:05 UTC] PHP   4. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[22-Mar-2021 19:16:05 UTC] PHP   5. wp_ajax_wp_privacy_export_personal_data() /Users/hellofromtonya/Local/wpcore/app/public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:16:05 UTC] PHP   6. apply_filters() .../public/wp-admin/includes/ajax-actions.php:4911
[22-Mar-2021 19:16:05 UTC] PHP   7. WP_Hook->apply_filters() .../public/wp-includes/plugin.php:212
[22-Mar-2021 19:16:05 UTC] PHP   8. wp_privacy_process_personal_data_export_page() .../public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:16:05 UTC] PHP   9. do_action() .../public/wp-admin/includes/privacy-tools.php:862
[22-Mar-2021 19:16:05 UTC] PHP  10. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[22-Mar-2021 19:16:05 UTC] PHP  11. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[22-Mar-2021 19:16:05 UTC] PHP  12. wp_privacy_generate_personal_data_export_file() .../public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:16:05 UTC] PHP  13. _doing_it_wrong() .../public/wp-admin/includes/privacy-tools.php:411
[22-Mar-2021 19:16:05 UTC] PHP  14. trigger_error() .../public/wp-includes/functions.php:5313

In the downloaded zip file:

• export.json:

{{{json
{"Personal Data Export for tester2@…":null}
}}}

• `index.html

{{{html
<!DOCTYPE html>
<html>
<head>
<meta http-equiv='Content-Type' content='text/html; charset=UTF-8' />
<style type='text/css'>body { color: black; font-family: Arial, sans-serif; font-size: 11pt; margin: 15px auto; width: 860px; }table { background: #f0f0f0; border: 1px solid #ddd; margin-bottom: 20px; width: 100%; }th { padding: 5px; text-align: left; width: 20%; }td { padding: 5px; }tr:nth-child(odd) { background-color: #fafafa; }.return-to-top { text-align: right; }</style><title>Personal Data Export for tester2@…</title></head>
<body>
<h1 id="top">Personal Data Export</h1></body>
</html>
}}}

### PHP 8.0

Results:

• Export file downloaded ✅
• No PHP Fatal error
• 1 PHP Notice

<img width="1069" alt="Screen Shot 2021-03-22 at 2 23 30 PM" src="​https://user-images.githubusercontent.com/7284611/112046632-48b27e80-8b1a-11eb-8c88-89b4d2fc3c2f.png">

Error log:

[22-Mar-2021 19:23:22 UTC] PHP Notice:  wp_privacy_generate_personal_data_export_file was called <strong>incorrectly</strong>. The <code>'_export_data_grouped'</code> post meta must be an array. Please see <a href="https://wordpress.org/support/article/debugging-in-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 5.8.0.) in .../public/wp-includes/functions.php on line 5313
[22-Mar-2021 19:23:22 UTC] PHP Stack trace:
[22-Mar-2021 19:23:22 UTC] PHP   1. {main}().../public/wp-admin/admin-ajax.php:0
[22-Mar-2021 19:23:22 UTC] PHP   2. do_action() .../public/wp-admin/admin-ajax.php:187
[22-Mar-2021 19:23:22 UTC] PHP   3. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[22-Mar-2021 19:23:22 UTC] PHP   4. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[22-Mar-2021 19:23:22 UTC] PHP   5. wp_ajax_wp_privacy_export_personal_data() .../public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:23:22 UTC] PHP   6. apply_filters() .../public/wp-admin/includes/ajax-actions.php:4911
[22-Mar-2021 19:23:22 UTC] PHP   7. WP_Hook->apply_filters() .../public/wp-includes/plugin.php:212
[22-Mar-2021 19:23:22 UTC] PHP   8. wp_privacy_process_personal_data_export_page() .../public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:23:22 UTC] PHP   9. do_action() .../public/wp-admin/includes/privacy-tools.php:862
[22-Mar-2021 19:23:22 UTC] PHP  10. WP_Hook->do_action() .../public/wp-includes/plugin.php:484
[22-Mar-2021 19:23:22 UTC] PHP  11. WP_Hook->apply_filters() .../public/wp-includes/class-wp-hook.php:316
[22-Mar-2021 19:23:22 UTC] PHP  12. wp_privacy_generate_personal_data_export_file() .../public/wp-includes/class-wp-hook.php:292
[22-Mar-2021 19:23:22 UTC] PHP  13. _doing_it_wrong() .../public/wp-admin/includes/privacy-tools.php:411
[22-Mar-2021 19:23:22 UTC] PHP  14. trigger_error() .../public/wp-includes/functions.php:5313

In the downloaded zip file:

• export.json:

{{{json
{"Personal Data Export for tester2@…":null}
}}}

• `index.html

{{{html
<!DOCTYPE html>
<html>
<head>
<meta http-equiv='Content-Type' content='text/html; charset=UTF-8' />
<style type='text/css'>body { color: black; font-family: Arial, sans-serif; font-size: 11pt; margin: 15px auto; width: 860px; }table { background: #f0f0f0; border: 1px solid #ddd; margin-bottom: 20px; width: 100%; }th { padding: 5px; text-align: left; width: 20%; }td { padding: 5px; }tr:nth-child(odd) { background-color: #fafafa; }.return-to-top { text-align: right; }</style><title>Personal Data Export for tester2@…</title></head>
<body>
<h1 id="top">Personal Data Export</h1></body>
</html>
}}}

@jrfnl This PR is ready for a final review. Since we last did a pair programming session on it, I discovered our solution caused a breaking change. Why? When the post meta does not exist or it's value is not an array, no about group content is generated in the downloaded zip file.

Why? See the updated notes in the description of this PR which includes 3v4l links to show it in action.

What's different since you and I last worked on it?

• See here ​https://github.com/WordPress/wordpress-develop/pull/758/commits/24839151602aff935b89f2f0ac14cf9f33a58985
• Updated the tests
• And consolidated a couple of individual ones, pulling the test data into the data provider, i.e. for consistency

[SergeyBiryukov]

In 50563:

Code Modernization: Correct expected data type for WP_User_Search::$page property.

This fixes erroneous parentheses placement and applies the type cast to the variable it was intended for.

Follow-up to [3864].

Props hellofromTonya, jrf, xknown.
See #51423.

Merged via changeset https://core.trac.wordpress.org/changeset/50563

Merged via changeset https://core.trac.wordpress.org/changeset/50563

Trac ticket: https://core.trac.wordpress.org/ticket/51423

Function: wp_privacy_generate_personal_data_export_file()

PHPStan flagged a type mismatch if false is passed into fwrite for groups JSON:
{{{php
fwrite( $file, $groups_json );
}}}

The groups JSON is assigned from wp_json_encode:
{{{php
$groups_json = wp_json_encode( $groups );
}}}

The ​return signatures of `wp_json_encode` are string or false:

_(string|false)_ The JSON encoded string, or false if it cannot be encoded.

### Is Passing false a problem?
See it in action here ​https://3v4l.org/bpmqG

• No errors, warnings, or notices happen in PHP 5.6 to 8.0.3
• Generates invalid JSON as false writes nothing into the file:

{{{json
"{Personal Data Export for tester@…:}"
}}}

This is the current behavior 👆, where generating invalid JSON is a bug.

### What can cause JSON encoding to fail?
json_encode returns false when:

• attempting to encode a non-UTF-8 string
• a resource is given

WordPress automatically remedies the encoding with _wp_json_sanity_check where it converts string(s) to UTF-8.

A resource could exist an element of the groups _when_ the post meta data is filtered. Using this ​test script reproduces the failed JSON encoding and invalid JSON report.

[hellofromTonya]

@SergeyBiryukov ​PR 758 is for ready for commit. @jrf and I have reviewed and tested it. She's approved it.

tl;dr

• Function: wp_privacy_generate_personal_data_export_file()
• Problem: When/if the '_export_data_grouped' post meta exists but is not an array, a PHP Warning (< 8)/Error (8+) is thrown for array_merge argument 2 type mismatch.
• Fix: _doing_it_wrong and sets vars to retain current behavior
• Also refactors tests to
  • Reduce redundant code
  • Switch to data provider
  • Test on the full HTML output instead of select pieces of the output
  • Expands unhappy path coverage

[SergeyBiryukov]

In 50613:

Code Modernization: Check if the _export_data_grouped post meta is an array when generating a personal data export file.

This avoids a fatal error on PHP 8 in wp_privacy_generate_personal_data_export_file() if the _export_data_grouped post meta exists but is not an array.

Additionally, refactor unit tests for the function to:

• Reduce redundant code
• Switch to data provider
• Test on the full HTML output instead of select pieces of the output
• Expand unhappy path coverage

Follow-up to [43012], [44786], [47146], [47278].

Props hellofromTonya, jrf, xknown.
See #51423.

[SergeyBiryukov]

Replying to hellofromTonya:

​PR 758 is for ready for commit.

This looks great, thanks for the test improvements!

Would it be possible to also add documentation for the ::setup_export_contents_test() method?

Merged via changeset https://core.trac.wordpress.org/changeset/50613.

[hellofromTonya]

Comments Tests_Privacy_WpPrivacyGeneratePersonalDataExportFile::setup_export_contents_test

[hellofromTonya]

Comments Tests_Privacy_WpPrivacyGeneratePersonalDataExportFile::setup_export_contents_test

[hellofromTonya]

Replying to SergeyBiryukov:

Would it be possible to also add documentation for the ::setup_export_contents_test() method?

Great suggestion Sergey. Patch 51423-setup_export_contents_test.diff documents it.

[SergeyBiryukov]

In 50616:

Docs: Add documentation for the ::setup_export_contents_test method used in personal data export tests.

Follow-up to [50613].

Props hellofromTonya.
See #51423.

[hellofromTonya]

Removing commit as commit ready PRs/patches are now committed.

​Review in slack by polyglots ✅

Moved creating the test class for get_bookmark() to PR #1180.

[hellofromTonya]

Marking ​PR 1100 ready for commit. cc @SergeyBiryukov.

[SergeyBiryukov]

Replying to hellofromTonya:

Marking ​PR 1100 ready for commit. cc @SergeyBiryukov.

Thanks for the PR!

• I don't see how get_site_option( 'limited_email_domains' ) can return null in normal workflow, so it seems a bit random to specifically guard against null here. Even if the option does not exist, it returns false. So it looks like the only way for that to happen is if someone specifically filters the value to null. Instead of targeting null, I think the existing check for a truthy value would be enough here.
• That said, I agree with some other points of the PR, we can check for is_array() before calling implode(). We should also do it consistently for the banned_email_domains and illegal_names options, though.
• The str_replace() call was added in ​mu:1285 / ​mu:#426 when converting limited_email_domains from an input field to a textarea. This would benefit from a comment to avoid any future confusion of why it is applied for limited_email_domains, but not for banned_email_domains.

So something like 51423.wp-admin-network-settings.diff​ would be my suggestion here.

[hellofromTonya]

Replying to SergeyBiryukov:

• I don't see how get_site_option( 'limited_email_domains' ) can return null in normal workflow, so it seems a bit random to specifically guard against null here. Even if the option does not exist, it returns false. So it looks like the only way for that to happen is if someone specifically filters the value to null. Instead of targeting null, I think the existing check for a truthy value would be enough here.

Checking for truthy would result in different value if the the returned filtered option was a string 0 or 0.0. ​See it in action here. I know that's a very very very far edge case. But here we were seeking to not change the existing behavior for backwards compatibility for any 3rd parties that are filtering the option.

Why guard for null?

In PHP 8.1, it will generate a deprecation notice. But in less than PHP 8.1, all falsey values will work with str_replace as ​shown here.

The PR could guard against the falsey values except for string 0 and 0.0. However, it might be confusing why. So we opted to guard specifically to prevent the future PHP 8.1 deprecation notice.

@SergeyBiryukov what do you think?

[SergeyBiryukov]

Replying to hellofromTonya:

Checking for truthy would result in different value if the the returned filtered option was a string 0 or 0.0. ​See it in action here. I know that's a very very very far edge case. But here we were seeking to not change the existing behavior for backwards compatibility for any 3rd parties that are filtering the option.

Thanks for the additional details! While I understand the intention to keep backward compatibility here, I think we should consider the context when making these changes and, if possible, write code that accounts for the intended values without being extra specific for edge cases or particular PHP versions.

We already have a truthy check here, so I think we can just move the str_replace() call into it.

My thinking when reading the current PR includes: "Checking for null? OK, but why would that value ever be null? Is that normal or some kind of back-compat? Do we need to account for that in other instances where it's used?" This kind of checks for specific edge cases could create confusion later and seem a bit hard to maintain the long run.

Looking at this again, I'd like to make adjustments to my previous patch. Since we mostly care about taking an array of values here and converting it to a string, I think we can do exactly that, see 51423.wp-admin-network-settings.2.diff​.

Here's the current code: ​https://3v4l.org/kmiZo
Here's the proposed code: ​https://3v4l.org/VTVcF

As you can see, the results almost exactly the same, except for an empty array, where the former code inadvertently passes an array instead of a string further down, so the proposed code fixes that as well.

[hellofromTonya]

Replying to SergeyBiryukov:

My thinking when reading the current PR includes: "Checking for null? OK, but why would that value ever be null? Is that normal or some kind of back-compat? Do we need to account for that in other instances where it's used?" This kind of checks for specific edge cases could create confusion later and seem a bit hard to maintain the long run.

Good point. Down the road, this could be confusing. As I read it now, it does cause me to stop when reading the code. Even with the comment, it causes one to stop and wonder.

Looking at this again, I'd like to make adjustments to my previous patch. Since we mostly care about taking an array of values here and converting it to a string, I think we can do exactly that, see 51423.wp-admin-network-settings.2.diff​.

Interesting approach. It does retain the values. However, it's less performant, i.e. when falsey/empty does unnecessary type casting and replace/implode.

While I understand the intention to keep backward compatibility here, I think we should consider the context when making these changes and, if possible, write code that accounts for the intended values without being extra specific for edge cases or particular PHP versions.

How can the code guard and be robust, stable, and performant?

If the edge case values are removed and the code guards itself while staying in context, it could check for empty() in the first conditional guard and skip the prep code altogether. Edgy case falsey values are converted into and empty string. In this context, that's okay as a string 0 is not an email domain.

PR is updated with this guarding pattern for the 3 instances.

[SergeyBiryukov]

Replying to hellofromTonya:

How can the code guard and be robust, stable, and performant?

If the edge case values are removed and the code guards itself while staying in context, it could check for empty() in the first conditional guard and skip the prep code altogether. Edgy case falsey values are converted into and empty string. In this context, that's okay as a string 0 is not an email domain.

PR is updated with this guarding pattern for the 3 instances.

That looks better to me, thanks! As demonstrated by 51423.type-casting-vs-empty-check-test.php​ over a million iterations, it is indeed about three times more performant than unnecessary type casting:

type casting: 1.03507 seconds
empty() & is_array() checks: 0.34766 seconds

However, one other consideration I have when making this kind of changes is whether the code can also be as concise as possible while remaining easy to understand and without a significant performance degradation.

Since it's a relatively rarely accessed settings page and the value is only displayed once per page load and does not affect performance in a noticeable way, I think the performance difference is negligible here, so I would prefer a more compact code over more verbose (1 line vs. 6 lines).

What do you think? If you feel strongly the performance difference, we can go with the empty() and is_array() checks as you suggest. Otherwise, keeping the type casting and just addressing the initial str_replace() issue seems like a good compromise.

[hellofromTonya]

@SergeyBiryukov Great job running the performance benchmarks on the 2 approaches!

What do I think?

Type casting code: it's more concise with less lines of code, but IMO less understandable for what happens and why with different values or lack of value.

Why? When I read it, my first thought is: why is it type casting and doing the processing when there are no emails to be processed.

I'd vote for the empty/is_array version for quick understanding, eliminating unnecessary processing, and clear showing the processing paths for different values (which helps with building happy and unhappy path testing scenarios).

[hellofromTonya]

​PR 722 is ready for commit. cc @SergeyBiryukov

File: wp-admin/setup-config.php

• Fixes a fwrite handle type mismatch which happens if fopen fails, i.e. returns false
• Adds error messaging for it

[SergeyBiryukov]

In 50772:

Code Modernization: Bring consistency to preparing some fields on Network Settings screen:

• illegal_names
• limited_email_domains
• banned_email_domains

If any of these options have a falsey value, treat it as an empty string. This addresses a PHP 8.1+ deprecation notice when passing a null value to str_replace().

Additionally, avoid unnecessary type casting for better performance.

Props hellofromTonya, jrf, SergeyBiryukov.
See #51423.

Merged via changeset https://core.trac.wordpress.org/changeset/50772

[peterwilsoncc]

In 50775:

Upgrade/Install: Prevent possible type errors during installation.

Prevent a TypeError from occurring during installation if wp-config.php is not writable. In PHP 8.0 this can cause a fatal error, in earlier versions of PHP a warning would be thrown.

Account for a change in type returned by fopen() coming in a future version of PHP. Minor coding standards fixes in the /wp-admin/setup-config.php file.

Props xknown, hellofromTonya, jrf, peterwilsoncc.
See #51423.

Committed in https://core.trac.wordpress.org/changeset/50775

Committed with changeset https://core.trac.wordpress.org/changeset/50713

[hellofromTonya]

Today is 5.8 Beta 1. The remaining work in this ticket will be completed in 5.9. Removing keywords as there are no PRs to commit at this time.

We noticed quite a few PHP warnings related triggered by set_theme_mod
calls. It turns out that some of these sites used to have empty strings
stored in the mods_CURRENT_THEME/theme_mods_CURRENT_THEME_SLUG
options.

The warnings happen because we are attempting to use an invalid offset
on `$mods[ $name ] = apply_filters( "pre_set_theme_mod_{$name}", $value,
$old_value );`

Trac ticket: https://core.trac.wordpress.org/ticket/51423

[SergeyBiryukov]

In 51524:

Themes: Make sure get_theme_mods() always returns an array.

This avoids a "Cannot access offset of type string on string" fatal error in set_theme_mod() on PHP 8 if the theme_mods_$theme_slug option has an incorrect value, e.g. an empty string instead of an array.

With this change, set_theme_mod() should be able to resolve the issue by saving a correct value.

Follow-up to [15736], [15739], [30672], [32629], [32632].

Props xknown.
See #51423.

Thanks for the PR! Merged in https://core.trac.wordpress.org/changeset/51524.

[hellofromTonya]

5.9 Beta 1 is in < 19 hours. PHP 8.1 consumed the time to work further on this ticket. Moving it to 6.0 and will refocus to finish it.

[kafleg]

This ticket came up during bug scrub. What is left here? The discussion is long. ;)

[hellofromTonya]

There's a lot more work to do, though none got done in 6.0. Moving this to 6.1.

[hellofromTonya]

Unfortunately, no work happened during the 6.1 cycle. Moving this one to 6.2.

[costdev]

This ticket was discussed during the recent bug scrub. It looks like it's unlikely that work will be done on this during the 6.2 cycle.

@SergeyBiryukov @hellofromTonya Is this still possible to land in 6.2, or should it be moved to Future Release for now?

[hellofromTonya]

Let's keep in the milestone for now. It's blessed. If there's time, might be able to land some of the fixes.

Thinking too that the scope of this ticket is likely too massive. Hard to know what is a false report vs fix vs status. I'll take a look at how to wrangle what's remaining and how better to track it.

[hellofromTonya]

I've opened #57782 to continue this work in the 6.3 cycle.

Why a new ticket?
It's difficult to ascertain the current state and scope of what needs to be fixed vs what's a false report. A new ticket offers:

• a way forward to continue the work started in this ticket with a focus on first assessing the current state of trunk with the knowledge gained from the work done here.
• improved tracking of fixes needed.

Given that 6.2 RC1 is fast approaching, a new PHPStan report is needed to scope fixes, and there's been no work done in this ticket since 5.9, I'm closing this ticket. Work will restart with focused effort in #57782.

Thank you everyone for your contributions! I invite you to join the continued effort in #57782. If someone has time, a good starting point is to rerun the PHPStan report and share it in #57782.