Changeset 59105

Timestamp:

09/27/2024 05:51:49 PM (9 months ago)

Author:

hellofromTonya

Message:

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.

Location:

trunk

Files:

• src/wp-includes/Text/Diff.php (modified) (2 diffs)
• src/wp-includes/Text/Exception.php (added)
• src/wp-includes/wp-diff.php (modified) (1 diff)
• tests/phpunit/tests/diff/Text_Diff_Check_Test.php (modified) (2 diffs)

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

@@ -261 +261 @@
     {
         if (serialize($from_lines) != serialize($this->getOriginal())) {
-            trigger_error("Reconstructed original does not match", E_USER_ERROR);
+            throw new Text_Exception("Reconstructed original does not match");
         }
         if (serialize($to_lines) != serialize($this->getFinal())) {
-            trigger_error("Reconstructed final does not match", E_USER_ERROR);
+            throw new Text_Exception("Reconstructed final does not match");
         }
 
         $rev = $this->reverse();
         if (serialize($to_lines) != serialize($rev->getOriginal())) {
-            trigger_error("Reversed original does not match", E_USER_ERROR);
+            throw new Text_Exception("Reversed original does not match");
         }
         if (serialize($from_lines) != serialize($rev->getFinal())) {
-            trigger_error("Reversed final does not match", E_USER_ERROR);
+            throw new Text_Exception("Reversed final does not match");
         }
 
@@ -278 +278 @@
         foreach ($this->_edits as $edit) {
             if ($prevtype !== null && $edit instanceof $prevtype) {
-                trigger_error("Edit sequence is non-optimal", E_USER_ERROR);
+                throw new Text_Exception("Edit sequence is non-optimal");
             }
             $prevtype = get_class($edit);

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

@@ -16 +16 @@
     /** Text_Diff_Renderer_inline class */
     require ABSPATH . WPINC . '/Text/Diff/Renderer/inline.php';
+    /** Text_Exception class */
+    require ABSPATH . WPINC . '/Text/Exception.php';
 }
 

trunk/tests/phpunit/tests/diff/Text_Diff_Check_Test.php

@@ -24 +24 @@
     public static function set_up_before_class() {
         require_once ABSPATH . 'wp-includes/Text/Diff.php';
+        require_once ABSPATH . 'wp-includes/Text/Exception.php';
     }
 
@@ -35 +36 @@
         $this->assertTrue( $diff->_check( self::FILE_A, self::FILE_B ) );
     }
+
+    public function test_check_throws_exception_when_from_is_not_same_as_original() {
+        $this->expectException( Text_Exception::class );
+        $this->expectExceptionMessage( 'Reconstructed original does not match' );
+
+        $diff = new Text_Diff( 'auto', array( self::FILE_A, self::FILE_B ) );
+        $diff->_check( self::FILE_B, self::FILE_B );
+    }
+
+    public function test_check_throws_exception_when_to_is_not_same_as_final() {
+        $this->expectException( Text_Exception::class );
+        $this->expectExceptionMessage( 'Reconstructed final does not match' );
+
+        $diff = new Text_Diff( 'auto', array( self::FILE_A, self::FILE_B ) );
+        $diff->_check( self::FILE_A, self::FILE_A );
+    }
 }