Skip to content

Commit 9a9ca41

Browse files
hellofromtonyahellofromtonya
committed
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: * 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 Follow-up to [59070], [52978], [7747]. Props jrf. See #62061. git-svn-id: https://develop.svn.wordpress.org/trunk@59105 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 2550224 commit 9a9ca41

File tree

4 files changed

+35
-5
lines changed

4 files changed

+35
-5
lines changed

‎src/wp-includes/Text/Diff.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,24 +260,24 @@ static function _getTempDir()
260260
function _check($from_lines, $to_lines)
261261
{
262262
if (serialize($from_lines) != serialize($this->getOriginal())) {
263-
trigger_error("Reconstructed original does not match", E_USER_ERROR);
263+
throw new Text_Exception("Reconstructed original does not match");
264264
}
265265
if (serialize($to_lines) != serialize($this->getFinal())) {
266-
trigger_error("Reconstructed final does not match", E_USER_ERROR);
266+
throw new Text_Exception("Reconstructed final does not match");
267267
}
268268

269269
$rev = $this->reverse();
270270
if (serialize($to_lines) != serialize($rev->getOriginal())) {
271-
trigger_error("Reversed original does not match", E_USER_ERROR);
271+
throw new Text_Exception("Reversed original does not match");
272272
}
273273
if (serialize($from_lines) != serialize($rev->getFinal())) {
274-
trigger_error("Reversed final does not match", E_USER_ERROR);
274+
throw new Text_Exception("Reversed final does not match");
275275
}
276276

277277
$prevtype = null;
278278
foreach ($this->_edits as $edit) {
279279
if ($prevtype !== null && $edit instanceof $prevtype) {
280-
trigger_error("Edit sequence is non-optimal", E_USER_ERROR);
280+
throw new Text_Exception("Edit sequence is non-optimal");
281281
}
282282
$prevtype = get_class($edit);
283283
}

‎src/wp-includes/Text/Exception.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
/**
3+
* Exception for errors from the Text_Diff package.
4+
*
5+
* {@internal This is a WP native addition to the external Text_Diff package.}
6+
*
7+
* @package WordPress
8+
* @subpackage Text_Diff
9+
*/
10+
11+
class Text_Exception extends Exception {}

‎src/wp-includes/wp-diff.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
require ABSPATH . WPINC . '/Text/Diff/Renderer.php';
1616
/** Text_Diff_Renderer_inline class */
1717
require ABSPATH . WPINC . '/Text/Diff/Renderer/inline.php';
18+
/** Text_Exception class */
19+
require ABSPATH . WPINC . '/Text/Exception.php';
1820
}
1921

2022
require ABSPATH . WPINC . '/class-wp-text-diff-renderer-table.php';

‎tests/phpunit/tests/diff/Text_Diff_Check_Test.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ final class Text_Diff_Check_Test extends WP_UnitTestCase {
2323

2424
public static function set_up_before_class() {
2525
require_once ABSPATH . 'wp-includes/Text/Diff.php';
26+
require_once ABSPATH . 'wp-includes/Text/Exception.php';
2627
}
2728

2829
/**
@@ -34,4 +35,20 @@ public function test_check_passes_when_passed_same_input() {
3435
$diff = new Text_Diff( 'auto', array( self::FILE_A, self::FILE_B ) );
3536
$this->assertTrue( $diff->_check( self::FILE_A, self::FILE_B ) );
3637
}
38+
39+
public function test_check_throws_exception_when_from_is_not_same_as_original() {
40+
$this->expectException( Text_Exception::class );
41+
$this->expectExceptionMessage( 'Reconstructed original does not match' );
42+
43+
$diff = new Text_Diff( 'auto', array( self::FILE_A, self::FILE_B ) );
44+
$diff->_check( self::FILE_B, self::FILE_B );
45+
}
46+
47+
public function test_check_throws_exception_when_to_is_not_same_as_final() {
48+
$this->expectException( Text_Exception::class );
49+
$this->expectExceptionMessage( 'Reconstructed final does not match' );
50+
51+
$diff = new Text_Diff( 'auto', array( self::FILE_A, self::FILE_B ) );
52+
$diff->_check( self::FILE_A, self::FILE_A );
53+
}
3754
}

0 commit comments

Comments
 (0)