From baab1e1ab72284b1701e1e4170a4dc3ca0552842 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Mon, 29 Aug 2016 13:39:47 +0200
Subject: [PATCH] Various fixes for sonarqube warnings

---
 include/class_IconTheme.inc                   | 20 +++++-
 include/class_SnapshotHandler.inc             | 23 +++----
 include/class_config.inc                      | 62 +++++--------------
 include/class_exceptions.inc                  | 16 ++---
 .../attributes/class_FileAttribute.inc        |  7 ---
 include/simpleplugin/class_Attribute.inc      |  6 +-
 .../simpleplugin/class_simpleManagement.inc   | 13 ++--
 include/simpleplugin/class_simplePlugin.inc   |  3 +-
 include/simpleplugin/class_simpleTabs.inc     |  9 ++-
 include/simpleplugin/simple-list.xml          | 10 +--
 10 files changed, 75 insertions(+), 94 deletions(-)

diff --git a/include/class_IconTheme.inc b/include/class_IconTheme.inc
index e7e8caf80..895d8e830 100644
--- a/include/class_IconTheme.inc
+++ b/include/class_IconTheme.inc
@@ -18,14 +18,30 @@
   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
 */
 
-class ThemeFileParsingException extends Exception {};
+class ThemeFileParsingException extends Exception {}
 
 class IconThemeDir
 {
+  /* Nominal (unscaled) size of the icons in this directory.
+   * Required. */
   private $Size;
+
+  /* Specifies the minimum (unscaled) size that the icons in this directory can be scaled to.
+   * Defaults to the value of Size if not present. */
   private $MinSize;
+
+  /* Specifies the maximum (unscaled) size that the icons in this directory can be scaled to.
+   * Defaults to the value of Size if not present. */
   private $MaxSize;
+
+  /* The type of icon sizes for the icons in this directory.
+   * Valid types are Fixed, Scalable and Threshold.
+   * The type decides what other keys in the section are used.
+   * If not specified, the default is Threshold. */
   private $Type      = 'Threshold';
+
+  /* The icons in this directory can be used if the size differ at most this much from the desired (unscaled) size.
+   * Defaults to 2 if not present. */
   private $Threshold = 2;
 
   function __construct($infos)
@@ -50,6 +66,7 @@ class IconThemeDir
     switch ($this->Type) {
       case 'Fixed':
         return ($this->Size == $size);
+      default:
       case 'Threshold':
       case 'Scalable':
         return (($this->MinSize <= $size) && ($size <= $this->MaxSize));
@@ -61,6 +78,7 @@ class IconThemeDir
     switch ($this->Type) {
       case 'Fixed':
         return abs($this->Size - $size);
+      default:
       case 'Threshold':
       case 'Scalable':
         if ($size < $this->MinSize) {
diff --git a/include/class_SnapshotHandler.inc b/include/class_SnapshotHandler.inc
index f119b4332..815ad12cd 100644
--- a/include/class_SnapshotHandler.inc
+++ b/include/class_SnapshotHandler.inc
@@ -244,32 +244,29 @@ class SnapshotHandler {
     foreach ($dns as $tmp_dn) {
       $data .= $ldap->generateLdif(LDAP::fix($tmp_dn), '(!(objectClass=gosaDepartment))', 'sub');
       if (!$ldap->success()) {
-        msg_dialog::display(_("LDAP error"), msgPool::ldaperror($ldap->get_error(), $tmp_dn, "", get_class()), LDAP_ERROR);
+        msg_dialog::display(_('LDAP error'), msgPool::ldaperror($ldap->get_error(), $tmp_dn, '', get_class()), LDAP_ERROR);
       }
     }
 
-    $newName  = str_replace(".", "", $sec."-".$usec);
     $target   = array();
 
-    $target['objectClass']            = array("top", "gosaSnapshotObject");
+    $target['objectClass']            = array('top', 'gosaSnapshotObject');
     $target['gosaSnapshotData']       = gzcompress($data, 6);
     $target['gosaSnapshotDN']         = $dn;
     $target['description']            = $description;
-    $target['gosaSnapshotTimestamp']  = $newName;
 
     /* Insert the new snapshot
        But we have to check first, if the given gosaSnapshotTimestamp
        is already used, in this case we should increment this value till there is
        an unused value. */
-    $new_dn = "gosaSnapshotTimestamp=".$newName.",".$new_base;
-    $ldap->cat($new_dn);
-    while ($ldap->count()) {
+    do {
+      $target['gosaSnapshotTimestamp']  = str_replace('.', '', $sec.'-'.$usec);
+      $new_dn                           = 'gosaSnapshotTimestamp='.$target['gosaSnapshotTimestamp'].','.$new_base;
       $ldap->cat($new_dn);
-      $newName = str_replace(".", "", $sec."-".(++$usec));
-      $new_dn                           = "gosaSnapshotTimestamp=".$newName.",".$new_base;
-      $target['gosaSnapshotTimestamp']  = $newName;
-    }
-    /* Inset this new snapshot */
+      $usec++;
+    } while ($ldap->count());
+
+    /* Insert this new snapshot */
     $ldap->cd($this->snapshotRDN);
     $ldap->create_missing_trees($this->snapshotRDN);
     $ldap->create_missing_trees($new_base);
@@ -277,7 +274,7 @@ class SnapshotHandler {
     $ldap->add($target);
 
     if (!$ldap->success()) {
-      msg_dialog::display(_("LDAP error"), msgPool::ldaperror($ldap->get_error(), $new_base, "", get_class()), LDAP_ERROR);
+      msg_dialog::display(_('LDAP error'), msgPool::ldaperror($ldap->get_error(), $new_base, '', get_class()), LDAP_ERROR);
     }
   }
 
diff --git a/include/class_config.inc b/include/class_config.inc
index 044f87775..5c285f8ef 100644
--- a/include/class_config.inc
+++ b/include/class_config.inc
@@ -199,26 +199,22 @@ class config  {
 
     /* Look through attributes */
     switch ($this->tags[$this->level - 1]) {
-
       /* Handle location */
       case 'LOCATION':
         if ($this->tags[$this->level - 2] == 'MAIN') {
-          $name           = $attrs['NAME'];
-          $name           = preg_replace("/[<>\"']/", "", $name);
-          $attrs['NAME']  = $name;
+          $attrs['NAME'] = preg_replace('/[<>"\']/', '', $attrs['NAME']);
 
-          $this->currentLocation = $name;
+          $this->currentLocation = $attrs['NAME'];
 
           /* Add location elements */
-          $this->data['LOCATIONS'][$name] = $attrs;
+          $this->data['LOCATIONS'][$attrs['NAME']] = $attrs;
         }
         break;
 
       /* Handle referral tags */
       case 'REFERRAL':
         if ($this->tags[$this->level - 2] == 'LOCATION') {
-          $url    = $attrs['URI'];
-          $server = preg_replace('!^([^:]+://[^/]+)/.*$!', '\\1', $url);
+          $server = preg_replace('!^([^:]+://[^/]+)/.*$!', '\\1', $attrs['URI']);
 
           /* Add location elements */
           if (!isset($this->data['LOCATIONS'][$this->currentLocation]['REFERRAL'])) {
@@ -233,6 +229,10 @@ class config  {
       case 'MAIN':
         $this->data['MAIN'] = array_merge ($this->data['MAIN'], $attrs);
         break;
+
+      /* Ignore other tags */
+      default:
+        break;
     }
   }
 
@@ -424,7 +424,8 @@ class config  {
     }
 
     $debugLevel = $this->get_cfg_value('DEBUGLEVEL');
-    if ($debugLevel & DEBUG_CONFIG) { // value from LDAP can't activate DEBUG_CONFIG
+    if ($debugLevel & DEBUG_CONFIG) {
+      /* Value from LDAP can't activate DEBUG_CONFIG */
       $debugLevel -= DEBUG_CONFIG;
     }
     if (isset($this->data['MAIN']['DEBUGLEVEL'])) {
@@ -608,7 +609,8 @@ class config  {
     foreach ($types as $type) {
       $i = objects::infos($type);
       $filter         .= $i['filter'];
-      $ldap_values[]  = $i['mainAttr']; // Specific key for departement objectTypes
+      /* Add type main attr to fetched attributes list */
+      $ldap_values[]  = $i['mainAttr'];
     }
     $filter = '(|'.$filter.')';
 
@@ -750,42 +752,6 @@ class config  {
     return $ret;
   }
 
-  /*!
-   * \brief Check if there's the specified bool value set in the configuration
-   *
-   *  The function checks, weither the specified bool value is set to a true
-   *  value in the configuration file. Considered true are either true or yes,
-   *  case-insensitive.
-   *
-   *  Example usage:
-   *  \code
-   *  if ($config->boolValueIsTrue("main", "copyPaste")) {
-   *    echo "Copy Paste Handling is enabled";
-   *  }
-   *  \endcode
-   *
-   *  \param string $section Section in the configuration file.
-   *
-   *  \param string $value Key in the given section, which is subject to check
-   *
-   *  \return bool TRUE if option set in the config file
-   */
-  function boolValueIsTrue($section, $value)
-  {
-    $section  = strtoupper($section);
-    $value    = strtoupper($value);
-    if (isset($this->data[$section][$value])) {
-
-      $data = $this->data[$section][$value];
-      if (preg_match("/^true$/i", $data) || preg_match("/yes/i", $data)) {
-        return TRUE;
-      }
-
-    }
-
-    return FALSE;
-  }
-
   /*!
    * \brief Search for hooks
    *
@@ -920,7 +886,8 @@ class config  {
       if (isset($plInfo['plObjectType'])) {
         $entry = array('CLASS' => $class,'NAME' => $plInfo['plShortName']);
         foreach ($plInfo['plObjectType'] as $key => $value) {
-          if (is_numeric($key)) { // This is not the main tab
+          if (is_numeric($key)) {
+            /* This is not the main tab */
             $tabclass = strtoupper($value)."TABS";
             if (($tabclass == 'GROUPTABS') && class_available('mixedGroup')) {
               $tabclass = 'OGROUP-USERTABS';
@@ -931,6 +898,7 @@ class config  {
             }
             $this->data['TABS'][$tabclass][] = $entry;
           } else {
+            /* This is the main tab */
             if (isset($this->data['OBJECTS'][strtoupper($key)])) {
               die("duplicated object type ".strtoupper($key)." in ".$this->data['OBJECTS'][strtoupper($key)]['mainTab']." and $class");
             }
diff --git a/include/class_exceptions.inc b/include/class_exceptions.inc
index 3e11f8310..635ecc005 100644
--- a/include/class_exceptions.inc
+++ b/include/class_exceptions.inc
@@ -36,11 +36,11 @@ class LDIFImportException extends FusionDirectoryException {}
 /*! \class LdapGeneralizedTimeBadFormatException
     \brief Exception class which can be thrown by LdapGeneralizedTime if the format does not match
 */
-class LdapGeneralizedTimeBadFormatException extends FusionDirectoryException {};
-
-class NonExistingObjectTypeException extends FusionDirectoryException {};
-class NonExistingBranchException extends FusionDirectoryException {};
-class NonExistingLdapNodeException extends FusionDirectoryException {};
-class EmptyFilterException extends FusionDirectoryException {};
-class NoManagementClassException extends FusionDirectoryException {};
-class LDAPFailureException extends FusionDirectoryException {};
+class LdapGeneralizedTimeBadFormatException extends FusionDirectoryException {}
+
+class NonExistingObjectTypeException extends FusionDirectoryException {}
+class NonExistingBranchException extends FusionDirectoryException {}
+class NonExistingLdapNodeException extends FusionDirectoryException {}
+class EmptyFilterException extends FusionDirectoryException {}
+class NoManagementClassException extends FusionDirectoryException {}
+class LDAPFailureException extends FusionDirectoryException {}
diff --git a/include/simpleplugin/attributes/class_FileAttribute.inc b/include/simpleplugin/attributes/class_FileAttribute.inc
index 6e199910c..27a345826 100644
--- a/include/simpleplugin/attributes/class_FileAttribute.inc
+++ b/include/simpleplugin/attributes/class_FileAttribute.inc
@@ -151,13 +151,6 @@ class FileDownloadAttribute extends FileAttribute
 
 class FileTextAreaAttribute extends FileDownloadAttribute
 {
-  function __construct ($label, $description, $ldapName, $required = FALSE, $extension = '.txt', $upload = TRUE, $defaultValue = "", $acl = "")
-  {
-    parent::__construct(
-      $label, $description, $ldapName, $required,
-      $extension, $upload, $defaultValue, $acl
-    );
-  }
   /*! \brief Update this attributes postValue depending of the $_POST values
    */
   function loadPostValue ()
diff --git a/include/simpleplugin/class_Attribute.inc b/include/simpleplugin/class_Attribute.inc
index f04736e67..f6e88bdcf 100644
--- a/include/simpleplugin/class_Attribute.inc
+++ b/include/simpleplugin/class_Attribute.inc
@@ -559,7 +559,8 @@ class Attribute
         $type[] = $class;
         $class  = get_parent_class($class);
       }
-      $type[] = 'Attribute'; // To avoid empty array
+      /* Avoid empty array */
+      $type[] = 'Attribute';
       $infos = array(
         'htmlid'      => $this->getHtmlId(),
         'label'       => $this->getLabel(),
@@ -603,12 +604,13 @@ class Attribute
    */
   function getAclInfo ()
   {
-    if (empty($this->acl)) { // If acl is not empty, we use an acl that is not ours, we have no acl to create
+    if (empty($this->acl)) {
       return array(
         'name' => $this->getHtmlId(),
         'desc' => $this->getDescription()
       );
     } else {
+      /* If acl is not empty, we use an acl that is not ours, we have no acl to create */
       return FALSE;
     }
   }
diff --git a/include/simpleplugin/class_simpleManagement.inc b/include/simpleplugin/class_simpleManagement.inc
index c0abb3593..bb48976a6 100644
--- a/include/simpleplugin/class_simpleManagement.inc
+++ b/include/simpleplugin/class_simpleManagement.inc
@@ -66,7 +66,8 @@ class templateDialog
         trigger_error('redefining template object');
       }
       $this->template = new template($this->type, $_POST['template']);
-      unset($_POST['template']); // This method can loop if there are several targets
+      /* This method can loop if there are several targets */
+      unset($_POST['template']);
     }
     if (is_object($this->template)) {
       if ($this->target !== NULL) {
@@ -902,14 +903,12 @@ class simpleManagement extends management
 
   static function mainInc ($classname)
   {
-    global $remove_lock, $cleanup, $display, $config, $ui;
+    global $remove_lock, $cleanup, $display;
 
     /* Remove locks */
-    if ($remove_lock) {
-      if (session::is_set($classname)) {
-          $macl = session::get($classname);
-          $macl->remove_lock();
-      }
+    if ($remove_lock && session::is_set($classname)) {
+      $macl = session::get($classname);
+      $macl->remove_lock();
     }
 
     if ($cleanup) {
diff --git a/include/simpleplugin/class_simplePlugin.inc b/include/simpleplugin/class_simplePlugin.inc
index f25766237..5ed40c974 100644
--- a/include/simpleplugin/class_simplePlugin.inc
+++ b/include/simpleplugin/class_simplePlugin.inc
@@ -228,7 +228,8 @@ class simplePlugin extends plugin
     foreach ($this->attributesInfo as &$sectionInfo) {
       foreach ($sectionInfo['attrs'] as $name => &$attr) {
         if (in_array($name, $this->preInitAttributes)) {
-          continue; // skip the preInit ones
+          /* skip the preInit ones */
+          continue;
         }
         $attr->setParent($this);
         $attr->loadValue($this->attrs);
diff --git a/include/simpleplugin/class_simpleTabs.inc b/include/simpleplugin/class_simpleTabs.inc
index 571f21f46..87bf5d3b2 100644
--- a/include/simpleplugin/class_simpleTabs.inc
+++ b/include/simpleplugin/class_simpleTabs.inc
@@ -43,9 +43,11 @@ class simpleTabs
   var $by_object  = array();
   var $acl_category;
 
-  var $parent = NULL; // A parent object if available, e.g. a management class.
+  /* A parent object if available, e.g. a management class. */
+  var $parent = NULL;
 
-  var $read_only = FALSE; // Used when the entry is opened as "readonly" due to locks.
+  /* Used when the entry is opened as "readonly" due to locks. */
+  var $read_only = FALSE;
 
   var $baseclass = "";
 
@@ -155,7 +157,8 @@ class simpleTabs
       } else {
         $baseobject->base = dn2base(get_userinfo()->dn);
       }
-      if (!($baseobject instanceOf simplePlugin) && is_object($baseobject->baseSelector)) { // For some plugins not yet migrated to simple plugin.
+      if (!($baseobject instanceOf simplePlugin) && is_object($baseobject->baseSelector)) {
+        /* For some plugins not yet migrated to simple plugin. */
         $baseobject->baseSelector->setBase($baseobject->base);
       }
       @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $baseobject->base, 'Fixed base');
diff --git a/include/simpleplugin/simple-list.xml b/include/simpleplugin/simple-list.xml
index 54a985efc..81bbbece5 100644
--- a/include/simpleplugin/simple-list.xml
+++ b/include/simpleplugin/simple-list.xml
@@ -54,10 +54,10 @@
   <actionmenu>
 
     <action>
-     <type>sub</type>
-     <image>geticon.php?context=actions&amp;icon=document-new&amp;size=16</image>
-     <label>Create</label>
-     <acl>[c]</acl>
+      <type>sub</type>
+      <image>geticon.php?context=actions&amp;icon=document-new&amp;size=16</image>
+      <label>Create</label>
+      <acl>[c]</acl>
     </action>
 
     <action>
@@ -68,7 +68,7 @@
       <acl>[r]</acl>
     </action>
 
-     <action>
+    <action>
       <name>remove</name>
       <type>entry</type>
       <image>geticon.php?context=actions&amp;icon=edit-delete&amp;size=16</image>
-- 
GitLab