[Core: Config] Add type declarations to function signatures (#4127)

Author: johnsaigle

php/libraries/NDB_Config.class.inc

@@ -1,8 +1,8 @@
-<?php
+<?php declare(strict_types=1);
 /**
  * Provides an interface to the NeuroDB configuration
  *
- * PHP Version 5
+ * PHP Version 7
  *
  * @category Main
  * @package  Loris
@@ -49,11 +49,11 @@ class NDB_Config
     /**
      * The singleton design pattern - autoloads config file
      *
-     * @param string $configFile the neurodb config.xml config file
+     * @param string|null $configFile the neurodb config.xml config file
      *
-     * @return NDB_Config object
+     * @return \NDB_Config object
      */
-    static function &singleton($configFile = null)
+    static function &singleton(?string $configFile = null): \NDB_Config
     {
         static $config = null;
         if (is_null($config)) {
@@ -90,7 +90,7 @@ class NDB_Config
      *
      * @throws Exception
      */
-    function load($configFile = "../project/config.xml")
+    function load(string $configFile = "../project/config.xml"): void
     {
         // load the configuration data into a global variable $config
         $newroot = simplexml_load_file($configFile);
@@ -114,7 +114,7 @@ class NDB_Config
      *
      * @throws LorisException when any config file is missing
      */
-    function configFilePath($pathToFile = "config.xml")
+    function configFilePath(string $pathToFile = "config.xml"): string
     {
         // first check for a project/config.xml, then check for the config
         // file in a standard unix filesystem location ->  /etc/loris/config.xml
@@ -152,6 +152,12 @@ class NDB_Config
      * and this gets it into the same format that was previously used
      * for backwards compatibility.
      *
+     * FIXME Despite the name, this function does not just return arrays, but
+     * also returns a string. Crucially, this is done in the process of creating
+     * a DB connection and so we cannot add a return type to this function.
+     * Future refactoring should be done to better encapsulate this logic to
+     * allow for strong typing.
+     *
      * @param SimpleXMLElement $xml The root element of the parsed XML.
      *
      * @return array of XML configuration
@@ -246,13 +252,17 @@ class NDB_Config
     /**
      * Gets a setting from the database config tables
      *
-     * @param string  $name The name of the config setting to get
-     * @param integer $id   (optional) The ID of the config setting we're
+     * FIXME This function currently returns a string, array, or null.
+     * Refactoring should be done to better encapsulate the logic in this
+     * function to allow for strong typing.
+     *
+     * @param string   $name The name of the config setting to get
+     * @param int|null $id   (optional) The ID of the config setting we're
      *                      getting to avoid ambiguity.
      *
-     * @return string The value from the database, or null if nothing found.
+     * @return mixed The value from the database, or null if nothing found.
      */
-    function getSettingFromDB($name, $id=null)
+    function getSettingFromDB(string $name, ?int $id = null)
     {
         // These should never come from the DB
         switch($name) {
@@ -340,7 +350,10 @@ class NDB_Config
             foreach ($configSetting as $childSetting) {
                 $childName        = $childSetting['Name'];
                 $childID          = $childSetting['ChildID'];
-                $childValue       = $this->getSettingFromDB($childName, $childID);
+                $childValue       = $this->getSettingFromDB(
+                    $childName,
+                    intval($childID)
+                );
                 $tree[$childName] = $childValue;
             }
             return $tree;
@@ -354,9 +367,9 @@ class NDB_Config
      *
      * @param string $name The name of the XML node to retrieve.
      *
-     * @return string The value from the config.xml
+     * @return array|string The value from the config.xml
      */
-    function getSettingFromXML($name)
+    function getSettingFromXML(string $name)
     {
         // loop over the settings, and find the node
         foreach ($this->_settings as $key => $value) {
@@ -375,13 +388,13 @@ class NDB_Config
     }
 
     /**
-    * Gets a setting by name
-    *
-    * @param string $name The name of the setting to retrieve
-    *
-    * @return mixed The contents of the node identified by $name
-    */
-    function getSetting($name)
+     * Gets a setting by name
+     *
+     * @param string $name The name of the setting to retrieve
+     *
+     * @return mixed The contents of the node identified by $name
+     */
+    function getSetting(string $name)
     {
         try {
             $XMLValue = $this->getSettingFromXML($name);
@@ -402,22 +415,22 @@ class NDB_Config
     }
 
     /**
-    * Get list of projects for this projects, given a ProjectID.
-    *
-    * @param integer $ProjectID The ProjectID we want settings for
-    *
-    * @return array of settings for this project.
-    */
-    function getProjectSettings($ProjectID)
+     * Get list of projects for this projects, given a ProjectID.
+     *
+     * @param integer $ProjectID The ProjectID we want settings for
+     *
+     * @return array of settings for this project.
+     */
+    function getProjectSettings(int $ProjectID): array
     {
         $factory = NDB_Factory::singleton();
         $DB      = $factory->database();
         try
         {
-             $info =$DB->pselectRow(
-                 "SELECT * FROM Project WHERE ProjectID=:sp",
-                 array('sp' => $ProjectID)
-             );
+            $info =$DB->pselectRow(
+                "SELECT * FROM Project WHERE ProjectID=:sp",
+                array('sp' => $ProjectID)
+            );
         } catch (DatabaseException $e)
         {
             return null;
@@ -441,19 +454,15 @@ class NDB_Config
      *
      * @return array of settings for this subprojectID
      */
-    function getSubprojectSettings($subprojectID)
+    function getSubprojectSettings(int $subprojectID): array
     {
         $factory = NDB_Factory::singleton();
         $DB      = $factory->database();
 
-        try {
-            $info = $DB->pselectRow(
-                "SELECT * FROM subproject WHERE SubprojectID=:sp",
-                array('sp' => $subprojectID)
-            );
-        } catch(DatabaseException $e) {
-            return null;
-        }
+        $info = $DB->pselectRow(
+            "SELECT * FROM subproject WHERE SubprojectID=:sp",
+            array('sp' => $subprojectID)
+        );
         // Format the results the same way it was formatted in config.xml
         // This variable assignment was done for phpcs... if anyone figures out
         // a way around this, please correct it
@@ -469,6 +478,7 @@ class NDB_Config
                     'RecruitmentTarget' => $info['RecruitmentTarget'],
                    );
         }
+        return array();
     }
 
     /**
@@ -480,7 +490,7 @@ class NDB_Config
      *
      * @return boolean true if the user can access the menu item
      */
-    static function checkMenuPermission($menuID)
+    static function checkMenuPermission(int $menuID): bool
     {
         $DB   = Database::singleton();
         $user = User::singleton();
@@ -509,16 +519,16 @@ class NDB_Config
      * of using $this->getSetting() to give more options for how to load
      * the menus, exactly (config.xml, different database tables, etc..)
      *
-     * @param integer $parent The parent ID of the tree. This should not
+     * @param int|null $parent The parent ID of the tree. This should not
      *                        be passed. It's used to build the tree recursively
      *                        from the LorisMenu table.
      *
      * @return array          Label, Visible, Link, and ID to build the menu
      *                        for the currently logged in user
      */
-    static function getMenuTabs($parent = null)
+    static function getMenuTabs(?int $parent = null): array
     {
-        $DB = Database::singleton();
+        $DB = \Database::singleton();
         if ($parent === null) {
             $thisLevel = $DB->pselect(
                 "SELECT Label,
@@ -532,7 +542,7 @@ class NDB_Config
             );
 
             foreach ($thisLevel as &$thisRow) {
-                $nextLevel = NDB_Config::getMenuTabs($thisRow['ID']);
+                $nextLevel = NDB_Config::getMenuTabs(intval($thisRow['ID']));
 
                 if (!empty($nextLevel)) {
                     $thisRow['subtabs'] = $nextLevel;
@@ -555,14 +565,14 @@ class NDB_Config
             $LevelWithPerm = array_filter(
                 $thisLevel,
                 function ($el) {
-                    return NDB_Config::checkMenuPermission($el['ID']);
+                    return NDB_Config::checkMenuPermission(intval($el['ID']));
                 }
             );
 
             if (!empty($LevelWithPerm)) {
                 return $LevelWithPerm;
             }
-            return;
+            return array();
         }
     }
 
@@ -573,7 +583,7 @@ class NDB_Config
      *
      * @return array of Label => URL
      */
-    function getExternalLinks($type)
+    function getExternalLinks(string $type): array
     {
         $factory = NDB_Factory::singleton();
         $db      = $factory->database();

php/libraries/TimePoint.class.inc

@@ -137,7 +137,7 @@ class TimePoint
         if (is_array($row) && count($row) > 0) {
             $this->_timePointInfo = $row;
             $subprojectSettings   = $config->getSubprojectSettings(
-                $row['SubprojectID']
+                intval($row['SubprojectID'])
             );
             $this->_timePointInfo['SubprojectTitle'] = $subprojectSettings['title'];
         } else {
@@ -941,11 +941,13 @@ class TimePoint
      */
     function getEffectiveDateOfBirth(): ?string
     {
-        $config    = NDB_Config::singleton();
-        $candidate = Candidate::singleton($this->getCandID());
-        $settings  = $config->getSubprojectSettings($this->getSubprojectID());
-        if ($settings['options']['useEDC'] == "true") {
-            return $candidate->getCandidateEDC();
+        $candidate = \Candidate::singleton($this->getCandID());
+        if (!is_null($this->getSubprojectID())) {
+            $settings = \NDB_Config::singleton()
+                ->getSubprojectSettings($this->getSubprojectID());
+            if ($settings['options']['useEDC'] == "true") {
+                return $candidate->getCandidateEDC();
+            }
         }
 
         return $candidate->getCandidateDoB();