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 --- .../hooks/translation-global.inc.php | 5 +--- .../inc/getpermissiondata.inc.php | 12 ++++----- .../inc/permissiondbupdate.inc.php | 21 ++++++++-------- .../permissionmanager/inc/permissionutil.inc.php | 29 +++++++++++----------- modules-available/permissionmanager/page.inc.php | 28 +++++++++++---------- 5 files changed, 48 insertions(+), 47 deletions(-) (limited to 'modules-available/permissionmanager') diff --git a/modules-available/permissionmanager/hooks/translation-global.inc.php b/modules-available/permissionmanager/hooks/translation-global.inc.php index 4810a719..cf2166bc 100644 --- a/modules-available/permissionmanager/hooks/translation-global.inc.php +++ b/modules-available/permissionmanager/hooks/translation-global.inc.php @@ -18,11 +18,8 @@ if (file_exists('modules/' . $moduleName . '/permissions/permissions.json')) { /** * Configuration categories. - * - * @param \Module $module - * @return array */ - $HANDLER['grep_permissions'] = function ($module) { + $HANDLER['grep_permissions'] = function (Module $module): array { $file = 'modules/' . $module->getIdentifier() . '/permissions/permissions.json'; if (!file_exists($file)) return []; diff --git a/modules-available/permissionmanager/inc/getpermissiondata.inc.php b/modules-available/permissionmanager/inc/getpermissiondata.inc.php index ead1e020..a51619e0 100644 --- a/modules-available/permissionmanager/inc/getpermissiondata.inc.php +++ b/modules-available/permissionmanager/inc/getpermissiondata.inc.php @@ -11,7 +11,7 @@ class GetPermissionData * * @return array array of users (each with userid, username and roles (each with roleid and rolename)) */ - public static function getUserData() + public static function getUserData(): array { $res = Database::simpleQuery("SELECT user.userid AS userid, user.login AS login, role.rolename AS rolename, role.roleid AS roleid FROM user @@ -42,7 +42,7 @@ class GetPermissionData * * @return array array of locations (each including the roles that have permissions for them) */ - public static function getLocationData() + public static function getLocationData(): array { $res = Database::simpleQuery("SELECT role.roleid AS roleid, rolename, GROUP_CONCAT(COALESCE(locationid, 0)) AS locationids FROM role INNER JOIN role_x_location ON role.roleid = role_x_location.roleid GROUP BY roleid ORDER BY rolename ASC"); @@ -70,7 +70,7 @@ class GetPermissionData * @param int $flags Bitmask specifying additional data to fetch (WITH_* constants of this class) * @return array array roles (each with roleid and rolename) */ - public static function getRoles($flags = 0) + public static function getRoles(int $flags = 0): array { $cols = $joins = ''; if ($flags & self::WITH_USER_COUNT) { @@ -93,15 +93,15 @@ class GetPermissionData * Get permissions and locations for a given role. * * @param string $roleid id of the role - * @return array|false array containing an array of permissions and an array of locations, false if not found + * @return ?array array containing an array of permissions and an array of locations, null if not found */ - public static function getRoleData($roleid) + public static function getRoleData(string $roleid): ?array { $data = self::getRole($roleid); $res = Database::simpleQuery("SELECT roleid, locationid FROM role_x_location WHERE roleid = :roleid", array("roleid" => $roleid)); if ($res === false) - return false; + return null; $data["locations"] = array(); foreach ($res as $row) { $data["locations"][] = $row['locationid']; diff --git a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php index 0cd89b3a..49988420 100644 --- a/modules-available/permissionmanager/inc/permissiondbupdate.inc.php +++ b/modules-available/permissionmanager/inc/permissiondbupdate.inc.php @@ -9,14 +9,14 @@ class PermissionDbUpdate * @param int[] $users userids * @param int[] $roles roleids */ - public static function addRoleToUser($users, $roles) + public static function addRoleToUser(array $users, array $roles): int { if (empty($users) || empty($roles)) return 0; - $arg = array(); + $arg = []; foreach ($users AS $userid) { foreach ($roles AS $roleid) { - $arg[] = compact('userid', 'roleid'); + $arg[] = ['userid' => $userid, 'roleid' => $roleid]; } } return Database::exec("INSERT IGNORE INTO role_x_user (userid, roleid) VALUES :arg", @@ -29,12 +29,12 @@ class PermissionDbUpdate * @param int[] $users userids * @param int[] $roles roleids */ - public static function removeRoleFromUser($users, $roles) + public static function removeRoleFromUser(array $users, array $roles): int { if (empty($users) || empty($roles)) return 0; $query = "DELETE FROM role_x_user WHERE userid IN (:users) AND roleid IN (:roles)"; - return Database::exec($query, array("users" => $users, "roles" => $roles)); + return Database::exec($query, ["users" => $users, "roles" => $roles]); } /** @@ -44,7 +44,7 @@ class PermissionDbUpdate * @param int[] $users list of user ids * @param int[] $roles list of role ids */ - public static function setRolesForUser($users, $roles) + public static function setRolesForUser(array $users, array $roles): int { $count = Database::exec("DELETE FROM role_x_user WHERE userid in (:users) AND roleid NOT IN (:roles)", compact('users', 'roles')); @@ -56,7 +56,7 @@ class PermissionDbUpdate * * @param int $roleid roleid */ - public static function deleteRole($roleid) + public static function deleteRole(int $roleid): int { return Database::exec("DELETE FROM role WHERE roleid = :roleid", array("roleid" => $roleid)); } @@ -69,7 +69,8 @@ class PermissionDbUpdate * @param string[] $permissions array of permissions * @param int|null $roleId roleid or null if the role does not exist yet */ - public static function saveRole($roleName, $roleDescription, $locations, $permissions, $roleId = null) + public static function saveRole(string $roleName, string $roleDescription, array $locations, array $permissions, + ?int $roleId = null): void { foreach ($permissions as &$permission) { $permission = strtolower($permission); @@ -92,14 +93,14 @@ class PermissionDbUpdate if (!empty($locations)) { $arg = array_map(function ($loc) use ($roleId) { - return compact('roleId', 'loc'); + return ['roleId' => $roleId, 'loc' => $loc]; }, $locations); Database::exec("INSERT IGNORE INTO role_x_location (roleid, locationid) VALUES :arg", ['arg' => $arg]); } if (!empty($permissions)) { $arg = array_map(function ($perm) use ($roleId) { - return compact('roleId', 'perm'); + return ['roleId' => $roleId, 'perm' => $perm]; }, $permissions); Database::exec("INSERT IGNORE INTO role_x_permission (roleid, permissionid) VALUES :arg", ['arg' => $arg]); } diff --git a/modules-available/permissionmanager/inc/permissionutil.inc.php b/modules-available/permissionmanager/inc/permissionutil.inc.php index d3f20b4f..170fd699 100644 --- a/modules-available/permissionmanager/inc/permissionutil.inc.php +++ b/modules-available/permissionmanager/inc/permissionutil.inc.php @@ -14,7 +14,7 @@ class PermissionUtil * @param string|false $wildcard if $permission is a wildcard string this returns the matching variant * @param int|false $wclen if $permission is a wildcard string, this is the length of the matching variant */ - private static function makeComparisonVariants($permission, &$compare, &$wildcard, &$wclen) + private static function makeComparisonVariants($permission, ?array &$compare, &$wildcard, &$wclen): void { if (!is_array($permission)) { $permission = explode('.', $permission); @@ -46,12 +46,12 @@ class PermissionUtil /** * Check if the user has the given permission (for the given location). * - * @param string $userid userid to check + * @param int $userid userid to check * @param string $permissionid permissionid to check * @param int|null $locationid locationid to check or null if the location should be disregarded * @return bool true if user has permission, false if not */ - public static function userHasPermission($userid, $permissionid, $locationid) + public static function userHasPermission(int $userid, string $permissionid, ?int $locationid): bool { $permissionid = strtolower($permissionid); self::validatePermission($permissionid); @@ -121,6 +121,8 @@ class PermissionUtil // Compare to database result if ($cacheAll) { $allLocs = Location::getLocationsAssoc(); + } else { + $allLocs = []; } self::makeComparisonVariants($parts, $compare, $wildcard, $wclen); $retval = false; @@ -152,11 +154,11 @@ class PermissionUtil /** * Get all locations where the user has the given permission. * - * @param string $userid userid to check + * @param int $userid userid to check * @param string $permissionid permissionid to check * @return array array of locationids where the user has the given permission */ - public static function getAllowedLocations($userid, $permissionid) + public static function getAllowedLocations(int $userid, string $permissionid): array { $permissionid = strtolower($permissionid); self::validatePermission($permissionid); @@ -199,10 +201,10 @@ class PermissionUtil * Extend an array of locations by adding all sublocations. * * @param array $tree tree of all locations (structured like Location::getTree()) - * @param array $allowedLocations the array of locationids to extend + * @param int[] $allowedLocations the array of locationids to extend * @return array extended array of locationids */ - public static function getSublocations($tree, $allowedLocations) + public static function getSublocations(array $tree, array $allowedLocations): array { $result = $allowedLocations; foreach ($tree as $location) { @@ -224,7 +226,7 @@ class PermissionUtil * * @param string $permissionId permission to check */ - private static function validatePermission($permissionId) + private static function validatePermission(string $permissionId): void { if (!CONFIG_DEBUG || $permissionId === '*') return; @@ -253,7 +255,7 @@ class PermissionUtil * * @return array permission tree as a multidimensional array */ - public static function getPermissions() + public static function getPermissions(): array { $permissions = array(); foreach (glob("modules/*/permissions/permissions.json", GLOB_NOSORT) as $file) { @@ -291,18 +293,17 @@ class PermissionUtil /** * Get all existing roles. * - * @param int|false $userid Which user to consider, false = none - * @param bool $onlyMatching true = filter roles the user doesn't have + * @param ?int $userid Which user to consider, false = none (list all) * @return array list of roles */ - public static function getRoles($userid = false, $onlyMatching = true) + public static function getRoles(?int $userid = null): array { if ($userid === false) { return Database::queryAll('SELECT roleid, rolename FROM role ORDER BY rolename ASC'); } $ret = Database::queryAll('SELECT r.roleid, r.rolename, u.userid AS hasRole FROM role r LEFT JOIN role_x_user u ON (r.roleid = u.roleid AND u.userid = :userid) - GROUP BY r.roleid + GROUP BY r.roleid, r.rolename ORDER BY rolename ASC', ['userid' => $userid]); foreach ($ret as &$role) { settype($role['hasRole'], 'bool'); @@ -318,7 +319,7 @@ class PermissionUtil * @param string $description the description of the permission * @param array $tree the permission tree to modify */ - private static function putInPermissionTree($permission, $locationAware, $description, &$tree) + private static function putInPermissionTree(string $permission, bool $locationAware, string $description, array &$tree): void { $subPermissions = explode('.', $permission); foreach ($subPermissions as $subPermission) { diff --git a/modules-available/permissionmanager/page.inc.php b/modules-available/permissionmanager/page.inc.php index b431d9c9..7e9f17e4 100644 --- a/modules-available/permissionmanager/page.inc.php +++ b/modules-available/permissionmanager/page.inc.php @@ -18,13 +18,13 @@ class Page_PermissionManager extends Page $action = Request::any('action', 'show', 'string'); if ($action === 'addRoleToUser') { User::assertPermission('users.edit-roles'); - $users = Request::post('users', ''); - $roles = Request::post('roles', ''); + $users = Request::post('users', [], 'array'); + $roles = Request::post('roles', [], 'array'); PermissionDbUpdate::addRoleToUser($users, $roles); } elseif ($action === 'removeRoleFromUser') { User::assertPermission('users.edit-roles'); - $users = Request::post('users', ''); - $roles = Request::post('roles', ''); + $users = Request::post('users', [], 'array'); + $roles = Request::post('roles', [], 'array'); PermissionDbUpdate::removeRoleFromUser($users, $roles); } elseif ($action === 'deleteRole') { User::assertPermission('roles.edit'); @@ -115,7 +115,7 @@ class Page_PermissionManager extends Page $roleid = Request::get("roleid", false, 'int'); if ($roleid !== false) { $role = GetPermissionData::getRoleData($roleid); - if ($role === false) { + if ($role === null) { Message::addError('invalid-role-id', $roleid); Util::redirect('?do=permissionmanager'); } @@ -147,7 +147,8 @@ class Page_PermissionManager extends Page * @param string $permString the prefix permission string with which all permissions in the permission tree should start * @return string generated html code */ - private static function generatePermissionHTML($permissions, $selectedPermissions = array(), $selectAll = false, $permString = "", $tags = []) + private static function generatePermissionHTML(array $permissions, array $selectedPermissions = [], + bool $selectAll = false, string $permString = "", array $tags = []): string { $res = ""; $toplevel = $permString == ""; @@ -203,11 +204,12 @@ class Page_PermissionManager extends Page * * @param array $locations the location tree * @param array $selectedLocations locations that should be preselected - * @param array $selectAll true if all locations should be preselected, false if only those in $selectedLocations - * @param array $toplevel true if the location tree are the children of the root location, false if not + * @param bool $selectAll true if all locations should be preselected, false if only those in $selectedLocations + * @param bool $toplevel true if the location tree are the children of the root location, false if not * @return string generated html code */ - private static function generateLocationHTML($locations, $selectedLocations = array(), $selectAll = false, $toplevel = true, $tags = []) + private static function generateLocationHTML(array $locations, array $selectedLocations = [], + bool $selectAll = false, bool $toplevel = true, array $tags = []): string { $res = ""; if ($toplevel && in_array(0, $selectedLocations)) { @@ -242,7 +244,7 @@ class Page_PermissionManager extends Page * @param array $locations the locationid array * @return array the locationid array without redundant locationids */ - private static function processLocations($locations) + private static function processLocations(array $locations): array { if (in_array(0, $locations)) return array(null); @@ -267,7 +269,7 @@ class Page_PermissionManager extends Page * @param array $permissions the permissionid array * @return array the permissionid array without redundant permissionids */ - private static function processPermissions($permissions) + private static function processPermissions(array $permissions): array { if (in_array("*", $permissions)) return array("*"); @@ -287,7 +289,7 @@ class Page_PermissionManager extends Page * @param array $permissions multidimensional array of permissionids * @return array flat array of permissionids */ - private static function extractPermissions($permissions) + private static function extractPermissions(array $permissions): array { $result = array(); foreach ($permissions as $permission => $a) { @@ -306,7 +308,7 @@ class Page_PermissionManager extends Page return $result; } - private function denyActionIfBuiltin($roleID) + private function denyActionIfBuiltin(string $roleID): void { if ($roleID) { $existing = GetPermissionData::getRole($roleID); -- cgit v1.2.3-55-g7522