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

[jrfnl]

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:

• https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_passing_e_user_error_to_trigger_error
• https://www.php.net/manual/en/migration80.incompatible.php

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 Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions[bot]]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jrf, hellofromtonya.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions[bot]]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance, it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[hellofromtonya]

Text_Diff::_check(): add basic test + fix a bug was committed via https://core.trac.wordpress.org/changeset/59070 ✅ for Trac https://core.trac.wordpress.org/ticket/62083.

[jrfnl]

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

[hellofromtonya]

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.

[hellofromtonya]

Fixing Text_Diff::_check() https://github.com/WordPress/wordpress-develop/pull/7375/commits/f4804bcdeeb0498a2f6ce2ffa07600d41fba23c5 was committed via https://core.trac.wordpress.org/changeset/59105 ✅

[hellofromtonya]

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!

[jrfnl]

Thanks @hellofromtonya!