From 5280d070e38eb93eccba081b5a6b7cc44d91d7f0 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Fri, 6 Jan 2023 19:24:57 +0100 Subject: [PATCH] Fix technical debt --- ChangeLog | 2 +- htdocs/commande/card.php | 2 +- htdocs/commande/class/commande.class.php | 18 ++++--- htdocs/compta/facture/class/facture.class.php | 10 +++- .../core/class/commondocgenerator.class.php | 47 +++++++++---------- htdocs/core/class/commonobject.class.php | 13 +++-- htdocs/core/class/dolgraph.class.php | 11 +++-- htdocs/core/class/html.formsetup.class.php | 17 +++++-- htdocs/core/class/lessc.class.php | 7 +-- htdocs/core/login/functions_ldap.php | 2 +- .../class/expensereport.class.php | 10 +++- htdocs/fichinter/class/fichinter.class.php | 27 ++++++----- .../template/test/phpunit/MyObjectTest.php | 2 - htdocs/societe/class/societe.class.php | 13 +++-- 14 files changed, 110 insertions(+), 71 deletions(-) diff --git a/ChangeLog b/ChangeLog index ef974dfde12..9ec9fb4525a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,7 +9,7 @@ WARNING: Following changes may create regressions for some external modules, but were necessary to make Dolibarr better: * The deprecated method escapeunderscore() of database handlers has been removed. You must use escapeforlike instead. - +* The method nb_expedition() has been renamed into countNbOfShipments() diff --git a/htdocs/commande/card.php b/htdocs/commande/card.php index 3e24285a1a7..ccf706c9ac7 100644 --- a/htdocs/commande/card.php +++ b/htdocs/commande/card.php @@ -2871,7 +2871,7 @@ if ($action == 'create' && $usercancreate) { // Ship $numshipping = 0; if (isModEnabled('expedition')) { - $numshipping = $object->nb_expedition(); + $numshipping = $object->countNbOfShipments(); if ($object->statut > Commande::STATUS_DRAFT && $object->statut < Commande::STATUS_CLOSED && ($object->getNbOfProductsLines() > 0 || !empty($conf->global->STOCK_SUPPORTS_SERVICES))) { if ((isModEnabled('expedition_bon') && $user->rights->expedition->creer) || ($conf->delivery_note->enabled && $user->rights->expedition->delivery->creer)) { diff --git a/htdocs/commande/class/commande.class.php b/htdocs/commande/class/commande.class.php index 25c234cc66d..29f990476ed 100644 --- a/htdocs/commande/class/commande.class.php +++ b/htdocs/commande/class/commande.class.php @@ -1173,6 +1173,8 @@ class Commande extends CommonOrder return -1; } } + + return 0; } else { dol_print_error($this->db); $this->db->rollback(); @@ -2307,17 +2309,13 @@ class Commande extends CommonOrder } } - // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps /** - * Returns a array with expeditions lines number + * Returns an array with expeditions lines number * * @return int Nb of shipments - * - * TODO deprecate, move to Shipping class */ - public function nb_expedition() + public function countNbOfShipments() { - // phpcs:enable $sql = 'SELECT count(*)'; $sql .= ' FROM '.MAIN_DB_PREFIX.'expedition as e'; $sql .= ', '.MAIN_DB_PREFIX.'element_element as el'; @@ -2333,6 +2331,8 @@ class Commande extends CommonOrder } else { dol_print_error($this->db); } + + return 0; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps @@ -2501,6 +2501,8 @@ class Commande extends CommonOrder return -1 * $error; } } + + return 0; } @@ -2565,6 +2567,8 @@ class Commande extends CommonOrder return -1 * $error; } } + + return 0; } @@ -3433,7 +3437,7 @@ class Commande extends CommonOrder } // Test we can delete - if ($this->nb_expedition() != 0) { + if ($this->countNbOfShipments() != 0) { $this->errors[] = $langs->trans('SomeShipmentExists'); $error++; } diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php index 9af2a06f7d4..69b2ea1c293 100644 --- a/htdocs/compta/facture/class/facture.class.php +++ b/htdocs/compta/facture/class/facture.class.php @@ -4224,7 +4224,7 @@ class Facture extends CommonInvoice * @param User $user User that set discount * @param double $remise Discount * @param int $notrigger 1=Does not execute triggers, 0= execute triggers - * @return int <0 if ko, >0 if ok + * @return int <0 if KO, >0 if OK */ public function set_remise($user, $remise, $notrigger = 0) { @@ -4239,7 +4239,7 @@ class Facture extends CommonInvoice * @param User $user User that set discount * @param double $remise Discount * @param int $notrigger 1=Does not execute triggers, 0= execute triggers - * @return int <0 if ko, >0 if ok + * @return int <0 if KO, >0 if OK */ public function setDiscount($user, $remise, $notrigger = 0) { @@ -4291,6 +4291,8 @@ class Facture extends CommonInvoice return -1 * $error; } } + + return 0; } @@ -4356,6 +4358,8 @@ class Facture extends CommonInvoice return -1 * $error; } } + + return 0; } /** @@ -5742,6 +5746,8 @@ class Facture extends CommonInvoice } else { dol_print_error($this->db); } + + return array(); } } diff --git a/htdocs/core/class/commondocgenerator.class.php b/htdocs/core/class/commondocgenerator.class.php index 69ec9d029ef..e142e5c8d61 100644 --- a/htdocs/core/class/commondocgenerator.class.php +++ b/htdocs/core/class/commondocgenerator.class.php @@ -1016,12 +1016,10 @@ abstract class CommonDocGenerator * @param int $hidedetails Do not show line details * @param int $hidedesc Do not show desc * @param int $hideref Do not show ref - * @return null + * @return void */ public function prepareArrayColumnField($object, $outputlangs, $hidedetails = 0, $hidedesc = 0, $hideref = 0) { - global $conf; - $this->defineColumnField($object, $outputlangs, $hidedetails, $hidedesc, $hideref); @@ -1175,7 +1173,7 @@ abstract class CommonDocGenerator * @param float $curY curent Y position * @param string $colKey the column key * @param string $columnText column text - * @return null + * @return void */ public function printStdColumnContent($pdf, &$curY, $colKey, $columnText = '') { @@ -1206,22 +1204,24 @@ abstract class CommonDocGenerator // restore cell padding $pdf->setCellPaddings($curentCellPaddinds['L'], $curentCellPaddinds['T'], $curentCellPaddinds['R'], $curentCellPaddinds['B']); } + + return; } /** * print description column content * - * @param TCPDF $pdf pdf object - * @param float $curY curent Y position - * @param string $colKey the column key - * @param object $object CommonObject - * @param int $i the $object->lines array key - * @param Translate $outputlangs Output language - * @param int $hideref hide ref - * @param int $hidedesc hide desc - * @param int $issupplierline if object need supplier product - * @return null + * @param TCPDF $pdf pdf object + * @param float $curY curent Y position + * @param string $colKey the column key + * @param object $object CommonObject + * @param int $i the $object->lines array key + * @param Translate $outputlangs Output language + * @param int $hideref hide ref + * @param int $hidedesc hide desc + * @param int $issupplierline if object need supplier product + * @return void */ public function printColDescContent($pdf, &$curY, $colKey, $object, $i, $outputlangs, $hideref = 0, $hidedesc = 0, $issupplierline = 0) { @@ -1265,7 +1265,7 @@ abstract class CommonDocGenerator global $hookmanager; if (empty($object->table_element)) { - return; + return ''; } $extrafieldsKeyPrefix = "options_"; @@ -1317,10 +1317,10 @@ abstract class CommonDocGenerator /** * display extrafields columns content * - * @param object $object line of common object - * @param Translate $outputlangs Output language - * @param array $params array of additionals parameters - * @return double max y value + * @param object $object line of common object + * @param Translate $outputlangs Output language + * @param array $params array of additionals parameters + * @return string Html string */ public function getExtrafieldsInHtml($object, $outputlangs, $params = array()) { @@ -1330,7 +1330,7 @@ abstract class CommonDocGenerator return; } - // Load extrafiels if not allready does + // Load extrafields if not allready done if (empty($this->extrafieldsCache)) { $this->extrafieldsCache = new ExtraFields($this->db); } @@ -1366,7 +1366,6 @@ abstract class CommonDocGenerator $params = $params + $defaultParams; - /** * @var $extrafields ExtraFields */ @@ -1593,12 +1592,10 @@ abstract class CommonDocGenerator * @param object $object common object det * @param Translate $outputlangs langs * @param int $hidedetails Do not show line details - * @return null + * @return void */ public function defineColumnExtrafield($object, $outputlangs, $hidedetails = 0) { - global $conf; - if (!empty($hidedetails)) { return; } @@ -1674,5 +1671,7 @@ abstract class CommonDocGenerator $this->insertNewColumnDef("options_".$key, $def); } } + + return; } } diff --git a/htdocs/core/class/commonobject.class.php b/htdocs/core/class/commonobject.class.php index 48e5124f213..a23c0611589 100644 --- a/htdocs/core/class/commonobject.class.php +++ b/htdocs/core/class/commonobject.class.php @@ -4539,6 +4539,8 @@ abstract class CommonObject $row = $this->db->fetch_row($resql); return $row[0]; } + + return 0; } /** @@ -5772,6 +5774,9 @@ abstract class CommonObject } // TODO Ad here a scan into table llx_overwrite_default with a filter on $this->element and $fieldname + // store content into $conf->cache['overwrite_default'] + + return ''; } @@ -7828,7 +7833,7 @@ abstract class CommonObject * clear validation message result for a field * * @param string $fieldKey Key of attribute to clear - * @return null + * @return void */ public function clearFieldError($fieldKey) { @@ -7841,12 +7846,14 @@ abstract class CommonObject * * @param string $fieldKey Key of attribute * @param string $msg the field error message - * @return null + * @return void */ public function setFieldError($fieldKey, $msg = '') { global $langs; - if (empty($msg)) { $msg = $langs->trans("UnknowError"); } + if (empty($msg)) { + $msg = $langs->trans("UnknowError"); + } $this->error = $this->validateFieldsErrors[$fieldKey] = $msg; } diff --git a/htdocs/core/class/dolgraph.class.php b/htdocs/core/class/dolgraph.class.php index 196364a84f8..1d60d03a652 100644 --- a/htdocs/core/class/dolgraph.class.php +++ b/htdocs/core/class/dolgraph.class.php @@ -221,8 +221,8 @@ class DolGraph /** * Set y label * - * @param string $label Y label - * @return boolean|null True + * @param string $label Y label + * @return void */ public function SetYLabel($label) { @@ -235,7 +235,7 @@ class DolGraph * Set width * * @param int|string $w Width (Example: 320 or '100%') - * @return boolean|null True + * @return void */ public function SetWidth($w) { @@ -717,7 +717,7 @@ class DolGraph * * @param string $file Image file name to use to save onto disk (also used as javascript unique id) * @param string $fileurl Url path to show image if saved onto disk - * @return integer|null + * @return mixed|boolean */ public function draw($file, $fileurl = '') { @@ -736,7 +736,8 @@ class DolGraph dol_syslog(get_class($this) . "::draw " . $this->error, LOG_WARNING); } $call = "draw_" . $this->_library; - call_user_func_array(array($this, $call), array($file, $fileurl)); + + return call_user_func_array(array($this, $call), array($file, $fileurl)); } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps diff --git a/htdocs/core/class/html.formsetup.class.php b/htdocs/core/class/html.formsetup.class.php index eb389454f82..60499845de0 100644 --- a/htdocs/core/class/html.formsetup.class.php +++ b/htdocs/core/class/html.formsetup.class.php @@ -339,8 +339,8 @@ class FormSetup /** * Method used to test module builder convertion to this form usage * - * @param array $params an array of arrays of params from old modulBuilder params - * @return null + * @param array $params an array of arrays of params from old modulBuilder params + * @return void */ public function addItemsFromParamsArray($params) { @@ -683,7 +683,8 @@ class FormSetupItem /** * Save const value based on htdocs/core/actions_setmoduleoptions.inc.php - * @return int -1 if KO, 1 if OK + * + * @return int -1 if KO, 1 if OK */ public function saveConfValue() { @@ -714,10 +715,13 @@ class FormSetupItem return 1; } } + + return 0; } /** * Set an override function for saving data + * * @param callable $callBack a callable function * @return void */ @@ -1005,6 +1009,7 @@ class FormSetupItem * set the type from string : used for old module builder setup conf style conversion and tests * because this two class will quickly evolve it's important to not set directly $this->type (will be protected) so this method exist * to be sure we can manage evolution easily + * * @param string $type possible values based on old module builder setup : 'string', 'textarea', 'category:'.Categorie::TYPE_CUSTOMER', 'emailtemplate', 'thirdparty_type' * @deprecated yes this setTypeFromTypeString came deprecated because it exists only for manage setup convertion * @return bool @@ -1012,11 +1017,13 @@ class FormSetupItem public function setTypeFromTypeString($type) { $this->type = $type; + return true; } /** * Add error + * * @param array|string $errors the error text * @return null */ @@ -1034,7 +1041,9 @@ class FormSetupItem } /** - * @return bool|string Generate the output html for this item + * generateOutputField + * + * @return bool|string Generate the output html for this item */ public function generateOutputField() { diff --git a/htdocs/core/class/lessc.class.php b/htdocs/core/class/lessc.class.php index cb6caf838e2..1b764e20a35 100644 --- a/htdocs/core/class/lessc.class.php +++ b/htdocs/core/class/lessc.class.php @@ -1011,7 +1011,7 @@ class Lessc if ($list[0] == "list" && isset($list[2][$idx - 1])) { return $list[2][$idx - 1]; } - return; + return null; } protected function lib_isnumber($value) @@ -1306,7 +1306,7 @@ class Lessc if (!is_null($color = $this->coerceColor($value))) { return isset($color[4]) ? $color[4] : 1; } - return; + return null; } // set the alpha of the color @@ -1958,7 +1958,8 @@ class Lessc if ($op == '+' || $op == '*') { return $this->op_color_number($op, $rgt, $lft); } - return; + + return null; } protected function op_color_number($op, $lft, $rgt) diff --git a/htdocs/core/login/functions_ldap.php b/htdocs/core/login/functions_ldap.php index faf0024d801..0db325736a6 100644 --- a/htdocs/core/login/functions_ldap.php +++ b/htdocs/core/login/functions_ldap.php @@ -61,7 +61,7 @@ function check_user_password_ldap($usertotest, $passwordtotest, $entitytotest) $langs->loadLangs(array('main', 'other')); $_SESSION["dol_loginmesg"] = $langs->transnoentitiesnoconv("ErrorLDAPFunctionsAreDisabledOnThisPHP").' '.$langs->transnoentitiesnoconv("TryAnotherConnectionMode"); - return; + return ''; } if ($usertotest) { diff --git a/htdocs/expensereport/class/expensereport.class.php b/htdocs/expensereport/class/expensereport.class.php index 1305be68bc2..63cbeaad67b 100644 --- a/htdocs/expensereport/class/expensereport.class.php +++ b/htdocs/expensereport/class/expensereport.class.php @@ -985,6 +985,8 @@ class ExpenseReport extends CommonObject return -1; } } + + return 0; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps @@ -1328,8 +1330,6 @@ class ExpenseReport extends CommonObject public function set_save_from_refuse($fuser) { // phpcs:enable - global $conf, $langs; - // Sélection de la date de début de la NDF $sql = 'SELECT date_debut'; $sql .= ' FROM '.MAIN_DB_PREFIX.$this->table_element; @@ -1357,6 +1357,8 @@ class ExpenseReport extends CommonObject } else { dol_syslog(get_class($this)."::set_save_from_refuse expensereport already with save status", LOG_WARNING); } + + return 0; } /** @@ -1465,6 +1467,8 @@ class ExpenseReport extends CommonObject } else { dol_syslog(get_class($this)."::setDeny expensereport already with refuse status", LOG_WARNING); } + + return 0; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps @@ -1531,6 +1535,8 @@ class ExpenseReport extends CommonObject } else { dol_syslog(get_class($this)."::set_unpaid expensereport already with unpaid status", LOG_WARNING); } + + return 0; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps diff --git a/htdocs/fichinter/class/fichinter.class.php b/htdocs/fichinter/class/fichinter.class.php index 1f191684d11..b072f4c756d 100644 --- a/htdocs/fichinter/class/fichinter.class.php +++ b/htdocs/fichinter/class/fichinter.class.php @@ -501,6 +501,8 @@ class Fichinter extends CommonObject $this->db->free($resql); return 1; } + + return 0; } else { $this->error = $this->db->lasterror(); return -1; @@ -515,8 +517,6 @@ class Fichinter extends CommonObject */ public function setDraft($user) { - global $langs, $conf; - $error = 0; // Protection @@ -664,6 +664,8 @@ class Fichinter extends CommonObject return -1; } } + + return 0; } /** @@ -904,8 +906,6 @@ class Fichinter extends CommonObject */ public function info($id) { - global $conf; - $sql = "SELECT f.rowid,"; $sql .= " f.datec,"; $sql .= " f.tms as date_modification,"; @@ -1074,13 +1074,11 @@ class Fichinter extends CommonObject * * @param User $user Object user who define * @param integer $date_delivery date of delivery - * @return int <0 if ko, >0 if ok + * @return int <0 if KO, >0 if OK */ public function set_date_delivery($user, $date_delivery) { // phpcs:enable - global $conf; - if ($user->rights->ficheinter->creer) { $sql = "UPDATE ".MAIN_DB_PREFIX."fichinter "; $sql .= " SET datei = '".$this->db->idate($date_delivery)."'"; @@ -1096,6 +1094,8 @@ class Fichinter extends CommonObject return -1; } } + + return 0; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps @@ -1109,8 +1109,6 @@ class Fichinter extends CommonObject public function set_description($user, $description) { // phpcs:enable - global $conf; - if ($user->rights->ficheinter->creer) { $sql = "UPDATE ".MAIN_DB_PREFIX."fichinter "; $sql .= " SET description = '".$this->db->escape($description)."',"; @@ -1126,6 +1124,8 @@ class Fichinter extends CommonObject return -1; } } + + return 0; } @@ -1135,13 +1135,11 @@ class Fichinter extends CommonObject * * @param User $user Object user who modify * @param int $contractid Description - * @return int <0 if ko, >0 if ok + * @return int <0 if KO, >0 if OK */ public function set_contrat($user, $contractid) { // phpcs:enable - global $conf; - if ($user->rights->ficheinter->creer) { $sql = "UPDATE ".MAIN_DB_PREFIX."fichinter "; $sql .= " SET fk_contrat = ".((int) $contractid); @@ -1155,6 +1153,7 @@ class Fichinter extends CommonObject return -1; } } + return -2; } @@ -1261,7 +1260,7 @@ class Fichinter extends CommonObject { dol_syslog(get_class($this)."::addline $fichinterid, $desc, $date_intervention, $duration"); - if ($this->statut == 0) { + if ($this->statut == self::STATUS_DRAFT) { $this->db->begin(); // Insertion ligne @@ -1288,6 +1287,8 @@ class Fichinter extends CommonObject return -1; } } + + return 0; } diff --git a/htdocs/modulebuilder/template/test/phpunit/MyObjectTest.php b/htdocs/modulebuilder/template/test/phpunit/MyObjectTest.php index 31084cf7acc..b31a3ca08d5 100644 --- a/htdocs/modulebuilder/template/test/phpunit/MyObjectTest.php +++ b/htdocs/modulebuilder/template/test/phpunit/MyObjectTest.php @@ -56,8 +56,6 @@ class MyObjectTest extends PHPUnit\Framework\TestCase /** * Constructor * We save global variables into local variables - * - * @return MyObjectTest */ public function __construct() { diff --git a/htdocs/societe/class/societe.class.php b/htdocs/societe/class/societe.class.php index 3bf780ba106..dbc9245fd71 100644 --- a/htdocs/societe/class/societe.class.php +++ b/htdocs/societe/class/societe.class.php @@ -1271,7 +1271,7 @@ class Societe extends CommonObject // Clean parameters $this->id = $id; $this->entity = ((isset($this->entity) && is_numeric($this->entity)) ? $this->entity : $conf->entity); - $this->name = $this->name ?trim($this->name) : trim($this->nom); + $this->name = $this->name ? trim($this->name) : trim($this->nom); $this->nom = $this->name; // For backward compatibility $this->name_alias = trim($this->name_alias); $this->ref_ext = trim($this->ref_ext); @@ -2217,6 +2217,7 @@ class Societe extends CommonObject } $this->db->commit(); + return 1; } } @@ -2278,6 +2279,8 @@ class Societe extends CommonObject $this->db->commit(); return 1; } + + return -1; } // phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps @@ -3091,6 +3094,8 @@ class Societe extends CommonObject } else { dol_print_error($this->db); } + + return ''; } @@ -3473,9 +3478,9 @@ class Societe extends CommonObject } else { return -1; } - } else { - return -1; } + + return -1; } /** @@ -4819,6 +4824,8 @@ class Societe extends CommonObject } elseif ($status == 3) { return $langs->trans("ProspectCustomer"); } + + return ''; }