refactor: Modernize code with rector

REUSE.toml

@@ -30,7 +30,7 @@ SPDX-FileCopyrightText = "2018 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
 
 [[annotations]]
-path = ["composer.json", "composer.lock"]
+path = ["composer.json", "composer.lock", "vendor-bin/*/composer.json", "vendor-bin/*/composer.lock"]
 precedence = "aggregate"
 SPDX-FileCopyrightText = "2019 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"

composer.json

@@ -17,6 +17,9 @@
 		}
 	},
 	"scripts": {
+		"post-install-cmd": [
+			"@composer bin all install --ansi"
+		],
 		"lint": "find . -name \\*.php -not -path './vendor/*' -print0 | xargs -0 -n1 php -l",
 		"cs:check": "php-cs-fixer fix --dry-run --diff",
 		"cs:fix": "php-cs-fixer fix",
@@ -25,16 +28,19 @@
 		"psalm:update-baseline:force": "psalm --threads=1 --update-baseline --set-baseline=tests/psalm-baseline.xml",
 		"psalm:clear": "psalm --clear-cache && psalm --clear-global-cache",
 		"psalm:fix": "psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,InvalidFalsableReturnType",
-		"test:unit": "vendor/bin/phpunit -c phpunit.xml"
+		"test:unit": "vendor/bin/phpunit -c phpunit.xml",
+		"rector": "rector && composer cs:fix"
 	},
 	"require": {
-		"php": ">=8.1 <=8.4"
+		"php": ">=8.1 <=8.4",
+		"bamarni/composer-bin-plugin": "^1.9"
 	},
 	"require-dev": {
 		"nextcloud/coding-standard": "^1.3.2",
 		"nextcloud/ocp": "dev-stable30",
 		"php-parallel-lint/php-parallel-lint": "^1.2",
 		"phpunit/phpunit": "^9",
+		"roave/security-advisories": "dev-latest",
 		"vimeo/psalm": "^5.26.1"
 	}
 }

lib/AppConfigOverwrite.php

@@ -13,7 +13,7 @@
 class AppConfigOverwrite extends AppConfig {
 
 	/** @var string[][] */
-	private $overWrite = [];
+	private array $overWrite = [];
 
 	public function setOverwrite(array $overwrite): void {
 		$this->overWrite = $overwrite;

lib/AppInfo/Application.php

@@ -65,9 +65,10 @@ public function boot(IBootContext $context): void {
 	}
 
 	private function setupGuestManagement(ContainerInterface $container, ContainerInterface $server): void {
+		/** @var Hooks $hookManager */
 		$hookManager = $container->get(Hooks::class);
-		$server->get(IEventDispatcher::class)->addListener(ShareCreatedEvent::class, [$hookManager, 'handlePostShare']);
-		$server->get(IEventDispatcher::class)->addListener(UserFirstTimeLoggedInEvent::class, [$hookManager, 'handleFirstLogin']);
+		$server->get(IEventDispatcher::class)->addListener(ShareCreatedEvent::class, $hookManager->handlePostShare(...));
+		$server->get(IEventDispatcher::class)->addListener(UserFirstTimeLoggedInEvent::class, $hookManager->handleFirstLogin(...));
 	}
 
 	private function setupGuestRestrictions(ContainerInterface $container, ContainerInterface $server): void {
@@ -81,7 +82,7 @@ private function setupGuestRestrictions(ContainerInterface $container, Container
 			$restrictionManager->verifyAccess();
 			$restrictionManager->setupRestrictions();
 		} else {
-			$userSession->listen('\OC\User', 'postLogin', function () use ($restrictionManager) {
+			$userSession->listen('\OC\User', 'postLogin', function () use ($restrictionManager): void {
 				$restrictionManager->verifyAccess();
 				$restrictionManager->setupRestrictions();
 			});

lib/AppWhitelist.php

@@ -22,30 +22,23 @@
  * @package OCA\Guests
  */
 class AppWhitelist {
-	private string $baseUrl;
-	private int $baseUrlLength;
+	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';
 
 	public const DEFAULT_WHITELIST = 'files_trashbin,files_versions,files_sharing,files_texteditor,text,activity,firstrunwizard,photos,notifications,dashboard,user_status,weather_status';
 
 	/**
 	 * AppWhitelist constructor.
-	 *
-	 * @param Config $config
-	 * @param GuestManager $guestManager
-	 * @param IL10N $l10n
-	 * @param IAppManager $appManager
-	 * @param IURLGenerator $urlGenerator
-	 * @param LoggerInterface $logger
 	 */
 	public function __construct(
 		IURLGenerator $urlGenerator,
-		private Config $config,
-		private GuestManager $guestManager,
-		private IL10N $l10n,
-		private IAppManager $appManager,
-		private LoggerInterface $logger,
+		private readonly Config $config,
+		private readonly GuestManager $guestManager,
+		private readonly IL10N $l10n,
+		private readonly IAppManager $appManager,
+		private readonly LoggerInterface $logger,
 	) {
 		$this->baseUrl = $urlGenerator->getBaseUrl();
 		$this->baseUrlLength = strlen($this->baseUrl);
@@ -62,9 +55,6 @@ public function isWhitelistEnabled(): bool {
 		return $this->config->useWhitelist();
 	}
 
-	/**
-	 * @param false|string $url
-	 */
 	public function isUrlAllowed(IUser $user, string|false $url): bool {
 		if ($this->guestManager->isGuest($user) && $this->isWhitelistEnabled()) {
 			$app = $this->getRequestedApp($url);
@@ -98,45 +88,43 @@ public function verifyAccess(IUser $user, IRequest $request): void {
 	/**
 	 * Core has \OC::$REQUESTEDAPP but it isn't set until the routes are matched
 	 * taken from \OC\Route\Router::match()
-	 *
-	 * @param false|string $url
 	 */
 	private function getRequestedApp(string|false $url): string {
 		if (substr($url, 0, $this->baseUrlLength) === $this->baseUrl) {
 			$url = substr($url, $this->baseUrlLength);
 		}
-		if (strpos($url, '/index.php/') === 0) {
+		if (str_starts_with($url, '/index.php/')) {
 			$url = substr($url, 10);
 		}
-		if (substr($url, 0, 6) === '/apps/') {
+		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') {
 			return 'core';
-		} elseif (substr($url, 0, 6) === '/core/') {
+		} elseif (str_starts_with($url, '/core/')) {
 			return 'core';
-		} elseif (substr($url, 0, 4) === '/js/') {
+		} elseif (str_starts_with($url, '/js/')) {
 			return 'core';
-		} elseif (substr($url, 0, 5) === '/css/') {
+		} elseif (str_starts_with($url, '/css/')) {
 			return 'core';
-		} elseif (substr($url, 0, 6) === '/login') {
+		} elseif (str_starts_with($url, '/login')) {
 			return 'core';
-		} elseif (substr($url, 0, 7) === '/logout') {
+		} elseif (str_starts_with($url, '/logout')) {
 			return 'core';
-		} elseif (substr($url, 0, 3) === '/f/') {
+		} elseif (str_starts_with($url, '/f/')) {
 			return 'files';
-		} elseif (substr($url, 0, 8) === '/webdav/') {
+		} elseif (str_starts_with($url, '/webdav/')) {
 			return 'dav';
-		} elseif (substr($url, 0, 5) === '/dav/') {
+		} elseif (str_starts_with($url, '/dav/')) {
 			return 'dav';
-		} elseif (substr($url, 0, 6) === '/call/') {
+		} elseif (str_starts_with($url, '/call/')) {
 			return 'spreed';
-		} elseif (substr($url, 0, 10) === '/settings/') {
+		} elseif (str_starts_with($url, '/settings/')) {
 			return 'settings';
-		} elseif (substr($url, 0, 8) === '/avatar/') {
+		} elseif (str_starts_with($url, '/avatar/')) {
 			return 'avatar';
-		} elseif (substr($url, 0, 10) === '/heartbeat') {
+		} elseif (str_starts_with($url, '/heartbeat')) {
 			return 'heartbeat';
 		}
 		return 'core';

lib/BackgroundJob/TransferJob.php

@@ -25,13 +25,13 @@
 class TransferJob extends QueuedJob {
 	public function __construct(
 		ITimeFactory $time,
-		private IUserManager $userManager,
-		private ISecureRandom $secureRandom,
-		private NotificationManager $notificationManager,
-		private IURLGenerator $urlGenerator,
-		private TransferService $transferService,
-		private TransferMapper $transferMapper,
-		private LoggerInterface $logger,
+		private readonly IUserManager $userManager,
+		private readonly ISecureRandom $secureRandom,
+		private readonly NotificationManager $notificationManager,
+		private readonly IURLGenerator $urlGenerator,
+		private readonly TransferService $transferService,
+		private readonly TransferMapper $transferMapper,
+		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($time);
 	}

lib/Capabilities.php

@@ -19,7 +19,7 @@ class Capabilities implements ICapability {
 	/**
 	 * @inheritDoc
 	 */
-	public function getCapabilities() {
+	public function getCapabilities(): array {
 		return [
 			'guests' => [
 				'enabled' => true,

lib/Command/AddCommand.php

@@ -12,6 +12,7 @@
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Mail\IMailer;
+use Override;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Input\InputArgument;
@@ -21,21 +22,16 @@
 use Symfony\Component\Console\Question\Question;
 
 class AddCommand extends Command {
-	/** @var IUserManager */
-	private $userManager;
-	/** @var GuestManager */
-	private $guestManager;
-	/** @var IMailer */
-	private $mailer;
-
-	public function __construct(IUserManager $userManager, IMailer $mailer, GuestManager $guestManager) {
+	public function __construct(
+		private readonly IUserManager $userManager,
+		private readonly IMailer $mailer,
+		private readonly GuestManager $guestManager,
+	) {
 		parent::__construct();
-		$this->userManager = $userManager;
-		$this->guestManager = $guestManager;
-		$this->mailer = $mailer;
 	}
 
-	protected function configure() {
+	#[Override]
+	protected function configure(): void {
 		$this
 			->setName('guests:add')
 			->setDescription('Add a new guest account')
@@ -78,24 +74,25 @@ protected function configure() {
 		parent::configure();
 	}
 
-	protected function execute(InputInterface $input, OutputInterface $output) {
+	#[Override]
+	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$creatorUser = $this->userManager->get($input->getArgument('created-by'));
 		if ($creatorUser === null) {
 			$output->writeln('<error>The user "' . $input->getArgument('created-by') . '" does not exist.</error>');
-			return 1;
+			return self::FAILURE;
 		}
 
 		// same behavior like in the UsersController
 		$uid = $input->getArgument('email');
 		if ($this->userManager->userExists($uid)) {
 			$output->writeln('<error>The user "' . $uid . '" already exists.</error>');
-			return 1;
+			return self::FAILURE;
 		}
 
 		$email = $input->getArgument('email');
 		if (!$this->mailer->validateMailAddress($email)) {
 			$output->writeln('<error>Invalid email address "' . $email . '".</error>');
-			return 1;
+			return self::FAILURE;
 		}
 
 		$password = null;
@@ -104,7 +101,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 				$password = getenv('OC_PASS');
 				if (!$password) {
 					$output->writeln('<error>--password-from-env given, but OC_PASS is empty!</error>');
-					return 1;
+					return self::FAILURE;
 				}
 			} elseif ($input->isInteractive()) {
 				/** @var QuestionHelper $helper */
@@ -120,11 +117,11 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 
 				if ($password !== $confirm) {
 					$output->writeln('<error>Passwords did not match!</error>');
-					return 1;
+					return self::FAILURE;
 				}
 			} else {
 				$output->writeln('<error>Interactive input or --password-from-env is needed for entering a password!</error>');
-				return 1;
+				return self::FAILURE;
 			}
 		}
 
@@ -139,10 +136,10 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 
 		if ($user instanceof IUser) {
 			$output->writeln('<info>The guest account user "' . $user->getUID() . '" was created successfully</info>');
-			return 0;
+			return self::SUCCESS;
 		} else {
 			$output->writeln('<error>An error occurred while creating the user</error>');
-			return 1;
+			return self::FAILURE;
 		}
 	}
 }

lib/Command/ListCommand.php

@@ -10,25 +10,27 @@
 
 use OC\Core\Command\Base;
 use OCA\Guests\GuestManager;
+use Override;
 use Symfony\Component\Console\Helper\Table;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class ListCommand extends Base {
-	private $guestManager;
-
-	public function __construct(GuestManager $guestManager) {
+	public function __construct(
+		private readonly GuestManager $guestManager,
+	) {
 		parent::__construct();
-		$this->guestManager = $guestManager;
 	}
 
+	#[Override]
 	protected function configure(): void {
 		$this
 			->setName('guests:list')
 			->setDescription('List created guests');
 		parent::configure();
 	}
 
+	#[Override]
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$guests = $this->guestManager->getGuestsInfo();
 
@@ -39,7 +41,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			} else {
 				$output->writeln('<info>No guests created</info>');
 			}
-			return 0;
+			return self::SUCCESS;
 		}
 
 		if ($outputType === self::OUTPUT_FORMAT_JSON || $outputType === self::OUTPUT_FORMAT_JSON_PRETTY) {
@@ -50,6 +52,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			$table->setRows($guests);
 			$table->render();
 		}
-		return 0;
+		return self::SUCCESS;
 	}
 }

lib/Config.php

@@ -16,11 +16,11 @@
 
 class Config {
 	public function __construct(
-		private IConfig $config,
-		private IAppConfig $appConfig,
-		private ISubAdmin $subAdmin,
-		private IUserSession $userSession,
-		private IGroupManager $groupManager,
+		private readonly IConfig $config,
+		private readonly IAppConfig $appConfig,
+		private readonly ISubAdmin $subAdmin,
+		private readonly IUserSession $userSession,
+		private readonly IGroupManager $groupManager,
 	) {
 	}