Issues in onupdate.inc.php - New Issues - Inspection of "updates" - XoopsModules25x/mylinks - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#8)

by Michael

created 2017-05-25 06:58 UTC

include/onupdate.inc.php (4 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
/**
 * mylinks install functions.php
 *
 * LICENSE
 *
 * You may not change or alter any portion of this comment or credits
 * of supporting developers from this source code or any supporting source code
 * which is considered copyrighted (c) material of the original comment or credit authors.
 *
 * @copyright:: {@link http://xoops.org/ XOOPS Project}
 * @license  ::    {@link http://www.gnu.org/licenses/gpl-2.0.html GNU Public License}
 * @package  ::   mylinks
 * @author   ::    zyspec ([email protected])
 */
defined('XOOPS_ROOT_PATH') or die('Restricted access');

$moduleDirName = basename(dirname(__DIR__));

/**
 * @param XoopsModule $xoopsModule
 * @param $prev_version
 * @return bool
 */
function xoops_module_update_mylinks_base(XoopsModule $xoopsModule, $prev_version)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
{
    $minUpgradeFrom = '0.0.0';  //minimum version of module supported for upgrade
    $success        = false;

    $ref = xoops_getenv('HTTP_REFERER');  //referer check
    if ($ref == '' || strpos($ref, XOOPS_URL . '/modules/system/admin.php') === 0) {
        /* module specific part */
        $minValueArray       = explode('.', $minUpgradeFrom);
        $installedVersion    = (int)$prev_version;
        $minSupportedVersion = ($minValueArray[0] * 100) + ($minValueArray[1] * 10) + $minValueArray[2];
        $modErrMsg           = "<span style='color: red; font-weight: bold;'>This module cannot be upgraded from version {$installedVersion}.</span>";

        if ($installedVersion < $minSupportedVersion) {
            $success = false;
            $xoopsModule->setErrors($modErrMsg);
        } else {
            require_once XOOPS_ROOT_PATH . '/modules/' . $xoopsModule->getVar('dirname') . '/include/oninstall.inc.php';
            $success = xoops_module_pre_install_mylinks($xoopsModule);
        }
    }

    global $xoopsDB;
function myFunction($a, $b) {
    // Do something
}
    if ($prev_version < 312) {
        // delete old html template files
        $templateDirectory = $GLOBALS['xoops']->path('modules/' . $xoopsModule->getVar('dirname', 'n') . '/templates/');
        $templateList      = array_diff(scandir($templateDirectory), array('..', '.'));
        foreach ($templateList as $k => $v) {

            $fileInfo = new SplFileInfo($templateDirectory . $v);
            if ($fileInfo->getExtension() === 'html' && $fileInfo->getFilename() !== 'index.html') {
                if (file_exists($templateDirectory . $v)) {
                    unlink($templateDirectory . $v);
                }
            }
        }
        // delete old block html template files
        $templateDirectory = $GLOBALS['xoops']->path('modules/' . $xoopsModule->getVar('dirname', 'n') . '/templates/blocks/');
        $templateList      = array_diff(scandir($templateDirectory), array('..', '.'));
        foreach ($templateList as $k => $v) {

            $fileInfo = new SplFileInfo($templateDirectory . $v);
            if ($fileInfo->getExtension() === 'html' && $fileInfo->getFilename() !== 'index.html') {
                if (file_exists($templateDirectory . $v)) {
                    unlink($templateDirectory . $v);
                }
            }
        }

        //delete .html entries from the tpl table
        $sql = 'DELETE FROM ' . $xoopsDB->prefix('tplfile') . " WHERE `tpl_module` = '" . $xoopsModule->getVar('dirname', 'n') . "' AND `tpl_file` LIKE '%.html%'";
        $xoopsDB->queryF($sql);
    }

    return $success;
}

/**
 * eval functions to support module relocation (directory renaming)
 */
eval('function xoops_module_update_' . $moduleDirName . '($module=NULL, $prev_version)
        {
        return xoops_module_update_mylinks_base($module, $prev_version);
        }
    ');


1		<?php
2		/**
3		* mylinks install functions.php
4		*
5		* LICENSE
6		*
7		* You may not change or alter any portion of this comment or credits
8		* of supporting developers from this source code or any supporting source code
9		* which is considered copyrighted (c) material of the original comment or credit authors.
10		*
11		* @copyright:: {@link http://xoops.org/ XOOPS Project}
12		* @license :: {@link http://www.gnu.org/licenses/gpl-2.0.html GNU Public License}
13		* @package :: mylinks
14		* @author :: zyspec ([email protected])
15		*/
16		defined('XOOPS_ROOT_PATH') or die('Restricted access');
17
18		$moduleDirName = basename(dirname(__DIR__));
19
20		/**
21		* @param XoopsModule $xoopsModule
22		* @param $prev_version
23		* @return bool
24		*/
25		function xoops_module_update_mylinks_base(XoopsModule $xoopsModule, $prev_version)
		0 ignored issues – show Coding Style introduced 2017-05-25 07:01 UTC by Report Bug Copy Issue Report Show Similar Issues like this `xoops_module_update_mylinks_base` uses the super-global variable `$GLOBALS` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
26		{
27		$minUpgradeFrom = '0.0.0'; //minimum version of module supported for upgrade
28		$success = false;
29
30		$ref = xoops_getenv('HTTP_REFERER'); //referer check
31		if ($ref == '' \|\| strpos($ref, XOOPS_URL . '/modules/system/admin.php') === 0) {
32		/* module specific part */
33		$minValueArray = explode('.', $minUpgradeFrom);
34		$installedVersion = (int)$prev_version;
35		$minSupportedVersion = ($minValueArray[0] * 100) + ($minValueArray[1] * 10) + $minValueArray[2];
36		$modErrMsg = "<span style='color: red; font-weight: bold;'>This module cannot be upgraded from version {$installedVersion}.</span>";
37
38		if ($installedVersion < $minSupportedVersion) {
39		$success = false;
40		$xoopsModule->setErrors($modErrMsg);
41		} else {
42		require_once XOOPS_ROOT_PATH . '/modules/' . $xoopsModule->getVar('dirname') . '/include/oninstall.inc.php';
43		$success = xoops_module_pre_install_mylinks($xoopsModule);
44		}
45		}
46
47		global $xoopsDB;
		0 ignored issues – show Compatibility Best Practice introduced 2017-05-25 07:01 UTC by Report Bug Copy Issue Report Show Similar Issues like this Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
48		if ($prev_version < 312) {
49		// delete old html template files
50		$templateDirectory = $GLOBALS['xoops']->path('modules/' . $xoopsModule->getVar('dirname', 'n') . '/templates/');
51		$templateList = array_diff(scandir($templateDirectory), array('..', '.'));
52	View Code Duplication	foreach ($templateList as $k => $v) {
		0 ignored issues – show Duplication introduced 2017-05-25 07:01 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
53		$fileInfo = new SplFileInfo($templateDirectory . $v);
54		if ($fileInfo->getExtension() === 'html' && $fileInfo->getFilename() !== 'index.html') {
55		if (file_exists($templateDirectory . $v)) {
56		unlink($templateDirectory . $v);
57		}
58		}
59		}
60		// delete old block html template files
61		$templateDirectory = $GLOBALS['xoops']->path('modules/' . $xoopsModule->getVar('dirname', 'n') . '/templates/blocks/');
62		$templateList = array_diff(scandir($templateDirectory), array('..', '.'));
63	View Code Duplication	foreach ($templateList as $k => $v) {
		0 ignored issues – show Duplication introduced 2017-05-25 07:01 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
64		$fileInfo = new SplFileInfo($templateDirectory . $v);
65		if ($fileInfo->getExtension() === 'html' && $fileInfo->getFilename() !== 'index.html') {
66		if (file_exists($templateDirectory . $v)) {
67		unlink($templateDirectory . $v);
68		}
69		}
70		}
71
72		//delete .html entries from the tpl table
73		$sql = 'DELETE FROM ' . $xoopsDB->prefix('tplfile') . " WHERE `tpl_module` = '" . $xoopsModule->getVar('dirname', 'n') . "' AND `tpl_file` LIKE '%.html%'";
74		$xoopsDB->queryF($sql);
75		}
76
77		return $success;
78		}
79
80		/**
81		* eval functions to support module relocation (directory renaming)
82		*/
83		eval('function xoops_module_update_' . $moduleDirName . '($module=NULL, $prev_version)
84		{
85		return xoops_module_update_mylinks_base($module, $prev_version);
86		}
87		');
88

XoopsModules25x / mylinks

Pull Request — master (#8)

include/onupdate.inc.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like