Pass through PHP notices, warnings, and errors to test harness and fix resulting failures

[Deltik]

Motivation and Context

e107's workaround for the many PHP notices, warnings, and errors has been to suppress them. This is especially detrimental for PHP major version changes (forward compatibility) because deprecated behavior would not be discovered until the code breaks.

Other less critical factors are type mismatches, resulting in PHP notices that get ignored. These are code smells or signs of problematic code. These notices can be resolved by being explicit about the desired type and more clearly conveys the developer's intention of what the variable should be.

Description

Overview

• Turns off error suppression for e107 in command line (CLI) mode. This allows the test harness to fail on every PHP error.
• Resolves every PHP notice, warning, and error from error_reporting(E_ALL) for all supported versions of PHP (versions 5.6 through 7.4).
• Fixes bugs in the e107 core caught by the newly failing tests
• Finally replaced the deprecated mysql extension with mysqli in the e_db_mysql class

Breaking Changes

• [b9d4961] e107 in CLI mode no longer enables its own error handler (error_handler::handle_error())
  when $_E107['cli'] or $_E107['debug'] is not false.
  • In $_E107['debug'] mode, the error reporting level is E_ALL.
  • Otherwise, in $_E107['cli'] mode, the error reporting level is E_ALL & ~E_STRICT & ~E_NOTICE.
• [ef34ef7] Removed obsolete ALLOW_AUTO_FIELD_DEFS constant
• [62a547a] e107::coreLan() now loads the lan_admin.php file if the $admin argument is true
• [622be85] e107::getTemplate() now accepts blank strings for the plugin name to mean that a core template should be loaded.
• [c789767] Removed PDO from e_db_mysql
• [4a26ac5] Removed unused USE_PERSISTANT_DB constant
• [8c528de] e_db_mysql: Replaced mysql with mysqli. Return types have changed but no tests needed to be modified, so client code should remain stable.
• [8b354ad] Silently ignore failures to e_db_mysql::close(); if it's failing, it's probably already closed

Bug Fixes

• [c232613] Fix mkdir() failure in e107::_init() if parent directory does not exist
• [bcba1e0] Blank IP address eIPHandler::$serverIP in CLI mode
• [24fe5c8] Updated pclzip.lib.php to version 2.8.4 to fix math error
• [24fe5c8] Suppress mkdir() error in e_file::unzipGithubArchive()
• [f5f1454] e107::getTemplate() could be run without the expected plugin LANs
• [524229b] Fixed a bunch of PHP 7.4 syntax deprecation warnings. All files that trigger this notice have been corrected:

  Array and string offset access syntax with curly braces is deprecated

• [524229b] Removed pointless (and invalid) destructor, LinkedIn::__destruct()
• [4454b01] Fix PHP 7.3 deprecation warning in lan_ren_help.php caused by this commit on 21 September 2004.
• [be8f2bb] userlogin::login() had a continue that should have been a continue 2 inside a switch.
• [52116ad] Silenced e_array::unserialize() HTML vomit in CLI mode
• [52116ad] Fixed e107_debug_shutdown() HTML vomit due to $error_handler not being global
• [72d3f07] e_db_mysql::rowCount() could try to use mysql_num_rows() to count rows of a non-resource
• [72d3f07] e_db_mysql::delete() now stores the number of deleted rows in e_db_mysql::$mySQLrows
• [72d3f07] e_db_mysql::getServerInfo() should now actually get a version number

Variable and Type Corrections

• [be36462] Null checks for $_SERVER keys in:
  • e107::prepare_request()
  • e107::set_constants()
  • e107::set_urls()
• [bcba1e0] Null checks for $_SERVER keys in:
  • eIPHandler::__construct()
  • eIPHandler::getCurrentIP()
• [6fe4bf1] Null checks for $_SERVER keys in:
  • e_online::goOnline()
• [4321c1b] Null checks for $_SERVER keys in:
  • e_session::getValidateData()
  • e_core_session::challenge()
• [62a547a] Fixed variable scope of $eplug_folder in e107plugin::uninstall()
• [8e0b047] array_diff_recursive() type check during recursion
• [c604b30] Removed unnecessary array type check in e_form::option_multi()
• [98911f0] Multiple type checks for e_db::insert() implementations
• [b2bd676] Array type check for e_bbcode::imgToBBcode()
• [b2bd676] Optional query string in e_parse::thumbUrlDecode()
• [b2bd676] Null checks for e107TinyMceParser
• [1561992] Removed unused variable from e107plugin::XmlAdminLinks()
• [82499f7] Debug variable scope fix in e107plugin::XmlTables()
• [1d72d48] Type checks for db_verify::prepareResults()
• [509b9ff] Null check for online_shortcodes::sc_online_member_user()
• [d2d0105] Null check for UserHandler::userClassUpdate()
• [15de2e2] Null check for optional unit in e_file::file_size_decode()
• [622be85] e_form::progressBar() now supports input values that already have % at the end
• [622be85] Null check for $options['list'] in e_form::progressBar()
• [91660a2] Null check in e_db_pdo::db_Query()
• [f5f1454] Empty check in e107Test::testGetTemplate()
• [cf8dc0b] Null checks in the e_theme constructor
• [d6eafdc] Null check for e_user_model::isBot()
• [a1560b1] Null check during child recursion of e_tree_model::flattenTree()
• [9506f98] Empty check for e_tohtml_linkwords::linksproc()
• [76c0f7e] Type validation for navigation_shortcode()
• [638412a] Null check for optional variable in e107_core/bbcodes/link.bb
• [d1bdfb8] Corrected broken type checks for e_parse::thumbSrcSet()
• [be8f2bb] Null check in e_db_pdo::makeTableDef()
• [be8f2bb] Null check in e_db_mysql::makeTableDef()
• [970f65b] Null check for optional query string in e107::url()
• [8b354ad] Changed class2.php's $sql variable to be hinted as an e_db instead of e_db_mysql

Constant Definitions

• [a49b532] Stop stepping on E107_DBG_* constants in tests
• [bfad3f7] Suppressed constant already defined errors in e_db_mysqlTest
• [62a547a] class2.php: Missing ADMINPERMS constant in CLI mode
• [b2bd676] Do not redefine e_ADMIN_AREA in parser.php
• [ce51015] Removed useless STRPTIME_COMPAT constant
• [fc6b81f] Fix undefined constant by importing theme LAN in e_marketplace. Apparently e_marketplace depends on the admin area theme LAN
• [f56bf44] Ignore redefines of EUF_* constants in the e107_user_extended constructor
• [72d3f07] Don't redefine MYSQL_* constants in e_db_pdo_class.php

Test Harness Changes

• [62a547a] Fixed isset() check order in pluginsTest::makePluginReport()
• [78a5c2a] Disabled e_onlineTest::testGoOnline() because it has no assertions
• [66a9765] Corrected array subset check in user_classTest::testGetUsersInClass()
• [b2bd676] Silence attempts to redefine TINYMCE_UNIT_TEST
• [dbdb5f4] Don't set e_DEBUG in e_arrayTest::testUnserialize()
• [46b5418] Fixed array key absence check in e_db_abstractTest::testDb_Fetch()
• [622be85] Test rounding in e_formTest::testProgressBar()
• [91660a2] Test isolation fixes for e_db_abstractTest:
  • testDb_Mark_Time() needed the debug log to be reset
  • testDb_IsLang() needed the admin LAN to be loaded
• [b8d6b9e] Type and null checks for pluginsTest
• [a0f4489] Disabled db_verifyTest::testGetIndex() because it has no assertions
• [f5f1454] e107Test::testGetInstance() included e107_config.php too many times
• [a1560b1] TreeModelTest::testTreeParentsAreAssignedCorrectly() apparently never worked until now because the wrong index was used
• [d55fe8a] Import LAN for e_formTest::testRenderElement()
• [be8f2bb] Silenced compact() in e107Test::testInitCore()
• [510d8e2] Fix flaky e_pluginTest::testBuildAddonPrefList() due to the "gallery" plugin possibly being uninstalled
• [72d3f07] e_db_abstractTest::testDb_Query() was fetching in PDO mode but shouldn't have been
• [72d3f07] Fixed typos in e_db_abstractTest::testDelete()
• [72d3f07] Moved PDO-exclusive testBackup() from e_db_abstractTest to e_db_pdoTest
• [72d3f07] e_db_mysqlTest now works in PHP 5.6 if the main e_db instance was in PDO mode but the test class initializes in legacy mode
• [72d3f07] e_db_mysqlTest now asserts that PDO mode is not in use
• [72d3f07] e_db_mysqlTest::testGetLastErrorNumber() has a different error code compared to PDO. This is now accounted for.
• [72d3f07] e_db_mysqlTest::testEscape() should actually be escaped by mysql_real_escape_string()
• [1fd0a17] GitHub Actions now builds with the PHP mysqli extension so that e_db_mysql can be tested.
• [8b354ad] Don't spam database server with connections: Close the PDO or mysqli connection after each e_db_abstractTest test.
• [f2a7590] Workaround for phpunit/php-code-coverage < 6.0.8 missing mkdir(). Fixes test coverage generation in PHP 5.6.

How Has This Been Tested?

See "Test Harness Changes" above.

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

• My code adheres to the e107 code style standard.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests pass.

[codeclimate]

Code Climate has analyzed commit f2a7590 and detected 19 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	8
Duplication	9
Bug Risk	2

Note: there is 1 critical issue.

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

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

View more on Code Climate.

[CaMer0n]

@Deltik Could you tell me more about getTemplate() now loading core templates.. while we already have getCoreTemplate() ?

[CaMer0n]

Looks good. 👍

[CaMer0n]

@Deltik lan_admin.php is no longer loaded early enough. We now have this on plugin admin pages:

Use of undefined constant LAN_MANAGE - assumed 'LAN_MANAGE' (this will throw an Error in a future version of PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php
--
Warning Use of undefined constant LAN_MANAGE - assumed 'LAN_MANAGE' (this will throw an Error in a future version of PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php
Warning Use of undefined constant LAN_CREATE - assumed 'LAN_CREATE' (this will throw an Error in a future version of PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php
Warning  Use of undefined constant LAN_CREATE_CATEGORY - assumed  'LAN_CREATE_CATEGORY' (this will throw an Error in a future version of  PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php
Warning Use of undefined constant LAN_PREFS - assumed 'LAN_PREFS' (this will throw an Error in a future version of PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php
Warning Use of undefined constant LAN_MANAGE - assumed 'LAN_MANAGE' (this will throw an Error in a future version of PHP), Line 237 of W:\www\e107v2\e107_plugins\download\admin_download.php

I have restored the early loading of lan_admin.php

[Deltik]

@Deltik Could you tell me more about getTemplate() now loading core templates.. while we already have getCoreTemplate() ?

@CaMer0n: I made this change because of e_formTest::testRenderElement() with $field = "bbarea_001".

This then leads to e_form::renderElement() on this line.

And from there, e_form::bbarea() is called, which has a third parameter that defaults to an empty string.

That empty string is passed on as the first argument to e107::getBB()->renderButtons(…), and that ends up here, where e107::getTemplate(…)'s first argument is a blank string.

e107::getCoreTemplate() then resolves an actual template path, …/e107_core/templates/bbcode_template.php, so I figured this was the desired behavior. The old behavior tried to load an invalid path, triggering a warning in e107::_getTemplate().