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

9 files changed, 63 insertions, 85 deletions

diff --git a/modules-available/statistics/inc/hardwareinfo.inc.php b/modules-available/statistics/inc/hardwareinfo.inc.php
index 56ea38cd..7e0bdba8 100644
--- a/modules-available/statistics/inc/hardwareinfo.inc.php
+++ b/modules-available/statistics/inc/hardwareinfo.inc.php
@@ -21,9 +21,8 @@ class HardwareInfo
 	 * the resulting machine that has the greatest "lastseen" value will be used.
 	 * @param ?string $uuid UUID of machine
 	 * @param ?string $mac MAC of machine
-	 * @return string
 	 */
-	public static function getKclModifications($uuid = null, $mac = null): string
+	public static function getKclModifications(?string $uuid = null, ?string $mac = null): string
 	{
 		if ($uuid === null && $mac === null) {
 			$uuid = Request::get('uuid', '', 'string');
diff --git a/modules-available/statistics/inc/hardwareparser.inc.php b/modules-available/statistics/inc/hardwareparser.inc.php
index 1c549976..428f7d55 100644
--- a/modules-available/statistics/inc/hardwareparser.inc.php
+++ b/modules-available/statistics/inc/hardwareparser.inc.php
@@ -38,8 +38,6 @@ class HardwareParser
 
 	/**
 	 * Decode JEDEC ID to according manufacturer
-	 * @param string $string
-	 * @return string
 	 */
 	public static function decodeJedec(string $string): string
 	{
@@ -79,10 +77,7 @@ class HardwareParser
 		return $string;
 	}
 
-	/**
-	 * @return ?string
-	 */
-	private static function decodeBankAndId(array $out, bool $bankFirst)
+	private static function decodeBankAndId(array $out, bool $bankFirst): ?string
 	{
 		// 16bit encoding from DDR3+: lower byte is number of 0x7f bytes, upper byte is id within bank
 		$id = hexdec(str_replace(' ', '', $out[1]));
@@ -100,10 +95,7 @@ class HardwareParser
 		return self::lookupJedec($bank, $id);
 	}
 
-	/**
-	 * @return ?string
-	 */
-	private static function lookupJedec(int $bank, int $id)
+	private static function lookupJedec(int $bank, int $id): ?string
 	{
 		static $data = false;
 		if ($data === false) {
@@ -119,11 +111,9 @@ class HardwareParser
 	 * base representation, meant for comparison. For example, Voltages are converted
 	 * to Millivolts, Anything measured in [KMGT]Bytes (per second) to bytes, GHz to
 	 * Hz, and so on.
-	 * @param string $key
-	 * @param string $value
 	 * @return ?int value, or null if not numeric
 	 */
-	private static function toNumeric(string $key, string $val)
+	private static function toNumeric(string $key, string $val): ?int
 	{
 		$key = strtolower($key);
 		// Normalize voltage to mV
@@ -208,7 +198,7 @@ class HardwareParser
 	 * @param int $hwid hw global hw id
 	 * @param string $pathId unique identifier for the local instance of this hw, e.q. PCI slot, /dev path, something that handles the case that there are multiple instances of the same hardware in one machine
 	 * @param array $props KV-pairs of properties to write for this instance; can be empty
-	 * @return int
+	 * @return int ID of mapping in DB
 	 */
 	private static function writeLocalHardwareData(string $uuid, int $hwid, string $pathId, array $props): int
 	{
@@ -330,7 +320,7 @@ class HardwareParser
 	 * @param array $data Hardware info, deserialized assoc array.
 	 * @return ?array id44mb and id45mb as calculated from given HDD data
 	 */
-	public static function parseMachine(string $uuid, array $data)
+	public static function parseMachine(string $uuid, array $data): ?array
 	{
 		$version = $data['version'] ?? 0;
 		if ($version != 2) {
@@ -510,12 +500,12 @@ class HardwareParser
 			if (empty($dev['readlink'])) // This is the canonical entry name directly under /dev/, e.g. /dev/sda
 				continue;
 			// Use smartctl as the source of truth, lsblk as fallback if data is missing
-			if (!isset($dev['smartctl'])) {
+			if (!isset($dev['smartctl']) || !is_array($dev['smartctl'])) {
 				$smart = [];
 			} else {
 				$smart =& $dev['smartctl'];
 			}
-			if (!isset($dev['lsblk']['blockdevices'][0])) {
+			if (!isset($dev['lsblk']['blockdevices'][0]) || !is_array($dev['lsblk']['blockdevices'][0])) {
 				$lsblk = [];
 			} else {
 				$lsblk =& $dev['lsblk']['blockdevices'][0];
@@ -626,7 +616,7 @@ class HardwareParser
 			}
 			$table['unused'] = $size - $used;
 			$table['dev'] = $dev['readlink'];
-			$table += self::propsFromArray($smart + ($lsblk ?? []),
+			$table += self::propsFromArray($smart + $lsblk,
 				'serial_number', 'firmware_version',
 				'interface_speed//current//string',
 				'smart_status//passed', 'temperature//current', 'temperature//min', 'temperature//max',
diff --git a/modules-available/statistics/inc/hardwareparserlegacy.inc.php b/modules-available/statistics/inc/hardwareparserlegacy.inc.php
index 7ee39bce..a6ac6d5e 100644
--- a/modules-available/statistics/inc/hardwareparserlegacy.inc.php
+++ b/modules-available/statistics/inc/hardwareparserlegacy.inc.php
@@ -243,10 +243,11 @@ class HardwareParserLegacy
 					$ramOk = true;
 				}
 				if ($ramOk && preg_match('/^\s*Number Of Devices:\s+(\d+)\s*$/i', $line, $out)) {
-					$row['Memory Slot Count'] += $out[1];
+					$row['Memory Slot Count'] += (int)$out[1];
 				}
 				if ($ramOk && preg_match('/^\s*Maximum Capacity:\s+(\d.+)/i', $line, $out)) {
-					$row['Memory Maximum Capacity'] += HardwareParser::convertSize($out[1], 'G', false);
+					/** @var array{"Memory Slot Count": int} $row */
+					$row['Memory Maximum Capacity'] += (int)HardwareParser::convertSize($out[1], 'G', false);
 				}
 			} elseif ($section === 'Memory Device') {
 				if (preg_match('/^\s*Size:\s*(.*?)\s*$/i', $line, $out)) {
diff --git a/modules-available/statistics/inc/hardwarequery.inc.php b/modules-available/statistics/inc/hardwarequery.inc.php
index 4f0ec344..6b1b5043 100644
--- a/modules-available/statistics/inc/hardwarequery.inc.php
+++ b/modules-available/statistics/inc/hardwarequery.inc.php
@@ -45,25 +45,23 @@ class HardwareQuery
 	 */
 	public function addForeignJoin(bool $global, string $prop, string $jTable, string $jColumn, string $condColumn = '', $condVal = null)
 	{
-		if (isset($this->columns[$jTable]))
+		if (isset($this->columns["$jTable.$prop"]))
 			return;
-		if (!isset($this->columns[$prop])) {
-			if ($global) {
-				$srcTable = 'shw';
-				$table = 'statistic_hw_prop';
-				$column = 'hwid';
-			} else {
-				$srcTable = 'mxhw';
-				$table = 'machine_x_hw_prop';
-				$column = 'machinehwid';
-			}
-			$tid = $this->id();
-			$pid = $this->id();
-			$this->joins[$prop] = "INNER JOIN $table $tid ON ($srcTable.$column = $tid.$column
-				AND $tid.prop = :$pid)";
-			$this->args[$pid] = $prop;
-			$this->columns[$prop] = "$tid.`value` AS `$prop`";
+		if ($global) {
+			$srcTable = 'shw';
+			$table = 'statistic_hw_prop';
+			$column = 'hwid';
+		} else {
+			$srcTable = 'mxhw';
+			$table = 'machine_x_hw_prop';
+			$column = 'machinehwid';
 		}
+		$tid = $this->id();
+		$pid = $this->id();
+		$this->joins[$prop] = "INNER JOIN $table $tid ON ($srcTable.$column = $tid.$column
+			AND $tid.prop = :$pid)";
+		$this->args[$pid] = $prop;
+		$this->columns[$prop] = "$tid.`value` AS `$prop`";
 		$jtid = $this->id();
 		$cond = '';
 		if (!empty($condColumn)) {
@@ -106,10 +104,8 @@ class HardwareQuery
 
 	/**
 	 * Join the machine table and add the given column from it to the SELECT
-	 * @param string $column
-	 * @return void
 	 */
-	public function addMachineColumn(string $column)
+	public function addMachineColumn(string $column): void
 	{
 		if (isset($this->columns[$column]))
 			return;
@@ -163,13 +159,11 @@ class HardwareQuery
 		} else {
 			$groupBy = '';
 		}
-		$query = 'SELECT ' . implode(', ', $columns)
+		return 'SELECT ' . implode(', ', $columns)
 			. ' FROM statistic_hw shw '
 			. implode(' ', $this->joins)
 			. ' WHERE ' . implode(' AND ', $this->where)
 			. $groupBy;
-		//error_log($query);
-		return $query;
 	}
 
 }
diff --git a/modules-available/statistics/inc/statistics.inc.php b/modules-available/statistics/inc/statistics.inc.php
index d27c1a6a..2a9ca3c9 100644
--- a/modules-available/statistics/inc/statistics.inc.php
+++ b/modules-available/statistics/inc/statistics.inc.php
@@ -7,7 +7,7 @@ class Statistics
 
 	private static $machineFields = false;
 
-	private static function initFields($returnData)
+	private static function initFields(int $returnData): string
 	{
 		if (self::$machineFields === false) {
 			$r = new ReflectionClass('Machine');
@@ -25,17 +25,15 @@ class Statistics
 	}
 
 	/**
-	 * @param string $machineuuid
 	 * @param int $returnData What kind of data to return Machine::NO_DATA, Machine::RAW_DATA, ...
-	 * @return \Machine|false
 	 */
-	public static function getMachine($machineuuid, $returnData)
+	public static function getMachine(string $machineuuid, int $returnData): ?Machine
 	{
 		$fields = self::initFields($returnData);
 
 		$row = Database::queryFirst("SELECT $fields FROM machine WHERE machineuuid = :machineuuid", compact('machineuuid'));
 		if ($row === false)
-			return false;
+			return null;
 		$m = new Machine();
 		foreach ($row as $key => $val) {
 			$m->{$key} = $val;
@@ -44,12 +42,11 @@ class Statistics
 	}
 
 	/**
-	 * @param string $ip
 	 * @param int $returnData What kind of data to return Machine::NO_DATA, Machine::RAW_DATA, ...
 	 * @param string $sort something like 'lastseen ASC' - not sanitized, don't pass user input!
-	 * @return \Machine[] list of matches
+	 * @return Machine[] list of matches
 	 */
-	public static function getMachinesByIp($ip, $returnData, $sort = false)
+	public static function getMachinesByIp(string $ip, int $returnData, bool $sort = false): array
 	{
 		$fields = self::initFields($returnData);
 
@@ -74,7 +71,7 @@ class Statistics
 	const OFFLINE_LENGTH = '~offline-length';
 	const SUSPEND_LENGTH = '~suspend-length';
 
-	public static function logMachineState($uuid, $ip, $type, $start, $length, $username = '')
+	public static function logMachineState(string $uuid, string $ip, string $type, int $start, int $length, string $username = ''): int
 	{
 		return Database::exec('INSERT INTO statistic (dateline, typeid, machineuuid, clientip, username, data)'
 			. " VALUES (:start, :type, :uuid, :clientip, :username, :length)", array(
diff --git a/modules-available/statistics/inc/statisticsfilter.inc.php b/modules-available/statistics/inc/statisticsfilter.inc.php
index cfc724c1..5e6448c7 100644
--- a/modules-available/statistics/inc/statisticsfilter.inc.php
+++ b/modules-available/statistics/inc/statisticsfilter.inc.php
@@ -94,9 +94,7 @@ abstract class StatisticsFilter
 
 	/**
 	 * Called to get an instance of DatabaseFilter that binds the given $op and $argument to this filter.
-	 * @param string $op
 	 * @param string[]|string $argument
-	 * @return DatabaseFilter
 	 */
 	public function bind(string $op, $argument): DatabaseFilter { return new DatabaseFilter($this, $op, $argument); }
 
@@ -109,6 +107,7 @@ abstract class StatisticsFilter
 		if (empty($this->ops))
 			return;
 		if (!in_array($operator, $this->ops)) {
+			// Yes keep $this in this call, get_class() !== get_class($this)
 			ErrorHandler::traceError("Invalid op '$operator' for " . get_class($this) . '::' . $this->column);
 		}
 	}
@@ -179,10 +178,7 @@ abstract class StatisticsFilter
 		return $filters;
 	}
 
-	/**
-	 * @param \StatisticsFilterSet $filterSet
-	 */
-	public static function renderFilterBox($show, $filterSet)
+	public static function renderFilterBox(string $show, StatisticsFilterSet $filterSet): void
 	{
 		// Build location list, with permissions
 		if (Module::isAvailable('locations')) {
@@ -194,7 +190,7 @@ abstract class StatisticsFilter
 		foreach (self::$columns as $key => $filter) {
 			$col = [
 				'key' => $key,
-				'name' => Dictionary::translateFile('filters', $key, true),
+				'name' => Dictionary::translateFile('filters', $key),
 				'placeholder' => $filter->placeholder,
 			];
 			$bind = $filterSet->hasFilterKey($key);
@@ -207,6 +203,7 @@ abstract class StatisticsFilter
 				$col['inputclass'] = 'is-date';
 			} elseif ($filter->type() === 'enum') {
 				$col['enum'] = true;
+				/** @var EnumStatisticsFilter $filter */
 				$col['values'] = $filter->values;
 				if ($bind !== null) {
 					// Current value from GET
@@ -550,15 +547,14 @@ class StateStatisticsFilter extends EnumStatisticsFilter
 	public function whereClause(string $operator, $argument, array &$args, array &$joins): string
 	{
 		$map = [ 'on' => ['IDLE', 'OCCUPIED'], 'off' => ['OFFLINE'], 'idle' => ['IDLE'], 'occupied' => ['OCCUPIED'], 'standby' => ['STANDBY'] ];
-		$neg = $operator == '!=' ? 'NOT ' : '';
+		$neg = $operator === '!=' ? 'NOT ' : '';
 		if (array_key_exists($argument, $map)) {
 			$key = StatisticsFilter::getNewKey($this->column);
 			$args[$key] = $map[$argument];
 			return " m.state $neg IN ( :$key ) ";
-		} else {
-			Message::addError('invalid-filter-argument', 'state', $argument);
-			return ' 1';
 		}
+		Message::addError('invalid-filter-argument', 'state', $argument);
+		return ' 1';
 	}
 }
 
@@ -623,7 +619,7 @@ class IpStatisticsFilter extends StatisticsFilter
 		} elseif (strpos($argument, '/') !== false) {
 			// TODO: IPv6 CIDR
 			$range = IpUtil::parseCidr($argument);
-			if ($range === false) {
+			if ($range === null) {
 				Message::addError('invalid-cidr-notion', $argument);
 				return '0';
 			}
@@ -838,9 +834,6 @@ class DatabaseFilter
 
 	/**
 	 * Called from StatisticsFilterSet::makeFragments() to build the final query.
-	 * @param array $args
-	 * @param array $joins
-	 * @return string
 	 */
 	public function whereClause(array &$args, array &$joins): string
 	{
diff --git a/modules-available/statistics/inc/statisticsfilterset.inc.php b/modules-available/statistics/inc/statisticsfilterset.inc.php
index 102cba42..26595e93 100644
--- a/modules-available/statistics/inc/statisticsfilterset.inc.php
+++ b/modules-available/statistics/inc/statisticsfilterset.inc.php
@@ -9,7 +9,10 @@ class StatisticsFilterSet
 
 	private $cache = false;
 
-	public function __construct($filters)
+	/**
+	 * @param DatabaseFilter[] $filters
+	 */
+	public function __construct(array $filters)
 	{
 		$this->filters = $filters;
 	}
@@ -49,9 +52,9 @@ class StatisticsFilterSet
 
 	/**
 	 * @param string $type filter type (class name)
-	 * @return ?DatabaseFilter The filter, false if not found
+	 * @return ?DatabaseFilter The filter, null if not found
 	 */
-	public function hasFilter(string $type)
+	public function hasFilter(string $type): ?DatabaseFilter
 	{
 		foreach ($this->filters as $filter) {
 			if ($filter->isClass($type)) {
@@ -63,9 +66,9 @@ class StatisticsFilterSet
 
 	/**
 	 * @param string $type filter type key/id
-	 * @return ?DatabaseFilter The filter, false if not found
+	 * @return ?DatabaseFilter The filter, null if not found
 	 */
-	public function hasFilterKey(string $type)
+	public function hasFilterKey(string $type): ?DatabaseFilter
 	{
 		if (isset($this->filters[$type]))
 			return $this->filters[$type];
@@ -102,8 +105,8 @@ class StatisticsFilterSet
 	 */
 	public function getAllowedLocations()
 	{
-		if (isset($this->filters['permissions']->argument) && is_array($this->filters['permissions']->argument))
-			return $this->filters['permissions']->argument;
+		if (isset($this->filters['permissions']) && is_array($this->filters['permissions']->argument))
+			return (array)$this->filters['permissions']->argument;
 		return false;
 	}
 
diff --git a/modules-available/statistics/inc/statisticshooks.inc.php b/modules-available/statistics/inc/statisticshooks.inc.php
index de20d599..b17cf4a9 100644
--- a/modules-available/statistics/inc/statisticshooks.inc.php
+++ b/modules-available/statistics/inc/statisticshooks.inc.php
@@ -5,7 +5,7 @@ class StatisticsHooks
 
 	private static $row = false;
 
-	private static function getRow($machineuuid)
+	private static function getRow(string $machineuuid)
 	{
 		if (self::$row !== false)
 			return;
@@ -13,7 +13,7 @@ class StatisticsHooks
 			['machineuuid' => $machineuuid]);
 	}
 
-	public static function getBaseconfigName($machineuuid)
+	public static function getBaseconfigName(string $machineuuid): string
 	{
 		self::getRow($machineuuid);
 		if (self::$row === false)
@@ -21,7 +21,7 @@ class StatisticsHooks
 		return self::$row['hostname'] ?: self::$row['clientip'];
 	}
 
-	public static function baseconfigLocationResolver($machineuuid)
+	public static function baseconfigLocationResolver(string $machineuuid): int
 	{
 		self::getRow($machineuuid);
 		if (self::$row === false)
@@ -30,10 +30,11 @@ class StatisticsHooks
 	}
 
 	/**
-	 * Hook to get inheritance tree for all config vars
-	 * @param int $machineuuid MachineUUID currently being edited
+	 * Hook to get inheritance tree for all config vars.
+	 *
+	 * @param string $machineuuid MachineUUID currently being edited
 	 */
-	public static function baseconfigInheritance($machineuuid)
+	public static function baseconfigInheritance(string $machineuuid): array
 	{
 		self::getRow($machineuuid);
 		if (self::$row === false)
diff --git a/modules-available/statistics/inc/statisticsstyling.inc.php b/modules-available/statistics/inc/statisticsstyling.inc.php
index 1843fe30..0e158026 100644
--- a/modules-available/statistics/inc/statisticsstyling.inc.php
+++ b/modules-available/statistics/inc/statisticsstyling.inc.php
@@ -3,7 +3,7 @@
 class StatisticsStyling
 {
 
-	public static function ramColorClass($mb)
+	public static function ramColorClass(int $mb): string
 	{
 		if ($mb < 2500) {
 			return 'danger';
@@ -15,7 +15,7 @@ class StatisticsStyling
 		return '';
 	}
 
-	public static function kvmColorClass($state)
+	public static function kvmColorClass(string $state): string
 	{
 		if ($state === 'DISABLED') {
 			return 'danger';
@@ -27,7 +27,7 @@ class StatisticsStyling
 		return '';
 	}
 
-	public static function hddColorClass($gb)
+	public static function hddColorClass(int $gb): string
 	{
 		if ($gb < 7) {
 			return 'danger';