Add function param/return types, fix a lot more phpstorm complaints

7 files changed, 38 insertions, 36 deletions

diff --git a/modules-available/dozmod/api.inc.php b/modules-available/dozmod/api.inc.php
index 90e663aa..afc53853 100644
--- a/modules-available/dozmod/api.inc.php
+++ b/modules-available/dozmod/api.inc.php
@@ -10,6 +10,8 @@
 **/
 
 
+use JetBrains\PhpStorm\NoReturn;
+
 if (!Module::isAvailable('locations')) {
 	die('require locations module');
 }
@@ -21,17 +23,17 @@ $availableRessources = ['list', 'netrules', 'metadata', 'imagemeta'];
 
 /* BEGIN: A simple caching mechanism ---------------------------- */
 
-function cache_hash($obj)
+function cache_hash($obj): string
 {
 	return md5(serialize($obj));
 }
 
-function cache_key_to_filename($key)
+function cache_key_to_filename(string $key): string
 {
 	return "/tmp/bwlp-slxadmin-cache-$key";
 }
 
-function cache_put($key, $value)
+function cache_put(string $key, string $value): void
 {
 	$filename = cache_key_to_filename($key);
 	// Try to avoid another client concurrently accessing the cache seeing an empty file
@@ -40,7 +42,7 @@ function cache_put($key, $value)
 	rename($tmp, $filename);
 }
 
-function cache_has($key)
+function cache_has(string $key): bool
 {
 	$filename = cache_key_to_filename($key);
 	$mtime = @filemtime($filename);
@@ -49,21 +51,18 @@ function cache_has($key)
 		return false; // cache miss
 	}
 	$now = time();
-	if ($now < $mtime || $now - $mtime > CONFIG_DOZMOD_EXPIRE) {
-		return false;
-	} else {
-		return true;
-	}
+	return $now >= $mtime && $now - $mtime <= CONFIG_DOZMOD_EXPIRE;
 }
 
-function cache_get($key)
+function cache_get(string $key): string
 {
 	$filename = cache_key_to_filename($key);
 	return file_get_contents($filename);
 }
 
 /* good for large binary files */
-function cache_get_passthru($key)
+#[NoReturn]
+function cache_get_passthru(string $key): void
 {
 	$filename = cache_key_to_filename($key);
 	$fp = fopen($filename, "r");
@@ -88,11 +87,16 @@ function cache_get_passthru($key)
  * Takes raw lecture list xml, returns array of uuids.
  *
  * @param string $responseXML XML from dozmod server
- * @return array list of UUIDs, false on error
+ * @return array list of UUIDs
  */
-function xmlToLectureIds($responseXML)
+function xmlToLectureIds(string $responseXML): array
 {
-	$xml = new SimpleXMLElement($responseXML);
+	try {
+		$xml = new SimpleXMLElement($responseXML);
+	} catch (Exception $e) {
+		EventLog::warning('Error parsing XML response data from DMSD: ' . $e->getMessage(), $responseXML);
+		return [];
+	}
 	if (!isset($xml->eintrag))
 		return [];
 
@@ -105,7 +109,8 @@ function xmlToLectureIds($responseXML)
 	return $uuids;
 }
 
-function sendExamModeMismatch()
+#[NoReturn]
+function sendExamModeMismatch(): void
 {
 	Header('Content-Type: text/xml; charset=utf-8');
 	echo
@@ -145,7 +150,7 @@ BLA;
 }
 
 /** Caching wrapper around _getLecturesForLocations() */
-function getListForLocations($locationIds, $raw)
+function getListForLocations(array $locationIds, bool $raw)
 {
 	/* if in any of the locations there is an exam active, consider the client
 		 to be in "exam-mode" and only offer him exams (no lectures) */
@@ -199,20 +204,20 @@ function getListForLocations($locationIds, $raw)
 	return $list;
 }
 
-function getLectureUuidsForLocations($locationIds)
+function getLectureUuidsForLocations(array $locationIds)
 {
 	return getListForLocations($locationIds, false);
 }
 
-function outputLectureXmlForLocation($locationIds)
+function outputLectureXmlForLocation(array $locationIds)
 {
 	return getListForLocations($locationIds, true);
 }
 
-function _getVmData($lecture_uuid, $subResource = false)
+function _getVmData(string $lecture_uuid, string $subResource = null)
 {
 	$url = VMX_URL . '/' . $lecture_uuid;
-	if ($subResource !== false) {
+	if ($subResource !== null) {
 		$url .= '/' . $subResource;
 	}
 	$t = microtime(true);
@@ -229,7 +234,7 @@ function _getVmData($lecture_uuid, $subResource = false)
 }
 
 /** Caching wrapper around _getVmData() **/
-function outputResource($lecture_uuid, $resource)
+function outputResource(string $lecture_uuid, string $resource): void
 {
 	if ($resource === 'metadata') {
 		// HACK: config.tgz is compressed, don't use gzip output handler
@@ -250,7 +255,7 @@ function outputResource($lecture_uuid, $resource)
 	} else {
 		$value = _getVmData($lecture_uuid, $resource);
 		if ($value === false)
-			return false;
+			return;
 		if (is_int($value)) {
 			http_response_code($value);
 			exit;
@@ -258,16 +263,16 @@ function outputResource($lecture_uuid, $resource)
 		cache_put($key, $value);
 		die($value);
 	}
-	return false;
 }
 
+#[NoReturn]
 function fatalDozmodUnreachable()
 {
 	Header('HTTP/1.1 504 Gateway Timeout');
 	die('DMSD currently not available');
 }
 
-function readLectureParam($locationIds)
+function readLectureParam(array $locationIds): string
 {
 	$lecture = Request::get('lecture', false, 'string');
 	if ($lecture === false) {
@@ -290,7 +295,7 @@ function readLectureParam($locationIds)
 // in this context the lecture param is an image id (container), 
 // just read and check if valid.
 // TODO do we need to check if this is allowed?
-function readImageParam()
+function readImageParam(): string
 {
 	$image = Request::get('lecture', false, 'string');
 		
@@ -335,6 +340,3 @@ if ($resource === 'list') {
 	outputResource($lecture, $resource);
 }
 fatalDozmodUnreachable();
-
-Header('HTTP/1.1 400 Bad Request');
-die("I don't know how to give you that resource");
diff --git a/modules-available/dozmod/page.inc.php b/modules-available/dozmod/page.inc.php
index bf2a6fa4..4a43d881 100644
--- a/modules-available/dozmod/page.inc.php
+++ b/modules-available/dozmod/page.inc.php
@@ -68,7 +68,7 @@ class Page_DozMod extends Page
 		/* add sub-menus */
 		foreach ($this->validSections as $section) {
 			if ($section !== 'special' && User::hasPermission($section . '.*')) {
-				Dashboard::addSubmenu('?do=dozmod&section=' . $section, Dictionary::translate('submenu_' . $section, true));
+				Dashboard::addSubmenu('?do=dozmod&section=' . $section, Dictionary::translate('submenu_' . $section));
 			}
 		}
 	}
diff --git a/modules-available/dozmod/pages/actionlog.inc.php b/modules-available/dozmod/pages/actionlog.inc.php
index 0b6feb6f..182198c2 100644
--- a/modules-available/dozmod/pages/actionlog.inc.php
+++ b/modules-available/dozmod/pages/actionlog.inc.php
@@ -92,7 +92,7 @@ class SubPage
 		return $desc;
 	}
 
-	private static function addImageHeader()
+	private static function addImageHeader(): bool
 	{
 		$image = Database::queryFirst('SELECT o.userid AS ouserid, o.firstname AS ofirstname, o.lastname AS olastname,
 				u.userid AS uuserid, u.firstname AS ufirstname, u.lastname AS ulastname,
@@ -114,7 +114,7 @@ class SubPage
 		return $image !== false;
 	}
 
-	private static function addLectureHeader()
+	private static function addLectureHeader(): bool
 	{
 		$lecture = Database::queryFirst('SELECT o.userid AS ouserid, o.firstname AS ofirstname, o.lastname AS olastname,
 				u.userid AS uuserid, u.firstname AS ufirstname, u.lastname AS ulastname,
diff --git a/modules-available/dozmod/pages/expiredimages.inc.php b/modules-available/dozmod/pages/expiredimages.inc.php
index b61b863b..ab563273 100644
--- a/modules-available/dozmod/pages/expiredimages.inc.php
+++ b/modules-available/dozmod/pages/expiredimages.inc.php
@@ -8,7 +8,7 @@ class SubPage
 
 	}
 
-	private static function loadExpiredImages()
+	private static function loadExpiredImages(): array
 	{
 		$res = Database::simpleQuery("SELECT b.displayname,
 			own.firstname, own.lastname, own.userid,
diff --git a/modules-available/dozmod/pages/mailconfig.inc.php b/modules-available/dozmod/pages/mailconfig.inc.php
index 08205f2e..aa03a4d3 100644
--- a/modules-available/dozmod/pages/mailconfig.inc.php
+++ b/modules-available/dozmod/pages/mailconfig.inc.php
@@ -34,7 +34,7 @@ class SubPage
 		Util::redirect('?do=DozMod&section=mailconfig');
 	}
 
-	private static function cleanMailArray()
+	private static function cleanMailArray(): array
 	{
 		$keys = array('host', 'port', 'ssl', 'senderAddress', 'replyTo', 'username', 'password', 'serverName');
 		$data = array();
diff --git a/modules-available/dozmod/pages/templates.inc.php b/modules-available/dozmod/pages/templates.inc.php
index b857115f..b916e14c 100644
--- a/modules-available/dozmod/pages/templates.inc.php
+++ b/modules-available/dozmod/pages/templates.inc.php
@@ -69,7 +69,7 @@ class SubPage
 		]);
 	}
 
-	private static function forcmp($string)
+	private static function forcmp(string $string): string
 	{
 		return trim(str_replace("\r\n", "\n", $string));
 	}
diff --git a/modules-available/dozmod/pages/users.inc.php b/modules-available/dozmod/pages/users.inc.php
index cdf22b9d..fe00a71b 100644
--- a/modules-available/dozmod/pages/users.inc.php
+++ b/modules-available/dozmod/pages/users.inc.php
@@ -64,13 +64,13 @@ class SubPage
 			. ' ORDER BY displayname ASC');
 		$rows = array();
 		foreach ($res as $row) {
-			$row['canlogin'] = self::checked($row['canlogin']);
+			$row['canlogin'] = self::checked((bool)$row['canlogin']);
 			$rows[] = $row;
 		}
 		Render::addTemplate('orglist', array('organizations' => $rows));
 	}
 
-	private static function checked($val)
+	private static function checked(bool $val): string
 	{
 		if ($val)
 			return 'checked="checked"';