diff options
| author | Simon Rettberg | 2023-11-02 19:08:58 +0100 |
|---|---|---|
| committer | Simon Rettberg | 2023-11-02 19:08:58 +0100 |
| commit | c2d95ffaeaa289752b4c7b6664b6ca112a02e350 (patch) | |
| tree | 2c29b0157d14198aadbdbc9f8c18ded0bd4dadcd | |
| parent | [rebootcontrol] Remove API prototype (diff) | |
| download | slx-admin-c2d95ffaeaa289752b4c7b6664b6ca112a02e350.tar.gz slx-admin-c2d95ffaeaa289752b4c7b6664b6ca112a02e350.tar.xz slx-admin-c2d95ffaeaa289752b4c7b6664b6ca112a02e350.zip | |
Roundup of issues reported by PHPStorm
Mostly redundant checks, logic errors, dead code, etc.
57 files changed, 172 insertions, 252 deletions
diff --git a/Mustache/Autoloader.php b/Mustache/Autoloader.php index e8ea3f4a..c4cb6d96 100644 --- a/Mustache/Autoloader.php +++ b/Mustache/Autoloader.php @@ -53,7 +53,7 @@ class Mustache_Autoloader */ public static function register($baseDir = null) { - $key = $baseDir ? $baseDir : 0; + $key = $baseDir ?: 0; if (!isset(self::$instances[$key])) { self::$instances[$key] = new self($baseDir); diff --git a/inc/event.inc.php b/inc/event.inc.php index 57b4871e..e622f74f 100644 --- a/inc/event.inc.php +++ b/inc/event.inc.php @@ -64,7 +64,7 @@ class Event } else { $res = Taskmanager::waitComplete($ipxeId, 5000); if (Taskmanager::isFailed($res)) { - EventLog::failure('Update PXE Menu failed', $res['data']['error'] ?? $res['data']['error'] ?? ''); + EventLog::failure('Update PXE Menu failed', $res['data']['error'] ?? $res['statusCode'] ?? ''); $everythingFine = false; } } diff --git a/inc/module.inc.php b/inc/module.inc.php index ea3af1ba..042ea0c0 100644 --- a/inc/module.inc.php +++ b/inc/module.inc.php @@ -173,7 +173,7 @@ class Module if (isset($json['category']) && is_string($json['category'])) { $this->category = $json['category']; } - $this->collapse = isset($json['collapse']) && (bool)$json['collapse']; + $this->collapse = isset($json['collapse']) && $json['collapse']; if (isset($json['client-plugin'])) { $this->clientPlugin = (bool)$json['client-plugin']; } diff --git a/inc/request.inc.php b/inc/request.inc.php index bdbd32d5..7cf4a73f 100644 --- a/inc/request.inc.php +++ b/inc/request.inc.php @@ -75,7 +75,7 @@ class Request } return $default; } - if ($default === self::REQUIRED && is_string($array[$key]) && $array[$key] === '') { + if ($default === self::REQUIRED && $array[$key] === '') { Message::addError('main.parameter-empty', $key); Util::redirect('?do=' . $_REQUEST['do']); } diff --git a/inc/taskmanagercallback.inc.php b/inc/taskmanagercallback.inc.php index 84754de0..7c1e965e 100644 --- a/inc/taskmanagercallback.inc.php +++ b/inc/taskmanagercallback.inc.php @@ -180,7 +180,6 @@ class TaskmanagerCallback unset($data['storetype']); Property::setVmStoreConfig($data); } - return; } } diff --git a/install.php b/install.php index e403ad7e..d411d834 100644 --- a/install.php +++ b/install.php @@ -142,8 +142,7 @@ function tableExists($table) } function tableRename($old, $new) { - $res = Database::simpleQuery("RENAME TABLE $old TO $new", []); - return $res; + return Database::simpleQuery("RENAME TABLE $old TO $new", []); } diff --git a/modules-available/adduser/page.inc.php b/modules-available/adduser/page.inc.php index e81e9fb2..499436ff 100644 --- a/modules-available/adduser/page.inc.php +++ b/modules-available/adduser/page.inc.php @@ -32,10 +32,8 @@ class Page_AddUser extends Page $email = Request::post('email', '', 'string'); if (empty($login) || empty($pass1) || empty($pass2) || empty($fullname)) { Message::addError('main.empty-field'); - return; } elseif ($pass1 !== $pass2) { Message::addError('password-mismatch'); - return; } else { if (Database::queryFirst('SELECT userid FROM user LIMIT 1') !== false) { User::assertPermission('user.add'); @@ -69,7 +67,6 @@ class Page_AddUser extends Page } Message::addInfo('adduser-success'); $this->saveRoles($id); - return; } } diff --git a/modules-available/baseconfig/inc/validator.inc.php b/modules-available/baseconfig/inc/validator.inc.php index 7bbccc6d..ee883895 100644 --- a/modules-available/baseconfig/inc/validator.inc.php +++ b/modules-available/baseconfig/inc/validator.inc.php @@ -77,18 +77,19 @@ class Validator /** * Validate value against list. + * * @param string $list The list as a string of items, separated by "|" * @param string $displayValue The value to validate * @return boolean|string The value, if in list, false otherwise */ - private static function validateList($list, &$displayValue) + private static function validateList(string $list, string $displayValue) { $list = explode('|', $list); if (in_array($displayValue, $list)) return $displayValue; return false; } - private static function validateMultiList($list, &$displayValue) + private static function validateMultiList(string $list, string &$displayValue): string { $allowedValues = explode('|', $list); $values = []; @@ -101,7 +102,7 @@ class Validator return $displayValue; } - private static function validateMultiInput($list, &$displayValue) + private static function validateMultiInput($list, $displayValue) { return $displayValue; } diff --git a/modules-available/baseconfig/page.inc.php b/modules-available/baseconfig/page.inc.php index a1921084..8a9a7534 100644 --- a/modules-available/baseconfig/page.inc.php +++ b/modules-available/baseconfig/page.inc.php @@ -120,12 +120,11 @@ class Page_BaseConfig extends Page User::assertPermission('view', $lid); $editForbidden = !User::hasPermission('edit', $lid); // Get stuff that's set in DB already + $fields = ''; if ($this->targetModule !== false && isset($this->qry_extra['field'])) { - $fields = ''; $where = " WHERE {$this->qry_extra['field']} = :field_value"; $params = array('field_value' => $this->qry_extra['field_value']); } else { - $fields = ''; $where = ''; $params = array(); } @@ -136,7 +135,7 @@ class Page_BaseConfig extends Page // Remember missing variables $missing = $varsFromJson; // Populate structure with existing config from db - $this->fillSettings($varsFromJson, $settings, $missing, $this->qry_extra['table'], $fields, $where, $params, false); + $this->fillSettings($varsFromJson, $settings, $missing, $this->qry_extra['table'], $fields, $where, $params); // Add entries that weren't in the db (global), setup override checkbox (module specific) foreach ($varsFromJson as $key => $var) { if ($this->targetModule !== false && !isset($settings[$var['catid']]['settings'][$key])) { @@ -195,7 +194,7 @@ class Page_BaseConfig extends Page ) + $this->qry_extra); } - private function fillSettings($vars, &$settings, &$missing, $table, $fields, $where, $params, $sourceName) + private function fillSettings($vars, &$settings, &$missing, $table, $fields, $where, $params): void { $res = Database::simpleQuery("SELECT setting, value, displayvalue $fields FROM $table " . " {$where} ORDER BY setting ASC", $params); @@ -203,7 +202,7 @@ class Page_BaseConfig extends Page if (!isset($missing[$row['setting']])) continue; if (!isset($vars[$row['setting']]) || !is_array($vars[$row['setting']])) { - $unknown[] = $row['setting']; + //$unknown[] = $row['setting']; continue; } unset($missing[$row['setting']]); diff --git a/modules-available/dnbd3/baseconfig/getconfig.inc.php b/modules-available/dnbd3/baseconfig/getconfig.inc.php index b9091a34..eecb4642 100644 --- a/modules-available/dnbd3/baseconfig/getconfig.inc.php +++ b/modules-available/dnbd3/baseconfig/getconfig.inc.php @@ -36,7 +36,7 @@ foreach ($res as $row) { } else { $defPrio = 1000; } - $ip = $row['fixedip'] ? $row['fixedip'] : $row['clientip']; + $ip = $row['fixedip'] ?: $row['clientip']; // See if this server is meant for the client at all if (!is_null($row['locationid']) && !isset($locationsAssoc[$row['locationid']])) { $fallback[$ip] = true; diff --git a/modules-available/dnbd3/hooks/main-warning.inc.php b/modules-available/dnbd3/hooks/main-warning.inc.php index bee0a258..ead0a259 100644 --- a/modules-available/dnbd3/hooks/main-warning.inc.php +++ b/modules-available/dnbd3/hooks/main-warning.inc.php @@ -7,7 +7,7 @@ if (Dnbd3::isEnabled() && User::hasPermission('.dnbd3.access-page')) { WHERE errormsg IS NOT NULL'); foreach ($res as $row) { - $error = $row['errormsg'] ? $row['errormsg'] : '<unknown error>'; + $error = $row['errormsg'] ?: '<unknown error>'; $lastSeen = Util::prettyTime($row['dnbd3lastseen']); if ($row['fixedip'] === '<self>') { Message::addError('dnbd3.main-dnbd3-unreachable', true, $error, $lastSeen); diff --git a/modules-available/dnbd3/inc/dnbd3util.inc.php b/modules-available/dnbd3/inc/dnbd3util.inc.php index 9aaa5432..90940e8a 100644 --- a/modules-available/dnbd3/inc/dnbd3util.inc.php +++ b/modules-available/dnbd3/inc/dnbd3util.inc.php @@ -165,7 +165,7 @@ class Dnbd3Util { $private = array(); $public[$self] = $self; foreach ($res as $row) { - $ip = $row['fixedip'] ? $row['fixedip'] : $row['clientip']; + $ip = $row['fixedip'] ?: $row['clientip']; if ($ip === '<self>') { continue; } @@ -252,7 +252,7 @@ class Dnbd3Util { // $row['startaddr'] must lie before range start, otherwise we'd have hit the case above $row['endaddr'] = $ranges[$key]['endaddr']; unset($ranges[$key]); - continue; + //continue; } } $ranges[] = $row; diff --git a/modules-available/dnbd3/page.inc.php b/modules-available/dnbd3/page.inc.php index d60b3dfa..35f03199 100644 --- a/modules-available/dnbd3/page.inc.php +++ b/modules-available/dnbd3/page.inc.php @@ -34,7 +34,7 @@ class Page_Dnbd3 extends Page private function editServer() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); if (!isset($server['machineuuid'])) { Message::addError('not-automatic-server', $server['ip']); return; @@ -45,12 +45,15 @@ class Page_Dnbd3 extends Page $overrideIp = false; $sip = Request::post('fixedip', null, 'string'); if (empty($sip)) { + // Reset IP override $overrideIp = null; - } elseif ($server['fixedip'] !== $overrideIp) { + } elseif ($server['fixedip'] !== $sip) { + // IP override is set and different from current value if (Dnbd3Util::matchAddress($sip) === false) { Message::addError('invalid-ip', $sip); return; } + // Dupcheck $res = Database::queryFirst('SELECT serverid FROM dnbd3_server s LEFT JOIN machine m USING (machineuuid) WHERE s.fixedip = :ip OR m.clientip = :ip', ['ip' => $sip]); @@ -90,7 +93,7 @@ class Page_Dnbd3 extends Page private function saveServerLocations() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); $this->assertPermission($server); $locids = Request::post('location', [], 'array'); if (empty($locids)) { @@ -131,7 +134,7 @@ class Page_Dnbd3 extends Page private function deleteServer() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); $this->assertPermission($server); if ($server['fixedip'] === '<self>') return; @@ -264,7 +267,7 @@ class Page_Dnbd3 extends Page { User::assertPermission('view.details'); Module::isAvailable('js_stupidtable'); - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); Render::addTemplate('page-proxy-header', $server); $stats = Dnbd3Rpc::query($server['ip'], [Dnbd3Rpc::QUERY_STATS, Dnbd3Rpc::QUERY_CLIENTS, Dnbd3Rpc::QUERY_IMAGES, Dnbd3Rpc::QUERY_SPACE, Dnbd3Rpc::QUERY_CONFIG, Dnbd3Rpc::QUERY_ALTSERVERS]); @@ -411,7 +414,7 @@ class Page_Dnbd3 extends Page private function showServerLocationEdit() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); $this->assertPermission($server); // Get selected ones $res = Database::simpleQuery('SELECT locationid FROM dnbd3_server_x_location WHERE serverid = :serverid', @@ -437,11 +440,9 @@ class Page_Dnbd3 extends Page Render::addTemplate('page-server-locations', $server); } - private function getServerById($serverId = false) + private function getServerFromQuery(): array { - if ($serverId === false) { - $serverId = Request::any('server', false, 'int'); - } + $serverId = Request::any('server', false, 'int'); if ($serverId === false) { if (AJAX) die('Missing parameter'); @@ -533,7 +534,7 @@ class Page_Dnbd3 extends Page private function ajaxEditServer() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); if (!isset($server['machineuuid'])) { echo 'Not automatic server.'; return; @@ -565,7 +566,7 @@ class Page_Dnbd3 extends Page private function ajaxReboot() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); if (!isset($server['machineuuid'])) { die('Not automatic server.'); } @@ -604,7 +605,7 @@ class Page_Dnbd3 extends Page private function ajaxCacheMap() { - $server = $this->getServerById(); + $server = $this->getServerFromQuery(); $imgId = Request::any('id', 0, 'int'); if ($imgId <= 0) { Header('HTTP/1.1 400 Bad Request'); diff --git a/modules-available/dozmod/api.inc.php b/modules-available/dozmod/api.inc.php index 17298bb7..90e663aa 100644 --- a/modules-available/dozmod/api.inc.php +++ b/modules-available/dozmod/api.inc.php @@ -327,18 +327,14 @@ $location_ids = Location::getLocationRootChain($location_ids); if ($resource === 'list') { outputLectureXmlForLocation($location_ids); // Won't return on success... - fatalDozmodUnreachable(); - } elseif ($resource === 'imagemeta') { $image = readImageParam(); outputResource($image, $resource); - fatalDozmodUnreachable(); - } else { $lecture = readLectureParam($location_ids); outputResource($lecture, $resource); - fatalDozmodUnreachable(); } +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 a38c9d9d..bf2a6fa4 100644 --- a/modules-available/dozmod/page.inc.php +++ b/modules-available/dozmod/page.inc.php @@ -78,7 +78,6 @@ class Page_DozMod extends Page /* different pages for different sections */ if ($this->haveSubPage !== false) { SubPage::doRender(); - return; } } @@ -90,7 +89,6 @@ class Page_DozMod extends Page if ($this->haveSubPage !== false) { SubPage::doAjax(); - return; } } diff --git a/modules-available/eventlog/pages/log.inc.php b/modules-available/eventlog/pages/log.inc.php index a48b4a95..bfb16d11 100644 --- a/modules-available/eventlog/pages/log.inc.php +++ b/modules-available/eventlog/pages/log.inc.php @@ -43,12 +43,11 @@ class SubPage private static function typeToColor($type) { switch ($type) { - case 'info': - return ''; case 'warning': return 'orange'; case 'failure': return 'red'; + case 'info': default: return ''; } diff --git a/modules-available/locationinfo/api.inc.php b/modules-available/locationinfo/api.inc.php index ab1f2358..14be7718 100644 --- a/modules-available/locationinfo/api.inc.php +++ b/modules-available/locationinfo/api.inc.php @@ -137,8 +137,7 @@ function getLocationTree($idList) } $locations = Location::getTree(); - $ret = findLocations($locations, $idList); - return $ret; + return findLocations($locations, $idList); } function findLocations($locations, $idList) diff --git a/modules-available/locationinfo/exchange-includes/jamesiarmes/PhpEws/Autodiscover.php b/modules-available/locationinfo/exchange-includes/jamesiarmes/PhpEws/Autodiscover.php index 8198137d..8c60a4c8 100644 --- a/modules-available/locationinfo/exchange-includes/jamesiarmes/PhpEws/Autodiscover.php +++ b/modules-available/locationinfo/exchange-includes/jamesiarmes/PhpEws/Autodiscover.php @@ -891,6 +891,6 @@ class Autodiscover protected function tryViaUrl($url, $timeout = 6) { $result = $this->doNTLMPost($url, $timeout); - return ($result ? true : false); + return (bool)$result; } } diff --git a/modules-available/locationinfo/inc/coursebackend.inc.php b/modules-available/locationinfo/inc/coursebackend.inc.php index 72494214..04b5aa92 100644 --- a/modules-available/locationinfo/inc/coursebackend.inc.php +++ b/modules-available/locationinfo/inc/coursebackend.inc.php @@ -378,8 +378,7 @@ abstract class CourseBackend } return false; } - $array = json_decode(json_encode((array)$xml), true); - return $array; + return json_decode(json_encode((array)$xml), true); } } diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php index 738b5b14..55d5ed4b 100644 --- a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php +++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php @@ -48,8 +48,7 @@ class CourseBackend_HisInOne extends ICalCourseBackend $title = parent::toTitle($event); // His in one seems to prefix *some* (but *not* all) of the lectures by their ID/("Nummer") // No clue what that format is supposed to be, this regex is some guesswork after observing this for a while - $title = preg_replace('#^[0-9][0-9A-ZÄÖÜ]{3,9}-[A-Za-z0-9/_ÄÖÜäöüß.-]{4,30}\s+#u', '', $title); - return $title; + return preg_replace('#^[0-9][0-9A-ZÄÖÜ]{3,9}-[A-Za-z0-9/_ÄÖÜäöüß.-]{4,30}\s+#u', '', $title); } public function checkConnection(): bool diff --git a/modules-available/locationinfo/inc/infopanel.inc.php b/modules-available/locationinfo/inc/infopanel.inc.php index 84a74e80..14d2d949 100644 --- a/modules-available/locationinfo/inc/infopanel.inc.php +++ b/modules-available/locationinfo/inc/infopanel.inc.php @@ -165,7 +165,7 @@ class InfoPanel * @param array $array list of locations, indexed by locationId * @param int[] $idList list of locations */ - public static function appendOpeningTimes(&$array, $idList) + public static function appendOpeningTimes(array &$array, array $idList): void { // First, lets get all the parent ids for the given locations // in case we need to get inherited opening times @@ -207,7 +207,6 @@ class InfoPanel $currentId = $locations[$currentId]['parentlocationid']; } } - return; } diff --git a/modules-available/locationinfo/page.inc.php b/modules-available/locationinfo/page.inc.php index f81fca13..91de0012 100644 --- a/modules-available/locationinfo/page.inc.php +++ b/modules-available/locationinfo/page.inc.php @@ -100,7 +100,7 @@ class Page_LocationInfo extends Page /** * Deletes the server from the db. */ - private function deleteServer($id) + private function deleteServer($id): void { User::assertPermission('backend.edit'); if ($id === 0) { @@ -113,7 +113,7 @@ class Page_LocationInfo extends Page } } - private function deletePanel() + private function deletePanel(): void { $id = Request::post('uuid', false, 'string'); if ($id === false) { @@ -130,7 +130,7 @@ class Page_LocationInfo extends Page } } - private function getTime($str) + private function getTime(string $str): int { $str = explode(':', $str); if (count($str) !== 2) return false; @@ -138,17 +138,17 @@ class Page_LocationInfo extends Page return $str[0] * 60 + $str[1]; } - private function writeLocationConfig() + private function writeLocationConfig(): void { // Check locations $locationid = Request::post('locationid', false, 'int'); if ($locationid === false) { Message::addError('main.parameter-missing', 'locationid'); - return false; + return; } if (Location::get($locationid) === false) { Message::addError('location.invalid-location-id', $locationid); - return false; + return; } User::assertPermission('location.edit', $locationid); @@ -198,29 +198,23 @@ class Page_LocationInfo extends Page )); } } - - return true; } /** * Get all location ids from the locationids parameter, which is comma separated, then split - * and remove any ids that don't exist. The cleaned list will be returned + * and remove any ids that don't exist. The cleaned list will be returned. + * Will show error and redirect to main page if parameter is missing + * * @param bool $failIfEmpty Show error and redirect to main page if parameter is missing or list is empty * @return array list of locations from parameter */ - private function getLocationIdsFromRequest($failIfEmpty) + private function getLocationIdsFromRequest(): array { - $locationids = Request::post('locationids', false, 'string'); - if ($locationids === false) { - if (!$failIfEmpty) - return array(); - Message::addError('main.parameter-missing', 'locationids'); - Util::redirect('?do=locationinfo'); - } + $locationids = Request::post('locationids', Request::REQUIRED_EMPTY, 'string'); $locationids = explode(',', $locationids); $all = array_map(function ($item) { return $item['locationid']; }, Location::queryLocations()); $locationids = array_filter($locationids, function ($item) use ($all) { return in_array($item, $all); }); - if ($failIfEmpty && empty($locationids)) { + if (empty($locationids)) { Message::addError('main.parameter-empty', 'locationids'); Util::redirect('?do=locationinfo'); } @@ -230,7 +224,7 @@ class Page_LocationInfo extends Page /** * Updated the config in the db. */ - private function writePanelConfig() + private function writePanelConfig(): void { // UUID - existing or new $paneluuid = Request::post('uuid', false, 'string'); @@ -276,10 +270,10 @@ class Page_LocationInfo extends Page Util::redirect('?do=locationinfo'); } - private function preparePanelConfigDefault() + private function preparePanelConfigDefault(): array { // Check locations - $locationids = self::getLocationIdsFromRequest(true); + $locationids = self::getLocationIdsFromRequest(); if (count($locationids) > 4) { $locationids = array_slice($locationids, 0, 4); } @@ -331,7 +325,7 @@ class Page_LocationInfo extends Page return array('config' => $conf, 'locationids' => $locationids); } - private function preparePanelConfigUrl() + private function preparePanelConfigUrl(): array { $bookmarkNames = Request::post('bookmarkNames', [], 'array'); $bookmarkUrls = Request::post('bookmarkUrls', [], 'array'); @@ -354,14 +348,14 @@ class Page_LocationInfo extends Page 'split-login' => Request::post('split-login', 0, 'bool'), 'browser' => Request::post('browser', 'slx-browser', 'string'), 'interactive' => Request::post('interactive', '0', 'bool'), - 'bookmarks' => $bookmarkString ? $bookmarkString : '', + 'bookmarks' => $bookmarkString ?: '', 'allow-tty' => Request::post('allow-tty', '', 'string'), 'zoom-factor' => Request::post('zoom-factor', 100, 'int'), ); return array('config' => $conf, 'locationids' => []); } - private function preparePanelConfigSummary() + private function preparePanelConfigSummary(): array { // Build json structure $conf = array( @@ -374,14 +368,14 @@ class Page_LocationInfo extends Page $conf['panelupdate'] = 15; } // Check locations - $locationids = self::getLocationIdsFromRequest(true); + $locationids = self::getLocationIdsFromRequest(); return array('config' => $conf, 'locationids' => $locationids); } /** * Updates the server settings in the db. */ - private function updateServerSettings() + private function updateServerSettings(): void { User::assertPermission('backend.edit'); $serverid = Request::post('id', -1, 'int'); @@ -423,7 +417,7 @@ class Page_LocationInfo extends Page * * @param int $id Server id which connection should be checked. */ - private function checkConnection($serverid = 0) + private function checkConnection(int $serverid = 0): void { if ($serverid === 0) { ErrorHandler::traceError('checkConnection called with no server id'); @@ -449,7 +443,7 @@ class Page_LocationInfo extends Page LocationInfo::setServerError($serverid, $serverInstance->getErrors()); } - private function loadBackends() + private function loadBackends(): array { // Get a list of all the backend types. $servertypes = array(); @@ -487,7 +481,7 @@ class Page_LocationInfo extends Page /** * Show the list of backends */ - private function showBackendsTable($serverlist) + private function showBackendsTable(array $serverlist): void { User::assertPermission('backend.*'); $data = array( @@ -498,7 +492,7 @@ class Page_LocationInfo extends Page Render::addTemplate('page-servers', $data); } - private function showBackendLog() + private function showBackendLog(): void { $id = Request::get('serverid', false, 'int'); if ($id === false) { @@ -523,7 +517,7 @@ class Page_LocationInfo extends Page Render::addTemplate('page-server-log', $server); } - private function showLocationsTable() + private function showLocationsTable(): void { $allowedLocations = User::getAllowedLocations('location.edit'); if (empty($allowedLocations)) { @@ -578,7 +572,7 @@ class Page_LocationInfo extends Page )); } - private function showPanelsTable() + private function showPanelsTable(): void { $visibleLocations = User::getAllowedLocations('panel.list'); if (in_array(0, $visibleLocations)) { @@ -662,7 +656,7 @@ class Page_LocationInfo extends Page * * @param int $id Serverid */ - private function ajaxServerSettings($id) + private function ajaxServerSettings(int $id): void { User::assertPermission('backend.edit'); $oldConfig = Database::queryFirst('SELECT servername, servertype, credentials @@ -711,7 +705,7 @@ class Page_LocationInfo extends Page * * @param int $id id of the location */ - private function ajaxConfigLocation($id) + private function ajaxConfigLocation(int $id): void { User::assertPermission('location.edit', $id); $locConfig = Database::queryFirst("SELECT info.serverid, info.serverlocationid, loc.openingtime @@ -766,12 +760,6 @@ class Page_LocationInfo extends Page echo Render::parse('ajax-config-location', $data); } - private function fmtTime($time) - { - $t = explode(':', $time); - return sprintf('%02d:%02d', $t[0], $t[1]); - } - /** * Checks if simple mode or expert mode is active. * Tries to merge/compact the opening times schedule, and @@ -780,7 +768,7 @@ class Page_LocationInfo extends Page * * @return array new optimized openingtimes */ - private function compressTimes(&$array) + private function compressTimes(array $array): array { if (empty($array)) return []; @@ -825,9 +813,9 @@ class Page_LocationInfo extends Page /** * @param array $daysArray List of days, "Monday", "Tuesday" etc. Must not contain duplicates. - * @return string Human readable representation of list of days + * @return string Human-readable representation of list of days */ - private function buildDaysString(array $daysArray) + private function buildDaysString(array $daysArray): string { /* Dictionary::translate('monday') Dictionary::translate('tuesday') Dictionary::translate('wednesday') * Dictionary::translate('thursday') Dictionary::translate('friday') Dictionary::translate('saturday') @@ -884,7 +872,7 @@ class Page_LocationInfo extends Page // $start must lie before range start, otherwise we'd have hit the case above $e = $current[1]; unset($array[$day][$key]); - continue; + //continue; } } $array[$day][] = array($s, $e); @@ -892,10 +880,8 @@ class Page_LocationInfo extends Page /** * Ajax the config of a panel. - * - * @param $id Location ID */ - private function showPanelConfig() + private function showPanelConfig(): void { $id = Request::get('uuid', false, 'string'); if ($id === false) { @@ -1040,7 +1026,7 @@ class Page_LocationInfo extends Page } } - private function showPanel() + private function showPanel(): void { $uuid = Request::get('uuid', false, 'string'); if ($uuid === false) { @@ -1097,9 +1083,9 @@ class Page_LocationInfo extends Page /** * @param string|array $panelOrUuid UUID of panel, or array with keys paneltype and locationds * @param string $permission - * @param null|int[] $additionalLocations + * @param int[]|null $additionalLocations */ - private function assertPanelPermission($panelOrUuid, $permission, $additionalLocations = null) + private function assertPanelPermission($panelOrUuid, string $permission, array $additionalLocations = null): void { if (is_array($panelOrUuid)) { $panel = $panelOrUuid; diff --git a/modules-available/locations/inc/location.inc.php b/modules-available/locations/inc/location.inc.php index db4580be..f68d6821 100644 --- a/modules-available/locations/inc/location.inc.php +++ b/modules-available/locations/inc/location.inc.php @@ -254,8 +254,7 @@ class Location . 'FROM location ' . 'WHERE parentlocationid = :locationid', ['locationid' => $locationid]); $result = $result['isleaf']; - $result = (bool)$result; - return $result; + return (bool)$result; } public static function extractIds($tree) diff --git a/modules-available/locations/inc/locationutil.inc.php b/modules-available/locations/inc/locationutil.inc.php index acdaed2f..907bcc99 100644 --- a/modules-available/locations/inc/locationutil.inc.php +++ b/modules-available/locations/inc/locationutil.inc.php @@ -48,7 +48,7 @@ class LocationUtil if ($overlapOther) { $overlapOther = array(); foreach ($other as $entry) { - if (!isset($locs[$entry['lid1']]) || !isset($locs[$entry['lid2']])) + if (!isset($locs[$entry['lid1']]) && !isset($locs[$entry['lid2']])) continue; if (in_array($entry['lid1'], $locs[$entry['lid2']]['parents']) || in_array($entry['lid2'], $locs[$entry['lid1']]['parents'])) continue; diff --git a/modules-available/locations/pages/details.inc.php b/modules-available/locations/pages/details.inc.php index 77f96221..e75aaf7d 100644 --- a/modules-available/locations/pages/details.inc.php +++ b/modules-available/locations/pages/details.inc.php @@ -57,7 +57,7 @@ class SubPage $mangled = array(); foreach (array_keys($openingTimes) as $key) { $entry = $openingTimes[$key]; - if (!isset($entry['days']) || !is_array($entry['days']) || empty($entry['days'])) { + if (empty($entry['days']) || !is_array($entry['days'])) { Message::addError('ignored-line-no-days'); continue; } @@ -222,8 +222,6 @@ class SubPage if (!User::hasPermission('location.edit.subnets', $locationId)) return false; - $change = false; - // Deletion first $dels = Request::post('deletesubnet', false); $deleteCount = 0; @@ -242,8 +240,9 @@ class SubPage $starts = Request::post('startaddr', false); $ends = Request::post('endaddr', false); if (!is_array($starts) || !is_array($ends)) { - return $change; + return false; } + $change = false; $editCount = 0; $stmt = Database::prepare('UPDATE subnet SET startaddr = :start, endaddr = :end' . ' WHERE subnetid = :id'); @@ -282,12 +281,12 @@ class SubPage if (!User::hasPermission('location.edit.subnets', $locationId)) return false; - $change = false; $starts = Request::post('newstartaddr', false); $ends = Request::post('newendaddr', false); if (!is_array($starts) || !is_array($ends)) { - return $change; + return false; } + $change = false; $count = 0; $stmt = Database::prepare('INSERT INTO subnet SET startaddr = :start, endaddr = :end, locationid = :location'); foreach ($starts as $key => $start) { @@ -537,7 +536,7 @@ class SubPage // $start must lie before range start, otherwise we'd have hit the case above $e = $current[1]; unset($array[$day][$key]); - continue; + //continue; } } $array[$day][] = array($s, $e); diff --git a/modules-available/main/page.inc.php b/modules-available/main/page.inc.php index baea8350..016a510d 100644 --- a/modules-available/main/page.inc.php +++ b/modules-available/main/page.inc.php @@ -38,7 +38,7 @@ class Page_Main extends Page protected function doAjax() { - User::isLoggedIn(); + User::load(); die('Status: DB running'); } diff --git a/modules-available/minilinux/inc/linuxbootentryhook.inc.php b/modules-available/minilinux/inc/linuxbootentryhook.inc.php index 636f3dfe..03d7f11f 100644 --- a/modules-available/minilinux/inc/linuxbootentryhook.inc.php +++ b/modules-available/minilinux/inc/linuxbootentryhook.inc.php @@ -68,7 +68,7 @@ class LinuxBootEntryHook extends BootEntryHook } $group[] = new HookEntry($version['versionid'], $title, $valid); } - $array[] = new HookEntryGroup($branch['title'] ? $branch['title'] : $branch['branchid'], $group); + $array[] = new HookEntryGroup($branch['title'] ?: $branch['branchid'], $group); } } return $array; diff --git a/modules-available/news/page.inc.php b/modules-available/news/page.inc.php index a122d37c..bb74c711 100644 --- a/modules-available/news/page.inc.php +++ b/modules-available/news/page.inc.php @@ -201,8 +201,6 @@ class Page_News extends Page * * @param int $newsId ID of the news to be shown. * @param string $pageType type if news id is not given. - * - * @return bool true if loading that news worked */ private function loadNews($newsId, $pageType) { @@ -223,18 +221,15 @@ class Page_News extends Page ]); } if ($row === false) - return false; + return; // fetch the news to be shown - if ($row !== false) { - $this->newsId = $row['newsid']; - $this->newsTitle = $row['title']; - $this->newsContent = $row['content']; - $this->newsDateline = (int)$row['dateline']; - $this->newsExpires = (int)$row['expires']; - $this->pageType = $row['type']; - } - return true; + $this->newsId = $row['newsid']; + $this->newsTitle = $row['title']; + $this->newsContent = $row['content']; + $this->newsDateline = (int)$row['dateline']; + $this->newsExpires = (int)$row['expires']; + $this->pageType = $row['type']; } /** diff --git a/modules-available/permissionmanager/inc/permissionutil.inc.php b/modules-available/permissionmanager/inc/permissionutil.inc.php index 20023281..d3f20b4f 100644 --- a/modules-available/permissionmanager/inc/permissionutil.inc.php +++ b/modules-available/permissionmanager/inc/permissionutil.inc.php @@ -132,15 +132,13 @@ class PermissionUtil if (!$cacheAll) break; } - if ($cacheAll) { - $cache[$key][(int)$row['locationid']] = true; - $list = ($row['locationid'] === null) ? array_keys($allLocs) : $allLocs[(int)$row['locationid']]['children']; - foreach ($list as $lid) { - $cache[$key][$lid] = true; - } - if ($row['locationid'] === null) - break; + $cache[$key][(int)$row['locationid']] = true; + $list = ($row['locationid'] === null) ? array_keys($allLocs) : $allLocs[(int)$row['locationid']]['children']; + foreach ($list as $lid) { + $cache[$key][$lid] = true; } + if ($row['locationid'] === null) + break; } } if ($locationid === null) { diff --git a/modules-available/rebootcontrol/page.inc.php b/modules-available/rebootcontrol/page.inc.php index 872f27d4..c189e1fc 100644 --- a/modules-available/rebootcontrol/page.inc.php +++ b/modules-available/rebootcontrol/page.inc.php @@ -101,7 +101,6 @@ class Page_RebootControl extends Page if (Taskmanager::isTask($task)) { Util::redirect("?do=rebootcontrol&show=task&what=task&taskid=" . $task["id"]); } - return; } /** @@ -127,7 +126,6 @@ class Page_RebootControl extends Page if ($this->haveSubpage) { SubPage::doRender(); - return; } } diff --git a/modules-available/rebootcontrol/pages/jumphost.inc.php b/modules-available/rebootcontrol/pages/jumphost.inc.php index 90508e91..d9aae234 100644 --- a/modules-available/rebootcontrol/pages/jumphost.inc.php +++ b/modules-available/rebootcontrol/pages/jumphost.inc.php @@ -32,7 +32,6 @@ class SubPage if ($id !== false) { User::assertPermission('jumphost.edit'); self::deleteJumphost($id); - return; } } diff --git a/modules-available/roomplanner/page.inc.php b/modules-available/roomplanner/page.inc.php index 9f890518..7ac6890d 100644 --- a/modules-available/roomplanner/page.inc.php +++ b/modules-available/roomplanner/page.inc.php @@ -361,10 +361,8 @@ class Page_Roomplanner extends Page 'tutoruuid' => $tutorUuid ]); // See if the client is known, set run-mode - if (empty($managerIp)) { - RunMode::deleteMode(Page::getModule(), $this->locationid); - } else { - RunMode::deleteMode(Page::getModule(), $this->locationid); + RunMode::deleteMode(Page::getModule(), $this->locationid); + if (!empty($managerIp)) { $pc = Statistics::getMachinesByIp($managerIp, Machine::NO_DATA, 'lastseen DESC'); if (!empty($pc)) { $dedicated = (Request::post('dedimgr') === 'on'); diff --git a/modules-available/runmode/inc/runmode.inc.php b/modules-available/runmode/inc/runmode.inc.php index ccc432d2..65b444e7 100644 --- a/modules-available/runmode/inc/runmode.inc.php +++ b/modules-available/runmode/inc/runmode.inc.php @@ -297,7 +297,7 @@ class RunModeModuleConfig */ public $permission = false; - public function __construct($file) + public function __construct(string $file) { $data = json_decode(file_get_contents($file), true); if (!is_array($data)) @@ -313,19 +313,16 @@ class RunModeModuleConfig $this->loadType($data, 'permission', 'string'); } - private function loadType($data, $key, $type) + private function loadType(array $data, string $key, string $type): void { if (!isset($data[$key])) - return false; + return; if (is_string($type) && gettype($data[$key]) !== $type) - return false; - if (is_array($type) && !in_array(gettype($data[$key]), $type)) - return false; + return; $this->{$key} = $data[$key]; - return true; } - public function userHasPermission($locationId) + public function userHasPermission(?int $locationId): bool { return $this->permission === false || User::hasPermission($this->permission, $locationId); } diff --git a/modules-available/serversetup-bwlp-ipxe/inc/bootentryhook.inc.php b/modules-available/serversetup-bwlp-ipxe/inc/bootentryhook.inc.php index 73611b0a..060b3903 100644 --- a/modules-available/serversetup-bwlp-ipxe/inc/bootentryhook.inc.php +++ b/modules-available/serversetup-bwlp-ipxe/inc/bootentryhook.inc.php @@ -94,8 +94,8 @@ abstract class BootEntryHook public function renderExtraFields() { $list = $this->extraFields(); - foreach ($list as &$entry) { - $entry->currentValue = isset($this->data[$entry->name]) ? $this->data[$entry->name] : $entry->default; + foreach ($list as $entry) { + $entry->currentValue = $this->data[$entry->name] ?? $entry->default; $entry->hook = $this; } return $list; diff --git a/modules-available/serversetup-bwlp-ipxe/inc/pxelinux.inc.php b/modules-available/serversetup-bwlp-ipxe/inc/pxelinux.inc.php index 13c9a86c..cc4f9756 100644 --- a/modules-available/serversetup-bwlp-ipxe/inc/pxelinux.inc.php +++ b/modules-available/serversetup-bwlp-ipxe/inc/pxelinux.inc.php @@ -76,7 +76,7 @@ class PxeLinux } $section->helpText = $text; } elseif (self::handleKeyword($key, $val, $sectionPropMap, $section)) { - continue; + //continue; } } if ($section !== null) { diff --git a/modules-available/serversetup-bwlp-ipxe/page.inc.php b/modules-available/serversetup-bwlp-ipxe/page.inc.php index 4e104e78..8d4a366f 100644 --- a/modules-available/serversetup-bwlp-ipxe/page.inc.php +++ b/modules-available/serversetup-bwlp-ipxe/page.inc.php @@ -615,7 +615,7 @@ class Page_ServerSetup extends Page $error = $status['error'] ?? 'Unknown error'; } Render::addTemplate('ipaddress', array( - 'ips' => $this->addrListTask['data']['addresses'], + 'ips' => $this->addrListTask['data']['addresses'] ?? [], 'chooseHintClass' => $this->hasIpSet ? '' : 'alert alert-danger', 'disabled' => ($this->getCompileTask() === false) ? '' : 'disabled', 'versions' => $versions, @@ -626,13 +626,13 @@ class Page_ServerSetup extends Page // ----------------------------------------------------------------------------------------------- - private function getLocalAddresses() + private function getLocalAddresses(): void { $this->addrListTask = Taskmanager::submit('LocalAddressesList', array()); if ($this->addrListTask === false) { $this->addrListTask['data']['addresses'] = false; - return false; + return; } if (!Taskmanager::isFinished($this->addrListTask)) { // TODO: Async if just displaying @@ -641,12 +641,12 @@ class Page_ServerSetup extends Page if (Taskmanager::isFailed($this->addrListTask) || !isset($this->addrListTask['data']['addresses'])) { $this->addrListTask['data']['addresses'] = false; - return false; + return; } $sortIp = array(); foreach (array_keys($this->addrListTask['data']['addresses']) as $key) { - $item = & $this->addrListTask['data']['addresses'][$key]; + $item =& $this->addrListTask['data']['addresses'][$key]; if (!isset($item['ip']) || !preg_match('/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/', $item['ip']) || substr($item['ip'], 0, 4) === '127.') { unset($this->addrListTask['data']['addresses'][$key]); continue; @@ -659,7 +659,6 @@ class Page_ServerSetup extends Page } unset($item); array_multisort($sortIp, SORT_STRING, $this->addrListTask['data']['addresses']); - return true; } private function deleteBootEntry() @@ -836,7 +835,7 @@ class Page_ServerSetup extends Page { $newAddress = Request::post('ip', 'none', 'string'); $valid = false; - foreach ($this->addrListTask['data']['addresses'] as $item) { + foreach ($this->addrListTask['data']['addresses'] ?? [] as $item) { if ($item['ip'] !== $newAddress) continue; $valid = true; diff --git a/modules-available/statistics/inc/statisticsfilter.inc.php b/modules-available/statistics/inc/statisticsfilter.inc.php index 7b00fb12..cfc724c1 100644 --- a/modules-available/statistics/inc/statisticsfilter.inc.php +++ b/modules-available/statistics/inc/statisticsfilter.inc.php @@ -629,15 +629,7 @@ class IpStatisticsFilter extends StatisticsFilter } return 'INET_ATON(clientip) BETWEEN ' . $range['start'] . ' AND ' . $range['end']; } elseif (($num = substr_count($argument, ':')) !== 0 && $num <= 7) { - // IPv6, not yet in DB but let's prepare - if ($num > 7 || strpos($argument, '::') !== false) { // Too many :, or invalid compressed format - Message::addError('invalid-ip-address', $argument); - return '0'; - } elseif ($num <= 7 && substr($argument, -1) === ':') { - $argument .= '*'; - } elseif ($num < 7) { - $argument .= ':*'; - } + // TODO: Probably valid IPv6, not yet in DB } elseif (($num = substr_count($argument, '.')) !== 0 && $num <= 3) { if (substr($argument, -1) === '.') { $argument .= '*'; diff --git a/modules-available/statistics/inc/statisticshooks.inc.php b/modules-available/statistics/inc/statisticshooks.inc.php index 746bdabf..de20d599 100644 --- a/modules-available/statistics/inc/statisticshooks.inc.php +++ b/modules-available/statistics/inc/statisticshooks.inc.php @@ -18,7 +18,7 @@ class StatisticsHooks self::getRow($machineuuid); if (self::$row === false) return false; - return self::$row['hostname'] ? self::$row['hostname'] : self::$row['clientip']; + return self::$row['hostname'] ?: self::$row['clientip']; } public static function baseconfigLocationResolver($machineuuid) diff --git a/modules-available/statistics/pages/replace.inc.php b/modules-available/statistics/pages/replace.inc.php index 29e02292..50bfd6cf 100644 --- a/modules-available/statistics/pages/replace.inc.php +++ b/modules-available/statistics/pages/replace.inc.php @@ -18,7 +18,7 @@ class SubPage private static function handleReplace() { $replace = Request::post('replace', false, 'array'); - if ($replace === false || empty($replace)) { + if (empty($replace)) { Message::addError('main.parameter-empty', 'replace'); return; } diff --git a/modules-available/statistics_reporting/inc/queries.inc.php b/modules-available/statistics_reporting/inc/queries.inc.php index a7e6ce9c..2465e557 100644 --- a/modules-available/statistics_reporting/inc/queries.inc.php +++ b/modules-available/statistics_reporting/inc/queries.inc.php @@ -23,7 +23,7 @@ class Queries foreach ($machines as &$machine) { $machine['medianSessionLength'] = self::calcMedian($machine['sessions']); unset($machine['sessions']); - $machine['clientName'] = $machine['hostname'] ? $machine['hostname'] : $machine['clientip']; + $machine['clientName'] = $machine['hostname'] ?: $machine['clientip']; } return $machines; } @@ -327,22 +327,20 @@ class Queries // User Data: Name, Name(anonymized), Number of Logins public static function getUserStatistics($from, $to, $lowerTimeBound = 0, $upperTimeBound = 24) { - $res = Database::simpleQuery("SELECT username AS name, COUNT(*) AS 'count' + return Database::simpleQuery("SELECT username AS name, COUNT(*) AS 'count' FROM statistic WHERE typeid='.vmchooser-session-name' AND dateline >= $from and dateline <= $to AND FROM_UNIXTIME(dateline, '%H') >= $lowerTimeBound AND FROM_UNIXTIME(dateline, '%H') < $upperTimeBound GROUP BY username"); - return $res; } // Virtual Machine Data: Name, Number of Usages public static function getVMStatistics($from, $to, $lowerTimeBound = 0, $upperTimeBound = 24) { - $res = Database::simpleQuery("SELECT data AS name, COUNT(*) AS 'count' + return Database::simpleQuery("SELECT data AS name, COUNT(*) AS 'count' FROM statistic WHERE typeid='.vmchooser-session-name' AND dateline >= $from and dateline <= $to AND FROM_UNIXTIME(dateline, '%H') >= $lowerTimeBound AND FROM_UNIXTIME(dateline, '%H') < $upperTimeBound GROUP BY data"); - return $res; } public static function getDozmodStats(int $from, int $to) diff --git a/modules-available/statistics_reporting/page.inc.php b/modules-available/statistics_reporting/page.inc.php index c3e469fe..36081763 100644 --- a/modules-available/statistics_reporting/page.inc.php +++ b/modules-available/statistics_reporting/page.inc.php @@ -314,8 +314,7 @@ class Page_Statistics_Reporting extends Page } } // correct indexing of array after deletions - $data = array_values($data); - return $data; + return array_values($data); case 'client': $data = GetData::perClient($flags, Request::any('new', false, 'string')); // only show clients from locations which you have permission for @@ -326,8 +325,7 @@ class Page_Statistics_Reporting extends Page } } // correct indexing of array after deletions - $data = array_values($data); - return $data; + return array_values($data); case 'user': return GetData::perUser($flags); case 'vm': diff --git a/modules-available/sysconfig/addmodule.inc.php b/modules-available/sysconfig/addmodule.inc.php index 477326b8..c887f432 100644 --- a/modules-available/sysconfig/addmodule.inc.php +++ b/modules-available/sysconfig/addmodule.inc.php @@ -31,7 +31,7 @@ abstract class AddModule_Base Util::redirect('?do=SysConfig'); } self::$instance = new $step(); - if ($editId = $editId ? $editId : Request::any('edit')) { + if ($editId = $editId ?: Request::any('edit')) { self::$instance->edit = ConfigModule::get($editId); if (self::$instance->edit === false) ErrorHandler::traceError('Invalid module id for editing'); diff --git a/modules-available/sysconfig/addmodule_adauth.inc.php b/modules-available/sysconfig/addmodule_adauth.inc.php index 9f2a86f6..b7e1cca0 100644 --- a/modules-available/sysconfig/addmodule_adauth.inc.php +++ b/modules-available/sysconfig/addmodule_adauth.inc.php @@ -89,7 +89,6 @@ class AdAuth_CheckConnection extends AddModule_Base )); if (!isset($this->scanTask['id'])) { AddModule_Base::setStep('AdAuth_Start'); // Continues with AdAuth_Start for render() - return; } } @@ -519,7 +518,6 @@ class AdAuth_Finish extends AddModule_Base if ($this->edit === false) { AddModule_Base::setStep('AddModule_Assign', $module->id()); - return; } } diff --git a/modules-available/sysconfig/addmodule_branding.inc.php b/modules-available/sysconfig/addmodule_branding.inc.php index e6da9edb..f3a90e58 100644 --- a/modules-available/sysconfig/addmodule_branding.inc.php +++ b/modules-available/sysconfig/addmodule_branding.inc.php @@ -22,7 +22,6 @@ class Branding_ProcessFile extends AddModule_Base private $task; private $svgFile; - private $tarFile; protected function preprocessInternal() { @@ -53,9 +52,9 @@ class Branding_ProcessFile extends AddModule_Base Session::set('logo_name', $title); } chmod($this->svgFile, 0644); - $this->tarFile = '/tmp/bwlp-' . time() . '-' . mt_rand() . '.tgz'; + $tarFile = '/tmp/bwlp-' . time() . '-' . mt_rand() . '.tgz'; $this->task = Taskmanager::submit('BrandingGenerator', array( - 'tarFile' => $this->tarFile, + 'tarFile' => $tarFile, 'svgFile' => $this->svgFile )); $this->task = Taskmanager::waitComplete($this->task, 5000); @@ -64,7 +63,7 @@ class Branding_ProcessFile extends AddModule_Base Taskmanager::addErrorMessage($this->task); Util::redirect('?do=SysConfig&action=addmodule&step=Branding_Start'); } - Session::set('logo_tgz', $this->tarFile); + Session::set('logo_tgz', $tarFile); } protected function renderInternal() @@ -196,9 +195,9 @@ class Branding_Finish extends AddModule_Base protected function preprocessInternal() { $title = Request::post('title'); - if ($title === false || empty($title)) + if (empty($title)) $title = Session::get('logo_name'); - if ($title === false || empty($title)) { + if (empty($title)) { Message::addError('missing-title'); // TODO: Ask for title again instead of starting over Util::redirect('?do=SysConfig&action=addmodule&step=Branding_Start'); } diff --git a/modules-available/sysconfig/addmodule_ldapauth.inc.php b/modules-available/sysconfig/addmodule_ldapauth.inc.php index 88007f0c..fe5ff706 100644 --- a/modules-available/sysconfig/addmodule_ldapauth.inc.php +++ b/modules-available/sysconfig/addmodule_ldapauth.inc.php @@ -64,7 +64,6 @@ class LdapAuth_CheckConnection extends AddModule_Base )); if (!isset($this->scanTask['id'])) { AddModule_Base::setStep('LdapAuth_Start'); // Continues with LdapAuth_Start for render() - return; } } @@ -290,7 +289,6 @@ class LdapAuth_Finish extends AddModule_Base if ($this->edit === false) { AddModule_Base::setStep('AddModule_Assign', $module->id()); - return; } } diff --git a/modules-available/sysconfig/addmodule_screensaver.inc.php b/modules-available/sysconfig/addmodule_screensaver.inc.php index 9641bab3..24dfadd0 100644 --- a/modules-available/sysconfig/addmodule_screensaver.inc.php +++ b/modules-available/sysconfig/addmodule_screensaver.inc.php @@ -67,18 +67,17 @@ class Screensaver_Start extends AddModule_Base class Screensaver_Text extends AddModule_Base { private $session_data; - private $id; protected function preprocessInternal() { /* Load session data */ $this->session_data = Session::get('data'); - $this->id = Request::post('id', '', 'string'); + $id = Request::post('id', '', 'string'); - if ($this->id === 'start') { + if ($id === 'start') { Screensaver_Helper::processQssData($this->session_data); - } elseif ($this->id !== '') { - Screensaver_Helper::processScreensaverText($this->session_data, $this->id); + } elseif ($id !== '') { + Screensaver_Helper::processScreensaverText($this->session_data, $id); } $next = Request::post('next', $this->session_data['next'], 'string'); @@ -116,7 +115,7 @@ class Screensaver_Text extends AddModule_Base * Dictionary::translate('saver_DescriptionShutdown'); */ $data['title'] = Dictionary::translate('saver_Title' . $tag, true); - $data['description'] = Dictionary::translate('saver_Description' . $tag, true);; + $data['description'] = Dictionary::translate('saver_Description' . $tag, true); $data['msg_value'] = $this->session_data['messages']['General'][$next]; $data['msg_locked_value'] = $this->session_data['messages']['General'][$next . '-locked']; $data['text_value'] = $this->session_data['texts']['text-' . $next]; diff --git a/modules-available/sysconfig/inc/configmodule.inc.php b/modules-available/sysconfig/inc/configmodule.inc.php index 3b877d10..e414a6cf 100644 --- a/modules-available/sysconfig/inc/configmodule.inc.php +++ b/modules-available/sysconfig/inc/configmodule.inc.php @@ -470,11 +470,11 @@ abstract class ConfigModule $status = 'MISSING'; else $status = 'OUTDATED'; - return Database::exec("UPDATE configtgz_module SET filepath = :filename, status = :status WHERE moduleid = :id LIMIT 1", array( + Database::exec("UPDATE configtgz_module SET filepath = :filename, status = :status WHERE moduleid = :id LIMIT 1", array( 'id' => $this->moduleId, 'filename' => $this->moduleArchive, 'status' => $status - )) !== false; + )); } public function dateline_s() diff --git a/modules-available/sysconfig/inc/configmodule/branding.inc.php b/modules-available/sysconfig/inc/configmodule/branding.inc.php index 8990dbec..76968ff4 100644 --- a/modules-available/sysconfig/inc/configmodule/branding.inc.php +++ b/modules-available/sysconfig/inc/configmodule/branding.inc.php @@ -22,13 +22,12 @@ class ConfigModule_Branding extends ConfigModule if (!$this->validateConfig()) { return $this->archive() !== false && file_exists($this->archive()); // No new temp file given, old archive still exists, pretend it worked... } - $task = Taskmanager::submit('MoveFile', array( + return Taskmanager::submit('MoveFile', array( 'source' => $this->tmpFile, 'destination' => $tgz, 'parentTask' => $parent, 'failOnParentFail' => false )); - return $task; } protected function moduleVersion() diff --git a/modules-available/sysconfig/inc/configmodule/customodule.inc.php b/modules-available/sysconfig/inc/configmodule/customodule.inc.php index 8b968336..56aa9277 100644 --- a/modules-available/sysconfig/inc/configmodule/customodule.inc.php +++ b/modules-available/sysconfig/inc/configmodule/customodule.inc.php @@ -34,13 +34,12 @@ class ConfigModule_CustomModule extends ConfigModule // Nothing to do return true; } - $task = Taskmanager::submit('MoveFile', array( + return Taskmanager::submit('MoveFile', array( 'source' => $this->tmpFile, 'destination' => $tgz, 'parentTask' => $parent, 'failOnParentFail' => false )); - return $task; } protected function moduleVersion() diff --git a/modules-available/sysconfig/inc/configmodule/screensaver.inc.php b/modules-available/sysconfig/inc/configmodule/screensaver.inc.php index ed97941e..08a2c9a1 100644 --- a/modules-available/sysconfig/inc/configmodule/screensaver.inc.php +++ b/modules-available/sysconfig/inc/configmodule/screensaver.inc.php @@ -23,13 +23,11 @@ class ConfigModule_Screensaver extends ConfigModule /* Give the Taskmanager the job and create the tgz */ $taskId = 'xscreensaver' . mt_rand() . '-' . microtime(true); - $task = Taskmanager::submit('MakeTarball', array( + return Taskmanager::submit('MakeTarball', array( 'id' => $taskId, 'files' => $this->getFileArray(), 'destination' => $tgz, ), false); - - return $task; } protected function moduleVersion() diff --git a/modules-available/sysconfig/inc/configtgz.inc.php b/modules-available/sysconfig/inc/configtgz.inc.php index 67e02503..9ab3ffd3 100644 --- a/modules-available/sysconfig/inc/configtgz.inc.php +++ b/modules-available/sysconfig/inc/configtgz.inc.php @@ -10,7 +10,6 @@ class ConfigTgz private function __construct() { - ; } public function id() @@ -134,7 +133,7 @@ class ConfigTgz return $task['id']; } - public function delete() + public function delete(): bool { if ($this->configId === 0) ErrorHandler::traceError('ConfigTgz::delete called with invalid config id!'); @@ -150,14 +149,14 @@ class ConfigTgz return $ret !== false; } - public function markOutdated() + public function markOutdated(): void { if ($this->configId === 0) ErrorHandler::traceError('ConfigTgz::markOutdated called with invalid config id!'); - return $this->mark('OUTDATED'); + $this->mark('OUTDATED'); } - private function markUpdated($task) + private function markUpdated($task): void { if ($this->configId === 0) ErrorHandler::traceError('ConfigTgz::markUpdated called with invalid config id!'); @@ -186,27 +185,28 @@ class ConfigTgz 'status' => 'OK', 'warnings' => $warnings, ]); - return 'OK'; + return; } - return $this->mark('OUTDATED'); + $this->mark('OUTDATED'); } - private function markFailed() + private function markFailed(): void { if ($this->configId === 0) ErrorHandler::traceError('ConfigTgz::markFailed called with invalid config id!'); - if ($this->file === false || !file_exists($this->file)) - return $this->mark('MISSING'); - return $this->mark('OUTDATED'); + if ($this->file === false || !file_exists($this->file)) { + $this->mark('MISSING'); + } else { + $this->mark('OUTDATED'); + } } - private function mark($status) + private function mark($status): void { Database::exec("UPDATE configtgz SET status = :status WHERE configid = :configid LIMIT 1", [ 'configid' => $this->configId, 'status' => $status, ]); - return $status; } /* diff --git a/modules-available/sysconfig/inc/ppd.inc.php b/modules-available/sysconfig/inc/ppd.inc.php index 8e125ed7..75608c56 100644 --- a/modules-available/sysconfig/inc/ppd.inc.php +++ b/modules-available/sysconfig/inc/ppd.inc.php @@ -308,8 +308,8 @@ class Ppd $this->requiredKeywords = array(); // Parse - /* @var $rawOption \PpdOption */ - /* @var $currentBlock \PpdBlockInternal */ + /* @var PpdOption $rawOption */ + /* @var PpdBlockInternal $currentBlock */ $currentBlock = false; $inRawBlock = false; // True if in a multi-line InvocationValue or QuotedValue (3.6: Parsing Summary for Values) $wantsEnd = false; @@ -425,10 +425,7 @@ class Ppd return; } // TODO: Check unique - $nb = new PpdBlockInternal($value, $valueTranslation, 'Group', $currentBlock, $lStart); - if ($currentBlock !== false) { - $currentBlock->childBlocks[] = $nb; - } + $nb = new PpdBlockInternal($value, $valueTranslation, 'Group', false, $lStart); $currentBlock = $nb; continue; } elseif ($mainKeyword === 'OpenSubGroup') { @@ -438,9 +435,7 @@ class Ppd } // TODO: Check unique $nb = new PpdBlockInternal($value, $valueTranslation, 'SubGroup', $currentBlock, $lStart); - if ($currentBlock !== false) { - $currentBlock->childBlocks[] = $nb; - } + $currentBlock->childBlocks[] = $nb; $currentBlock = $nb; continue; } elseif ($mainKeyword === 'OpenUI' || $mainKeyword === 'JCLOpenUI') { @@ -542,7 +537,8 @@ class Ppd $option = $this->getOption(substr($mainKeyword, 7), $currentBlock); $option->default = new PpdOption($lStart, $len, $value, $valueTranslation); continue; - } elseif (substr($mainKeyword, 0, 17) === 'FoomaticRIPOption') { + } + if (substr($mainKeyword, 0, 17) === 'FoomaticRIPOption') { if ($optionKeyword === false) { $this->warn($no, "$mainKeyword with no option keyword"); } elseif ($currentBlock !== false && isset($this->settings[$optionKeyword])) { @@ -604,7 +600,7 @@ class Ppd } elseif (strlen(trim($line)) !== 0) { $this->warn($no, 'Invalid format; not empty and not starting with asterisk (*)'); } - } + } // end while loop over ppd contents // if ($currentBlock !== false) { $this->error = 'Block ' . $currentBlock->id . ' (' . $currentBlock->type . ') was never closed.'; @@ -691,10 +687,9 @@ class Ppd private function escapeBinaryArray($array) { - $chars = array_reduce(array_unique($array), function ($carry, $item) { + return array_reduce(array_unique($array), function ($carry, $item) { return $carry . '\x' . dechex(ord($item)); }, ''); - return $chars; } private function unhexTranslation($lineNo, $translation) @@ -957,7 +952,7 @@ class Ppd // $start must lie before range start, otherwise we'd have hit the case above $end = $ranges[$key][1]; unset($ranges[$key]); - continue; + //continue; } } $ranges[] = array($start, $end); @@ -1107,11 +1102,11 @@ class PpdBlockInternal public $translation; public $type; /** - * @var \PpdBlockInternal[] + * @var PpdBlockInternal[] */ public $childBlocks = array(); /** - * @var \PpdBlockInternal + * @var PpdBlockInternal|false */ public $parent; @@ -1148,10 +1143,10 @@ class PpdBlockInternal } /** - * @param \PpdBlockInternal $block some other PpdBlock instance + * @param PpdBlockInternal $block some other PpdBlock instance * @return bool true if this is a child of $block */ - public function isChildOf($block) + public function isChildOf(PpdBlockInternal $block) { $parent = $this->parent; while ($parent !== false) { diff --git a/modules-available/sysconfig/install.inc.php b/modules-available/sysconfig/install.inc.php index 6c9edba2..9d32137c 100644 --- a/modules-available/sysconfig/install.inc.php +++ b/modules-available/sysconfig/install.inc.php @@ -132,7 +132,7 @@ if ($list === false) { if ($ad->moduleType() === 'SshConfig') { // 2020-11-12: Split SshConfig into SshConfig and SshKey $pubkey = $ad->getData('publicKey'); - if ($pubkey !== false && !empty($pubkey)) { + if (!empty($pubkey)) { error_log('Legacy module with pubkey ' . $ad->id()); $key = ConfigModule::getInstance('SshKey'); if ($key !== false) { diff --git a/modules-available/systemstatus/inc/systemstatus.inc.php b/modules-available/systemstatus/inc/systemstatus.inc.php index 8bc67385..7f3e5d42 100644 --- a/modules-available/systemstatus/inc/systemstatus.inc.php +++ b/modules-available/systemstatus/inc/systemstatus.inc.php @@ -20,7 +20,7 @@ class SystemStatus return false; $task = Taskmanager::waitComplete($task, 3000); - if (!isset($task['data']['list']) || empty($task['data']['list'])) + if (empty($task['data']['list'])) return false; $wantedSource = Property::getVmStoreUrl(); $storeUsage = false; diff --git a/modules-available/translation/page.inc.php b/modules-available/translation/page.inc.php index ee1d2cde..da4205d2 100644 --- a/modules-available/translation/page.inc.php +++ b/modules-available/translation/page.inc.php @@ -477,9 +477,8 @@ class Page_Translation extends Page $tags[$p[1]] = $tag; } } - $tags = $this->loadTagsFromPhp('/Message\s*::\s*add\w+\s*\(\s*[\'"](?<tag>[^\'"\.]*)[\'"]\s*(?<data>\)|\,.*)/i', + return $this->loadTagsFromPhp('/Message\s*::\s*add\w+\s*\(\s*[\'"](?<tag>[^\'"\.]*)[\'"]\s*(?<data>\)|\,.*)/i', $this->getModulePhpFiles($module), $tags); - return $tags; } /** @@ -510,11 +509,9 @@ class Page_Translation extends Page return $tags; } - private function loadUsedMenuCategories($module = false) + private function loadUsedMenuCategories() { - if ($module === false) { - $module = $this->module; - } + $module = $this->module; $skip = strlen($module->getIdentifier()) + 1; $match = $module->getIdentifier() . '.'; $want = array(); @@ -740,15 +737,13 @@ class Page_Translation extends Page private function loadModuleEditArray() { $tags = $this->loadUsedModuleTags(); - $table = $this->buildTranslationTable('module', array_keys($tags), true); - return $table; + return $this->buildTranslationTable('module', array_keys($tags), true); } private function loadMenuCategoryEditArray() { $tags = $this->loadUsedMenuCategories(); - $table = $this->buildTranslationTable('categories', array_keys($tags), true); - return $table; + return $this->buildTranslationTable('categories', array_keys($tags), true); } /** @@ -760,8 +755,7 @@ class Page_Translation extends Page private function loadCustomEditArray() { $tags = $this->loadUsedCustomTags($this->subsection); - $table = $this->buildTranslationTable($this->subsection, array_keys($tags), true); - return $table; + return $this->buildTranslationTable($this->subsection, array_keys($tags), true); } /** diff --git a/tools/convert-modules.php b/tools/convert-modules.php index f8c7a1a5..aa43d951 100644 --- a/tools/convert-modules.php +++ b/tools/convert-modules.php @@ -181,7 +181,6 @@ echo "Processing\n"; foreach ($messages as $id => $modules) { asort($modules, SORT_NUMERIC); $modules = array_reverse($modules, true); - reset($modules); $topModule = key($modules); $topCount = $modules[$topModule]; $sum = 0; diff --git a/tools/global-candidates.php b/tools/global-candidates.php index 12e900a5..c42a43aa 100644 --- a/tools/global-candidates.php +++ b/tools/global-candidates.php @@ -58,6 +58,7 @@ foreach ($tags as $k => &$tag) { } } } +unset($tag); echo "\n\nDUPLICATE STRINGS WITH DIFFERENT NAMES:\n"; foreach ($strings as $text => $data) { |