From 80165811e0294616d40fbc2e07113bf4dc1fea83 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 27 Sep 2021 15:50:19 +0200 Subject: [PATCH] Clean code --- htdocs/compta/bank/class/account.class.php | 16 ++++----- htdocs/core/class/infobox.class.php | 40 ++++++++++----------- htdocs/core/class/link.class.php | 10 +++--- htdocs/product/class/productbatch.class.php | 1 + test/phpunit/CodingPhpTest.php | 7 ++-- 5 files changed, 36 insertions(+), 38 deletions(-) diff --git a/htdocs/compta/bank/class/account.class.php b/htdocs/compta/bank/class/account.class.php index 9fd88f44d4f..1eb764d7af8 100644 --- a/htdocs/compta/bank/class/account.class.php +++ b/htdocs/compta/bank/class/account.class.php @@ -1696,21 +1696,21 @@ class Account extends CommonObject /** * Function used to replace a thirdparty id with another one. * - * @param DoliDB $db Database handler + * @param DoliDB $dbs Database handler * @param int $origin_id Old thirdparty id * @param int $dest_id New thirdparty id - * @return bool + * @return bool True=SQL success, False=SQL error */ - public static function replaceThirdparty($db, $origin_id, $dest_id) + public static function replaceThirdparty($dbs, $origin_id, $dest_id) { $sql = "UPDATE ".MAIN_DB_PREFIX."bank_url SET url_id = ".((int) $dest_id)." WHERE url_id = ".((int) $origin_id)." AND type='company'"; - if (!$db->query($sql)) { - //if ($ignoreerrors) return true; // TODO Not enough. If there is A-B on kept thirdarty and B-C on old one, we must get A-B-C after merge. Not A-B. - //$this->errors = $db->lasterror(); - return false; - } else { + if ($dbs->query($sql)) { return true; + } else { + //if ($ignoreerrors) return true; // TODO Not enough. If there is A-B on kept thirdarty and B-C on old one, we must get A-B-C after merge. Not A-B. + //$this->errors = $dbs->lasterror(); + return false; } } } diff --git a/htdocs/core/class/infobox.class.php b/htdocs/core/class/infobox.class.php index fb59e2b7c42..bdd0f79a0aa 100644 --- a/htdocs/core/class/infobox.class.php +++ b/htdocs/core/class/infobox.class.php @@ -83,7 +83,7 @@ class InfoBox /** * Return array of boxes qualified for area and user * - * @param DoliDB $db Database handler + * @param DoliDB $dbs Database handler * @param string $mode 'available' or 'activated' * @param int $zone Name or area (-1 for all, 0 for Homepage, 1 for Accountancy, 2 for xxx, ...) * @param User|null $user Object user to filter @@ -91,7 +91,7 @@ class InfoBox * @param int $includehidden Include also hidden boxes * @return array Array of boxes */ - public static function listBoxes($db, $mode, $zone, $user = null, $excludelist = array(), $includehidden = 1) + public static function listBoxes($dbs, $mode, $zone, $user = null, $excludelist = array(), $includehidden = 1) { global $conf; @@ -119,12 +119,12 @@ class InfoBox } dol_syslog(get_class()."::listBoxes get default box list for mode=".$mode." userid=".(is_object($user) ? $user->id : '')."", LOG_DEBUG); - $resql = $db->query($sql); + $resql = $dbs->query($sql); if ($resql) { - $num = $db->num_rows($resql); + $num = $dbs->num_rows($resql); $j = 0; while ($j < $num) { - $obj = $db->fetch_object($resql); + $obj = $dbs->fetch_object($resql); if (!in_array($obj->box_id, $excludelist)) { $regs = array(); @@ -144,7 +144,7 @@ class InfoBox // Goal is to avoid making a "new" done for each boxes returned by select. dol_include_once($relsourcefile); if (class_exists($boxname)) { - $box = new $boxname($db, $obj->note); // Constructor may set properties like box->enabled. obj->note is note into box def, not user params. + $box = new $boxname($dbs, $obj->note); // Constructor may set properties like box->enabled. obj->note is note into box def, not user params. //$box=new stdClass(); // box properties @@ -204,8 +204,8 @@ class InfoBox $j++; } } else { - dol_syslog($db->lasterror(), LOG_ERR); - return array('error'=>$db->lasterror()); + dol_syslog($dbs->lasterror(), LOG_ERR); + return array('error'=>$dbs->lasterror()); } return $boxes; @@ -215,13 +215,13 @@ class InfoBox /** * Save order of boxes for area and user * - * @param DoliDB $db Database handler + * @param DoliDB $dbs Database handler * @param int $zone Name of area (0 for Homepage, ...) * @param string $boxorder List of boxes with correct order 'A:123,456,...-B:789,321...' * @param int $userid Id of user * @return int <0 if KO, 0=Nothing done, > 0 if OK */ - public static function saveboxorder($db, $zone, $boxorder, $userid = 0) + public static function saveboxorder($dbs, $zone, $boxorder, $userid = 0) { global $conf; @@ -235,18 +235,18 @@ class InfoBox return 0; } - $user = new User($db); + $user = new User($dbs); $user->id = $userid; - $db->begin(); + $dbs->begin(); // Save parameters to say user has a dedicated setup $tab = array(); $confuserzone = 'MAIN_BOXES_'.$zone; $tab[$confuserzone] = 1; - if (dol_set_user_param($db, $conf, $user, $tab) < 0) { - $error = $db->lasterror(); - $db->rollback(); + if (dol_set_user_param($dbs, $conf, $user, $tab) < 0) { + $error = $dbs->lasterror(); + $dbs->rollback(); return -3; } @@ -257,7 +257,7 @@ class InfoBox $sql .= " AND position = ".((int) $zone); dol_syslog(get_class()."::saveboxorder", LOG_DEBUG); - $result = $db->query($sql); + $result = $dbs->query($sql); if ($result) { $colonnes = explode('-', $boxorder); foreach ($colonnes as $collist) { @@ -279,12 +279,12 @@ class InfoBox $sql .= " values ("; $sql .= " ".((int) $id).","; $sql .= " ".((int) $zone).","; - $sql .= " '".$db->escape($colonne.$ii)."',"; + $sql .= " '".$dbs->escape($colonne.$ii)."',"; $sql .= " ".((int) $userid).","; $sql .= " ".((int) $conf->entity); $sql .= ")"; - $result = $db->query($sql); + $result = $dbs->query($sql); if ($result < 0) { $error++; break; @@ -297,10 +297,10 @@ class InfoBox } if ($error) { - $db->rollback(); + $dbs->rollback(); return -2; } else { - $db->commit(); + $dbs->commit(); return 1; } } diff --git a/htdocs/core/class/link.class.php b/htdocs/core/class/link.class.php index 2a5e0f99c64..eaf4804b213 100644 --- a/htdocs/core/class/link.class.php +++ b/htdocs/core/class/link.class.php @@ -274,24 +274,24 @@ class Link extends CommonObject /** * Return nb of links * - * @param DoliDb $db Database handler + * @param DoliDb $dbs Database handler * @param string $objecttype Type of the associated object in dolibarr * @param int $objectid Id of the associated object in dolibarr * @return int Nb of links, -1 if error **/ - public static function count($db, $objecttype, $objectid) + public static function count($dbs, $objecttype, $objectid) { global $conf; $sql = "SELECT COUNT(rowid) as nb FROM ".MAIN_DB_PREFIX."links"; - $sql .= " WHERE objecttype = '".$db->escape($objecttype)."' AND objectid = ".((int) $objectid); + $sql .= " WHERE objecttype = '".$dbs->escape($objecttype)."' AND objectid = ".((int) $objectid); if ($conf->entity != 0) { $sql .= " AND entity = ".$conf->entity; } - $resql = $db->query($sql); + $resql = $dbs->query($sql); if ($resql) { - $obj = $db->fetch_object($resql); + $obj = $dbs->fetch_object($resql); if ($obj) { return $obj->nb; } diff --git a/htdocs/product/class/productbatch.class.php b/htdocs/product/class/productbatch.class.php index f7f604b5399..9bd361858ba 100644 --- a/htdocs/product/class/productbatch.class.php +++ b/htdocs/product/class/productbatch.class.php @@ -437,6 +437,7 @@ class Productbatch extends CommonObject public static function findAll($db, $fk_product_stock, $with_qty = 0, $fk_product = 0) { global $langs, $conf; + $ret = array(); $sql = "SELECT"; diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index fea52610c2b..21a58f34159 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -205,8 +205,6 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase 'conf.class.php', 'html.form.class.php', 'html.formmail.class.php', - 'infobox.class.php', - 'link.class.php', 'translate.class.php', 'utils.class.php', 'modules_product.class.php', @@ -215,8 +213,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase 'multicurrency.class.php', 'productbatch.class.php', 'reception.class.php', - 'societe.class.php' , - 'account.class.php' + 'societe.class.php' ))) { // Must not found $db-> $ok=true; @@ -337,7 +334,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase // Check string sql|set...'".$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request. preg_match_all('/(sql|SET|WHERE|INSERT|VALUES).+\s*\'"\s*\.\s*\$(.........)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { - if (! in_array($val[2], array('this->db-', 'this->esc', 'db->escap', 'mydb->esc', 'dbsession', 'db->idate', 'escapedli', 'excludeGr', 'includeGr'))) { + if (! in_array($val[2], array('this->db-', 'this->esc', 'db->escap', 'dbs->esca', 'mydb->esc', 'dbsession', 'db->idate', 'escapedli', 'excludeGr', 'includeGr'))) { $ok=false; break; }