From 06bff0b9b84d47c43f9bc8aff06a29d85ebb7ed0 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Tue, 14 Nov 2023 14:47:55 +0100 Subject: Add function param/return types, fix a lot more phpstorm complaints --- .../statistics/inc/hardwareinfo.inc.php | 3 +- .../statistics/inc/hardwareparser.inc.php | 26 +++++--------- .../statistics/inc/hardwareparserlegacy.inc.php | 5 +-- .../statistics/inc/hardwarequery.inc.php | 40 +++++++++------------- .../statistics/inc/statistics.inc.php | 15 ++++---- .../statistics/inc/statisticsfilter.inc.php | 23 +++++-------- .../statistics/inc/statisticsfilterset.inc.php | 17 +++++---- .../statistics/inc/statisticshooks.inc.php | 13 +++---- .../statistics/inc/statisticsstyling.inc.php | 6 ++-- 9 files changed, 63 insertions(+), 85 deletions(-) (limited to 'modules-available/statistics/inc') 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'; -- cgit v1.2.3-55-g7522