Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: highlightFile() in BaseExceptionHandler for PHP 8.3 #8401

Merged
merged 6 commits into from
Jan 7, 2024

Conversation

michalsn
Copy link
Member

@michalsn michalsn commented Jan 5, 2024

Description
Supersedes #8399
Fixes #8398

This PR fixes issues with highlightFile() method.

The source of the problem is the fact that the highlight_file() function has slightly changed its behavior in PHP 8.3.

A nice summary can be found here:
https://php.watch/versions/8.3/highlight_file-highlight_string-html-changes

Except for the changes in the highlight_file() function, this PR also fixes some issues found in closing opened span tags.

I added some changes to the way $source variable is handled after noticing @kenjis PR: #8399.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 5, 2024
@kenjis
Copy link
Member

kenjis commented Jan 6, 2024

@michalsn Fixes: #8398 should be Fixes #8398.
See https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@kenjis
Copy link
Member

kenjis commented Jan 6, 2024

There were 4 failures:

1) CodeIgniter\Debug\ExceptionHandlerTest::testHighlightFile
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '<pre><code><span class="line"><span class="number"> 9</span> &nbsp;*&nbsp;the&nbsp;LICENSE&nbsp;file&nbsp;that&nbsp;was&nbsp;distributed&nbsp;with&nbsp;this&nbsp;source&nbsp;code.
 <span class="line"><span class="number">10</span> &nbsp;*/
 <span class="line"><span class="number">11</span> 
-<span class="line"><span class="number">12</span> </span><span style="color: #f1ce61;">namespace&nbsp;</span><span style="color: #c7c7c7">Tests\Support\Controllers</span><span style="color: #f1ce61;">;
+<span class="line"><span class="number">12</span> </span><span style="color: #f1ce61;">namespace&nbsp;</span><span style="color: #c7c7c7">Tests</span><span style="color: #f1ce61;">\</span><span style="color: #c7c7c7">Support</span><span style="color: #f1ce61;">\</span><span style="color: #c7c7c7">Controllers</span><span style="color: #f1ce61;">;
 <span class="line"><span class="number">13</span> 
-<span class="line"><span class="number">14</span> use&nbsp;</span><span style="color: #c7c7c7">CodeIgniter\Controller</span><span style="color: #f1ce61;">;
+<span class="line"><span class="number">14</span> use&nbsp;</span><span style="color: #c7c7c7">CodeIgniter</span><span style="color: #f1ce61;">\</span><span style="color: #c7c7c7">Controller</span><span style="color: #f1ce61;">;
 <span class="line"><span class="number">15</span> 
 <span class='line highlight'><span class='number'>16</span> class&nbsp;Hello&nbsp;extends&nbsp;Controller
 </span></span><span style="color: #c7c7c7"></span><span style="color: #f1ce61;"></span><span style="color: #c7c7c7"><span class="line"><span class="number">17</span> </span><span style="color: #f1ce61;">{

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Debug/ExceptionHandlerTest.php:257
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

2) CodeIgniter\Helpers\TextHelperTest::testHighlightCode
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<code><span style="color: #000000">
-<span style="color: #0000BB">&lt;?php&nbsp;var_dump</span><span style="color: #007700">(</span><span style="color: #0000BB">$this</span><span style="color: #007700">);&nbsp;</span><span style="color: #0000BB">?&gt;&nbsp;</span>
+'<code><span style="color: #06B">
+<span style="color: #c7c7c7">&lt;?php&nbsp;var_dump</span><span style="color: #f1ce61;">(</span><span style="color: #c7c7c7">$this</span><span style="color: #f1ce61;">);&nbsp;</span><span style="color: #c7c7c7">?&gt;&nbsp;</span>
 </span>
 </code>'

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Helpers/TextHelperTest.php:273
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

3) CodeIgniter\Test\IniTestTraitTest::testBackupAndRestoreIniValues
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'#0000BB'
+'#c7c7c7'

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Test/IniTestTraitTest.php:32
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

4) CodeIgniter\View\ParserFilterTest::testHighlightCode
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<code><span style="color: #000000">
-<span style="color: #0000BB">Sincerely&nbsp;</span>
+'<code><span style="color: #06B">
+<span style="color: #c7c7c7">Sincerely&nbsp;</span>
 </span>
 </code>'

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/View/ParserFilterTest.php:204
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

--

There were 2 skipped tests:

1) CodeIgniter\Debug\ExceptionsTest::testDeprecationsOnPhp81DoNotThrow
PHP >= 8.1 is required.

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Debug/ExceptionsTest.php:58
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Debug/ExceptionsTest.php:58
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

2) CodeIgniter\Test\TestCaseTest::testPHPUnitHeadersEmitted
CodeIgniter\Test\CIUnitTestCase::assertHeaderEmitted() requires xdebug.

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/CIUnitTestCase.php:512
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/CIUnitTestCase.php:417
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Test/TestCaseTest.php:98
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

FAILURES!
Tests: 5012, Assertions: 8463, Failures: 4, Skipped: 2.

https://github.com/codeigniter4/CodeIgniter4/actions/runs/7422018813/job/20196514237?pr=8401

@michalsn
Copy link
Member Author

michalsn commented Jan 6, 2024

Yes, I'm not sure why it fails on PHP 7.4... and I don't have it locally to test 😐

@michalsn
Copy link
Member Author

michalsn commented Jan 6, 2024

Silly me... the result for PHP 7.4 is slightly different again. Fixed.

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you.

@kenjis kenjis merged commit fc62f96 into codeigniter4:develop Jan 7, 2024
58 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Displaying Error Page with 4.4 with PHP 8.3
3 participants