Improve strictness of rector

lib/AppInfo/Application.php

@@ -1,10 +1,11 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-
 namespace OCA\Guests\AppInfo;
 
 use OCA\Files\Event\LoadAdditionalScriptsEvent;

lib/AppWhitelist.php

@@ -25,6 +25,7 @@
  */
 class AppWhitelist {
 	private readonly string $baseUrl;
+
 	private readonly int $baseUrlLength;
 
 	public const WHITELIST_ALWAYS = ',core,theming,settings,avatar,files,heartbeat,dav,guests,impersonate,accessibility,terms_of_service,dashboard,weather_status,user_status,apporder,twofactor_totp,twofactor_webauthn,twofactor_backupcodes,twofactor_nextcloud_notification';
@@ -60,15 +61,16 @@ public function isUrlAllowed(IUser $user, string|false $url): bool {
 
 			if ($this->isAppWhitelisted($app)) {
 				return true;
-			} else {
-				if ($url === '/apps/files_external/api/v1/mounts') {
-					// fake successful response
-					echo '[]';
-					exit;
-				}
-				$this->logger->notice("Blocking access to non-whitelisted app ($app) for guest", ['app' => 'guests']);
-				return false;
 			}
+
+			if ($url === '/apps/files_external/api/v1/mounts') {
+				// fake successful response
+				echo '[]';
+				exit;
+			}
+
+			$this->logger->notice('Blocking access to non-whitelisted app (' . $app . ') for guest', ['app' => 'guests']);
+			return false;
 		}
 
 		return true;
@@ -92,40 +94,69 @@ private function getRequestedApp(string|false $url): string {
 		if (substr($url, 0, $this->baseUrlLength) === $this->baseUrl) {
 			$url = substr($url, $this->baseUrlLength);
 		}
+
 		if (str_starts_with($url, '/index.php/')) {
 			$url = substr($url, 10);
 		}
+
 		if (str_starts_with($url, '/apps/')) {
 			// empty string / 'apps' / $app / rest of the route
 			[, , $app,] = explode('/', $url, 4);
 			return \OC_App::cleanAppId($app);
-		} elseif ($url === '/cron.php') {
+		}
+
+		if ($url === '/cron.php') {
 			return 'core';
-		} elseif (str_starts_with($url, '/core/')) {
+		}
+
+		if (str_starts_with($url, '/core/')) {
 			return 'core';
-		} elseif (str_starts_with($url, '/js/')) {
+		}
+
+		if (str_starts_with($url, '/js/')) {
 			return 'core';
-		} elseif (str_starts_with($url, '/css/')) {
+		}
+
+		if (str_starts_with($url, '/css/')) {
 			return 'core';
-		} elseif (str_starts_with($url, '/login')) {
+		}
+
+		if (str_starts_with($url, '/login')) {
 			return 'core';
-		} elseif (str_starts_with($url, '/logout')) {
+		}
+
+		if (str_starts_with($url, '/logout')) {
 			return 'core';
-		} elseif (str_starts_with($url, '/f/')) {
+		}
+
+		if (str_starts_with($url, '/f/')) {
 			return 'files';
-		} elseif (str_starts_with($url, '/webdav/')) {
+		}
+
+		if (str_starts_with($url, '/webdav/')) {
 			return 'dav';
-		} elseif (str_starts_with($url, '/dav/')) {
+		}
+
+		if (str_starts_with($url, '/dav/')) {
 			return 'dav';
-		} elseif (str_starts_with($url, '/call/')) {
+		}
+
+		if (str_starts_with($url, '/call/')) {
 			return 'spreed';
-		} elseif (str_starts_with($url, '/settings/')) {
+		}
+
+		if (str_starts_with($url, '/settings/')) {
 			return 'settings';
-		} elseif (str_starts_with($url, '/avatar/')) {
+		}
+
+		if (str_starts_with($url, '/avatar/')) {
 			return 'avatar';
-		} elseif (str_starts_with($url, '/heartbeat')) {
+		}
+
+		if (str_starts_with($url, '/heartbeat')) {
 			return 'heartbeat';
 		}
+
 		return 'core';
 	}
 

lib/BackgroundJob/TransferJob.php

@@ -74,6 +74,7 @@ private function fail(Transfer $transfer, ?IUser $targetUser = null): void {
 		if (!$targetUser instanceof IUser) {
 			return;
 		}
+
 		$result = $targetUser->delete(); // Rollback created user
 		if (!$result) {
 			$this->logger->error('Failed to delete target user', ['user' => $targetUser->getUID()]);
@@ -86,6 +87,7 @@ public function run($argument): void {
 
 		$transfer = $this->transferMapper->getById($id);
 		$transfer->setStatus(Transfer::STATUS_STARTED);
+
 		$this->transferMapper->update($transfer);
 
 		$source = $transfer->getSource();
@@ -99,7 +101,7 @@ public function run($argument): void {
 		}
 
 		if ($this->userManager->userExists($target)) {
-			$this->logger->error("Cannot transfer guest user \"$source\", target user \"$target\" already exists");
+			$this->logger->error('Cannot transfer guest user "' . $source . '", target user "' . $target . '" already exists');
 			$this->fail($transfer);
 			return;
 		}
@@ -119,8 +121,8 @@ public function run($argument): void {
 
 		try {
 			$this->transferService->transfer($sourceUser, $targetUser);
-		} catch (\Throwable $th) {
-			$this->logger->error($th->getMessage(), ['exception' => $th]);
+		} catch (\Throwable $throwable) {
+			$this->logger->error($throwable->getMessage(), ['exception' => $throwable]);
 			$this->fail($transfer, $targetUser);
 			return;
 		}

lib/Capabilities.php

@@ -20,6 +20,7 @@ class Capabilities implements ICapability {
 
 	/**
 	 * @inheritDoc
+	 * @return array<string, array<string, bool>>
 	 */
 	public function getCapabilities(): array {
 		return [

lib/Command/AddCommand.php

@@ -134,7 +134,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$input->getOption('language') ?? '',
 			$password
 		);
-
 		if ($user instanceof IUser) {
 			$output->writeln('<info>The guest account user "' . $user->getUID() . '" was created successfully</info>');
 			return self::SUCCESS;

lib/Command/ListCommand.php

@@ -35,12 +35,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$guests = $this->guestManager->getGuestsInfo();
 
 		$outputType = $input->getOption('output');
-		if (count($guests) === 0) {
+		if ($guests === []) {
 			if ($outputType === self::OUTPUT_FORMAT_JSON || $outputType === self::OUTPUT_FORMAT_JSON_PRETTY) {
 				$output->writeln('[]');
 			} else {
 				$output->writeln('<info>No guests created</info>');
 			}
+
 			return self::SUCCESS;
 		}
 
@@ -52,6 +53,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$table->setRows($guests);
 			$table->render();
 		}
+
 		return self::SUCCESS;
 	}
 }

lib/Config.php

@@ -65,6 +65,7 @@ public function setAppWhitelist(array|string $whitelist): void {
 		if (is_array($whitelist)) {
 			$whitelist = implode(',', $whitelist);
 		}
+
 		$this->appConfig->setAppValueString('whitelist', $whitelist);
 	}
 
@@ -87,10 +88,10 @@ public function canCreateGuests(): bool {
 		// Check if we have a group restriction
 		// and if the user belong to that group
 		$groupRestriction = $this->getCreateRestrictedToGroup();
-		if (!empty($groupRestriction)) {
+		if ($groupRestriction !== []) {
 			$userGroups = $this->groupManager->getUserGroupIds($user);
 			$groupRestriction = array_intersect($userGroups, $groupRestriction);
-			if (empty($groupRestriction)) {
+			if ($groupRestriction === []) {
 				return false;
 			}
 		}

lib/Controller/APIController.php

@@ -65,7 +65,7 @@ public function languages(): DataResponse {
 			// Put appropriate languages into appropriate arrays, to print them sorted
 			// common languages -> divider -> other languages
 			if (in_array($lang, Factory::COMMON_LANGUAGE_CODES)) {
-				$commonLanguages[array_search($lang, Factory::COMMON_LANGUAGE_CODES)] = $ln;
+				$commonLanguages[array_search($lang, Factory::COMMON_LANGUAGE_CODES, true)] = $ln;
 			} else {
 				$languages[] = $ln;
 			}
@@ -79,10 +79,12 @@ public function languages(): DataResponse {
 				// If a doesn't have a name, but b does, list b before a
 				return 1;
 			}
+
 			if ($a['code'] !== $a['name'] && $b['code'] === $b['name']) {
 				// If a does have a name, but b doesn't, list a before b
 				return -1;
 			}
+
 			// Otherwise compare the names
 			return strcmp((string)$a['name'], (string)$b['name']);
 		});
@@ -102,6 +104,7 @@ public function groups(): DataResponse {
 		} else {
 			$groups = $this->subAdmin->getSubAdminsGroups($user);
 		}
+
 		$groups = array_values(array_map(fn (IGroup $group): array => [
 			'gid' => $group->getGID(),
 			'name' => $group->getDisplayName(),

lib/Controller/UsersController.php

@@ -87,23 +87,25 @@ public function create(string $email, string $displayName, string $language, arr
 			if (!$group) {
 				return new DataResponse(
 					[
-						'errorMessages' => ["Group $groupId not found"],
+						'errorMessages' => ['Group ' . $groupId . ' not found'],
 					],
 					Http::STATUS_BAD_REQUEST
 				);
 			}
-			if (!($this->subAdmin->isSubAdminOfGroup($currentUser, $group) || $this->groupManager->isAdmin($currentUser->getUID()))) {
+
+			if (!$this->subAdmin->isSubAdminOfGroup($currentUser, $group) && !$this->groupManager->isAdmin($currentUser->getUID())) {
 				return new DataResponse(
 					[
-						'errorMessages' => ["You are not allowed to add users to group $groupId"],
+						'errorMessages' => ['You are not allowed to add users to group ' . $groupId],
 					],
 					Http::STATUS_FORBIDDEN
 				);
 			}
+
 			$groupObjects[] = $group;
 		}
 
-		if (empty($email) || !$this->mailer->validateMailAddress($email)) {
+		if ($email === '' || !$this->mailer->validateMailAddress($email)) {
 			$errorMessages['email'] = $this->l10n->t(
 				'Invalid mail address'
 			);
@@ -122,7 +124,7 @@ public function create(string $email, string $displayName, string $language, arr
 			);
 		}
 
-		if (!empty($errorMessages)) {
+		if ($errorMessages !== []) {
 			return new DataResponse(
 				[
 					'errorMessages' => $errorMessages,
@@ -132,18 +134,26 @@ public function create(string $email, string $displayName, string $language, arr
 		}
 
 		try {
-			$this->guestManager->createGuest($this->userSession->getUser(), $username, $email, $displayName, $language);
-			$guestUser = $this->userManager->get($username);
+			$guestUser = $this->guestManager->createGuest($this->userSession->getUser(), $username, $email, $displayName, $language);
+			if (!$guestUser instanceof IUser) {
+				return new DataResponse(
+					[
+						'errorMessages' => ['email' => 'Unable to create guest user ' . $username],
+					],
+					Http::STATUS_UNPROCESSABLE_ENTITY
+				);
+			}
 			if ($this->userManager instanceof PublicEmitter) {
 				$this->userManager->emit('\OC\User', 'assignedUserId', [$username]);
 			}
+
 			foreach ($groupObjects as $group) {
 				$group->addUser($guestUser);
 			}
-		} catch (\Exception $e) {
+		} catch (\Exception $exception) {
 			return new DataResponse(
 				[
-					'errorMessages' => ['email' => $e->getMessage()],
+					'errorMessages' => ['email' => $exception->getMessage()],
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);

lib/Db/Transfer.php

@@ -26,6 +26,7 @@
  */
 class Transfer extends Entity {
 	public const STATUS_WAITING = 'waiting';
+
 	public const STATUS_STARTED = 'started';
 
 	/** @var string */

lib/FilteredNavigationManager.php

@@ -28,6 +28,9 @@ public function getAll(string $type = 'link'): array {
 		return array_filter($items, $this->isEntryWhitelisted(...));
 	}
 
+	/**
+	 * @param array<string, mixed> $item
+	 */
 	private function isEntryWhitelisted(array $item): bool {
 		return $this->whitelist->isUrlAllowed($this->user, $item['href']);
 	}

lib/GroupBackend.php

@@ -34,7 +34,7 @@ public function __construct(
 	}
 
 	private function getMembers(): array {
-		if (empty($this->guestMembers)) {
+		if ($this->guestMembers === []) {
 			$this->guestMembers = $this->guestManager->listGuests();
 		}
 
@@ -111,6 +111,7 @@ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0): arra
 			if ($this->guestManager->isGuest() && $this->config->hideOtherUsers()) {
 				return [$this->userSession->getUser()->getUID()];
 			}
+
 			return $offset === 0 ? $this->getMembers() : [];
 		}
 
@@ -122,15 +123,18 @@ public function countUsersInGroup(string $gid, string $search = ''): int {
 			if ($this->guestManager->isGuest() && $this->config->hideOtherUsers()) {
 				return 1;
 			}
+
 			return count($this->getMembers());
 		}
+
 		return 0;
 	}
 
 	public function getGroupDetails(string $gid): array {
 		if ($gid === $this->groupName) {
 			return ['displayName' => 'Guests'];
 		}
+
 		return [];
 	}