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

PHP 8.0 compatibility #4269

Merged
merged 9 commits into from
Dec 2, 2020
Merged

PHP 8.0 compatibility #4269

merged 9 commits into from
Dec 2, 2020

Conversation

Deltik
Copy link
Member

@Deltik Deltik commented Nov 27, 2020

Motivation and Context

PHP 8.0 is now stable and released, but e107 won't run on it because of various syntax and logic errors.

Description

This pull request fixes all PHP 8.0 errors caught by the existing tests. Specifically:

Vendor Changes

  • tedivm/jshrink appears to be abandoned and times out while minifying any JavaScript. It has been replaced with matthiasmullie/minify, which seems to be a drop-in replacement.
  • The test framework is no longer pinned to PHP 5.6 to enable phpunit/phpunit to run on PHP 8.0.

Method Signatures

  • error_handler::handle_error() had a required fifth parameter, but set_error_handler() in PHP 8.0 only sends four arguments.
  • lancheck::check_lanfiles() had a required parameter after the optional parameters, which is a compile error in PHP 8.0.
  • lancheck::checkLog() had a required parameter after the optional parameters, which is a compile error in PHP 8.0.
  • The sitelinks_alt class had methods called statically without the methods being declared as static, but PHP 8.0 now requires static methods to be declared as such.
  • e107::getTemplateInfo() had a required parameter after the optional parameters, which is a compile error in PHP 8.0. The first parameter, $plug_name, must be provided now, even if it is null.
  • e_marketplace::getDownloadModal() had a required parameter after the optional parameters, which is a compile error in PHP 8.0.
  • themeHandler::renderTheme() had a required parameter after the optional parameters, which is a compile error in PHP 8.0.
  • e107forum::set_crumb() had a required parameter after the optional parameters, which is a compile error in PHP 8.0.

Variables and Constants

  • The BOOTSTRAP constant was sometimes referenced before being defined.
  • siteStats::renderTodaysVisits() tried to increment a number by a string, which is not possible in PHP 8.0.
  • siteStats::bar() tried to number_format() a string, which is not possible in PHP 8.0.
  • The USER_WIDTH constant may have been referenced before being defined.

Function Changes

  • In PHP 8.0, the built-in function \get_parent_class() now raises an Error if it is provided a string that does not represent a class, but to maintain compatibility with PHP 5.6, a shim is required to suppress the Error.

Logic Changes

  • e_form::option_multi() performed a weak equality check on which value is selected, but PHP 8.0 reworked their string to number comparisons, which makes the output different. The comparison is now strict to enforce consistent output.
  • user_class::getName() performed a weak comparison on the input parameter, which could be provided as an empty string instead of a number. Since PHP 8.0 reworked their string to number comparisons, the comparison in this method is now made consistent by casting the input to an int.

How Has This Been Tested?

The changes make the existing tests pass on all PHP major versions since version 5.6. The GitHub Actions automated testing has been expanded to test on PHP 5.6 through PHP 8.0 on MySQL 5.5 through MySQL 8.0 and MariaDB 10.0 through MariaDB 10.5.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist

@Deltik Deltik requested review from CaMer0n and Moc November 28, 2020 11:27
@Deltik Deltik force-pushed the php-8-compatibility branch 2 times, most recently from f9b0aae to 668ab7f Compare November 28, 2020 15:17
(Looks like tedivm/jshrink is abandoned)
```
[Sat Nov 28 07:42:08.146694 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: PHP Fatal error:  Uncaught Error: Non-static method e_date::convert_date() cannot be called statically in ~/public_html/e107_handlers/bbcode_handler.php(390) : eval()'d code:6
[Sat Nov 28 07:42:08.146832 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: Stack trace:
[Sat Nov 28 07:42:08.146841 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: #0 ~/public_html/e107_handlers/bbcode_handler.php(390): eval()
[Sat Nov 28 07:42:08.146848 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#1 ~/public_html/e107_handlers/bbcode_handler.php(202): e_bbcode->proc_bbcode()
[Sat Nov 28 07:42:08.146853 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#2 ~/public_html/e107_handlers/e_parse_class.php(1919): e_bbcode->parseBBCodes()
[Sat Nov 28 07:42:08.146859 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#3 ~/public_html/e107_core/shortcodes/batch/comment_shortcodes.php(350): e_parse->toHTML()
[Sat Nov 28 07:42:08.146882 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#4 ~/public_html/e107_handlers/shortcode_handler.php(1119): comment_shortcodes->sc_comment()
[Sat Nov 28 07:42:08.146888 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#5 [internal function]: e_parse_shortcode->doCode()
[Sat Nov 28 07:42:08.146908 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#6 ~/public_html/e107_handlers/shortcode_handler.php(986): preg_replace_callback()
[Sat Nov 28 07:42:08.146914 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#7 ~/public_html/e107_handlers/e_parse_class.php(883): e_parse_shortcode->parseCodes()
[Sat Nov 28 07:42:08.146920 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#8 ~/public_html/e107_handlers/comment_class.php(534): e_parse->parseTemplate()
[Sat Nov 28 07:42:08.146926 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#9 ~/public_html/e107_handlers/comment_class.php(1301): comment->render_comment()
[Sat Nov 28 07:42:08.146931 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#10 ~/public_html/e107_handlers/comment_class.php(1125): comment->getComments()
[Sat Nov 28 07:42:08.146938 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#11 ~/public_html/e107_plugins/news/news.php(1299): comment->compose_comment()
[Sat Nov 28 07:42:08.146943 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#12 ~/public_html/e107_plugins/news/news.php(236): news_front->renderComments()
[Sat Nov 28 07:42:08.146950 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#13 ~/public_html/e107_plugins/news/news.php(1895): news_front->render()
[Sat Nov 28 07:42:08.146955 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#14 ~/public_html/news.php(23): require_once('...')
[Sat Nov 28 07:42:08.146961 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#15 ~/public_html/index.php(103): include('...')
[Sat Nov 28 07:42:08.146967 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#16 {main}
[Sat Nov 28 07:42:08.146972 2020] [fcgid:warn] [pid 224726:tid 140236713551616] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr:   thrown in ~/public_html/e107_handlers/bbcode_handler.php(390) : eval()'d code on line 6
```
Method plugin_forum_viewforum_shortcodes::sc_newthreadbuttonx()
```
[Mon Nov 30 03:41:56.019794 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: PHP Fatal error:  Uncaught TypeError: array_multisort(): Argument e107inc#1 ($array) must be an array or a sort flag in ~/public_html/e107_plugins/log/stats.php:808
[Mon Nov 30 03:41:56.019853 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: Stack trace:
[Mon Nov 30 03:41:56.019861 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: #0 ~/public_html/e107_plugins/log/stats.php(808): array_multisort()
[Mon Nov 30 03:41:56.019867 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#1 ~/public_html/e107_plugins/log/stats.php(1620): siteStats->arraySort()
[Mon Nov 30 03:41:56.019885 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#2 ~/public_html/e107_plugins/log/stats.php(194): siteStats->renderQueries()
[Mon Nov 30 03:41:56.019891 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr: e107inc#3 {main}
[Mon Nov 30 03:41:56.019897 2020] [fcgid:warn] [pid 3259290:tid 140500652779264] [client xxx.xxx.xxx.xxx:0] mod_fcgid: stderr:   thrown in ~/public_html/e107_plugins/log/stats.php on line 808
```
@Deltik Deltik force-pushed the php-8-compatibility branch from 6edba11 to 28ae487 Compare November 30, 2020 11:58
@codeclimate
Copy link

codeclimate bot commented Nov 30, 2020

Code Climate has analyzed commit 28ae487 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 20.9% (80% is the threshold).

This pull request will bring the total coverage in the repository to 8.4% (0.0% change).

View more on Code Climate.

@CaMer0n CaMer0n merged commit ce30a09 into e107inc:master Dec 2, 2020
@Deltik Deltik deleted the php-8-compatibility branch December 2, 2020 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants