Maintenance: Started work for PHPStan Level 4

.gitignore

@@ -2,6 +2,7 @@
 /node_modules
 /.vscode
 /composer
+/composer.phar
 /coverage
 Homestead.yaml
 .env

app/Access/EmailConfirmationService.php

@@ -5,6 +5,7 @@
 use BookStack\Access\Notifications\ConfirmEmailNotification;
 use BookStack\Exceptions\ConfirmationEmailException;
 use BookStack\Users\Models\User;
+use Exception;
 
 class EmailConfirmationService extends UserTokenService
 {
@@ -16,6 +17,7 @@ class EmailConfirmationService extends UserTokenService
      * Also removes any existing old ones.
      *
      * @throws ConfirmationEmailException
+     * @throws Exception
      */
     public function sendConfirmation(User $user): void
     {

app/Access/LoginService.php

@@ -71,7 +71,7 @@ public function reattemptLoginFor(User $user): void
         }
 
         $lastLoginDetails = $this->getLastLoginAttemptDetails();
-        $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember'] ?? false);
+        $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember']);
     }
 
     /**

app/Access/Mfa/MfaValue.php

@@ -48,17 +48,16 @@ public static function upsertWithValue(User $user, string $method, string $value
     }
 
     /**
-     * Easily get the decrypted MFA value for the given user and method.
+     * Get the decrypted MFA value for the given user and method.
      */
     public static function getValueForUser(User $user, string $method): ?string
     {
-        /** @var MfaValue $mfaVal */
         $mfaVal = static::query()
             ->where('user_id', '=', $user->id)
             ->where('method', '=', $method)
             ->first();
 
-        return $mfaVal ? $mfaVal->getValue() : null;
+        return $mfaVal?->getValue();
     }
 
     /**

app/Access/Oidc/OidcJwtSigningKey.php

@@ -9,26 +9,21 @@
 
 class OidcJwtSigningKey
 {
-    /**
-     * @var PublicKey
-     */
-    protected $key;
+    protected PublicKey $key;
 
     /**
      * Can be created either from a JWK parameter array or local file path to load a certificate from.
      * Examples:
      * 'file:///var/www/cert.pem'
      * ['kty' => 'RSA', 'alg' => 'RS256', 'n' => 'abc123...'].
      *
-     * @param array|string $jwkOrKeyPath
-     *
      * @throws OidcInvalidKeyException
      */
-    public function __construct($jwkOrKeyPath)
+    public function __construct(array|string $jwkOrKeyPath)
     {
         if (is_array($jwkOrKeyPath)) {
             $this->loadFromJwkArray($jwkOrKeyPath);
-        } elseif (is_string($jwkOrKeyPath) && strpos($jwkOrKeyPath, 'file://') === 0) {
+        } elseif (str_starts_with($jwkOrKeyPath, 'file://')) {
             $this->loadFromPath($jwkOrKeyPath);
         } else {
             throw new OidcInvalidKeyException('Unexpected type of key value provided');
@@ -38,7 +33,7 @@ public function __construct($jwkOrKeyPath)
     /**
      * @throws OidcInvalidKeyException
      */
-    protected function loadFromPath(string $path)
+    protected function loadFromPath(string $path): void
     {
         try {
             $key = PublicKeyLoader::load(
@@ -58,7 +53,7 @@ protected function loadFromPath(string $path)
     /**
      * @throws OidcInvalidKeyException
      */
-    protected function loadFromJwkArray(array $jwk)
+    protected function loadFromJwkArray(array $jwk): void
     {
         // 'alg' is optional for a JWK, but we will still attempt to validate if
         // it exists otherwise presume it will be compatible.
@@ -82,7 +77,7 @@ protected function loadFromJwkArray(array $jwk)
             throw new OidcInvalidKeyException('A "n" parameter on the provided key is expected');
         }
 
-        $n = strtr($jwk['n'] ?? '', '-_', '+/');
+        $n = strtr($jwk['n'], '-_', '+/');
 
         try {
             $key = PublicKeyLoader::load([

app/Access/Oidc/OidcJwtWithClaims.php

@@ -102,12 +102,12 @@ public function replaceClaims(array $claims): void
     protected function validateTokenStructure(): void
     {
         foreach (['header', 'payload'] as $prop) {
-            if (empty($this->$prop) || !is_array($this->$prop)) {
+            if (empty($this->$prop)) {
                 throw new OidcInvalidTokenException("Could not parse out a valid {$prop} within the provided token");
             }
         }
 
-        if (empty($this->signature) || !is_string($this->signature)) {
+        if (empty($this->signature)) {
             throw new OidcInvalidTokenException('Could not parse out a valid signature within the provided token');
         }
     }

app/Access/Oidc/OidcUserDetails.php

@@ -39,7 +39,7 @@ public function populate(
     ): void {
         $this->externalId = $claims->getClaim($idClaim) ?? $this->externalId;
         $this->email = $claims->getClaim('email') ?? $this->email;
-        $this->name = static::getUserDisplayName($displayNameClaims, $claims) ?? $this->name;
+        $this->name = static::getUserDisplayName($displayNameClaims, $claims) ?: $this->name;
         $this->groups = static::getUserGroups($groupsClaim, $claims) ?? $this->groups;
         $this->picture = static::getPicture($claims) ?: $this->picture;
     }

app/Access/Saml2Service.php

@@ -266,7 +266,7 @@ protected function getExternalId(array $samlAttributes, string $defaultValue)
     /**
      * Extract the details of a user from a SAML response.
      *
-     * @return array{external_id: string, name: string, email: string, saml_id: string}
+     * @return array{external_id: string, name: string, email: string|null, saml_id: string}
      */
     protected function getUserDetails(string $samlID, $samlAttributes): array
     {
@@ -357,7 +357,7 @@ public function processLoginCallback(string $samlID, array $samlAttributes): Use
             ]);
         }
 
-        if ($userDetails['email'] === null) {
+        if (empty($userDetails['email'])) {
             throw new SamlException(trans('errors.saml_no_email_address'));
         }
 

app/Access/SocialAuthService.php

@@ -117,14 +117,14 @@ public function handleLoginCallback(string $socialDriver, SocialUser $socialUser
         }
 
         // When a user is logged in and the social account exists and is already linked to the current user.
-        if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id === $currentUser->id) {
+        if ($isLoggedIn && $socialAccount->user->id === $currentUser->id) {
             session()->flash('error', trans('errors.social_account_existing', ['socialAccount' => $titleCaseDriver]));
 
             return redirect('/my-account/auth#social_accounts');
         }
 
         // When a user is logged in, A social account exists but the users do not match.
-        if ($isLoggedIn && $socialAccount !== null && $socialAccount->user->id != $currentUser->id) {
+        if ($isLoggedIn && $socialAccount->user->id != $currentUser->id) {
             session()->flash('error', trans('errors.social_account_already_used_existing', ['socialAccount' => $titleCaseDriver]));
 
             return redirect('/my-account/auth#social_accounts');

app/Activity/Notifications/NotificationManager.php

@@ -15,14 +15,14 @@
 class NotificationManager
 {
     /**
-     * @var class-string<NotificationHandler>[]
+     * @var array<string, class-string<NotificationHandler>[]>
      */
-    protected array $handlers = [];
+    protected array $handlersByActivity = [];
 
     public function handle(Activity $activity, string|Loggable $detail, User $user): void
     {
         $activityType = $activity->type;
-        $handlersToRun = $this->handlers[$activityType] ?? [];
+        $handlersToRun = $this->handlersByActivity[$activityType] ?? [];
         foreach ($handlersToRun as $handlerClass) {
             /** @var NotificationHandler $handler */
             $handler = new $handlerClass();
@@ -35,12 +35,12 @@ public function handle(Activity $activity, string|Loggable $detail, User $user):
      */
     public function registerHandler(string $activityType, string $handlerClass): void
     {
-        if (!isset($this->handlers[$activityType])) {
-            $this->handlers[$activityType] = [];
+        if (!isset($this->handlersByActivity[$activityType])) {
+            $this->handlersByActivity[$activityType] = [];
         }
 
-        if (!in_array($handlerClass, $this->handlers[$activityType])) {
-            $this->handlers[$activityType][] = $handlerClass;
+        if (!in_array($handlerClass, $this->handlersByActivity[$activityType])) {
+            $this->handlersByActivity[$activityType][] = $handlerClass;
         }
     }
 

app/Api/ApiDocsGenerator.php

@@ -17,7 +17,14 @@
 
 class ApiDocsGenerator
 {
+    /**
+     * @var array<string, ReflectionClass>
+     */
     protected array $reflectionClasses = [];
+
+    /**
+     * @var array<string, ApiController>
+     */
     protected array $controllerClasses = [];
 
     /**
@@ -107,7 +114,6 @@ protected function loadDetailsFromControllers(Collection $routes): Collection
      */
     protected function getBodyParamsFromClass(string $className, string $methodName): ?array
     {
-        /** @var ApiController $class */
         $class = $this->controllerClasses[$className] ?? null;
         if ($class === null) {
             $class = app()->make($className);
@@ -153,7 +159,7 @@ protected function parseDescriptionFromDocBlockComment(string $comment): string
         $matches = [];
         preg_match_all('/^\s*?\*\s?($|((?![\/@\s]).*?))$/m', $comment, $matches);
 
-        $text = implode(' ', $matches[1] ?? []);
+        $text = implode(' ', $matches[1]);
         return str_replace('  ', "\n", $text);
     }
 

app/Api/ApiEntityListFormatter.php

@@ -78,7 +78,7 @@ public function withTags(): self
     public function withParents(): self
     {
         $this->withField('book', function (Entity $entity) {
-            if ($entity instanceof BookChild && $entity->book) {
+            if ($entity instanceof BookChild) {
                 return $entity->book->only(['id', 'name', 'slug']);
             }
             return null;

app/Api/ApiTokenGuard.php

@@ -16,30 +16,15 @@ class ApiTokenGuard implements Guard
 {
     use GuardHelpers;
 
-    /**
-     * The request instance.
-     */
-    protected $request;
-
-    /**
-     * @var LoginService
-     */
-    protected $loginService;
-
     /**
      * The last auth exception thrown in this request.
-     *
-     * @var ApiAuthException
      */
-    protected $lastAuthException;
+    protected ApiAuthException|null $lastAuthException = null;
 
-    /**
-     * ApiTokenGuard constructor.
-     */
-    public function __construct(Request $request, LoginService $loginService)
-    {
-        $this->request = $request;
-        $this->loginService = $loginService;
+    public function __construct(
+        protected Request $request,
+        protected LoginService $loginService
+    ) {
     }
 
     /**
@@ -67,7 +52,7 @@ public function user()
     }
 
     /**
-     * Determine if current user is authenticated. If not, throw an exception.
+     * Determine if the current user is authenticated. If not, throw an exception.
      *
      * @throws ApiAuthException
      *
@@ -121,7 +106,7 @@ protected function validateTokenHeaderValue(string $authToken): void
             throw new ApiAuthException(trans('errors.api_no_authorization_found'));
         }
 
-        if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) {
+        if (!str_contains($authToken, ':') || !str_starts_with($authToken, 'Token ')) {
             throw new ApiAuthException(trans('errors.api_bad_authorization_format'));
         }
     }
@@ -155,7 +140,7 @@ protected function validateToken(?ApiToken $token, string $secret): void
     /**
      * {@inheritdoc}
      */
-    public function validate(array $credentials = [])
+    public function validate(array $credentials = []): bool
     {
         if (empty($credentials['id']) || empty($credentials['secret'])) {
             return false;
@@ -175,7 +160,7 @@ public function validate(array $credentials = [])
     /**
      * "Log out" the currently authenticated user.
      */
-    public function logout()
+    public function logout(): void
     {
         $this->user = null;
     }

app/Console/Commands/AssignSortRuleCommand.php

@@ -32,7 +32,7 @@ class AssignSortRuleCommand extends Command
      */
     public function handle(BookSorter $sorter): int
     {
-        $sortRuleId = intval($this->argument('sort-rule')) ?? 0;
+        $sortRuleId = intval($this->argument('sort-rule'));
         if ($sortRuleId === 0) {
             return $this->listSortRules();
         }

app/Console/Commands/CopyShelfPermissionsCommand.php

@@ -32,6 +32,7 @@ public function handle(PermissionsUpdater $permissionsUpdater, BookshelfQueries
     {
         $shelfSlug = $this->option('slug');
         $cascadeAll = $this->option('all');
+        $noInteraction = boolval($this->option('no-interaction'));
         $shelves = null;
 
         if (!$cascadeAll && !$shelfSlug) {
@@ -41,14 +42,16 @@ public function handle(PermissionsUpdater $permissionsUpdater, BookshelfQueries
         }
 
         if ($cascadeAll) {
-            $continue = $this->confirm(
-                'Permission settings for all shelves will be cascaded. ' .
-                        'Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. ' .
-                        'Are you sure you want to proceed?'
-            );
+            if (!$noInteraction) {
+                $continue = $this->confirm(
+                    'Permission settings for all shelves will be cascaded. ' .
+                    'Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. ' .
+                    'Are you sure you want to proceed?',
+                );
 
-            if (!$continue && !$this->hasOption('no-interaction')) {
-                return 0;
+                if (!$continue) {
+                    return 0;
+                }
             }
 
             $shelves = $queries->start()->get(['id']);

app/Entities/Models/Entity.php

@@ -479,6 +479,7 @@ public static function instanceFromType(string $type): self
             'chapter' => new Chapter(),
             'book' => new Book(),
             'bookshelf' => new Bookshelf(),
+            default => throw new \InvalidArgumentException("Invalid entity type: {$type}"),
         };
     }
 }

app/Entities/Models/Page.php

@@ -23,7 +23,7 @@
  * @property bool         $draft
  * @property int          $revision_count
  * @property string       $editor
- * @property Chapter      $chapter
+ * @property Chapter|null $chapter
  * @property Collection   $attachments
  * @property Collection   $revisions
  * @property PageRevision $currentRevision

app/Entities/Repos/PageRepo.php

@@ -60,7 +60,7 @@ public function getNewDraftPage(Entity $parent): Page
             $page->book_id = $parent->id;
         }
 
-        $defaultTemplate = $page->chapter?->defaultTemplate()->get() ?? $page->book?->defaultTemplate()->get();
+        $defaultTemplate = $page->chapter?->defaultTemplate()->get() ?? $page->book->defaultTemplate()->get();
         if ($defaultTemplate) {
             $page->forceFill([
                 'html'  => $defaultTemplate->html,

app/Http/Controller.php

@@ -62,7 +62,7 @@ protected function showPermissionError(string $redirectLocation = '/'): never
      */
     protected function checkPermission(string|Permission $permission): void
     {
-        if (!user() || !user()->can($permission)) {
+        if (!user()->can($permission)) {
             $this->showPermissionError();
         }
     }