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 --- .../roomplanner/inc/composedroom.inc.php | 21 ++++---- .../roomplanner/inc/pvsgenerator.inc.php | 14 +++-- modules-available/roomplanner/inc/room.inc.php | 49 ++++++++--------- .../roomplanner/inc/simpleroom.inc.php | 18 ++++--- modules-available/roomplanner/page.inc.php | 61 ++++++++++------------ 5 files changed, 79 insertions(+), 84 deletions(-) (limited to 'modules-available/roomplanner') diff --git a/modules-available/roomplanner/inc/composedroom.inc.php b/modules-available/roomplanner/inc/composedroom.inc.php index cdd50984..3ee892db 100644 --- a/modules-available/roomplanner/inc/composedroom.inc.php +++ b/modules-available/roomplanner/inc/composedroom.inc.php @@ -98,7 +98,7 @@ class ComposedRoom extends Room return $this->orientation; } - public function subLocationIds() + public function subLocationIds(): array { return $this->list; } @@ -108,7 +108,7 @@ class ComposedRoom extends Room return $this->controlRoom; } - public function machineCount() + public function machineCount(): int { $sum = 0; foreach ($this->list as $lid) { @@ -117,10 +117,9 @@ class ComposedRoom extends Room return $sum; } - public function getSize(&$width, &$height) + public function getSize(?int &$width, ?int &$height) { $horz = ($this->orientation == 'horizontal'); - $width = $height = 0; foreach ($this->list as $locId) { self::$rooms[$locId]->getSize($w, $h); $width = $horz ? $width + $w : max($width, $w); @@ -128,7 +127,7 @@ class ComposedRoom extends Room } } - public function getIniClientSection(&$i, $offX = 0, $offY = 0) + public function getIniClientSection(int &$i, int $offX = 0, int $offY = 0) { if (!$this->enabled) return false; @@ -154,10 +153,10 @@ class ComposedRoom extends Room return $out; } - public function getShiftedArray($offX = 0, $offY = 0) + public function getShiftedArray(int $offX = 0, int $offY = 0): ?array { if (!$this->enabled) - return false; + return null; if ($this->orientation == 'horizontal') { $x = 1; $y = 0; @@ -168,7 +167,7 @@ class ComposedRoom extends Room $ret = []; foreach ($this->list as $locId) { $new = self::$rooms[$locId]->getShiftedArray($offX, $offY); - if ($new !== false) { + if ($new !== null) { $ret = array_merge($ret, $new); self::$rooms[$locId]->getSize($w, $h); $offX += $w * $x; @@ -176,7 +175,7 @@ class ComposedRoom extends Room } } if (empty($ret)) - return false; + return null; return $ret; } @@ -194,12 +193,12 @@ class ComposedRoom extends Room return false; } - public function isLeaf() + public function isLeaf(): bool { return false; } - public function shouldSkip() + public function shouldSkip(): bool { return !$this->enabled; } diff --git a/modules-available/roomplanner/inc/pvsgenerator.inc.php b/modules-available/roomplanner/inc/pvsgenerator.inc.php index f23dcb20..f3c5c838 100644 --- a/modules-available/roomplanner/inc/pvsgenerator.inc.php +++ b/modules-available/roomplanner/inc/pvsgenerator.inc.php @@ -3,7 +3,7 @@ class PvsGenerator { - public static function generate() + public static function generate(): string { /* collect names and build room blocks - filter empty rooms while at it */ $roomNames = array(); @@ -36,7 +36,7 @@ class PvsGenerator * @param Room $room room/location data as fetched from db * @return string|false .ini section for room, or false if room is empty */ - private static function generateRoomBlock($room) + private static function generateRoomBlock(Room $room) { $room->getSize($sizeX, $sizeY); if ($sizeX === 0 || $sizeY === 0) @@ -91,13 +91,13 @@ class PvsGenerator } // Load room $room = Room::get($locationId); - if ($room === false) + if ($room === null) return false; $room->getSize($sizeX, $sizeY); if ($sizeX === 0 || $sizeY === 0) return false; // Empty - $machines = $room->getShiftedArray(); + $machines = $room->getShiftedArray() ?? []; $ORIENTATION = ['north' => 2, 'east' => 3, 'south' => 0, 'west' => 1]; if (is_string($highlightUuid)) { $highlightUuid = strtoupper($highlightUuid); @@ -176,14 +176,12 @@ class PvsGenerator /** * Get display name for manager of given locationId. * Hook for "runmode" module to resolve mode name. - * @param $locationId - * @return bool|string */ - public static function getManagerName($locationId) + public static function getManagerName(int $locationId): ?string { $names = Location::getNameChain($locationId); if ($names === false) - return false; + return null; return implode(' / ', $names); } diff --git a/modules-available/roomplanner/inc/room.inc.php b/modules-available/roomplanner/inc/room.inc.php index 1a7a80ae..880cb6d0 100644 --- a/modules-available/roomplanner/inc/room.inc.php +++ b/modules-available/roomplanner/inc/room.inc.php @@ -29,10 +29,10 @@ abstract class Room FROM location_roomplan lr LEFT JOIN machine m ON (lr.tutoruuid = m.machineuuid)'); foreach ($ret as $row) { - $row = self::loadSingleRoom($row); - if ($row === false) + $room = self::loadSingleRoom($row); + if ($room === null) continue; - self::$rooms[$row->locationId] = $row; + self::$rooms[$room->locationId] = $room; } foreach (self::$rooms as $room) { $room->sanitize(); @@ -42,14 +42,14 @@ abstract class Room /** * Instantiate ComposedRoom or MachineGroup depending on contents of $row * @param array $row DB row from location_roomplan. - * @return Room|false Room instance, false on error + * @return ?Room Room instance, null on error */ - private static function loadSingleRoom($row) + private static function loadSingleRoom(array $row): ?Room { $locations = Location::getLocationsAssoc(); settype($row['locationid'], 'int'); if (!isset($locations[$row['locationid']])) - return false; + return null; if ($locations[$row['locationid']]['isleaf']) return new SimpleRoom($row); return new ComposedRoom($row, false); @@ -59,7 +59,7 @@ abstract class Room * Get array of all rooms with room plan * @return Room[] */ - public static function getAll() + public static function getAll(): array { self::init(); return self::$rooms; @@ -68,9 +68,9 @@ abstract class Room /** * Get room instance for given location * @param int $locationId room to get - * @return Room|false requested room, false if not configured or not found + * @return ?Room requested room, false if not configured or not found */ - public static function get($locationId) + public static function get(int $locationId): ?Room { if (self::$rooms === null) { $room = Database::queryFirst( @@ -79,8 +79,10 @@ abstract class Room LEFT JOIN machine m ON (lr.tutoruuid = m.machineuuid) WHERE lr.locationid = :lid', ['lid' => $locationId]); if ($room === false) - return false; + return null; $room = self::loadSingleRoom($room); + if ($room === null) + return null; // If it's a leaf room we probably don't need any other rooms, return it if ($room->isLeaf()) return $room; @@ -89,7 +91,7 @@ abstract class Room } if (isset(self::$rooms[$locationId])) return self::$rooms[$locationId]; - return false; + return null; } public function __construct($row) @@ -102,35 +104,34 @@ abstract class Room /** * @return int number of machines in this room */ - abstract public function machineCount(); + abstract public function machineCount(): int; /** * Size of this room, returned by reference. - * @param int $width OUT width of room - * @param int $height OUT height of room + * + * @param int|null $width OUT width of room + * @param int|null $height OUT height of room */ - abstract public function getSize(&$width, &$height); + abstract public function getSize(?int &$width, ?int &$height); /** * Get clients in this room in .ini format for PVS. * Adjusted so the top/left client is at (0|0), which * is further adjustable with $offX and $offY. + * * @param int $i offset for indexing clients * @param int $offX positional X offset for clients * @param int $offY positional Y offset for clients * @return string|false */ - abstract public function getIniClientSection(&$i, $offX = 0, $offY = 0); + abstract public function getIniClientSection(int &$i, int $offX = 0, int $offY = 0); /** * Get clients in this room as array. * Adjusted so the top/left client is at (0|0), which *is further adjustable with $offX and $offY. - * @param int $offX - * @param int $offY - * @return array */ - abstract public function getShiftedArray($offX = 0, $offY = 0); + abstract public function getShiftedArray(int $offX = 0, int $offY = 0): ?array; /** * @return string|false IP address of manager. @@ -145,12 +146,12 @@ abstract class Room /** * @return bool true if this is a simple/leaf room, false for composed rooms. */ - abstract public function isLeaf(); + abstract public function isLeaf(): bool; /** * @return bool should this room be skipped from output? true for empty SimpleRoom or disabled ComposedRoom. */ - abstract public function shouldSkip(); + abstract public function shouldSkip(): bool; /** * Sanitize this room's data. @@ -160,7 +161,7 @@ abstract class Room /** * @return string get room's name. */ - public function locationName() + public function locationName(): string { return $this->locationName; } @@ -168,7 +169,7 @@ abstract class Room /** * @return int get room's id. */ - public function locationId() + public function locationId(): int { return $this->locationId; } diff --git a/modules-available/roomplanner/inc/simpleroom.inc.php b/modules-available/roomplanner/inc/simpleroom.inc.php index 43ae43ca..b4d3e744 100644 --- a/modules-available/roomplanner/inc/simpleroom.inc.php +++ b/modules-available/roomplanner/inc/simpleroom.inc.php @@ -11,6 +11,7 @@ class SimpleRoom extends Room private $tutorIp = false; + /** @var ?string */ private $managerIp = false; public function __construct($row) @@ -55,27 +56,29 @@ class SimpleRoom extends Room } } - public function machineCount() + public function machineCount(): int { return count($this->machines); } - public function getSize(&$width, &$height) + public function getSize(?int &$width, ?int &$height) { if (empty($this->machines)) { $width = $height = 0; return; } + $minX = $minY = $maxX = $maxY = 0; $this->boundingBox($minX, $minY, $maxX, $maxY); // client's size that cannot be configured as of today $width = max($maxX - $minX + self::CLIENT_SIZE, 1); $height = max($maxY - $minY + self::CLIENT_SIZE, 1); } - public function getIniClientSection(&$i, $offX = 0, $offY = 0) + public function getIniClientSection(int &$i, int $offX = 0, int $offY = 0): string { /* output individual client positions, shift coordinates to requested position */ $out = ''; + $minX = $minY = $maxX = $maxY = 0; $this->boundingBox($minX, $minY, $maxX, $maxY); foreach ($this->machines as $pos) { $i++; @@ -86,10 +89,11 @@ class SimpleRoom extends Room return $out; } - public function getShiftedArray($offX = 0, $offY = 0) + public function getShiftedArray(int $offX = 0, int $offY = 0): ?array { /* output individual client positions, shift coordinates to requested position */ $ret = []; + $minX = $minY = $maxX = $maxY = 0; $this->boundingBox($minX, $minY, $maxX, $maxY); foreach ($this->machines as $pos) { $pos['gridCol'] += $offX - $minX; @@ -100,7 +104,7 @@ class SimpleRoom extends Room return $ret; } - private function boundingBox(&$minX, &$minY, &$maxX, &$maxY) + private function boundingBox(int &$minX, int &$minY, int &$maxX, int &$maxY): void { if ($this->bb !== false) { $minX = $this->bb[0]; @@ -130,12 +134,12 @@ class SimpleRoom extends Room return $this->tutorIp; } - public function isLeaf() + public function isLeaf(): bool { return true; } - public function shouldSkip() + public function shouldSkip(): bool { return empty($this->machines); } diff --git a/modules-available/roomplanner/page.inc.php b/modules-available/roomplanner/page.inc.php index 7ac6890d..53e8bf0b 100644 --- a/modules-available/roomplanner/page.inc.php +++ b/modules-available/roomplanner/page.inc.php @@ -82,22 +82,17 @@ class Page_Roomplanner extends Page $config = Database::queryFirst('SELECT roomplan, managerip, tutoruuid FROM location_roomplan WHERE locationid = :locationid', ['locationid' => $this->locationid]); + if ($config === false) { + $config = ['dedicatedmgr' => false, 'managerip' => '']; + } $runmode = RunMode::getForMode(Page::getModule(), $this->locationid, true); - if (empty($runmode)) { - $config['dedicatedmgr'] = false; - } else { + if (!empty($runmode)) { $runmode = array_pop($runmode); $config['managerip'] = $runmode['clientip']; $config['manageruuid'] = $runmode['machineuuid']; $data = json_decode($runmode['modedata'], true); $config['dedicatedmgr'] = (isset($data['dedicatedmgr']) && $data['dedicatedmgr']); } - if ($config !== false) { - $managerIp = $config['managerip'] ?? ''; - $dediMgr = $config['dedicatedmgr'] ? 'checked' : ''; - } else { - $dediMgr = $managerIp = ''; - } $furniture = $this->getFurniture($config); $subnetMachines = $this->getPotentialMachines(); $machinesOnPlan = $this->getMachinesOnPlan($config['tutoruuid'] ?? ''); @@ -105,8 +100,8 @@ class Page_Roomplanner extends Page $canEdit = User::hasPermission('edit', $this->locationid); $params = [ 'location' => $this->location, - 'managerip' => $managerIp, - 'dediMgrChecked' => $dediMgr, + 'managerip' => $config['managerip'], + 'dediMgrChecked' => $config['dedicatedmgr'] ? 'checked' : '', 'subnetMachines' => json_encode($subnetMachines), 'locationid' => $this->locationid, 'roomConfiguration' => json_encode($roomConfig), @@ -229,11 +224,9 @@ class Page_Roomplanner extends Page if ($leaf !== $this->isLeaf) { if ($isAjax) { die('Leaf mode mismatch. Did you restructure locations while editing this room?'); - } else { - Message::addError('leaf-mode-mismatch'); - Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } - return; + Message::addError('leaf-mode-mismatch'); + Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } if ($this->isLeaf) { $this->saveLeafRoom($isAjax); @@ -250,10 +243,9 @@ class Page_Roomplanner extends Page if (!is_array($config) || !isset($config['furniture']) || !isset($config['computers'])) { if ($isAjax) { die('JSON data incomplete'); - } else { - Message::addError('json-data-invalid'); - Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } + Message::addError('json-data-invalid'); + Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } $tutorUuid = Request::post('tutoruuid', '', 'string'); if (empty($tutorUuid)) { @@ -263,10 +255,9 @@ class Page_Roomplanner extends Page if ($ret === false) { if ($isAjax) { die('Invalid tutor UUID'); - } else { - Message::addError('invalid-tutor-uuid'); - Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } + Message::addError('invalid-tutor-uuid'); + Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } } $this->saveRoomConfig($config['furniture'], $tutorUuid); @@ -282,10 +273,9 @@ class Page_Roomplanner extends Page if ($res === false) { if ($isAjax) { die('Error writing config to DB'); - } else { - Message::addError('db-error'); - Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } + Message::addError('db-error'); + Util::redirect("?do=roomplanner&locationid={$this->locationid}&action=show"); } } @@ -302,7 +292,7 @@ class Page_Roomplanner extends Page * @param array $computers Deserialized json from browser with all the computers * @param array $oldComputers Deserialized old roomplan from database, used to find removed computers */ - protected function saveComputerConfig($computers, $oldComputers) + protected function saveComputerConfig(array $computers, array $oldComputers) { $oldUuids = []; @@ -323,12 +313,12 @@ class Page_Roomplanner extends Page if (!isset($computer['gridRow'])) { $computer['gridRow'] = 0; } else { - $this->sanitizeNumber($computer['gridRow'], 0, 32 * 4); + Util::clamp($computer['gridRow'], 0, 32 * 4); } if (!isset($computer['gridCol'])) { $computer['gridCol'] = 0; } else { - $this->sanitizeNumber($computer['gridCol'], 0, 32 * 4); + Util::clamp($computer['gridCol'], 0, 32 * 4); } $position = json_encode(['gridRow' => $computer['gridRow'], @@ -374,17 +364,20 @@ class Page_Roomplanner extends Page } } - protected function getFurniture($config) + protected function getFurniture(array $config): array { - if ($config === false) - return array(); - $config = json_decode($config['roomplan'] ?? '', true); + if (empty($config['roomplan'])) + return []; + $config = json_decode($config['roomplan'], true); if (!is_array($config)) - return array(); + return []; return $config; } - protected function getMachinesOnPlan($tutorUuid) + /** + * @return array{computers: array} + */ + protected function getMachinesOnPlan(string $tutorUuid): array { $result = Database::simpleQuery('SELECT machineuuid, macaddr, clientip, hostname, position FROM machine @@ -418,7 +411,7 @@ class Page_Roomplanner extends Page return ['computers' => $machines]; } - protected function getPotentialMachines() + protected function getPotentialMachines(): array { $result = Database::simpleQuery('SELECT m.machineuuid, m.macaddr, m.clientip, m.hostname, l.locationname AS otherroom, m.fixedlocationid FROM machine m -- cgit v1.2.3-55-g7522