From 5e78dc6ed54aeb6d3d2565334074286a0e249b78 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 8 Apr 2026 21:03:20 +0100 Subject: [PATCH] Maintenance: Updated PHPStan to Level 4 (#6085) --- .gitignore | 1 + app/Access/EmailConfirmationService.php | 2 ++ app/Access/LoginService.php | 2 +- app/Access/Mfa/MfaValue.php | 5 ++- app/Access/Oidc/OidcJwtSigningKey.php | 17 ++++------ app/Access/Oidc/OidcJwtWithClaims.php | 4 +-- app/Access/Oidc/OidcUserDetails.php | 2 +- app/Access/Saml2Service.php | 4 +-- app/Access/SocialAuthService.php | 4 +-- .../Notifications/NotificationManager.php | 14 ++++---- app/Api/ApiDocsGenerator.php | 10 ++++-- app/Api/ApiEntityListFormatter.php | 7 ++-- app/Api/ApiTokenGuard.php | 33 +++++-------------- .../Commands/AssignSortRuleCommand.php | 2 +- .../Commands/CopyShelfPermissionsCommand.php | 17 ++++++---- app/Entities/Models/Book.php | 2 +- app/Entities/Models/Entity.php | 1 + app/Entities/Models/Page.php | 2 +- app/Entities/Repos/PageRepo.php | 2 +- app/Entities/Tools/PageContent.php | 2 +- app/Entities/Tools/PermissionsUpdater.php | 2 +- app/Exports/ExportFormatter.php | 4 +-- app/Exports/ZipExports/ZipImportRunner.php | 14 ++++---- app/Exports/ZipExports/ZipReferenceParser.php | 4 --- app/Http/Controller.php | 2 +- app/Permissions/JointPermissionBuilder.php | 3 +- app/Search/SearchOptions.php | 12 +++---- app/Sorting/BookSorter.php | 3 +- app/Uploads/ImageRepo.php | 2 +- app/Uploads/UserAvatars.php | 2 +- app/Users/Models/User.php | 3 +- phpstan.neon.dist | 4 +-- tests/Api/SearchApiTest.php | 1 + .../CopyShelfPermissionsCommandTest.php | 18 ++++++++++ 34 files changed, 105 insertions(+), 102 deletions(-) diff --git a/.gitignore b/.gitignore index b545d161f..06a8723c5 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /node_modules /.vscode /composer +/composer.phar /coverage Homestead.yaml .env diff --git a/app/Access/EmailConfirmationService.php b/app/Access/EmailConfirmationService.php index 1a5156d3e..e950c5504 100644 --- a/app/Access/EmailConfirmationService.php +++ b/app/Access/EmailConfirmationService.php @@ -5,6 +5,7 @@ namespace BookStack\Access; 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 { diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php index c81e95572..0c32b538f 100644 --- a/app/Access/LoginService.php +++ b/app/Access/LoginService.php @@ -71,7 +71,7 @@ class LoginService } $lastLoginDetails = $this->getLastLoginAttemptDetails(); - $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember'] ?? false); + $this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember']); } /** diff --git a/app/Access/Mfa/MfaValue.php b/app/Access/Mfa/MfaValue.php index dd3e04618..b0f08e826 100644 --- a/app/Access/Mfa/MfaValue.php +++ b/app/Access/Mfa/MfaValue.php @@ -48,17 +48,16 @@ class MfaValue extends Model } /** - * 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(); } /** diff --git a/app/Access/Oidc/OidcJwtSigningKey.php b/app/Access/Oidc/OidcJwtSigningKey.php index 3dab3e442..3edba12b3 100644 --- a/app/Access/Oidc/OidcJwtSigningKey.php +++ b/app/Access/Oidc/OidcJwtSigningKey.php @@ -9,10 +9,7 @@ use phpseclib3\Math\BigInteger; 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. @@ -20,15 +17,13 @@ class OidcJwtSigningKey * '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 @@ class OidcJwtSigningKey /** * @throws OidcInvalidKeyException */ - protected function loadFromPath(string $path) + protected function loadFromPath(string $path): void { try { $key = PublicKeyLoader::load( @@ -58,7 +53,7 @@ class OidcJwtSigningKey /** * @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 @@ class OidcJwtSigningKey throw new OidcInvalidKeyException('A "n" parameter on the provided key is expected'); } - $n = strtr($jwk['n'] ?? '', '-_', '+/'); + $n = strtr($jwk['n'], '-_', '+/'); try { $key = PublicKeyLoader::load([ diff --git a/app/Access/Oidc/OidcJwtWithClaims.php b/app/Access/Oidc/OidcJwtWithClaims.php index 06c04d81e..9d7eeead1 100644 --- a/app/Access/Oidc/OidcJwtWithClaims.php +++ b/app/Access/Oidc/OidcJwtWithClaims.php @@ -102,12 +102,12 @@ class OidcJwtWithClaims implements ProvidesClaims 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'); } } diff --git a/app/Access/Oidc/OidcUserDetails.php b/app/Access/Oidc/OidcUserDetails.php index 7a422a58d..b1736f97d 100644 --- a/app/Access/Oidc/OidcUserDetails.php +++ b/app/Access/Oidc/OidcUserDetails.php @@ -39,7 +39,7 @@ class OidcUserDetails ): 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; } diff --git a/app/Access/Saml2Service.php b/app/Access/Saml2Service.php index 106a7a229..5572d2104 100644 --- a/app/Access/Saml2Service.php +++ b/app/Access/Saml2Service.php @@ -266,7 +266,7 @@ class Saml2Service /** * 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 @@ class Saml2Service ]); } - if ($userDetails['email'] === null) { + if (empty($userDetails['email'])) { throw new SamlException(trans('errors.saml_no_email_address')); } diff --git a/app/Access/SocialAuthService.php b/app/Access/SocialAuthService.php index c3c20587d..bdcfb45c8 100644 --- a/app/Access/SocialAuthService.php +++ b/app/Access/SocialAuthService.php @@ -117,14 +117,14 @@ class SocialAuthService } // 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'); diff --git a/app/Activity/Notifications/NotificationManager.php b/app/Activity/Notifications/NotificationManager.php index 8a6c26ffb..38da2c552 100644 --- a/app/Activity/Notifications/NotificationManager.php +++ b/app/Activity/Notifications/NotificationManager.php @@ -15,14 +15,14 @@ use BookStack\Users\Models\User; class NotificationManager { /** - * @var class-string[] + * @var array[]> */ - 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 @@ class NotificationManager */ 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; } } diff --git a/app/Api/ApiDocsGenerator.php b/app/Api/ApiDocsGenerator.php index eb8f5508c..a59cb8198 100644 --- a/app/Api/ApiDocsGenerator.php +++ b/app/Api/ApiDocsGenerator.php @@ -17,7 +17,14 @@ use ReflectionMethod; class ApiDocsGenerator { + /** + * @var array + */ protected array $reflectionClasses = []; + + /** + * @var array + */ protected array $controllerClasses = []; /** @@ -107,7 +114,6 @@ class ApiDocsGenerator */ 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 @@ class ApiDocsGenerator $matches = []; preg_match_all('/^\s*?\*\s?($|((?![\/@\s]).*?))$/m', $comment, $matches); - $text = implode(' ', $matches[1] ?? []); + $text = implode(' ', $matches[1]); return str_replace(' ', "\n", $text); } diff --git a/app/Api/ApiEntityListFormatter.php b/app/Api/ApiEntityListFormatter.php index 3c94d96ee..23073bfc2 100644 --- a/app/Api/ApiEntityListFormatter.php +++ b/app/Api/ApiEntityListFormatter.php @@ -74,18 +74,21 @@ class ApiEntityListFormatter /** * Include parent book/chapter info in the formatted data. + * These functions are careful to not load the relation themselves, since they should + * have already been loaded in a more efficient manner, with permissions applied, by the time + * the parent fields are handled here. */ public function withParents(): self { $this->withField('book', function (Entity $entity) { - if ($entity instanceof BookChild && $entity->book) { + if ($entity instanceof BookChild && $entity->relationLoaded('book') && $entity->getRelationValue('book')) { return $entity->book->only(['id', 'name', 'slug']); } return null; }); $this->withField('chapter', function (Entity $entity) { - if ($entity instanceof Page && $entity->chapter) { + if ($entity instanceof Page && $entity->relationLoaded('chapter') && $entity->getRelationValue('chapter')) { return $entity->chapter->only(['id', 'name', 'slug']); } return null; diff --git a/app/Api/ApiTokenGuard.php b/app/Api/ApiTokenGuard.php index 9f4537b29..f1a3f0dc8 100644 --- a/app/Api/ApiTokenGuard.php +++ b/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 @@ class ApiTokenGuard implements Guard } /** - * 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 @@ class ApiTokenGuard implements Guard 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 @@ class ApiTokenGuard implements Guard /** * {@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 @@ class ApiTokenGuard implements Guard /** * "Log out" the currently authenticated user. */ - public function logout() + public function logout(): void { $this->user = null; } diff --git a/app/Console/Commands/AssignSortRuleCommand.php b/app/Console/Commands/AssignSortRuleCommand.php index c438d0783..f00df8383 100644 --- a/app/Console/Commands/AssignSortRuleCommand.php +++ b/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(); } diff --git a/app/Console/Commands/CopyShelfPermissionsCommand.php b/app/Console/Commands/CopyShelfPermissionsCommand.php index c5e2d504e..1207621de 100644 --- a/app/Console/Commands/CopyShelfPermissionsCommand.php +++ b/app/Console/Commands/CopyShelfPermissionsCommand.php @@ -32,6 +32,7 @@ class CopyShelfPermissionsCommand extends Command { $shelfSlug = $this->option('slug'); $cascadeAll = $this->option('all'); + $noInteraction = boolval($this->option('no-interaction')); $shelves = null; if (!$cascadeAll && !$shelfSlug) { @@ -41,14 +42,16 @@ class CopyShelfPermissionsCommand extends Command } 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']); diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index 1909dbd56..10f04695a 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -17,7 +17,7 @@ use Illuminate\Support\Collection; * * @property string $description * @property string $description_html - * @property int $image_id + * @property ?int $image_id * @property ?int $default_template_id * @property ?int $sort_rule_id * @property \Illuminate\Database\Eloquent\Collection $chapters diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 47e134626..27cfccaa8 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -479,6 +479,7 @@ abstract class Entity extends Model implements 'chapter' => new Chapter(), 'book' => new Book(), 'bookshelf' => new Bookshelf(), + default => throw new \InvalidArgumentException("Invalid entity type: {$type}"), }; } } diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php index a1d3fc7b4..d3a392da6 100644 --- a/app/Entities/Models/Page.php +++ b/app/Entities/Models/Page.php @@ -23,7 +23,7 @@ use Illuminate\Database\Eloquent\Relations\HasOne; * @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 diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index bc590785d..375bf1d2b 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -60,7 +60,7 @@ class PageRepo $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, diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 8d89a86cf..6a7649900 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -359,7 +359,7 @@ class PageContent { $contentHash = md5($html); $contentId = $this->page->id; - $contentTime = $this->page->updated_at?->timestamp ?? time(); + $contentTime = $this->page->updated_at->timestamp ?? time(); $appVersion = AppVersion::get(); $filterConfig = config('app.content_filtering') ?? ''; return "page-content-cache::{$filterConfig}::{$appVersion}::{$contentId}::{$contentTime}::{$contentHash}"; diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php index fa9ae753c..f3165b603 100644 --- a/app/Entities/Tools/PermissionsUpdater.php +++ b/app/Entities/Tools/PermissionsUpdater.php @@ -47,7 +47,7 @@ class PermissionsUpdater { if (isset($data['role_permissions'])) { $entity->permissions()->where('role_id', '!=', 0)->delete(); - $rolePermissionData = $this->formatPermissionsFromApiRequestToEntityPermissions($data['role_permissions'] ?? [], false); + $rolePermissionData = $this->formatPermissionsFromApiRequestToEntityPermissions($data['role_permissions'], false); $entity->permissions()->createMany($rolePermissionData); } diff --git a/app/Exports/ExportFormatter.php b/app/Exports/ExportFormatter.php index c5973eace..6bf0a05ad 100644 --- a/app/Exports/ExportFormatter.php +++ b/app/Exports/ExportFormatter.php @@ -208,7 +208,7 @@ class ExportFormatter preg_match_all("/\/i", $htmlContent, $imageTagsOutput); // Replace image src with base64 encoded image strings - if (isset($imageTagsOutput[0]) && count($imageTagsOutput[0]) > 0) { + if (count($imageTagsOutput[0]) > 0) { foreach ($imageTagsOutput[0] as $index => $imgMatch) { $oldImgTagString = $imgMatch; $srcString = $imageTagsOutput[2][$index]; @@ -225,7 +225,7 @@ class ExportFormatter preg_match_all("/\/i", $htmlContent, $linksOutput); // Update relative links to be absolute, with instance url - if (isset($linksOutput[0]) && count($linksOutput[0]) > 0) { + if (count($linksOutput[0]) > 0) { foreach ($linksOutput[0] as $index => $linkMatch) { $oldLinkString = $linkMatch; $srcString = $linksOutput[2][$index]; diff --git a/app/Exports/ZipExports/ZipImportRunner.php b/app/Exports/ZipExports/ZipImportRunner.php index 382e4073e..9fa7dec3a 100644 --- a/app/Exports/ZipExports/ZipImportRunner.php +++ b/app/Exports/ZipExports/ZipImportRunner.php @@ -82,10 +82,8 @@ class ZipImportRunner $entity = $this->importBook($exportModel, $reader); } else if ($exportModel instanceof ZipExportChapter) { $entity = $this->importChapter($exportModel, $parent, $reader); - } else if ($exportModel instanceof ZipExportPage) { - $entity = $this->importPage($exportModel, $parent, $reader); } else { - throw new ZipImportException(['No importable data found in import data.']); + $entity = $this->importPage($exportModel, $parent, $reader); } $this->references->replaceReferences(); @@ -132,7 +130,7 @@ class ZipImportRunner 'name' => $exportBook->name, 'description_html' => $exportBook->description_html ?? '', 'image' => $exportBook->cover ? $this->zipFileToUploadedFile($exportBook->cover, $reader) : null, - 'tags' => $this->exportTagsToInputArray($exportBook->tags ?? []), + 'tags' => $this->exportTagsToInputArray($exportBook->tags), ]); if ($book->coverInfo()->getImage()) { @@ -151,7 +149,7 @@ class ZipImportRunner foreach ($children as $child) { if ($child instanceof ZipExportChapter) { $this->importChapter($child, $book, $reader); - } else if ($child instanceof ZipExportPage) { + } else { $this->importPage($child, $book, $reader); } } @@ -166,7 +164,7 @@ class ZipImportRunner $chapter = $this->chapterRepo->create([ 'name' => $exportChapter->name, 'description_html' => $exportChapter->description_html ?? '', - 'tags' => $this->exportTagsToInputArray($exportChapter->tags ?? []), + 'tags' => $this->exportTagsToInputArray($exportChapter->tags), ], $parent); $exportPages = $exportChapter->pages; @@ -199,7 +197,7 @@ class ZipImportRunner 'name' => $exportPage->name, 'markdown' => $exportPage->markdown ?? '', 'html' => $exportPage->html ?? '', - 'tags' => $this->exportTagsToInputArray($exportPage->tags ?? []), + 'tags' => $this->exportTagsToInputArray($exportPage->tags), ]); $this->references->addPage($page, $exportPage); @@ -302,7 +300,7 @@ class ZipImportRunner array_push($chapters, ...$exportModel->chapters); } else if ($exportModel instanceof ZipExportChapter) { $chapters[] = $exportModel; - } else if ($exportModel instanceof ZipExportPage) { + } else { $pages[] = $exportModel; } diff --git a/app/Exports/ZipExports/ZipReferenceParser.php b/app/Exports/ZipExports/ZipReferenceParser.php index a6560e3f2..9bb069ab7 100644 --- a/app/Exports/ZipExports/ZipReferenceParser.php +++ b/app/Exports/ZipExports/ZipReferenceParser.php @@ -68,10 +68,6 @@ class ZipReferenceParser $matches = []; preg_match_all($referenceRegex, $content, $matches); - if (count($matches) < 3) { - return $content; - } - for ($i = 0; $i < count($matches[0]); $i++) { $referenceText = $matches[0][$i]; $type = strtolower($matches[1][$i]); diff --git a/app/Http/Controller.php b/app/Http/Controller.php index 1a0f5932e..796505795 100644 --- a/app/Http/Controller.php +++ b/app/Http/Controller.php @@ -62,7 +62,7 @@ abstract class Controller extends BaseController */ protected function checkPermission(string|Permission $permission): void { - if (!user() || !user()->can($permission)) { + if (!user()->can($permission)) { $this->showPermissionError(); } } diff --git a/app/Permissions/JointPermissionBuilder.php b/app/Permissions/JointPermissionBuilder.php index 56b22ad16..94f18916d 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -61,8 +61,7 @@ class JointPermissionBuilder return; } - /** @var BookChild $entity */ - if ($entity->book) { + if ($entity instanceof BookChild) { $entities[] = $entity->book; } diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index 83af2d043..cfd068386 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -121,13 +121,11 @@ class SearchOptions foreach ($patterns as $termType => $pattern) { $matches = []; preg_match_all($pattern, $searchString, $matches); - if (count($matches) > 0) { - foreach ($matches[1] as $index => $value) { - $negated = str_starts_with($matches[0][$index], '-'); - $terms[$termType][] = $constructors[$termType]($value, $negated); - } - $searchString = preg_replace($pattern, '', $searchString); + foreach ($matches[1] as $index => $value) { + $negated = str_starts_with($matches[0][$index], '-'); + $terms[$termType][] = $constructors[$termType]($value, $negated); } + $searchString = preg_replace($pattern, '', $searchString); } // Unescape exacts and backslash escapes @@ -261,7 +259,7 @@ class SearchOptions $userFilters = ['updated_by', 'created_by', 'owned_by']; $unsupportedFilters = ['is_template', 'sort_by']; foreach ($this->filters->all() as $filter) { - if (in_array($filter->getKey(), $userFilters, true) && $filter->value !== null && $filter->value !== 'me') { + if (in_array($filter->getKey(), $userFilters, true) && $filter->value && $filter->value !== 'me') { $options[] = $filter; } else if (in_array($filter->getKey(), $unsupportedFilters, true)) { $options[] = $filter; diff --git a/app/Sorting/BookSorter.php b/app/Sorting/BookSorter.php index b4f93d47b..0862aaa88 100644 --- a/app/Sorting/BookSorter.php +++ b/app/Sorting/BookSorter.php @@ -125,9 +125,8 @@ class BookSorter */ protected function applySortUpdates(BookSortMapItem $sortMapItem, array $modelMap): void { - /** @var BookChild $model */ $model = $modelMap[$sortMapItem->type . ':' . $sortMapItem->id] ?? null; - if (!$model) { + if (!($model instanceof BookChild)) { return; } diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index a16b87bd7..e87e22b3a 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -91,7 +91,7 @@ class ImageRepo $parentFilter = function (Builder $query) use ($filterType, $contextPage) { if ($filterType === 'page') { $query->where('uploaded_to', '=', $contextPage->id); - } else if ($filterType === 'book') { + } else { $validPageIds = $contextPage->book->pages() ->scopes('visible') ->pluck('id') diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php index 0cc640f22..8fcf63580 100644 --- a/app/Uploads/UserAvatars.php +++ b/app/Uploads/UserAvatars.php @@ -148,7 +148,7 @@ class UserAvatars $responseCount++; $isRedirect = ($response->getStatusCode() === 301 || $response->getStatusCode() === 302); $url = $response->getHeader('Location')[0] ?? ''; - } while ($responseCount < 3 && $isRedirect && is_string($url) && str_starts_with($url, 'http')); + } while ($responseCount < 3 && $isRedirect && str_starts_with($url, 'http')); if ($responseCount === 3) { throw new HttpFetchException("Failed to fetch image, max redirect limit of 3 tries reached. Last fetched URL: {$url}"); diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 50efdcdad..b9289a5a7 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -222,8 +222,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon public function getAvatar(int $size = 50): string { $default = url('/user_avatar.png'); - $imageId = $this->image_id; - if ($imageId === 0 || $imageId === '0' || $imageId === null) { + if ($this->image_id === 0) { return $default; } diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 72189222f..bab28ea0e 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -7,11 +7,11 @@ parameters: - app # The level 8 is the highest level - level: 3 + level: 4 phpVersion: min: 80200 - max: 80400 + max: 80500 bootstrapFiles: - bootstrap/phpstan.php diff --git a/tests/Api/SearchApiTest.php b/tests/Api/SearchApiTest.php index 517c5d8e4..5d0ce53ec 100644 --- a/tests/Api/SearchApiTest.php +++ b/tests/Api/SearchApiTest.php @@ -106,6 +106,7 @@ class SearchApiTest extends TestCase $this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]); $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue'); + $resp->assertOk(); $resp->assertJsonPath('data.0.id', $page->id); $resp->assertJsonPath('data.0.book.name', $book->name); $resp->assertJsonMissingPath('data.0.chapter'); diff --git a/tests/Commands/CopyShelfPermissionsCommandTest.php b/tests/Commands/CopyShelfPermissionsCommandTest.php index 5c21a2e34..d5f9677a2 100644 --- a/tests/Commands/CopyShelfPermissionsCommandTest.php +++ b/tests/Commands/CopyShelfPermissionsCommandTest.php @@ -2,6 +2,7 @@ namespace Tests\Commands; +use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; use Tests\TestCase; @@ -61,4 +62,21 @@ class CopyShelfPermissionsCommandTest extends TestCase 'view' => true, 'update' => true, 'create' => false, 'delete' => false, ]); } + + public function test_copy_shelf_permissions_command_using_slug_without_interaction() + { + $shelf = $this->entities->shelfHasBooks(); + $editorRole = $this->users->editor()->roles()->first(); + /** @var Book $child */ + $child = $shelf->books()->first(); + $child->shelves()->where('id', '!=', $shelf->id)->delete(); + + $this->assertFalse($child->hasPermissions()); + + $this->permissions->setEntityPermissions($shelf, ['view', 'update'], [$editorRole]); + $this->artisan('bookstack:copy-shelf-permissions --all --no-interaction'); + + $child->refresh(); + $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted'); + } }