Make WordPress Core

Opened 10 days ago

Last modified 23 minutes ago

#62061 new task (blessed)

Prepare for PHP 8.4

Reported by: jrf's profile jrf Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: General Keywords: php-84 has-patch has-unit-tests needs-dev-note
Focuses: php-compatibility Cc:

Description (last modified by jrf)

This is a meta ticket to track the efforts to prepare for PHP 8.4.

For PHP 8.0/8.1/8.2/8.3 specific fixes, please refer to the generic WP 6.7 PHP 8.x ticket: #59654

Please link patches related to a specific PHP 8.4 related task to the appropriate dedicated issue, if there is one (see the links in the description below).

Generic/one-off PHP 8.4 related patches can be linked to this ticket.

Previous PHP meta tickets: #59231 (PHP 8.3), #56009 (PHP 8.2)


PHP 8.4: Important dates

PHP 8.4 is expected to be released on November 21 2024.

Other note-worthy dates:

  • The first alpha was released on July 4th 2024.
  • Soft Feature freeze started on August 13, 2024.
  • Hard Feature freeze will be in effect from September 26, 2024.

Please note that PHP 8.4 is the first release following the new release cycle policy.

Changes of note:

  • Implementations for accepted RFCs are now formally allowed to still be merged during the beta period, even after (soft) feature freeze.
  • Minor improvements which do not require an RFC, such as adding constants, parameters, config options, or even small functions or methods, are now also allowed to still be merged during the beta period, even after (soft) feature freeze.

Note:
The below represents the status per September 16, 2024 and is based on PHP 8.4-beta5.
Even though PHP 8.4 is in feature freeze, these statuses are subject to change due to the extended beta period and allowance for small changes during the Beta period.

Readiness of essential tooling

Composer

Current status:

  • CI for Composer itself is being run against PHP 8.4 and is passing.
  • Tests pass on PHP 8.4, though there are a few deprecation notices coming from an underlying dependency, which Composer uses at an older version due to their minimum PHP requirements. Nothing to be concerned about though and nothing blocking.
  • PHPCompatibility (bleeding edge): found no significant issues.
  • Conclusion:: The only "real" issues I've managed to identify are in the test suite of Composer, which has no impact on end-users of Composer.

Note about plugins/themes and the composer/installers package:

Plugins often use the Composer Installers package to allow Composer to install the plugin within the directory layout expected by WordPress.
The Composer Installers package is compatible with PHP 8.4 when using the 2.x range.

Plugins which still require v1.x of the package will need to widen their requirements to ^1.0 || ^2.0 to allow the installers to run without deprecation notices on PHP 8.4 (and to prevent installation conflicts with plugins which haven't widened their requirements yet).

PHPUnit

Current status:

  • CI for PHPUnit itself is being run against PHP 8.4.
  • Tests pass on PHP 8.4 for all relevant PHPUnit versions.
  • PHPCompatibility (bleeding edge): found no significant issues.
  • WordPress is behind (see #62004) and uses PHPUnit 8 + 9 (while 10 and 11 are already out), but that's not problematic for PHP 8.4 compatibility as PHPUnit 8 and 9 have "life support", which means they will be made compatible with new PHP versions for the time being.
  • Conclusion: No known issues in the relevant release for the WP test suite (9.6.20).

PHPUnit Polyfills

Current status:

  • CI for PHPUnit Polyfills itself is being run against PHP 8.4.
  • Tests pass on PHP 8.4 for all relevant PHPUnit Polyfills versions.
  • PHPCompatibility (bleeding edge): found no issues.
  • WordPress is behind (see #62004) and uses PHPUnit Polyfills 1.x (while 2.x and 3.x are available), but that's not problematic for PHP 8.4 compatibility. The PHPUnit Polyfills package follows the PHPUnit support policy, which means that older majors will continue to be supported and made compatible with new PHP versions for as long as the PHPUnit versions the major supports, are still supported by PHPUnit.
  • Conclusion: No known issues in the relevant release for the WP test suite (1.1.2).

WP-CLI

Current status:

  • CI for the packages is being run against PHP 8.4 and CI for all packages is run daily. Also see: https://wp-cli.org/dashboard/
  • However, it looks like CI isn't set up to fail on PHP deprecations in all circumstances, which allows for new issues to be introduced without anybody noticing for some of the packages. /cc @schlessera
  • PHPCompatibility (bleeding edge): found a couple of issues. PRs have been opened for each of these and most are merged by now.
  • A critical in-depth look at the latest test runs showed a number of additional PHP compatibility issues, including some related to PHP 8.1 and 8.3. Again, PRs have been opened for each of these.
  • Conclusion: While there are some issues, these should (hopefully) be fixed in the next release, which WP will automatically use in the CI jobs, once available.

Other tooling

Other (PHP) tooling doesn't necessarily have to run against PHP 8.4 (yet), so has not been evaluated.

Having said that, tooling like PHP_CodeSniffer, WordPressCS and PHPCompatibility have no known runtime PHP 8.4 issues at this time, though it may still be quite a while before full syntax support is available for new PHP 8.4 syntaxes.

Initial DevOps Tasks

Typical tasks which need to be executed to allow WordPress to prepare for PHP 8.4:

Docker

Add PHP 8.4 to the Docker images. A PR for this was merged on July 26, 2024.

Notes:

  • There is no Docker image with Xdebug 3.4.0 (PHP 8.4 compatible) available yet.
  • There is no Imagick release available yet which is compatible with PHP 8.3 or 8.4.

👉🏻 Once either of the above statuses change, the Docker image(s) will need further updates.

GitHub Actions

Add PHP 8.4 to the GitHub Actions phpunit-tests.yml configuration. GH PR #7379 (won't pass until other PRs have been merged, see task list below)

Notes:

  • Test failures on PHP 8.4 should not (yet) fail the build, but as the actual script to run the tests has been moved, it is currently impossible to use continue-on-error as that keyword is not supported when calling a reusable workflow... /cc @desrosj
  • There is a small project ongoing in the background to change how the PHP tests are being run, including significant adjustments to the workflows and matrixes. This project does not directly affect this ticket, but this ticket does affect that project.

PHP 8.4 deprecations for which WordPress needs to prepare

Deprecate implicitly nullable parameter types

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

This is a high impact deprecation, with ~880 of the top 2000 Packagist packages being affected by this deprecation.

As the use of type declarations is relatively low in the WP codebase, this deprecation didn't hit WP that hard. Initial fixes for this deprecation were made in #60786, but some new ones have slipped in. GH PR #7369

This deprecation also affected Requests (fixed in v2.0.11), Sodium Compat (fixed in 1.21.x) and SimplePie (PR open).

The typical fix for this deprecation is to make the typed parameter explicitly nullable. As WP has dropped support for PHP < 7.2, this is a viable solution for WP, but it may not be a viable path for affected plugin/themes.

When a plugin/theme still supports PHP < 7.1, they will need to apply a different solution. Which solution will need to be evaluated on a case-by-case basis.

Some examples of potential solutions:

  • If the type declaration is an array or scalar type declaration, make the default value comply with the expected type.
  • If the type declaration is class/interface name based, drop the type declaration in favour of doing in-function type checking. Mind: for non-global functions, non-private or non-final methods, this is a breaking change (signature change which will cause a fatal error for overloaded non-final, non-private methods).
  • In some cases, dropping the default value and making the parameter required may also be a solution, though again, this could be a breaking change.
  • Raise the minimum supported PHP version to PHP 7.1 and use the nullable operator.
  • etc

PHPCompatibility (develop) can detect code affected by this deprecation.

Generic deprecations for PHP 8.4

The PHP 8.4 Deprecations RFC was huge with a whopping 26 proposals which went to vote, of which 22 passed.

Based on initial (bleeding edge) PHPCompatibility scans + the tests, WP will be affected by a number of these deprecations.

I'm going to selectively highlight those deprecations which are most relevant in the context of WordPress.
For full information, please have a read through the RFC itself.

xml_set_object() and xml_set_*_handler() with string method names

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

<?php
xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for a long time and is cross-version compatible. So the above code is 100% equivalent to:

<?php
xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This deprecation affects WordPress in a number of places. Patches for these issues are available. GH PR #7370, GH PR #7371, GH PR #7372, GH PR #7373

PHPCompatibility (develop) can detect code affected by this deprecation.

Deprecate passing E_USER_ERROR to trigger_error()

PHP 8.4 deprecates passing E_USER_ERROR to trigger_error(), with the recommendation being to replace these type of calls with either throwing an Exception or an exit statement.

WP has its own wp_trigger_error() function, which under the hood calls trigger_error(). If passed E_USER_ERROR as the $error_level, this will hit the PHP 8.4 deprecation.

As a general rule of thumb, I recommend using exceptions to fix this type of deprecation, if only because it allows the code to remain testable.

There are circumstances in which a hard exit() is warranted, but those are few and far between.

This deprecation affects WordPress in a number of places, with wp_trigger_error() being the primary issue.
Patches for these issues are available. GH PR #7375, GH PR #7376, GH PR #7377

Note:
For those people tempted to silence this deprecation notice until PHP 9.0:
This is not a good idea as it will not be PHP 7 - 8 cross-version compatible.

Prior to PHP 8.0, error silencing would apply to all errors - including the error being thrown.
As of PHP 8.0, error silencing will no longer apply to fatal errors, so would only silence the deprecation notice (intended behaviour).
Also see: https://3v4l.org/ghYGk

PHPCompatibility (develop) can detect code affected by this deprecation.

Deprecation of mysqli_ping() and mysqli::ping()

The mysqli_ping() function is deprecated as of PHP 8.4, though, in reality, the function wasn't working according to spec anymore since PHP 8.2 when the libmysql driver was dropped in favour of libmysqlnd, which was already the default (and recommended) driver since PHP 5.4.

The mysqli_ping() method was also not really correctly named as its functionality was to reconnect to the database, not just ping.

The alternative is to "manually" ping the database by sending a DO 1 query (the cheapest possible SQL query).

This issue affects the wpdb class in a minor way. Patch available. GH PR #7374

It is not expected that (properly coded) plugins or themes are affected by this change as those should all be using $wpdb.

PHPCompatibility (develop) will be able to detect code affected by this deprecation (PR open).

Deprecate mysqli_refresh() + Deprecate mysqli_kill()

WordPress itself does not use either of these functions and it is not expected that (properly coded) plugins or themes are affected by this change as those should all be using $wpdb.

PHPCompatibility (develop) will be able to detect code affected by this deprecation (PR upcoming).

Deprecate the second parameter to mysqli_store_result()

WordPress itself does not this function and it is not expected that (properly coded) plugins or themes are affected by this change as those should all be using $wpdb.

PHPCompatibility (develop) can detect code affected by this deprecation.

Deprecation of unserialize()'s 'S' tag

Even though supported in PHP itself, this tag was never actually in use as it is an artifact of the never-released PHP 6.

WordPress' is_serialized() function does not take the S tag into account.

Conclusion: nothing to do.

Deprecate proprietary CSV escaping mechanism

The current implementation of this deprecation is not according to the specs outlined in the RFC and basically makes the $escape parameter in the affected functions - fputcsv(), fgetcsv() and str_getcsv() and their SplFileObject equivalents - a (soft) required parameter.

In the context of WordPress, this affects WP-CLI.

I've challenged the current implementation as it doesn't comply with the spec outlined in the RFC and I recommend not fixing any related deprecation until the current discussion has reached a conclusion (or until the hard feature freeze comes into effect, whichever comes first).

PHPCompatibility (develop) will likely be able to detect code affected by this deprecation, but won't until the final implementation is settled on.

Remove E_STRICT error level and deprecate E_STRICT constant

While this proposal has been voted in, the PR to make the actual change in PHP looks to have stalled and it is uncertain whether this deprecation will take effect in PHP 8.4.

I recommend not taking any action on this deprecation until the change has been committed into PHP Core.
If and when this happens, there is one usage of the constant in WP Core which will need to be handled.

[Update 2024-09-18] This change has now been committed to PHP. Patch available in GH PR #7397

PHPCompatibility (develop) will be able to detect code affected by this deprecation once it lands in PHP Core.

Deprecate returning non-string values from a user output handler + Deprecate producing output in a user output handler

While both these proposals have been voted in, at this time, no PR to make the actual change in PHP Core has been opened yet. This makes it exceedingly unlikely that these changes will take effect in PHP 8.4.

I recommend not taking any action on this deprecation until the change has been committed into PHP and the real world implementation details of the deprecation are known.

Side-note: as a rule of thumb, it should be considered strongly discouraged to use output buffering in the context of WordPress anyway, let alone in combination with a custom output handler, as this gets very messy very quickly once output buffers are stacked and plugins/Core are retrieving a different output buffer than the one they expect.

For a more detailed explanation about this, see WPCS #1422.

PHPCompatibility (develop) may be able to detect code affected by this deprecation, but whether it can or can't won't be clear until the final implementation details are known.

Unbundle the imap, pspell, and oci8 extensions

The IMAP, Pspell and OCI8 extensions have been unbundled from PHP and moved to PECL, making it less likely for these extensions to be installed.

By the looks of it, IMAP is the only extension used in WP and only in the PHPMailer dependency, which already has a fall-back in place.

PHPCompatibility (develop) can detect code affected by this deprecation.

Other PHP 8.4 changes to be aware of

The changes listed below do not directly impact WP Core based on this initial investigation, but these may impact the larger eco-system.

These changes are included in this ticket mostly to raise awareness.

Raising zero to the power of negative number

This comes down to certain mathematically unsound calculations now throwing a deprecation notice.

<?php
//PHP 8.4
var_dump(0 ** -1); // Deprecated: Zero raised to a negative power is deprecated (was float(INF))
var_dump(0 ** -1.1); //Deprecated: Zero raised to a negative power is deprecated (was float(INF))

var_dump(pow(0, -1)); //Deprecated: Zero raised to a negative power is deprecated (was float(INF))
var_dump(pow(0, -1.1)); //Deprecated: Zero raised to a negative power is deprecated (was float(INF))

var_dump(1 / 0); //DivisionByZeroError: Division by zero (not changed)

To retain the old behaviour, the new PHP 8.4+ fpow() function can be used.

It does not look like WP is impacted by this deprecation. Still good to be aware of, in case bugs get reported which hit these edge cases.

Exit as a function

This is largely a semantic change and typically affects the type juggling behaviour for exit and die when passed a parameter between parentheses.
It also means that exit and die can now be used as a callable, respect strict_types and can take named arguments.

Take note of this unchanged behaviour:

  • exit and die can (still) not be disabled via disable_functions
  • exit and die are still reserved keywords and can not be used for function names, whether namespaced or not.

PHPCompatibility (develop) will be able to detect code affected by the changed type juggling behaviour (PR upcoming).

Add http_(get|clear)_last_response_headers() function

These functions are intended to eventually replace the "magic" function-local variable `$http_response_header`, which is expected to be removed in a future PHP version.

Path to Saner Increment/Decrement operators

The second step in this PHP 8.3 RFC - "Deprecate using the increment operator with non-numeric strings" - should have been enacted in PHP 8.4, but it looks like this may have been delayed. I've asked for clarification on the PHP Internals mailinglist.

Two deprecations related to PHP native session handling

In the context of WordPress, using PHP native session handling is strongly discouraged. Having said that, in the wider eco-system, it is known there are plugins/themes out there which persist in using this anyway.

PHP native session handling is subject to two separate deprecations in PHP 8.4 and developers who use PHP session handling in their code should read up on these to evaluate whether these deprecations will impact their code.

Ref:

Fix up BCMath Number Class / Change GMP bool cast behavior

A GMP object will now behave like an integer when cast.

This means loosy comparisons against a GMP object will behave different, in particular for the case where the object value would evaluate to 0.

<?php
if ( $gmp ) {}

Highlights of PHP 8.4

PHP 8.4, of course, also offers various new features. These can't be used by WP until support for PHP < 8.4 has been dropped, though select features could be singled-out for polyfilling ahead of time.

The below highlights some of the more interesting new features for future use in WordPress.

`#[\Deprecated` Attribute]

An attribute to mark functions and (class-)constants as deprecated and throw standardized deprecation notices about these.

This feature is interesting, but not relevant for WordPress until the minimum supported PHP version would be PHP 8.4, as polyfilling the behaviour for older PHP versions would be quite involved and not worth the effort.

RFC1867 for non-POST HTTP verbs

This RFC introduces a new request_parse_body() function to parse multipart/form-data received via, for example, PUT or PATCH HTTP requests.

This may be interesting in the future, in particular for the REST API, but there is a gotcha: request_parse_body() may not be called twice, as it destructively consumes sapi_module.read_post().

array_find, array_find_key, array_any and array_all

PHP 8.4 will come with four new functions "which are helper functions for common patterns of checking an array for the existence of elements matching a specific condition".

It might be worth polyfilling these functions as I suspect that will allow for getting rid of some code pattern duplications across the code base. Worth looking into for sure.

XML_OPTION_PARSE_HUGE

This solves the problem that parsing large input data (> 10 Mb) is no longer allowed by default since libxml2 v 2.7.0.

This might be worth looking into for the WordPress Importer plugin (WXR parser) if any users have reported problems with large imports.

Add stream open functions to XML{Reader,Writer}

These improvements will help in resource (memory) management when handling large XML files. Unfortunately, it is unlikely that this functionality can be polyfilled.

Various improvements to the DOM extension

These are likely to be very interesting for various parts of WordPress once they can be used, but it is unlikely that the functionality can be polyfilled.

Refs:

Status of External Dependencies

Gutenberg 🚨

While not strictly an external dependency, I have scanned Gutenberg in the same way as other dependencies as it is being developed in a separate repo.

Current status:

  • Tests are NOT yet being run against PHP 8.4 (PHP 8.3 is also missing). PR to enable running against PHP 8.3 is open.
  • Test coverage is unknown (not monitored via an online service, not locally checkable).
  • PHPCompatibility (bleeding edge): found no issues.
  • Package is using at least one outdated PHP dependency which needs an update for PHP 8.4 compatibility.
  • Running the tests against PHP 8.4 locally turned out to be impossible as the available instructions in the contributing docs don't actually work.
  • Trying to get a test run against PHP 8.4 working on GH Actions also turned out to be impossible.
  • In other words: status unknown.

GetID3

Current status:

  • Linting is enabled against PHP 8.4. The build passes without finding any PHP 8.4 related issues.
  • Test coverage: there are no tests (0%).
  • PHPCompatibility (bleeding edge): found no issues.
  • Important: as the project has no test suite, the linting passing on PHP 8.4 is only a small comfort and does not provide any real safeguards. On the plus side, the project does use PHPStan, which should still be able to catch at least some issues.
  • In other words: status unknown.
  • WordPress is using the latest version (1.9.23), see #59683
  • There have been various bug fix/PHP compatibility related commits since the last release, so would be nice if a new release were tagged, but by the looks of it, there have been no fixes committed directly related to PHP 8.4.

PHPMailer

PHPMailer uses the IMAP extension, which in PHP 8.4 is moved from PHP to PECL.
The PHPMailer code already contains a fall-back for this (and has for a long time).

Current status:

  • Linting and tests are being run against PHP 8.4 and pass.
  • I have a PR lined up to ensure that the tests will also be run without IMAP available to ensure the fallback code is tested. This PR will be pulled once another open PR has been merged.
  • Test coverage is 76% (non-strict).
  • PHPCompatibility (bleeding edge): found no issues other than the IMAP code.
  • WordPress is using the latest version (6.9.1), see #59966
  • There have been various commits since the last release, but nothing significant for PHP compatibility.

Requests

Current status:

  • There were 2 known PHP 8.4 issues, both of which were already fixed in the 2.0.11 release from March 2024.
  • Linting and tests are being run against PHP 8.4.
  • Test coverage is 94% (non-strict).
  • PHPCompatibility (bleeding edge): found no issues.
  • WordPress is using the latest relevant version 2.0.11, see #60838. (Requests 2.0.12 only updated the certificates bundle, while WP uses its own)

SimplePie 🚨

Current status:

  • Tests are NOT yet being run against PHP 8.4. PR to enable this is open.
  • Test coverage is ~46% for the 1.8 branch (non-strict), ~49% for the master branch.
  • PHPCompatibility (bleeding edge): found multiple issues. Multiple PRs open #880, #881.
  • WordPress is behind and is still using version 1.5.8, while the latest release is 1.8.0, see #55604 (ticket is now over two years old...).
  • SimplePie 1.8.0 does have some deprecation notices still on PHP 8.4, aside from the newly found issues. Some fixes have already been made in the master branch, but that branch is for 2.0. A PR to backport the PHP 8.4 fixes to the 1.8 branch is open, but will need to be updated with the above mentioned additional fixes.
  • All in all, SimplePie is nowhere near ready and it'll be interesting to see if a new version gets tagged in time for PHP 8.4 / WP 6.7. That is, providing WP actually updates to v 1.8.0 in time to even be able to update to the next tag.

Note: There is no expectation that any fixes will be backported to branches before the 1.8 branch.

Sodium Compat

Current status:

  • Tests are being run against PHP 8.4.
  • Test coverage is 48% (non-strict).
  • PHPCompatibility (bleeding edge): found no issues.
  • No known issues in the last release (1.21.1).
  • WordPress is using the latest version (1.21.1), see #61686.

PHPass

Current status:

  • Tests are being run against PHP 8.4.
  • Test coverage is 92% (non-strict).
  • PHPCompatibility (bleeding edge): found no issues.
  • No known issues in the current main branch, which translates to the 0.5.4 version.
  • WordPress is using version 0.5.0, but the script is a little out of sync with upstream, though not in a way that it impacts the running of WP on PHP 8.4. Still a good idea to sync with upstream anyway. See #62058.

WordPress Importer plugin

This plugin is used in the test suite.

Current status:

  • Tests are NOT (yet) being run against PHP 8.4.
  • Test coverage is ~68% (non-strict).
  • PHPCompatibility (bleeding edge): found one issue. PR to fix this has been submitted.
  • The plugin should tag a new version once this PR has been merged.
  • WordPress automatically installs the latest master of the plugin when the Docker container is installed, so once the above mentioned PR has been merged, the related Core automated tests will be able to pass.

Important:
Contributors not using Docker for their local dev environment, will need to update their copy of the plugin as used for the tests. This should be mentioned in a dev-note.
Also see: https://make.wordpress.org/core/handbook/contribute/git/#unit-tests

Task list

  • [x] [@jrf] Once current open PR is merged, submit PR to PHPMailer to run tests against optimal/minimal dependencies. PHPMailer PR#3096
  • [x] Update PHPass library, see #62058, patch available.
  • [ ] Review and merge remaining PR(s) for WP-CLI.
  • [ ] Review and merge PR(s) for the WordPress Importer plugin.
  • [ ] Review and commit patches submitted via PRs for WP Core as attached to this ticket.
    • [x] Fixes related to deprecation of implicitly nullable parameters
    • [ ] Fixes related to deprecation of XML Parser custom callables
    • [ ] Fixes related to deprecation of msqli_ping()
    • [ ] Fixes related to deprecation of trigger_error() with E_USER_ERROR
    • [ ] Fix related to E_STRICT
  • [ ] Update to SimplePie 1.8.0, see #55604
  • [ ] Update SimplePie to _next_ release once available (and verify that all PHP 8.4 fixes were included).
  • [ ] Enable running tests against PHP 8.4 for the WordPress Importer plugin
  • [ ] Enable running tests against PHP 8.4 for WordPress Core. This will only be possible once both the Importer plugin as well as SimplePie have released new versions and WP has updated (for SimplePie). Alternatively, we could already turn it on after the WP specific PRs have been merged, and temporarily skip the Importer tests and the tests which would fail on SimplePie.

Change History (50)

This ticket was mentioned in PR #7369 on WordPress/wordpress-develop by @jrf.


10 days ago
#1

  • Keywords has-patch has-unit-tests added

### PHP 8.4 | WP_HTML_Processor: fix implicitly nullable parameter [1]

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

This commit the one instance of this in the WP_HTML_Processor class.

Fixed by adding the nullability operator to the type, which is supported since PHP 7.1, so we can use it now the minimum supported PHP version is PHP 7.2.

As this deprecation is thrown at compile time, it can be seen at the top of the test output when running on PHP 8.4 (which will be gone once this change has been committed). It is not possible to write a test to cover this.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

### PHP 8.4 | Tests_HtmlApi_WpHtmlProcessorComments: fix implicitly nullable parameters [2]

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

The Tests_HtmlApi_WpHtmlProcessorComments test class contains one problematic parameter in the test_comment_processing() method declaration.

While this could be fixed by adding the nullability operator, I've elected not to do this, but to remove the type declarations in the test method instead, including other type declarations for this method and the second test method, which were not affected by the deprecation.

The reason for this is quite straight-forward: using type declarations in tests is bad practice and inhibits defense-in-depth type testing.

Using type declarations in tests prevents being able to test the "code under test" with unexpected input types as the values with unexpected (scalar) types will be juggled to the expected type between the data provider and the test method and the _real_ data value would therefore never reach the method under test.

The knock-on effects of this are:

  • That the input handling of the "code under test" can not be safeguarded, whether this input handling is done via in-function type checking or via a type declaration in the "code under test".
  • That if such "unexpected data type" tests are added to the data provider, they will silently pass (due to the type being juggled before reaching the "code under test"), giving a false sense of security, while in actual fact, these data sets would not be testing anything at all and if, for instance, the type declaration in the "code under test" would be removed, these tests would still pass, while by rights they should start failing.

Also note that this problem would only be exacerbated if the file would be put under strict_types.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

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

This ticket was mentioned in PR #7370 on WordPress/wordpress-develop by @jrf.


10 days ago
#2

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the TestXMLParser helper utility. In this case, the callbacks were already using the recommended format and the call to xml_set_object() was completely redundant.

As this is a test utility and was already causing pre-existing tests using the utility to fail, there is no need for dedicated tests to cover this change.

Refs:

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

This ticket was mentioned in PR #7371 on WordPress/wordpress-develop by @jrf.


10 days ago
#3

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the IXR_Message::parse() method.

This change is safeguarded via the newTests_XMLRPC_Message::test_parse_sets_handlers() test method.

Note: I recognize that this is "officially" an external library, but AFAIK, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.

Refs:

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

This ticket was mentioned in PR #7372 on WordPress/wordpress-develop by @jrf.


10 days ago
#4

Note: the first commit is necessary to allow the tests being added in the second commit to pass.

---

### PHP 8.2 | AtomParser: explicitly declare all properties

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

  • If it's an accidental typo for a declared property: fix the typo.
  • For known properties: declare them on the class.
  • For unknown properties: add the magic __get(), __set() et al methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods build in.
  • For unknown _use of_ dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

In this case, the property added are explicitly referenced in this class, so fall in the "known property" category.

Refs:

### PHP 8.4 | Remove use of xml_set_object() [3]

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the AtomParser::parse() method.

This change is safeguarded via the new AtomParser_Parse_Test class.

Notes:

  • I recognize that this is "officially" an external library, but AFAIK, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.
  • It appears that this class is not actually used by WP Core itself, so it could be considered to deprecate the class. However, as the class is not currently deprecated, safeguarding the change with a test seemed prudent.
  • The fixture used for the test reuses a fixture from the original package: https://code.google.com/archive/p/phpatomlib/source/default/source
  • The new test class follows the recommended test format (naming convention of the class, @covers tag at class level, only testing one method) as per Trac tickets 62004 / 53010.

Refs:

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

This ticket was mentioned in PR #7373 on WordPress/wordpress-develop by @jrf.


10 days ago
#5

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the MagpieRSS::__construct() method.

The change has not been not covered by tests. This class has been deprecated since WP 3.0.0 and is not covered by tests at all. Adding those now seems superfluous, all the more as the principle of the fix is no different than for the other files, so we can be sure it works anyway.

Note: I recognize that this is "officially" an external library, but AFAIK, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.

Refs:

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

This ticket was mentioned in PR #7374 on WordPress/wordpress-develop by @jrf.


10 days ago
#6

The mysqli_ping() function is deprecated as of PHP 8.4, though, in reality, the function wasn't working according to spec anymore since PHP 8.2 when the libmysql driver was dropped in favour of libmysqlnd, which was already the default (and recommended) driver since PHP 5.4.

The mysqli_ping() method was also not really correctly named as its functionality was to reconnect to the database, not just ping.

The alternative is to "manually" ping the database by sending a DO 1 query (the cheapest possible SQL query).

Now, I considered adding a PHP version based toggle, but as mentioned above, the default driver has been libmysqlnd since PHP 5.4 and in that case, the function never worked anyway, so in reality mysqli_ping() was only really functional for the odd custom PHP compilation where mysqli was build against libmysql AND reconnect was not disabled.

With this in mind, I'm proposing to replace the call to mysqli_ping() with the DO 1 query completely. If that query succeeds, we can conclude the database connection is still alive. This solution should be the most stable as it will work for both PHP 7.2 <= 8.1, independently of which driver mysqli was compiled with, as well as for PHP 8.2+.

Note: It could also be considered to remove the function call to mysqli_ping() completely and rely on standard error handling in case the connection would have dropped, as after all, the fact that the connection existed at the moment the "ping" happened, is no guarantee that the connection will still exist when the next query is send.... I choose not to do so as WP has custom error handling and does not use the PHP native mysqli exceptions for this, which would make implementing this more awkward.

Includes a test to verify that the connection check works when there is a valid connection (this was previously not covered by tests).

Refs:

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

This ticket was mentioned in PR #7375 on WordPress/wordpress-develop by @jrf.


10 days ago
#7

### Text_Diff::_check(): add basic test + fix a bug

This commit adds a simple test for the Text_Diff::_check() method and as that test could never pass with the code as-is, it also fixes the bug discoved by this test.

Bug symptom: Error: Class name must be a valid object or a string on line 279.

### PHP 8.4 | Text_Diff::_check(): fix trigger_error() with E_USER_ERROR is deprecated

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

Considering this is an unmaintained external dependency, I'm propose we fix this in the WP specific copy of the dependency.

Now, there were basically three options:

  • Silence the deprecation until PHP 9.0 and delay properly solving this until then.

This would lead to an awkward solution, as prior to PHP 8.0, error silencing would apply to all errors, while, as of PHP 8.0, it will no longer apply to fatal errors.
It also would only buy us some time and wouldn't actually solve anything.

  • Use exit($status).

This would make the code untestable and would disable handling of these errors via custom error handlers, which makes this an undesirable solution.

  • Throw an exception.

This makes for the most elegant solution with the least BC-breaking impact.

This commit implements the third option. It introduces a new Text_Exception class and starts using that in the Text_Diff::_check() method in all applicable places.

It also adds tests for the first two error conditions.

Unfortunately, I have not been able to get a test set up to test the other three exceptions, as without hacking the object under test, i.e. manually editing the $_edits property between the instantiating the object and running the _check(), I have not found a way to trigger those exceptions.

Ref:

### PHP 8.4 | Text_Diff_Op::reverse(): fix trigger_error() with E_USER_ERROR is deprecated

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

Considering this is an unmaintained external dependency, I'm propose we fix this in the WP specific copy of the dependency.

In this case, the trigger_error() call looks to be a remnant of the PHP 4 era before a class could be declared as abstract, so I've fixed this in the most straight-forward manner: by making both the method as well as the class abstract and removing the call to trigger_error().

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

This ticket was mentioned in PR #7376 on WordPress/wordpress-develop by @jrf.


10 days ago
#8

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing). The recommended replacements are either to use exceptions or to do a hard exit.

WP has its own wp_trigger_error() function, which under the hood calls trigger_error(). If passed E_USER_ERROR as the $error_level, this will hit the PHP 8.4 deprecation.

Now, there were basically three options:

  • Silence the deprecation until PHP 9.0 and delay properly solving this until then. This would lead to an awkward solution, as prior to PHP 8.0, error silencing would apply to all errors, while, as of PHP 8.0, it will no longer apply to fatal errors. It also would only buy us some time and wouldn't actually solve anything.
  • Use exit($status) when wp_trigger_error() is called with E_USER_ERROR. This would make the code untestable and would disable handling of these errors via custom error handlers, which makes this an undesirable solution.
  • Throw an exception when wp_trigger_error() is called with E_USER_ERROR. This makes for the most elegant solution with the least BC-breaking impact, though it does open it up to the error potential being "caught" via a try-catch. In my opinion, that's not actually a bad thing and is likely to only happen for those errors which can be worked around, in which case, it's a bonus that that's now possible.

So, this commit implements the third option. It introduces a new WP_Exception class and starts using that in the wp_trigger_error() function is the $error_level is set to E_USER_ERROR.

This change is covered by pre-existing tests, which have been updated to expect the exception instead of a PHP error.

Now, why did I not use WP_Error ?

Well, for one, this would lead to completely different behaviour as WP_Error doesn't extend Exception, which means that the program would not be stopped, but would continue running, which would be a much bigger breaking change and carries security risks. WP_Error also doesn't natively trigger displaying/logging of the error message, so in that case, it would still need an exit with the error message, bringing us back to point 2 above.

Basically, WP_Error is an arcane left-over from the PHP 4 days, before PHP natively supported try-catch statements with Exceptions. If it were up to me, we'd burn it with fire, but considering how much would break if we would (any and all plugins/themes/Core checks which check a function return value for is_wp_error() instead of using try-catch), that's not really an option.

Having said that, I would strongly recommend, going forward, to not allow any _new_ code to return a WP_Error and to encourage the use of dedicated exception classes instead (for new code). I'd recommend for additional exception classes to be placed in a new wp-includes/exceptions directory and for these, in principle, to extend WP_Exception.

_Note: this change will need to be mentioned in the WP 6.7 dev-note!_

Ref:

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

This ticket was mentioned in PR #7377 on WordPress/wordpress-develop by @jrf.


10 days ago
#9

### PHP 8.4 | WP_Test_Stream::open(): fix trigger_error() with E_USER_ERROR is deprecated

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

As this is a test-only class, we don't have to take BC-breaks into account.

Also, as this is a test helper, throwing a exception is the most appropriate solution.

Ref:

### PHP 8.4 | TestXMLParser::parse(): fix trigger_error() with E_USER_ERROR is deprecated

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

As this is a test-only class, we don't have to take BC-breaks into account.

Also, as this is a test helper, throwing a exception is the most appropriate solution.

Ref:

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

This ticket was mentioned in PR #7379 on WordPress/wordpress-develop by @jrf.


10 days ago
#10

:warning: Please read Trac ticket #62061 - this PR can't be committed until other patches have been committed first! :warning:

PHP 8.4 is expected to be released at the end of November 2024.

Enabling the tests to run in CI on PHP 8.4 allows WordPress core to start getting ready.

Note: Xdebug-related tests will not be run on PHP 8.4 at this time as the Docker image for PHP 8.4 does not contain Xdebug yet. Once a stable release of Xdebug 3.4.0 is available, it can be added to the Docker image and the test step can then be enabled for PHP 8.4.

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

#11 @jrf
10 days ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core-php by jrf. View the logs.


10 days ago

This ticket was mentioned in Slack in #cli by jrf. View the logs.


10 days ago

#14 @hellofromTonya
9 days ago

In 59052:

Tests: Fix implicitly nullable parameters in Tests_HtmlApi_WpHtmlProcessorComments.

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

The Tests_HtmlApi_WpHtmlProcessorComments test class contains one problematic parameter in the test_comment_processing() method declaration.

While this could be fixed by adding the nullability operator, the type declarations in the test method is removed instead, including other type declarations for this method and the second test method, which were not affected by the deprecation.

The reason for this is quite straight-forward: using type declarations in tests is bad practice and inhibits defense-in-depth type testing.

Using type declarations in tests prevents being able to test the "code under test" with unexpected input types as the values with unexpected (scalar) types will be juggled to the expected type between the data provider and the test method and the _real_ data value would therefore never reach the method under test.

The knock-on effects of this are:

  • That the input handling of the "code under test" can not be safeguarded, whether this input handling is done via in-function type checking or via a type declaration in the "code under test".
  • That if such "unexpected data type" tests are added to the data provider, they will silently pass (due to the type being juggled before reaching the "code under test"), giving a false sense of security, while in actual fact, these data sets would not be testing anything at all and if, for instance, the type declaration in the "code under test" would be removed, these tests would still pass, while by rights they should start failing.

Also note that this problem would only be exacerbated if the file would be put under strict_types.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

Follow-up to [58734].

Props jrf.
See #62061.

#15 @hellofromTonya
9 days ago

In 59053:

Code Modernization: Fix implicitly nullable parameter in WP_HTML_Processor.

PHP 8.4 deprecates implicitly nullable parameters, i.e. typed parameters with a null default value, which are not explicitly declared as nullable.

This commit the one instance of this in the WP_HTML_Processor class.

Fixed by adding the nullability operator to the type, which is supported since PHP 7.1, so we can use it now the minimum supported PHP version is PHP 7.2.

As this deprecation is thrown at compile time, it can be seen at the top of the test output when running on PHP 8.4 (which will be gone once this change has been committed). It is not possible to write a test to cover this.

Ref: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

Follow-up to [58867], [58769], [58304], [58192].

Props jrf.
See #62061.

@jrf commented on PR #7369:


9 days ago
#17

Thanks @hellofromtonya!

This ticket was mentioned in PR #7397 on WordPress/wordpress-develop by @jrf.


9 days ago
#18

The E_STRICT constant is deprecated as of PHP 8.4 and will be removed in PHP 9.0.

The error level hasn't been in use since PHP 8.0 anyway, so removing the exclusion from the error_reporting() setting in the install.php script used in the tests should make no difference in practice.

Ref:

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

#19 @jrf
9 days ago

  • Description modified (diff)

Updated with new info about E_STRICT and linked the relevant patch in the ticket.

#20 @hellofromTonya
9 days ago

In 59055:

Tests: Remove use of xml_set_object() in TestXMLParser.

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the TestXMLParser helper utility. In this case, the callbacks were already using the recommended format and the call to xml_set_object() was completely redundant.

As this is a test utility and was already causing pre-existing tests using the utility to fail, there is no need for dedicated tests to cover this change.

Refs:

Follow-up to [25002].

Props jrf.
See #62061.

#22 @hellofromTonya
9 days ago

In 59056:

Code Modernization: Remove xml_set_object() in IXR_Message::parse().

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the IXR_Message::parse() method.

This change is safeguarded via the newTests_XMLRPC_Message::test_parse_sets_handlers() test method.

Note: Though this is "officially" an external library, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.

Refs:

Follow-up to [15612], [1346].

Props jrf, hellofromTonya.
See #62061.

#25 @hellofromTonya
9 days ago

In 59062:

Code Modernization: Remove xml_set_object() in AtomParser::parse().

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the AtomParser::parse() method.

This change is safeguarded via the new AtomParser_Parse_Test class.

Notes:

  • Though this is "officially" an external library, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.
  • It appears that this class is not actually used by WP Core itself, so it could be considered to deprecate the class. However, as the class is not currently deprecated, safeguarding the change with a test seemed prudent.
  • The fixture used for the test reuses a fixture from the original package: https://code.google.com/archive/p/phpatomlib/source/default/source
  • The new test class follows the recommended test format (naming convention of the class, @covers tag at class level, only testing one method) as per Trac tickets 62004 / 53010.

Refs:

Follow-up to [5951].

Props jrf.
See #62061.

#27 @hellofromTonya
9 days ago

In 59063:

Code Modernization: Remove xml_set_object() in MagpieRSS::construct().

The XML Parser extension still supports a quite dated mechanism for method based callbacks, where the object is first set via xml_set_object() and the callbacks are then set by passing only the name of the method to the relevant parameters on any of the xml_set_*_handler() functions.

xml_set_object( $parser, $my_obj );
xml_set_character_data_handler( $parser, 'method_name_on_my_obj' );

Passing proper callables to the xml_set_*_handler() functions has been supported for the longest time and is cross-version compatible. So the above code is 100% equivalent to:

xml_set_character_data_handler( $parser, [$my_obj, 'method_name_on_my_obj'] );

The mechanism of setting the callbacks with xml_set_object() has now been deprecated as of PHP 8.4, in favour of passing proper callables to the xml_set_*_handler() functions. This is also means that calling the xml_set_object() function is deprecated as well.

This commit fixes this deprecation for the MagpieRSS::__construct() method.

The change has not been not covered by tests. This class has been deprecated since WP 3.0.0 and is not covered by tests at all. Adding those now seems superfluous, all the more as the principle of the fix is no different than for the other files, so we can be sure it works anyway.

Note: Though this is "officially" an external library, this package is no longer externally maintained. The code style of the fix in the source file is in line with the existing code style for the file.

Refs:

Follow-up to [4399].

Props jrf.
See #62061.

#29 @hellofromTonya
8 days ago

In 59068:

Tests: Remove use of E_STRICT.

The E_STRICT constant is deprecated as of PHP 8.4 and will be removed in PHP 9.0.

The error level hasn't been in use since PHP 8.0 anyway, so removing the exclusion from the error_reporting() setting in the install.php script used in the tests should make no difference in practice.

Ref:

Follow-up to [25002].

Props jrf.
See #62061.

#31 @hellofromTonya
8 days ago

In 59069:

Code Modernization: handle mysqli_ping() deprecation in wpdb::check_connection().

The mysqli_ping() function is deprecated as of PHP 8.4, though, in reality, the function wasn't working according to spec anymore since PHP 8.2 when the libmysql driver was dropped in favour of libmysqlnd, which was already the default (and recommended) driver since PHP 5.4.

The mysqli_ping() method was also not really correctly named as its functionality was to reconnect to the database, not just ping.

The alternative is to "manually" ping the database by sending a DO 1 query (the cheapest possible SQL query).

Adding a PHP version based toggle was considered, but as mentioned above, the default driver has been libmysqlnd since PHP 5.4 and in that case, the function never worked anyway, so in reality mysqli_ping() was only really functional for the odd custom PHP compilation where mysqli was build against libmysql AND reconnect was not disabled.

With this in mind, this change replaces the call to mysqli_ping() with the DO 1 query completely. If that query succeeds, it concludes the database connection is still alive. This solution should be the most stable as it will work for both PHP 7.2 <= 8.1, independently of which driver mysqli was compiled with, as well as for PHP 8.2+.

Note: It could also be considered to remove the function call to mysqli_ping() completely and rely on standard error handling in case the connection would have dropped, as after all, the fact that the connection existed at the moment the "ping" happened, is no guarantee that the connection will still exist when the next query is send.... this approach was not chosen so as WP has custom error handling and does not use the PHP native mysqli exceptions for this, which would make implementing this more awkward.

Includes a test to verify that the connection check works when there is a valid connection (this was previously not covered by tests).

Refs:

Follow-up to [56475], [27250], [27075].

Props jrf, hellofromTonya.
See #62061.

This ticket was mentioned in Slack in #hosting by javier. View the logs.


2 days ago

#35 @hellofromTonya
2 days ago

Flagging #62110 where the OPs tests are failing for the new AtomParser_Parse_Test test added via [59062].

This ticket was mentioned in Slack in #cli by jrf. View the logs.


8 hours ago

@jrf commented on PR #7375:


5 hours ago
#37

@hellofromtonya Would you like me to rebase to get rid of the merge conflicts ?

@hellofromTonya commented on PR #7375:


5 hours ago
#38

Would you like me to rebase to get rid of the merge conflicts ?

Thanks for offering @jrfnl. But it's not necessary. I'm going commit by commit.

#39 @hellofromTonya
5 hours ago

In 59105:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in Text_Diff::_check().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing). The recommended replacements are either to use exceptions or to do a hard exit.

This is an unmaintained external dependency; thus, the fix is made in the WP specific copy of the dependency.

Now, there were basically three options:

  • Silence the deprecation until PHP 9.0 and delay properly solving this until then.

This would lead to an awkward solution, as prior to PHP 8.0, error silencing would apply to all errors, while, as of PHP 8.0, it will no longer apply to fatal errors.
It also would only buy us some time and wouldn't actually solve anything.

  • Use exit($status).

This would make the code untestable and would disable handling of these errors via custom error handlers, which makes this an undesirable solution.

  • Throw an exception.

This makes for the most elegant solution with the least BC-breaking impact.

The third option is implemented which:

  • Introduces a new Text_Exception class.
  • Starts using that in the Text_Diff::_check() method in all applicable places.
  • Adds tests for the first two error conditions.

References:

Follow-up to [59070], [52978], [7747].

Props jrf.
See #62061.

#41 @hellofromTonya
4 hours ago

In 59106:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in Text_Diff_Op::reverse().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

This is an unmaintained external dependency; thus, the fix is made in the WP specific copy of the dependency.

As trigger_error() call looks to be a remnant of the PHP 4 era before a class could be declared as abstract, fixed by making both the method as well as the class abstract and removing the call to trigger_error().

Ref:

Follow-up to [7747].

Props jrf.
See #62061.

@hellofromTonya commented on PR #7375:


4 hours ago
#42

Fixed Text_Diff_Op:reverse() https://github.com/WordPress/wordpress-develop/commit/1d059a390ec2d57cf44a943cc656f6aee5b5d73a via https://core.trac.wordpress.org/changeset/59106

With all commits now committed, closing this PR. Thank you @jrfnl!

#43 @hellofromTonya
3 hours ago

In 59107:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in wp_trigger_error().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing). The recommended replacements are either to use exceptions or to do a hard exit.

WP has its own wp_trigger_error() function, which under the hood calls trigger_error(). If passed E_USER_ERROR as the $error_level, this will hit the PHP 8.4 deprecation.

Now, there were basically three options:

  • Silence the deprecation until PHP 9.0 and delay properly solving this until then. This would lead to an awkward solution, as prior to PHP 8.0, error silencing would apply to all errors, while, as of PHP 8.0, it will no longer apply to fatal errors. It also would only buy us some time and wouldn't actually solve anything.
  • Use exit($status) when wp_trigger_error() is called with E_USER_ERROR. This would make the code untestable and would disable handling of these errors via custom error handlers, which makes this an undesirable solution.
  • Throw an exception when wp_trigger_error() is called with E_USER_ERROR. This makes for the most elegant solution with the least BC-breaking impact, though it does open it up to the error potential being "caught" via a try-catch. That's not actually a bad thing and is likely to only happen for those errors which can be worked around, in which case, it's a bonus that that's now possible.

The third option is implemented which:

  • Introduces a new WP_Exception class.
  • Starts using WP_Exception in the wp_trigger_error() function when the $error_level is set to E_USER_ERROR.

This change is covered by pre-existing tests, which have been updated to expect the exception instead of a PHP error.

Why not use WP_Error?

Well, for one, this would lead to completely different behaviour (BC).

As WP_Error doesn't extend Exception, the program would not be stopped, but would continue running, which would be a much bigger breaking change and carries security risks. WP_Error also doesn't natively trigger displaying/logging of the error message, so in that case, it would still need an exit with the error message, bringing us back to point 2 above.

Introducing WP_Exception provides (essentially) the same behaviour in that it retains the fatal error and error message displaying/logging behaviors. It also introduces a base Exception class, from which future exception classes can extend.

References:

Follow-up to [56530].

Props jrf, hellofromTonya.
See #62061.

@hellofromTonya commented on PR #7376:


3 hours ago
#44

Committed via https://core.trac.wordpress.org/changeset/59107

But I missed props in the commit for @costdev 🤦‍♀️ Manually added ✅

#45 @hellofromTonya
3 hours ago

  • Keywords needs-dev-note added

[59107] introduces WP_Exception and changes wp_trigger_error() to throw it when E_USER_ERROR is the error level. This change will need a dev note.

#46 @hellofromTonya
3 hours ago

In 59108:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in WP_Test_Stream::open().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

As this is a test-only class, do not have to take BC-breaks into account.

Also, as this is a test helper, throwing a exception is the most appropriate solution.

Reference:

Follow-up to [49230].

Props jrf.
See #62061.

#47 @hellofromTonya
2 hours ago

In 59109:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in TestXMLParser::parse().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing).
The recommended replacements are either to use exceptions or to do a hard exit.

As this is a test-only class, do not have to take BC-breaks into account.

Also, as this is a test helper, throwing a exception is the most appropriate solution.

Reference:

Follow-up to [25002].

Props jrf.
See #62061.

@jrf commented on PR #7375:


29 minutes ago
#49

Thanks @hellofromtonya!

@jrf commented on PR #7376:


23 minutes ago
#50

Thanks @hellofromtonya! Just noting here that this change will need a mention in the field guide/miscellaneous dev notes for WP 6.7.

Note: See TracTickets for help on using tickets.