From 651595469698dcb6146abd1ec1dfd3d1a9b1c4c8 Mon Sep 17 00:00:00 2001 From: Lung Date: Thu, 11 Jul 2024 23:06:55 +0200 Subject: [PATCH] fixed phpstan according to shipmonk strict rules --- composer.json | 1 + composer.lock | 59 +++++++++++++++++++- phpstan.neon | 5 +- src/ErrorHandlerGetter.php | 1 + src/Event/EventType/Obrok/EventTypeObrok.php | 2 +- src/FileHandler/LocalSaveFileHandler.php | 1 + src/FileHandler/UploadFileHandler.php | 6 +- src/Import/ImportSrs.php | 1 + src/Orm/Mapper.php | 2 + src/Participant/Admin/AdminController.php | 2 +- src/Participant/ParticipantRepository.php | 17 ++++-- src/Payment/PaymentService.php | 4 +- src/Settings/Settings.php | 1 + src/Settings/TwigExtension.php | 17 +++--- src/Skautis/SkautisController.php | 2 + src/User/UserController.php | 12 +++- src/User/UserRole.php | 45 +++++++++++++-- src/User/UserService.php | 5 +- 18 files changed, 156 insertions(+), 27 deletions(-) diff --git a/composer.json b/composer.json index 7aa63f79..c8f01b71 100755 --- a/composer.json +++ b/composer.json @@ -68,6 +68,7 @@ "phpunit/phpunit": "^11", "roave/security-advisories": "dev-latest", "shipmonk/composer-dependency-analyser": "^1.3", + "shipmonk/phpstan-rules": "^3.1", "symfony/var-dumper": "^7.0" }, "autoload": { diff --git a/composer.lock b/composer.lock index 1f435b43..ddab4b52 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "981769b18a54c9c3a2b49725f1b03e8f", + "content-hash": "c701a98db8d9679c2a985140da2eeb77", "packages": [ { "name": "aws/aws-crt-php", @@ -10294,6 +10294,63 @@ }, "time": "2024-07-02T20:09:30+00:00" }, + { + "name": "shipmonk/phpstan-rules", + "version": "3.1.0", + "source": { + "type": "git", + "url": "https://github.com/shipmonk-rnd/phpstan-rules.git", + "reference": "8ace7eaadb0622707f7a9284cfbc9496193edf8e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/shipmonk-rnd/phpstan-rules/zipball/8ace7eaadb0622707f7a9284cfbc9496193edf8e", + "reference": "8ace7eaadb0622707f7a9284cfbc9496193edf8e", + "shasum": "" + }, + "require": { + "php": "^7.4 || ^8.0", + "phpstan/phpstan": "^1.11.0" + }, + "require-dev": { + "editorconfig-checker/editorconfig-checker": "^10.6.0", + "ergebnis/composer-normalize": "^2.28", + "nette/neon": "^3.3.1", + "phpstan/phpstan-phpunit": "^1.4.0", + "phpstan/phpstan-strict-rules": "^1.6.0", + "phpunit/phpunit": "^9.5.20", + "shipmonk/composer-dependency-analyser": "^1.3.0", + "shipmonk/name-collision-detector": "^2.0.0", + "slevomat/coding-standard": "^8.0.1" + }, + "type": "phpstan-extension", + "extra": { + "phpstan": { + "includes": [ + "rules.neon" + ] + } + }, + "autoload": { + "psr-4": { + "ShipMonk\\PHPStan\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Various extra strict PHPStan rules we found useful in ShipMonk.", + "keywords": [ + "PHPStan", + "static analysis" + ], + "support": { + "issues": "https://github.com/shipmonk-rnd/phpstan-rules/issues", + "source": "https://github.com/shipmonk-rnd/phpstan-rules/tree/3.1.0" + }, + "time": "2024-07-10T15:23:53+00:00" + }, { "name": "symfony/finder", "version": "v7.1.1", diff --git a/phpstan.neon b/phpstan.neon index 8356ffd0..265ade86 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -30,4 +30,7 @@ parameters: exceptions: uncheckedExceptionRegexes: - '#Exception#' - treatPhpDocTypesAsCertain: false + treatPhpDocTypesAsCertain: false + shipmonkRules: + enforceReadonlyPublicProperty: + enabled: false diff --git a/src/ErrorHandlerGetter.php b/src/ErrorHandlerGetter.php index 5479cbfa..0cfd1319 100755 --- a/src/ErrorHandlerGetter.php +++ b/src/ErrorHandlerGetter.php @@ -40,6 +40,7 @@ public function getErrorHandler(): callable return function (Throwable $throwable, Inspector $inspector) { if ($throwable instanceof HttpNotFoundException) { http_response_code(404); + /** @phpstan-ignore-next-line shipmonk.checkedExceptionInCallable */ echo $this->twig->fetch('404.twig'); die; } diff --git a/src/Event/EventType/Obrok/EventTypeObrok.php b/src/Event/EventType/Obrok/EventTypeObrok.php index f2207994..ab388667 100755 --- a/src/Event/EventType/Obrok/EventTypeObrok.php +++ b/src/Event/EventType/Obrok/EventTypeObrok.php @@ -125,7 +125,7 @@ public function getTranslationFilePaths(): array } #[\Override] - public function getStylesheetNameWithoutLeadingSlash(): ?string + public function getStylesheetNameWithoutLeadingSlash(): string { return 'eventSpecificCss/stylesObrok.css'; } diff --git a/src/FileHandler/LocalSaveFileHandler.php b/src/FileHandler/LocalSaveFileHandler.php index 7751b7ef..0f98a979 100755 --- a/src/FileHandler/LocalSaveFileHandler.php +++ b/src/FileHandler/LocalSaveFileHandler.php @@ -17,6 +17,7 @@ public function getFile(string $filename): File { $mimeContentType = mime_content_type($this->uploadFolder . $filename); if ($mimeContentType === false) { + /** @phpstan-ignore shipmonk.variableTypeOverwritten */ $mimeContentType = 'unknown_mime_type'; } diff --git a/src/FileHandler/UploadFileHandler.php b/src/FileHandler/UploadFileHandler.php index 9d6c6567..d65e0416 100644 --- a/src/FileHandler/UploadFileHandler.php +++ b/src/FileHandler/UploadFileHandler.php @@ -34,7 +34,11 @@ public function resolveUploadedFile(Request $request): ?UploadedFile $uploadedFile = $uploadedFiles['uploadFile']; // check for too-big files - if ($uploadedFile->getSize() > 10_000_000) { // 10MB + $fileSize = $uploadedFile->getSize(); + if ($fileSize === null) { + throw new \RuntimeException('Uploaded file size is null.'); + } + if ($fileSize > 10_000_000) { // 10MB $this->flashMessages->warning($this->translator->trans('flash.warning.fileTooBig')); return null; diff --git a/src/Import/ImportSrs.php b/src/Import/ImportSrs.php index 80a573de..2c140ed2 100644 --- a/src/Import/ImportSrs.php +++ b/src/Import/ImportSrs.php @@ -45,6 +45,7 @@ public function importIst(UploadedFile $istsDataFile, Event $event): void $istsData = []; try { /** @var array> $istsData */ + /** @phpstan-ignore shipmonk.variableTypeOverwritten */ $istsData = $this->csvParser->parseCsv($istsDataFile); } catch (\UnexpectedValueException | LeagueCsvException) { $this->flashMessages->error($this->translator->trans('flash.error.importSrs.invalidCsv')); diff --git a/src/Orm/Mapper.php b/src/Orm/Mapper.php index d57dc0d7..bae9f0e8 100755 --- a/src/Orm/Mapper.php +++ b/src/Orm/Mapper.php @@ -144,6 +144,7 @@ protected function trimNamespace(string $class): string protected function toUnderScore(string $string): string { + /** @phpstan-ignore shipmonk.unknownClosureParamType */ $pregReplaced = preg_replace_callback('#(?<=.)([A-Z])#', fn ($m) => '_' . strtolower($m[1]), $string); if ($pregReplaced === null) { throw new \RuntimeException('preg_replace_callback failed'); @@ -154,6 +155,7 @@ protected function toUnderScore(string $string): string protected function toCamelCase(string $string): string { + /** @phpstan-ignore shipmonk.unknownClosureParamType */ $pregReplaced = preg_replace_callback('#_(.)#', fn ($m) => strtoupper($m[1]), $string); if ($pregReplaced === null) { throw new \RuntimeException('preg_replace_callback failed'); diff --git a/src/Participant/Admin/AdminController.php b/src/Participant/Admin/AdminController.php index eed2c56d..e538b4e5 100755 --- a/src/Participant/Admin/AdminController.php +++ b/src/Participant/Admin/AdminController.php @@ -285,7 +285,7 @@ public function denyParticipant( $this->flashMessages->info($this->translator->trans('flash.info.denied')); $this->logger->info('Denied registration for participant with ID ' - . $participantId . ' and role ' . $participant->role?->value . ' with reason: ' . $reason); + . $participantId . ' and role ' . ($participant->role?->value ?? 'missing') . ' with reason: ' . $reason); return $this->redirect($request, $response, 'admin-show-approving'); } diff --git a/src/Participant/ParticipantRepository.php b/src/Participant/ParticipantRepository.php index a928371b..4e7fe347 100755 --- a/src/Participant/ParticipantRepository.php +++ b/src/Participant/ParticipantRepository.php @@ -381,7 +381,9 @@ public function getParticipantsForEntry(Event $event): array foreach($rows as $row) { /** @var string $role */ $role = $row['role']; - $participants[$role][$row['id']] = $this->mapDataToEntryParticipant($row); + /** @var string $id */ + $id = $row['id']; + $participants[$role][$id] = $this->mapDataToEntryParticipant($row); } $rowsDependableParticipants = $this->getRowsForEntryParticipant($event, [ @@ -390,15 +392,22 @@ public function getParticipantsForEntry(Event $event): array ]); foreach ($rowsDependableParticipants as $rowDependableParticipant) { - $role = match ($rowDependableParticipant['role']) { + /** @var string $dpId */ + $dpId = $rowDependableParticipant['id']; + /** @var string $dpRole */ + $dpRole = $rowDependableParticipant['role']; + /** @var string $dpPatrolLeaderId */ + $dpPatrolLeaderId = $rowDependableParticipant['patrol_leader_id']; + + $role = match ($dpRole) { ParticipantRole::TroopParticipant->value => ParticipantRole::TroopLeader->value, ParticipantRole::PatrolParticipant->value => ParticipantRole::PatrolLeader->value, default => 'unknown', }; - $leader = $participants[$role][$rowDependableParticipant['patrol_leader_id']] ?? null; + $leader = $participants[$role][$dpPatrolLeaderId] ?? null; if ($leader instanceof EntryParticipant) { - $leader->participants[$rowDependableParticipant['id']] + $leader->participants[$dpId] = $this->mapDataToEntryParticipant($rowDependableParticipant); } } diff --git a/src/Payment/PaymentService.php b/src/Payment/PaymentService.php index cae89fd9..b86c9125 100755 --- a/src/Payment/PaymentService.php +++ b/src/Payment/PaymentService.php @@ -172,7 +172,7 @@ public function updatePayments(Event $event, int $limit = 10): void /** @var BankPayment $bankPayment */ foreach (array_slice($freshBankPayments, 0, $limit) as $bankPayment) { if (array_key_exists($bankPayment->variableSymbol ?? '', $participantKeydPayments)) { - $payment = $participantKeydPayments[$bankPayment->variableSymbol]; + $payment = $participantKeydPayments[$bankPayment->variableSymbol ?? '']; if ($payment->price === $bankPayment->price) { // match! $this->confirmPayment($payment); @@ -246,7 +246,7 @@ public function calculatePaymentDueDate( private function composeNote(Participant $participant, Event $event): string { if ($participant instanceof PatrolLeader) { - return $event->slug . ' ' . $participant->patrolName . ' ' . $participant->getFullName(); + return $event->slug . ' ' . ($participant->patrolName ?? '') . ' ' . $participant->getFullName(); } return $event->slug . ' ' . $participant->getFullName(); diff --git a/src/Settings/Settings.php b/src/Settings/Settings.php index 2d1d77da..88a0c0f7 100755 --- a/src/Settings/Settings.php +++ b/src/Settings/Settings.php @@ -303,6 +303,7 @@ public function getContainerDefinition(string $envPath, string $envFilename): ar Translator $translator, FlashMessagesBySession $flashMessages, ) { + /** @phpstan-ignore shipmonk.checkedExceptionInCallable */ $view = Twig::create( [ __DIR__ . '/../Templates/translatable', diff --git a/src/Settings/TwigExtension.php b/src/Settings/TwigExtension.php index aa5d2884..07f3aab5 100755 --- a/src/Settings/TwigExtension.php +++ b/src/Settings/TwigExtension.php @@ -4,6 +4,7 @@ namespace kissj\Settings; +use kissj\Participant\Participant; use kissj\Participant\Patrol\PatrolLeader; use kissj\Participant\Patrol\PatrolParticipant; use kissj\Participant\Troop\TroopLeader; @@ -22,35 +23,35 @@ public function getTests(): array return [ new TwigTest( 'PatrolLeader', - fn ($participant): bool => $participant instanceof PatrolLeader + fn (Participant $participant): bool => $participant instanceof PatrolLeader ), new TwigTest( 'PatrolParticipant', - fn ($participant): bool => $participant instanceof PatrolParticipant + fn (Participant $participant): bool => $participant instanceof PatrolParticipant ), new TwigTest( 'TroopLeader', - fn ($participant): bool => $participant instanceof TroopLeader + fn (Participant $participant): bool => $participant instanceof TroopLeader ), new TwigTest( 'TroopParticipant', - fn ($participant): bool => $participant instanceof TroopParticipant + fn (Participant $participant): bool => $participant instanceof TroopParticipant ), new TwigTest( 'Leader', - fn ($participant): bool => $participant instanceof PatrolLeader || $participant instanceof TroopLeader + fn (Participant $participant): bool => $participant instanceof PatrolLeader || $participant instanceof TroopLeader ), new TwigTest( 'Troop', - fn ($participant): bool => $participant instanceof TroopLeader || $participant instanceof TroopParticipant + fn (Participant $participant): bool => $participant instanceof TroopLeader || $participant instanceof TroopParticipant ), new TwigTest( 'hasUser', - fn ($participant): bool => !$participant instanceof PatrolParticipant + fn (Participant $participant): bool => !$participant instanceof PatrolParticipant ), new TwigTest( 'eligibleForShowTieCode', - fn ($participant): bool => ( + fn (Participant $participant): bool => ( $participant instanceof TroopLeader && $participant->getUserButNotNull()->status === UserStatus::Open ) || ( // TroopLeader TieCode is also used for pairing with some external services, so they have to be able to access it in paid status diff --git a/src/Skautis/SkautisController.php b/src/Skautis/SkautisController.php index 055eb83f..77a9d354 100755 --- a/src/Skautis/SkautisController.php +++ b/src/Skautis/SkautisController.php @@ -22,6 +22,8 @@ public function __construct( */ public function redirectFromSkautis(Request $request, Response $response): Response { + // TODO fix into method $this->getParsedBody(); + /** @phpstan-ignore shipmonk.forbiddenCast */ $this->skautisService->saveDataFromPost((array)$request->getParsedBody()); $eventSlug = $this->getParameterFromQuery($request, 'ReturnUrl'); diff --git a/src/User/UserController.php b/src/User/UserController.php index 893fa5ca..8d0c7b99 100755 --- a/src/User/UserController.php +++ b/src/User/UserController.php @@ -137,7 +137,17 @@ public function getDashboard(User $user, Request $request, Response $response): return match ($user->role) { UserRole::Participant => $this->redirect($request, $response, 'dashboard', $routerEventSlug), - default => $this->redirect($request, $response, 'admin-dashboard', $routerEventSlug), + UserRole::Admin, + UserRole::IstAdmin, + UserRole::ContingentAdminCs, + UserRole::ContingentAdminSk, + UserRole::ContingentAdminPl, + UserRole::ContingentAdminHu, + UserRole::ContingentAdminEu, + UserRole::ContingentAdminRo, + UserRole::ContingentAdminGb, + UserRole::ContingentAdminSw, + => $this->redirect($request, $response, 'admin-dashboard', $routerEventSlug), }; } diff --git a/src/User/UserRole.php b/src/User/UserRole.php index a0e6e61c..5e26aad5 100755 --- a/src/User/UserRole.php +++ b/src/User/UserRole.php @@ -40,24 +40,57 @@ public static function adminRoles(): array public function isEligibleToHandlePayments(): bool { return match ($this) { - self::Admin, self::ContingentAdminCs, self::ContingentAdminSk => true, - default => false, + self::Admin, + self::ContingentAdminCs, + self::ContingentAdminSk + => true, + self::Participant, + self::IstAdmin, + self::ContingentAdminPl, + self::ContingentAdminHu, + self::ContingentAdminEu, + self::ContingentAdminRo, + self::ContingentAdminGb, + self::ContingentAdminSw, + => false, }; } public function isEligibleToManageTroops(): bool { return match ($this) { - self::Admin, self::ContingentAdminCs, self::ContingentAdminSk => true, - default => false, + self::Admin, + self::ContingentAdminCs, + self::ContingentAdminSk + => true, + self::Participant, + self::IstAdmin, + self::ContingentAdminPl, + self::ContingentAdminHu, + self::ContingentAdminEu, + self::ContingentAdminRo, + self::ContingentAdminGb, + self::ContingentAdminSw, + => false, }; } public function isEligibleToImportIst(): bool { return match ($this) { - self::Admin => true, - default => false, + self::Admin + => true, + self::Participant, + self::IstAdmin, + self::ContingentAdminCs, + self::ContingentAdminSk, + self::ContingentAdminPl, + self::ContingentAdminHu, + self::ContingentAdminEu, + self::ContingentAdminRo, + self::ContingentAdminGb, + self::ContingentAdminSw, + => false, }; } } diff --git a/src/User/UserService.php b/src/User/UserService.php index 6e0e9caf..6e44f660 100755 --- a/src/User/UserService.php +++ b/src/User/UserService.php @@ -85,7 +85,10 @@ public function isLoginTokenValid(string $loginToken): bool return false; } - return $lastToken->createdAt > DateTimeUtils::getDateTime('now - 24 hours'); + /** @var \DateTimeInterface $createdAt */ + $createdAt = $lastToken->createdAt; + + return $createdAt > DateTimeUtils::getDateTime('now - 24 hours'); } public function getLoginTokenFromStringToken(string $token): LoginToken