From 876bc10d4d414c0680ea13e008b764c3ee70060f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 20 Nov 2021 14:03:56 +0000 Subject: [PATCH] Applied another set of static analysis improvements --- app/Actions/ActivityService.php | 7 ++++--- app/Auth/Access/ExternalBaseUserProvider.php | 15 +++++++-------- app/Auth/Permissions/RolePermission.php | 9 +++------ app/Console/Commands/RegenerateSearch.php | 2 +- app/Entities/Models/Deletion.php | 3 ++- app/Entities/Models/Entity.php | 4 +++- app/Entities/Models/PageRevision.php | 6 ++---- app/Entities/Repos/BaseRepo.php | 4 +++- app/Entities/Repos/PageRepo.php | 8 ++++++-- app/Entities/Tools/BookContents.php | 4 ++-- app/Entities/Tools/NextPreviousContentLocator.php | 2 +- app/Entities/Tools/PageContent.php | 6 +++--- app/Entities/Tools/SearchIndex.php | 2 +- app/Entities/Tools/SearchRunner.php | 5 ++++- app/Entities/Tools/SiblingFetcher.php | 3 ++- app/Entities/Tools/TrashCan.php | 10 ++++++---- app/Http/Controllers/RecycleBinController.php | 1 + app/Interfaces/Deletable.php | 14 ++++++++++++++ 18 files changed, 65 insertions(+), 40 deletions(-) create mode 100644 app/Interfaces/Deletable.php diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index bc7a6b6b7..33ed44b32 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -4,6 +4,7 @@ namespace BookStack\Actions; use BookStack\Auth\Permissions\PermissionService; use BookStack\Auth\User; +use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; @@ -100,13 +101,13 @@ class ActivityService */ public function entityActivity(Entity $entity, int $count = 20, int $page = 1): array { - /** @var [string => int[]] $queryIds */ + /** @var array $queryIds */ $queryIds = [$entity->getMorphClass() => [$entity->id]]; - if ($entity->isA('book')) { + if ($entity instanceof Book) { $queryIds[(new Chapter())->getMorphClass()] = $entity->chapters()->visible()->pluck('id'); } - if ($entity->isA('book') || $entity->isA('chapter')) { + if ($entity instanceof Book || $entity instanceof Chapter) { $queryIds[(new Page())->getMorphClass()] = $entity->pages()->visible()->pluck('id'); } diff --git a/app/Auth/Access/ExternalBaseUserProvider.php b/app/Auth/Access/ExternalBaseUserProvider.php index fde610c3e..dc765ddc5 100644 --- a/app/Auth/Access/ExternalBaseUserProvider.php +++ b/app/Auth/Access/ExternalBaseUserProvider.php @@ -4,6 +4,7 @@ namespace BookStack\Auth\Access; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Auth\UserProvider; +use Illuminate\Database\Eloquent\Model; class ExternalBaseUserProvider implements UserProvider { @@ -16,8 +17,6 @@ class ExternalBaseUserProvider implements UserProvider /** * LdapUserProvider constructor. - * - * @param $model */ public function __construct(string $model) { @@ -27,7 +26,7 @@ class ExternalBaseUserProvider implements UserProvider /** * Create a new instance of the model. * - * @return \Illuminate\Database\Eloquent\Model + * @return Model */ public function createModel() { @@ -41,7 +40,7 @@ class ExternalBaseUserProvider implements UserProvider * * @param mixed $identifier * - * @return \Illuminate\Contracts\Auth\Authenticatable|null + * @return Authenticatable|null */ public function retrieveById($identifier) { @@ -54,7 +53,7 @@ class ExternalBaseUserProvider implements UserProvider * @param mixed $identifier * @param string $token * - * @return \Illuminate\Contracts\Auth\Authenticatable|null + * @return Authenticatable|null */ public function retrieveByToken($identifier, $token) { @@ -64,7 +63,7 @@ class ExternalBaseUserProvider implements UserProvider /** * Update the "remember me" token for the given user in storage. * - * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param Authenticatable $user * @param string $token * * @return void @@ -79,7 +78,7 @@ class ExternalBaseUserProvider implements UserProvider * * @param array $credentials * - * @return \Illuminate\Contracts\Auth\Authenticatable|null + * @return Authenticatable|null */ public function retrieveByCredentials(array $credentials) { @@ -94,7 +93,7 @@ class ExternalBaseUserProvider implements UserProvider /** * Validate a user against the given credentials. * - * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param Authenticatable $user * @param array $credentials * * @return bool diff --git a/app/Auth/Permissions/RolePermission.php b/app/Auth/Permissions/RolePermission.php index 0a0e6ff17..f34de917c 100644 --- a/app/Auth/Permissions/RolePermission.php +++ b/app/Auth/Permissions/RolePermission.php @@ -4,6 +4,7 @@ namespace BookStack\Auth\Permissions; use BookStack\Auth\Role; use BookStack\Model; +use Illuminate\Database\Eloquent\Relations\BelongsToMany; /** * @property int $id @@ -13,19 +14,15 @@ class RolePermission extends Model /** * The roles that belong to the permission. */ - public function roles() + public function roles(): BelongsToMany { return $this->belongsToMany(Role::class, 'permission_role', 'permission_id', 'role_id'); } /** * Get the permission object by name. - * - * @param $name - * - * @return mixed */ - public static function getByName($name) + public static function getByName(string $name): ?RolePermission { return static::where('name', '=', $name)->first(); } diff --git a/app/Console/Commands/RegenerateSearch.php b/app/Console/Commands/RegenerateSearch.php index 62ee88fc0..20e3fc798 100644 --- a/app/Console/Commands/RegenerateSearch.php +++ b/app/Console/Commands/RegenerateSearch.php @@ -49,7 +49,7 @@ class RegenerateSearch extends Command DB::setDefaultConnection($this->option('database')); } - $this->searchIndex->indexAllEntities(function (Entity $model, int $processed, int $total) { + $this->searchIndex->indexAllEntities(function (Entity $model, int $processed, int $total): void { $this->info('Indexed ' . class_basename($model) . ' entries (' . $processed . '/' . $total . ')'); }); diff --git a/app/Entities/Models/Deletion.php b/app/Entities/Models/Deletion.php index 3face841b..97abb87ff 100644 --- a/app/Entities/Models/Deletion.php +++ b/app/Entities/Models/Deletion.php @@ -3,13 +3,14 @@ namespace BookStack\Entities\Models; use BookStack\Auth\User; +use BookStack\Interfaces\Deletable; use BookStack\Interfaces\Loggable; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\MorphTo; /** - * @property Model $deletable + * @property Deletable $deletable */ class Deletion extends Model implements Loggable { diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index 4c4e55bb8..eccec40b5 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -12,6 +12,7 @@ use BookStack\Auth\Permissions\JointPermission; use BookStack\Entities\Tools\SearchIndex; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Facades\Permissions; +use BookStack\Interfaces\Deletable; use BookStack\Interfaces\Favouritable; use BookStack\Interfaces\Sluggable; use BookStack\Interfaces\Viewable; @@ -44,7 +45,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @method static Builder withLastView() * @method static Builder withViewCount() */ -abstract class Entity extends Model implements Sluggable, Favouritable, Viewable +abstract class Entity extends Model implements Sluggable, Favouritable, Viewable, Deletable { use SoftDeletes; use HasCreatorAndUpdater; @@ -210,6 +211,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable /** * Check if this instance or class is a certain type of entity. * Examples of $type are 'page', 'book', 'chapter'. + * @deprecated Use instanceof instead. */ public static function isA(string $type): bool { diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php index 6856c23e1..a3c4379a6 100644 --- a/app/Entities/Models/PageRevision.php +++ b/app/Entities/Models/PageRevision.php @@ -84,11 +84,9 @@ class PageRevision extends Model * Included here to align with entities in similar use cases. * (Yup, Bit of an awkward hack). * - * @param $type - * - * @return bool + * @deprecated Use instanceof instead. */ - public static function isA($type) + public static function isA(string $type): bool { return $type === 'revision'; } diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 569918503..6b29dad7b 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -67,10 +67,12 @@ class BaseRepo /** * Update the given items' cover image, or clear it. * + * @param Entity&HasCoverImage $entity + * * @throws ImageUploadException * @throws \Exception */ - public function updateCoverImage(HasCoverImage $entity, ?UploadedFile $coverImage, bool $removeImage = false) + public function updateCoverImage($entity, ?UploadedFile $coverImage, bool $removeImage = false) { if ($coverImage) { $this->imageRepo->destroyImage($entity->cover); diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index f66f2beb8..ed5534f08 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -290,6 +290,8 @@ class PageRepo public function restoreRevision(Page $page, int $revisionId): Page { $page->revision_count++; + + /** @var PageRevision $revision */ $revision = $page->revisions()->where('id', '=', $revisionId)->first(); $page->fill($revision->toArray()); @@ -334,7 +336,8 @@ class PageRepo } $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : null; - $page->changeBook($parent instanceof Book ? $parent->id : $parent->book->id); + $newBookId = ($parent instanceof Chapter) ? $parent->book->id : $parent->id; + $page->changeBook($newBookId); $page->rebuildPermissions(); Activity::addForEntity($page, ActivityType::PAGE_MOVE); @@ -406,7 +409,7 @@ class PageRepo */ protected function changeParent(Page $page, Entity $parent) { - $book = ($parent instanceof Book) ? $parent : $parent->book; + $book = ($parent instanceof Chapter) ? $parent->book : $parent; $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : 0; $page->save(); @@ -467,6 +470,7 @@ class PageRepo { $parent = $page->getParent(); if ($parent instanceof Chapter) { + /** @var ?Page $lastPage */ $lastPage = $parent->pages('desc')->first(); return $lastPage ? $lastPage->priority + 1 : 0; diff --git a/app/Entities/Tools/BookContents.php b/app/Entities/Tools/BookContents.php index 8622d5e12..9b2190ca2 100644 --- a/app/Entities/Tools/BookContents.php +++ b/app/Entities/Tools/BookContents.php @@ -67,7 +67,7 @@ class BookContents $all->each(function (Entity $entity) use ($renderPages) { $entity->setRelation('book', $this->book); - if ($renderPages && $entity->isA('page')) { + if ($renderPages && $entity instanceof Page) { $entity->html = (new PageContent($entity))->render(); } }); @@ -151,7 +151,7 @@ class BookContents $priorityChanged = intval($model->priority) !== intval($sortMapItem->sort); $bookChanged = intval($model->book_id) !== intval($sortMapItem->book); - $chapterChanged = ($sortMapItem->type === 'page') && intval($model->chapter_id) !== $sortMapItem->parentChapter; + $chapterChanged = ($model instanceof Page) && intval($model->chapter_id) !== $sortMapItem->parentChapter; if ($bookChanged) { $model->changeBook($sortMapItem->book); diff --git a/app/Entities/Tools/NextPreviousContentLocator.php b/app/Entities/Tools/NextPreviousContentLocator.php index f70abd9b6..54b575935 100644 --- a/app/Entities/Tools/NextPreviousContentLocator.php +++ b/app/Entities/Tools/NextPreviousContentLocator.php @@ -64,7 +64,7 @@ class NextPreviousContentLocator /** @var Entity $item */ foreach ($bookTree->all() as $item) { $flatOrdered->push($item); - $childPages = $item->visible_pages ?? []; + $childPages = $item->getAttribute('visible_pages') ?? []; $flatOrdered = $flatOrdered->concat($childPages); } diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 45bfe8fa1..a6ba352a8 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -156,7 +156,7 @@ class PageContent /** * Parse a base64 image URI into the data and extension. * - * @return array{extension: array, data: string} + * @return array{extension: string, data: string} */ protected function parseBase64ImageUri(string $uri): array { @@ -230,7 +230,7 @@ class PageContent */ protected function setUniqueId(\DOMNode $element, array &$idMap): array { - if (get_class($element) !== 'DOMElement') { + if (!$element instanceof \DOMElement) { return ['', '']; } @@ -242,7 +242,7 @@ class PageContent return [$existingId, $existingId]; } - // Create an unique id for the element + // Create a unique id for the element // Uses the content as a basis to ensure output is the same every time // the same content is passed through. $contentId = 'bkmrk-' . mb_substr(strtolower(preg_replace('/\s+/', '-', trim($element->nodeValue))), 0, 20); diff --git a/app/Entities/Tools/SearchIndex.php b/app/Entities/Tools/SearchIndex.php index d748c1695..79139ec24 100644 --- a/app/Entities/Tools/SearchIndex.php +++ b/app/Entities/Tools/SearchIndex.php @@ -67,7 +67,7 @@ class SearchIndex * - The number that have been processed so far. * - The total number of that model to be processed. * - * @param callable(Entity, int, int)|null $progressCallback + * @param callable(Entity, int, int):void|null $progressCallback */ public function indexAllEntities(?callable $progressCallback = null) { diff --git a/app/Entities/Tools/SearchRunner.php b/app/Entities/Tools/SearchRunner.php index 04f4c5768..a4f131482 100644 --- a/app/Entities/Tools/SearchRunner.php +++ b/app/Entities/Tools/SearchRunner.php @@ -9,6 +9,7 @@ use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; use BookStack\Entities\Models\SearchTerm; +use Illuminate\Database\Connection; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -356,7 +357,9 @@ class SearchRunner // We have to do a raw sql query for this since otherwise PDO will quote the value and MySQL will // search the value as a string which prevents being able to do number-based operations // on the tag values. We ensure it has a numeric value and then cast it just to be sure. - $tagValue = (float) trim($query->getConnection()->getPdo()->quote($tagValue), "'"); + /** @var Connection $connection */ + $connection = $query->getConnection(); + $tagValue = (float) trim($connection->getPdo()->quote($tagValue), "'"); $query->whereRaw("value ${tagOperator} ${tagValue}"); } else { $query->where('value', $tagOperator, $tagValue); diff --git a/app/Entities/Tools/SiblingFetcher.php b/app/Entities/Tools/SiblingFetcher.php index 249e0038e..617ef4a62 100644 --- a/app/Entities/Tools/SiblingFetcher.php +++ b/app/Entities/Tools/SiblingFetcher.php @@ -5,6 +5,7 @@ namespace BookStack\Entities\Tools; use BookStack\Entities\EntityProvider; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; use Illuminate\Support\Collection; @@ -24,7 +25,7 @@ class SiblingFetcher } // Page in book or chapter - if (($entity instanceof Page && !$entity->chapter) || $entity->isA('chapter')) { + if (($entity instanceof Page && !$entity->chapter) || $entity instanceof Chapter) { $entities = $entity->book->getDirectChildren(); } diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index 8327c9489..fcf933726 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -235,13 +235,15 @@ class TrashCan { $shouldRestore = true; $restoreCount = 0; - $parent = $deletion->deletable->getParent(); - if ($parent && $parent->trashed()) { - $shouldRestore = false; + if ($deletion->deletable instanceof Entity) { + $parent = $deletion->deletable->getParent(); + if ($parent && $parent->trashed()) { + $shouldRestore = false; + } } - if ($shouldRestore) { + if ($deletion->deletable instanceof Entity && $shouldRestore) { $restoreCount = $this->restoreEntity($deletion->deletable); } diff --git a/app/Http/Controllers/RecycleBinController.php b/app/Http/Controllers/RecycleBinController.php index 1736023a5..1cffb161c 100644 --- a/app/Http/Controllers/RecycleBinController.php +++ b/app/Http/Controllers/RecycleBinController.php @@ -58,6 +58,7 @@ class RecycleBinController extends Controller $searching = false; } } + /** @var ?Deletion $parentDeletion */ $parentDeletion = ($currentDeletable === $deletion->deletable) ? null : $currentDeletable->deletions()->first(); diff --git a/app/Interfaces/Deletable.php b/app/Interfaces/Deletable.php new file mode 100644 index 000000000..ca59e04f3 --- /dev/null +++ b/app/Interfaces/Deletable.php @@ -0,0 +1,14 @@ +