Maintenance: Updated PHPStan to Level 4 (#6085)

This commit is contained in:
Dan Brown
2026-04-08 21:03:20 +01:00
committed by GitHub
parent c33853ed84
commit 5e78dc6ed5
34 changed files with 105 additions and 102 deletions

1
.gitignore vendored
View File

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

View File

@@ -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
{

View File

@@ -71,7 +71,7 @@ class LoginService
}
$lastLoginDetails = $this->getLastLoginAttemptDetails();
$this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember'] ?? false);
$this->login($user, $lastLoginDetails['method'], $lastLoginDetails['remember']);
}
/**

View File

@@ -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();
}
/**

View File

@@ -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([

View File

@@ -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');
}
}

View File

@@ -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;
}

View File

@@ -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'));
}

View File

@@ -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');

View File

@@ -15,14 +15,14 @@ use BookStack\Users\Models\User;
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 @@ 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;
}
}

View File

@@ -17,7 +17,14 @@ use ReflectionMethod;
class ApiDocsGenerator
{
/**
* @var array<string, ReflectionClass>
*/
protected array $reflectionClasses = [];
/**
* @var array<string, ApiController>
*/
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);
}

View File

@@ -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;

View File

@@ -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;
}

View File

@@ -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();
}

View File

@@ -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']);

View File

@@ -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

View File

@@ -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}"),
};
}
}

View File

@@ -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

View File

@@ -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,

View File

@@ -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}";

View File

@@ -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);
}

View File

@@ -208,7 +208,7 @@ class ExportFormatter
preg_match_all("/\<img.*?src\=(\'|\")(.*?)(\'|\").*?\>/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("/\<a.*href\=(\'|\")(.*?)(\'|\").*?\>/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];

View File

@@ -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;
}

View File

@@ -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]);

View File

@@ -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();
}
}

View File

@@ -61,8 +61,7 @@ class JointPermissionBuilder
return;
}
/** @var BookChild $entity */
if ($entity->book) {
if ($entity instanceof BookChild) {
$entities[] = $entity->book;
}

View File

@@ -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;

View File

@@ -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;
}

View File

@@ -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')

View File

@@ -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}");

View File

@@ -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;
}

View File

@@ -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

View File

@@ -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');

View File

@@ -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');
}
}