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

9 files changed, 66 insertions, 66 deletions

diff --git a/modules-available/locations/baseconfig/getconfig.inc.php b/modules-available/locations/baseconfig/getconfig.inc.php
index f2aa2b46..1bed5de7 100644
--- a/modules-available/locations/baseconfig/getconfig.inc.php
+++ b/modules-available/locations/baseconfig/getconfig.inc.php
@@ -1,5 +1,8 @@
 <?php
 
+/** @var ?string $uuid */
+/** @var ?string $ip */
+
 // Location handling: figure out location
 $locationId = false;
 if (BaseConfig::hasOverride('locationid')) {
@@ -12,7 +15,7 @@ if (BaseConfig::hasOverride('locationid')) {
 }
 
 if ($locationId === false) {
-	if (!$ip) // Required at this point, bail out if not given
+	if ($ip === null) // Required at this point, bail out if not given
 		return;
 	$locationId = Location::getFromIpAndUuid($ip, $uuid);
 }
diff --git a/modules-available/locations/inc/location.inc.php b/modules-available/locations/inc/location.inc.php
index f68d6821..59e6d2b6 100644
--- a/modules-available/locations/inc/location.inc.php
+++ b/modules-available/locations/inc/location.inc.php
@@ -17,7 +17,7 @@ class Location
 		return self::$treeCache;
 	}
 
-	public static function queryLocations()
+	public static function queryLocations(): array
 	{
 		$res = Database::simpleQuery("SELECT locationid, parentlocationid, locationname FROM location");
 		$rows = array();
@@ -29,10 +29,9 @@ class Location
 
 	/**
 	 * Return row from location table for $locationId.
-	 * @param $locationId
 	 * @return array|bool row from DB, false if not found
 	 */
-	public static function get($locationId)
+	public static function get(int $locationId)
 	{
 		return Database::queryFirst("SELECT * FROM location WHERE locationid = :locationId", compact('locationId'));
 	}
@@ -42,9 +41,8 @@ class Location
 	 * @param int $locationId id of location to get name for
 	 * @return string|false Name of location, false if locationId doesn't exist
 	 */
-	public static function getName($locationId)
+	public static function getName(int $locationId)
 	{
-		$locationId = (int)$locationId;
 		if (self::$assocLocationCache === false) {
 			self::getLocationsAssoc();
 		}
@@ -56,15 +54,13 @@ class Location
 	/**
 	 * Get all the names of the given location and its parents, up
 	 * to the root element. Array keys will be locationids, value the names.
-	 * @param int $locationId
 	 * @return array|false locations, from furthest to nearest or false if locationId doesn't exist
 	 */
-	public static function getNameChain($locationId)
+	public static function getNameChain(int $locationId)
 	{
 		if (self::$assocLocationCache === false) {
 			self::getLocationsAssoc();
 		}
-		$locationId = (int)$locationId;
 		if (!isset(self::$assocLocationCache[$locationId]))
 			return false;
 		$ret = array();
@@ -84,7 +80,7 @@ class Location
 		return self::$assocLocationCache;
 	}
 
-	private static function flattenTreeAssoc($tree, $parents = array(), $depth = 0)
+	private static function flattenTreeAssoc($tree, $parents = array(), $depth = 0): array
 	{
 		if ($depth > 20) {
 			ErrorHandler::traceError('Recursive location definition detected at ' . print_r($tree, true));
@@ -121,12 +117,12 @@ class Location
 
 	/**
 	 * @param int|int[] $selected Which locationIDs to mark as selected
-	 * @param int $excludeId Which locationID to explude
+	 * @param int $excludeId Which locationID to exclude
 	 * @param bool $addNoParent Add entry for "no location" at the top
 	 * @param bool $keepArrayKeys Keep location IDs as array index
 	 * @return array Locations
 	 */
-	public static function getLocations($selected = 0, $excludeId = 0, $addNoParent = false, $keepArrayKeys = false)
+	public static function getLocations($selected = 0, int $excludeId = 0, bool $addNoParent = false, bool $keepArrayKeys = false): array
 	{
 		if (self::$flatLocationCache === false) {
 			$rows = self::getTree();
@@ -173,7 +169,7 @@ class Location
 	 * @param bool $locationTree used in recursive calls, don't pass
 	 * @return array list of passed locations plus their children
 	 */
-	public static function getRecursive($idList, $locationTree = false)
+	public static function getRecursive($idList, bool $locationTree = false): array
 	{
 		if (!is_array($idList)) {
 			$idList = array($idList);
@@ -207,7 +203,7 @@ class Location
 		return $ret;
 	}
 
-	public static function buildTree($elements, $parentId = 0)
+	public static function buildTree(array $elements, int $parentId = 0): array
 	{
 		$branch = array();
 		$sort = array();
@@ -227,7 +223,7 @@ class Location
 		return $branch;
 	}
 
-	private static function flattenTree($tree, $depth = 0)
+	private static function flattenTree(array $tree, int $depth = 0): array
 	{
 		if ($depth > 20) {
 			ErrorHandler::traceError('Recursive location definition detected at ' . print_r($tree, true));
@@ -249,7 +245,8 @@ class Location
 		return $output;
 	}
 
-	public static function isLeaf($locationid) {
+	public static function isLeaf(int $locationid): bool
+	{
 		$result = Database::queryFirst('SELECT COUNT(locationid) = 0 AS isleaf '
 			. 'FROM location '
 			. 'WHERE parentlocationid = :locationid', ['locationid' => $locationid]);
@@ -257,7 +254,7 @@ class Location
 		return (bool)$result;
 	}
 
-	public static function extractIds($tree)
+	public static function extractIds(array $tree): array
 	{
 		$ids = array();
 		foreach ($tree as $node) {
@@ -271,10 +268,11 @@ class Location
 
 	/**
 	 * Get location id for given machine (by uuid)
+	 *
 	 * @param string $uuid machine uuid
-	 * @return bool|int locationid, false if no match
+	 * @return false|int locationid, false if no match
 	 */
-	public static function getFromMachineUuid($uuid)
+	public static function getFromMachineUuid(string $uuid)
 	{
 		// Only if we have the statistics module which supplies the machine table
 		if (Module::get('statistics') === false)
@@ -291,9 +289,9 @@ class Location
 	 *
 	 * @param string $ip IP address of client
 	 * @param bool $honorRoomPlanner consider a fixed location assigned manually by roomplanner
-	 * @return bool|int locationid, or false if no match
+	 * @return false|int locationid, or false if no match
 	 */
-	public static function getFromIp($ip, $honorRoomPlanner = false)
+	public static function getFromIp(string $ip, bool $honorRoomPlanner = false)
 	{
 		if (Module::get('statistics') !== false) {
 			// Shortcut - try to use subnetlocationid in machine table
@@ -322,17 +320,17 @@ class Location
 	 * client, so if it seems too fishy, the UUID will be ignored.
 	 *
 	 * @param string $ip IP address of client
-	 * @param string $uuid System-UUID of client
-	 * @return int|bool location id, or false if none matches
+	 * @param ?string $uuid System-UUID of client
+	 * @return int|false location id, or false if none matches
 	 */
-	public static function getFromIpAndUuid($ip, $uuid)
+	public static function getFromIpAndUuid(string $ip, ?string $uuid)
 	{
 		$locationId = false;
 		$ipLoc = self::getFromIp($ip);
 		if ($ipLoc !== false) {
 			// Set locationId to ipLoc for now, it will be overwritten later if another case applies.
 			$locationId = $ipLoc;
-			if ($uuid !== false) {
+			if ($uuid !== null) {
 				// Machine ip maps to a location, and we have a client supplied uuid (which might not be known if the client boots for the first time)
 				$uuidLoc = self::getFromMachineUuid($uuid);
 				if (self::isFixedLocationValid($uuidLoc, $ipLoc)) {
@@ -343,7 +341,7 @@ class Location
 		return $locationId;
 	}
 
-	public static function isFixedLocationValid($uuidLoc, $ipLoc)
+	public static function isFixedLocationValid($uuidLoc, $ipLoc): bool
 	{
 		if ($uuidLoc === false)
 			return false;
@@ -366,12 +364,10 @@ class Location
 	/**
 	 * Get all location IDs from the given location up to the root.
 	 *
-	 * @param int $locationId
 	 * @return int[] location ids, including $locationId
 	 */
-	public static function getLocationRootChain($locationId)
+	public static function getLocationRootChain(int $locationId): array
 	{
-		$locationId = (int)$locationId;
 		if (self::$assocLocationCache === false) {
 			self::getLocationsAssoc();
 		}
@@ -385,7 +381,7 @@ class Location
 	/**
 	 * @return array list of subnets as numeric array
 	 */
-	public static function getSubnets()
+	public static function getSubnets(): array
 	{
 		$res = Database::simpleQuery("SELECT startaddr, endaddr, locationid FROM subnet");
 		$subnets = array();
@@ -397,9 +393,9 @@ class Location
 	}
 
 	/**
-	 * @return array|bool assoc array mapping from locationid to subnets
+	 * @return array assoc array mapping from locationid to subnets
 	 */
-	public static function getSubnetsByLocation($recursive = false)
+	public static function getSubnetsByLocation($recursive = false): array
 	{
 		$locs = self::getLocationsAssoc();
 		$subnets = self::getSubnets();
@@ -433,9 +429,9 @@ class Location
 	 * random one will be returned.
 	 *
 	 * @param string $ip IP to look up
-	 * @return bool|int locationid ip matches, false = no match
+	 * @return false|int locationid ip matches, false = no match
 	 */
-	public static function mapIpToLocation($ip)
+	public static function mapIpToLocation(string $ip)
 	{
 		if (self::$subnetMapCache === false) {
 			self::$subnetMapCache = self::getSubnetsByLocation();
@@ -464,7 +460,10 @@ class Location
 		return (int)$best;
 	}
 
-	public static function updateMapIpToLocation($uuid, $ip)
+	/**
+	 * @return false|int newly determined location
+	 */
+	public static function updateMapIpToLocation(string $uuid, string $ip)
 	{
 		$loc = self::mapIpToLocation($ip);
 		if ($loc === false) {
diff --git a/modules-available/locations/inc/locationhooks.inc.php b/modules-available/locations/inc/locationhooks.inc.php
index 5ce3bbfe..fca85575 100644
--- a/modules-available/locations/inc/locationhooks.inc.php
+++ b/modules-available/locations/inc/locationhooks.inc.php
@@ -6,7 +6,7 @@ class LocationHooks
 	/**
 	 * Resolve baseconfig id to locationid -- noop in this case
 	 */
-	public static function baseconfigLocationResolver($id)
+	public static function baseconfigLocationResolver(int $id): int
 	{
 		return $id;
 	}
@@ -15,7 +15,7 @@ class LocationHooks
 	 * Hook to get inheritance tree for all config vars
 	 * @param int $id Locationid currently being edited
 	 */
-	public static function baseconfigInheritance($id)
+	public static function baseconfigInheritance(int $id): array
 	{
 		$locs = Location::getLocationsAssoc();
 		if ($locs === false || !isset($locs[$id]))
diff --git a/modules-available/locations/inc/locationutil.inc.php b/modules-available/locations/inc/locationutil.inc.php
index 907bcc99..17608203 100644
--- a/modules-available/locations/inc/locationutil.inc.php
+++ b/modules-available/locations/inc/locationutil.inc.php
@@ -159,19 +159,17 @@ class LocationUtil
 		}
 		if (empty($return))
 			return $return;
-		if ($locationId === 0) {
+		if ($locationId === 0)
 			return array_values($return);
-		} else {
-			return $return[$locationId];
-		}
+		return $return[$locationId];
 	}
 
-	private static function overlap($net1, $net2)
+	private static function overlap(array $net1, array $net2): bool
 	{
 		return ($net1['startaddr'] <= $net2['endaddr'] && $net1['endaddr'] >= $net2['startaddr']);
 	}
 
-	public static function rangeToLongVerbose($start, $end)
+	public static function rangeToLongVerbose(int $start, int $end): ?array
 	{
 		$result = self::rangeToLong($start, $end);
 		list($startLong, $endLong) = $result;
@@ -182,15 +180,15 @@ class LocationUtil
 			Message::addWarning('main.value-invalid', 'end addr', $start);
 		}
 		if ($startLong === false || $endLong === false)
-			return false;
+			return null;
 		if ($startLong > $endLong) {
 			Message::addWarning('main.value-invalid', 'range', $start . ' - ' . $end);
-			return false;
+			return null;
 		}
 		return $result;
 	}
 
-	public static function rangeToLong($start, $end)
+	public static function rangeToLong(int $start, int $end): array
 	{
 		$startLong = ip2long($start);
 		$endLong = ip2long($end);
diff --git a/modules-available/locations/inc/openingtimes.inc.php b/modules-available/locations/inc/openingtimes.inc.php
index 18e25063..74dae7c3 100644
--- a/modules-available/locations/inc/openingtimes.inc.php
+++ b/modules-available/locations/inc/openingtimes.inc.php
@@ -11,9 +11,8 @@ class OpeningTimes
 	 * "openingtime": "8:00",
 	 * "closingtime": "20:00"
 	 * }</pre>
-	 * @return array|null
 	 */
-	public static function forLocation(int $locationId)
+	public static function forLocation(int $locationId): ?array
 	{
 		static $openingTimesList = false;
 		if ($openingTimesList === false) {
diff --git a/modules-available/locations/pages/cleanup.inc.php b/modules-available/locations/pages/cleanup.inc.php
index a7562c50..423d6a6b 100644
--- a/modules-available/locations/pages/cleanup.inc.php
+++ b/modules-available/locations/pages/cleanup.inc.php
@@ -3,7 +3,7 @@
 class SubPage
 {
 
-	public static function doPreprocess($action)
+	public static function doPreprocess($action): bool
 	{
 		if ($action === 'resetmachines') {
 			self::resetMachines();
@@ -16,7 +16,7 @@ class SubPage
 		return false;
 	}
 
-	public static function doRender($action)
+	public static function doRender($action): bool
 	{
 		$list = self::loadForLocation();
 		if ($list === false)
@@ -29,7 +29,7 @@ class SubPage
 		return true;
 	}
 
-	public static function doAjax($action)
+	public static function doAjax($action): bool
 	{
 		return false;
 	}
diff --git a/modules-available/locations/pages/details.inc.php b/modules-available/locations/pages/details.inc.php
index e75aaf7d..279eee44 100644
--- a/modules-available/locations/pages/details.inc.php
+++ b/modules-available/locations/pages/details.inc.php
@@ -3,24 +3,25 @@
 class SubPage
 {
 
-	public static function doPreprocess($action)
+	public static function doPreprocess($action): bool
 	{
 		if ($action === 'updatelocation') {
 			self::updateLocation();
 			return true;
-		} else if ($action === 'updateOpeningtimes') {
+		}
+		if ($action === 'updateOpeningtimes') {
 			self::updateOpeningTimes();
 			return true;
 		}
 		return false;
 	}
 
-	public static function doRender($action)
+	public static function doRender($action): bool
 	{
 		return false;
 	}
 
-	public static function doAjax($action)
+	public static function doAjax($action): bool
 	{
 		if ($action === 'showlocation') {
 			self::ajaxShowLocation();
@@ -172,7 +173,7 @@ class SubPage
 		Util::redirect('?do=Locations');
 	}
 
-	private static function updateLocationData($location)
+	private static function updateLocationData(array $location): bool
 	{
 		$locationId = (int)$location['locationid'];
 		$newParent = Request::post('parentlocationid', false, 'integer');
@@ -216,7 +217,7 @@ class SubPage
 		return $newParent != $location['parentlocationid'];
 	}
 
-	private static function updateLocationSubnets()
+	private static function updateLocationSubnets(): bool
 	{
 		$locationId = Request::post('locationid', false, 'integer');
 		if (!User::hasPermission('location.edit.subnets', $locationId))
@@ -257,7 +258,7 @@ class SubPage
 				continue;
 			}
 			$range = LocationUtil::rangeToLongVerbose($start, $end);
-			if ($range === false)
+			if ($range === null)
 				continue;
 			list($startLong, $endLong) = $range;
 			if ($stmt->execute(array('id' => $subnetid, 'start' => $startLong, 'end' => $endLong))) {
@@ -275,7 +276,7 @@ class SubPage
 		return $change;
 	}
 
-	private static function addNewLocationSubnets($location)
+	private static function addNewLocationSubnets(array $location): bool
 	{
 		$locationId = (int)$location['locationid'];
 		if (!User::hasPermission('location.edit.subnets', $locationId))
diff --git a/modules-available/locations/pages/locations.inc.php b/modules-available/locations/pages/locations.inc.php
index 8ba4793e..78818328 100644
--- a/modules-available/locations/pages/locations.inc.php
+++ b/modules-available/locations/pages/locations.inc.php
@@ -3,7 +3,7 @@
 class SubPage
 {
 
-	public static function doPreprocess($action)
+	public static function doPreprocess($action): bool
 	{
 		if ($action === 'addlocations') {
 			self::addLocations();
@@ -12,7 +12,7 @@ class SubPage
 		return false;
 	}
 
-	public static function doRender($getAction)
+	public static function doRender($getAction): bool
 	{
 		if ($getAction === false) {
 			if (User::hasPermission('location.view')) {
@@ -32,7 +32,7 @@ class SubPage
 		return false;
 	}
 
-	public static function doAjax($action)
+	public static function doAjax($action): bool
 	{
 		return false;
 	}
diff --git a/modules-available/locations/pages/subnets.inc.php b/modules-available/locations/pages/subnets.inc.php
index e568c655..7628486b 100644
--- a/modules-available/locations/pages/subnets.inc.php
+++ b/modules-available/locations/pages/subnets.inc.php
@@ -3,7 +3,7 @@
 class SubPage
 {
 
-	public static function doPreprocess($action)
+	public static function doPreprocess(string $action): bool
 	{
 		if ($action === 'updatesubnets') {
 			self::updateSubnets();
@@ -42,7 +42,7 @@ class SubPage
 				continue;
 			}
 			$range = LocationUtil::rangeToLongVerbose($start, $end);
-			if ($range === false)
+			if ($range === null)
 				continue;
 			list($startLong, $endLong) = $range;
 			if ($stmt->execute(compact('startLong', 'endLong', 'loc', 'subnetid'))) {
@@ -59,7 +59,7 @@ class SubPage
 		Util::redirect('?do=Locations');
 	}
 
-	public static function doRender($getAction)
+	public static function doRender($getAction): bool
 	{
 		if ($getAction === false) {
 			User::assertPermission('subnets.edit', NULL, '?do=locations');
@@ -81,7 +81,7 @@ class SubPage
 		return false;
 	}
 
-	public static function doAjax($action)
+	public static function doAjax($action): bool
 	{
 		return false;
 	}