chore: enable phpstan, apply fixes

.github/workflows/backend.yml

@@ -0,0 +1,13 @@
+name: FoF Byōbu PHP
+
+on: [workflow_dispatch, push, pull_request]
+
+jobs:
+  run:
+    uses: flarum/framework/.github/workflows/REUSABLE_backend.yml@main
+    with:
+      enable_backend_testing: false
+      enable_phpstan: true
+      php_versions: '["8.0", "8.1", "8.2"]'
+
+      backend_directory: .

.github/workflows/build.yml → .github/workflows/frontend.yml

@@ -1,4 +1,4 @@
-name: Javascript
+name: FoF Byōbu JS
 
 on: [workflow_dispatch, push, pull_request]
 
@@ -8,11 +8,12 @@ jobs:
     with:
       enable_bundlewatch: false
       enable_prettier: true
-      enable_typescript: false
+      enable_typescript: true
 
       frontend_directory: ./js
       backend_directory: .
       js_package_manager: npm
       main_git_branch: master
+
     secrets:
       bundlewatch_github_token: ${{ secrets.BUNDLEWATCH_GITHUB_TOKEN }}

composer.json

@@ -54,10 +54,15 @@
                 "flarum/tags",
                 "flarum/approval",
                 "v17development/flarum-support"
-              ]
+            ]
         },
         "extiverse": {
             "discuss": "https://discuss.flarum.org/d/4762"
+        },
+        "flarum-cli": {
+            "modules": {
+                "githubActions": true
+            }
         }
     },
     "replace": {
@@ -70,6 +75,14 @@
     },
     "require-dev": {
         "fof/split": "*",
-        "flarum/flags": "*"
+        "flarum/flags": "*",
+        "flarum/phpstan": "*"
+    },
+    "scripts": {
+        "analyse:phpstan": "phpstan analyse",
+        "clear-cache:phpstan": "phpstan clear-result-cache"
+    },
+    "scripts-descriptions": {
+        "analyse:phpstan": "Run static analysis"
     }
 }

extend.php

@@ -37,6 +37,7 @@
     new Extend\Locales(__DIR__.'/resources/locale'),
 
     (new Extend\Model(Discussion::class))
+        ->cast('isByobu', 'boolean')
         ->relationship('recipientUsers', function ($discussion) {
             return $discussion->belongsToMany(User::class, 'recipients')
                 ->wherePivot('removed_at', null);
@@ -55,6 +56,7 @@
         }),
 
     (new Extend\Model(User::class))
+        ->cast('blocks_byobu_pd', 'boolean')
         ->relationship('privateDiscussions', function ($user) {
             return $user->belongsToMany(Discussion::class, 'recipients')
                 ->wherePivot('removed_at', null);

phpstan.neon

@@ -0,0 +1,13 @@
+includes:
+  - vendor/flarum/phpstan/extension.neon
+
+parameters:
+  # The level will be increased in Flarum 2.0
+  level: 5
+  paths:
+    - extend.php
+    - src
+  excludePaths:
+    - *.blade.php
+  checkMissingIterableValueType: false
+  databaseMigrationsPath: ['migrations']

src/Api/DiscussionDataAttributes.php

@@ -17,21 +17,10 @@
 
 class DiscussionDataAttributes
 {
-    /** @var Screener */
-    private $screener;
-
-    public function __construct(Screener $screener)
+    public function __construct(private Screener $screener)
     {
-        $this->screener = $screener;
     }
 
-    /**
-     * @param Flarum\Api\Serializer\UserSerializer $serializer
-     * @param Flarum\Discussion\Discussion         $discussion
-     * @param array                                $attributes
-     *
-     * @return array
-     */
     public function __invoke(DiscussionSerializer $serializer, Discussion $model, array $attributes): array
     {
         $attributes['isPrivateDiscussion'] = $this->screener->fromDiscussion($model)->isPrivate();

src/Api/DiscussionPermissionAttributes.php

@@ -17,23 +17,10 @@
 
 class DiscussionPermissionAttributes
 {
-    /**
-     * @var Screener
-     */
-    protected $screener;
-
-    public function __construct(Screener $screener)
+    public function __construct(protected Screener $screener)
     {
-        $this->screener = $screener;
     }
 
-    /**
-     * @param \Flarum\Api\Serializer\UserSerializer $serializer
-     * @param \Flarum\Discussion\Discussion         $discussion
-     * @param array                                 $attributes
-     *
-     * @return array
-     */
     public function __invoke(DiscussionSerializer $serializer, Discussion $model, array $attributes): array
     {
         $actor = $serializer->getActor();

src/Api/ForumPermissionAttributes.php

@@ -15,13 +15,6 @@
 
 class ForumPermissionAttributes
 {
-    /**
-     * @param Flarum\Api\Serializer\ForumSerializer $serializer
-     * @param mixed                                 $model
-     * @param array                                 $attributes
-     *
-     * @return array
-     */
     public function __invoke(ForumSerializer $serializer, $model, array $attributes): array
     {
         $actor = $serializer->getActor();

src/Database/RecipientsConstraint.php

@@ -24,7 +24,6 @@ trait RecipientsConstraint
     /**
      * @param Query|Eloquent $query
      * @param User           $user
-     * @param bool           $unify
      */
     public function constraint($query, User $user, bool $includeFlagged = true)
     {

src/Discussion/Screener.php

@@ -33,7 +33,9 @@ public function fromDiscussion(Discussion $discussion): Screener
     {
         $screener = new self();
 
+        /** @phpstan-ignore-next-line */
         $screener->users = $screener->currentUsers = $discussion->recipientUsers()->get();
+        /** @phpstan-ignore-next-line */
         $screener->groups = $screener->currentGroups = $discussion->recipientGroups()->get();
 
         return $screener;
@@ -42,7 +44,9 @@ public function fromDiscussion(Discussion $discussion): Screener
     public function whenSavingDiscussions(Saving $event): Screener
     {
         $screener = new self();
+        /** @phpstan-ignore-next-line */
         $screener->currentUsers = $event->discussion->recipientUsers()->get();
+        /** @phpstan-ignore-next-line */
         $screener->currentGroups = $event->discussion->recipientGroups()->get();
 
         $screener->users = static::getRecipientsFromPayload($event, 'users');
@@ -108,7 +112,7 @@ public function hasBlockingUsers(): bool
         return $this->users
             ->reject(function (User $user) {
                 // Reject currentUsers and allow the actor to create PDs even if they block incoming new PDs
-                return $this->currentUsers->contains($user) || ($user->id === $this->actor()?->id ?? false);
+                return $this->currentUsers->contains($user) || ($user->id === ($this->actor()?->id ?? false));
             })
             ->first(function (User $user) {
                 return boolval($user->blocks_byobu_pd);

src/Events/AbstractRecipientsEvent.php

@@ -12,23 +12,12 @@
 namespace FoF\Byobu\Events;
 
 use Flarum\Discussion\Discussion;
+use Flarum\User\User;
 use FoF\Byobu\Discussion\Screener;
 
 abstract class AbstractRecipientsEvent
 {
-    /**
-     * @var Discussion
-     */
-    public $discussion;
-
-    /**
-     * @var Screener
-     */
-    public $screener;
-
-    public function __construct(Discussion $discussion, Screener $screener)
+    public function __construct(public Discussion $discussion, public Screener $screener, public User $actor)
     {
-        $this->discussion = $discussion;
-        $this->screener = $screener;
     }
 }

src/Events/AllRecipientsLeft.php

@@ -16,19 +16,7 @@
 
 class AllRecipientsLeft extends AbstractRecipientsEvent
 {
-    /**
-     * @var Discussion
-     */
-    public $discussion;
-
-    /**
-     * @var User
-     */
-    public $actor;
-
-    public function __construct(Discussion $discussion, User $actor)
+    public function __construct(public Discussion $discussion, public User $actor)
     {
-        $this->discussion = $discussion;
-        $this->actor = $actor;
     }
 }

src/Events/SearchingRecipient.php

@@ -15,25 +15,7 @@
 
 class SearchingRecipient
 {
-    /**
-     * @var SearchState
-     */
-    public $search;
-
-    /**
-     * @var array
-     */
-    public $matches;
-
-    /**
-     * @var bool
-     */
-    public $negate;
-
-    public function __construct(SearchState $search, array $matches, $negate)
+    public function __construct(public SearchState $search, public array $matches, public bool $negate)
     {
-        $this->search = $search;
-        $this->matches = $matches;
-        $this->negate = $negate;
     }
 }

src/Gambits/Discussion/PrivacyGambit.php

@@ -24,16 +24,6 @@ public function getGambitPattern()
         return 'is:private';
     }
 
-    /**
-     * Apply conditions to the search, given that the gambit was matched.
-     *
-     * @param AbstractSearch $search  The search object.
-     * @param array          $matches An array of matches from the search bit.
-     * @param bool           $negate  Whether or not the bit was negated, and thus whether
-     *                                or not the conditions should be negated.
-     *
-     * @return mixed
-     */
     protected function conditions(SearchState $search, array $matches, $negate)
     {
         $actor = $search->getActor();

src/Jobs/SendNotificationWhenPostedInPrivateDiscussion.php

@@ -46,10 +46,12 @@ public function __construct(
 
     public function handle(NotificationSyncer $notifications)
     {
+        /** @phpstan-ignore-next-line */
         $recipientUsers = $this->post->discussion->recipientUsers->reject(function ($user) {
             return $user->id === $this->actor->id;
         });
 
+        /** @phpstan-ignore-next-line */
         $groups = $this->post->discussion->recipientGroups->pluck('id')->toArray();
 
         $recipientGroupUsers = User::leftJoin('group_user', 'users.id', 'group_user.user_id')

src/Listeners/AddRecipientsToSplitDiscussion.php

@@ -11,7 +11,7 @@
 
 namespace FoF\Byobu\Listeners;
 
-use Fof\Split\Events\DiscussionWasSplit;
+use FoF\Split\Events\DiscussionWasSplit;
 
 class AddRecipientsToSplitDiscussion
 {
@@ -20,10 +20,14 @@ class AddRecipientsToSplitDiscussion
      */
     public function handle(DiscussionWasSplit $event)
     {
+        /** @phpstan-ignore-next-line */
         $event->newDiscussion->recipientUsers()->sync(
+            /** @phpstan-ignore-next-line */
             $event->originalDiscussion->recipientUsers()->allRelatedIds()->all()
         );
+        /** @phpstan-ignore-next-line */
         $event->newDiscussion->recipientGroups()->sync(
+            /** @phpstan-ignore-next-line */
             $event->originalDiscussion->recipientGroups()->allRelatedIds()->all()
         );
     }

src/Listeners/IgnoreApprovals.php

@@ -26,6 +26,7 @@ public function handle(Saving $event)
         $screener = $screener->fromDiscussion($event->post->discussion);
 
         if ($this->extensionIsEnabled('flarum-approval') && $screener->isPrivate()) {
+            /** @phpstan-ignore-next-line */
             $event->post->is_approved = true;
         }
     }

src/Listeners/PersistRecipients.php

@@ -81,6 +81,7 @@ public function handle(Saving $event)
 
         // When discussions need approval and this is a private disucussion, ignore approvals.
         if ($this->screener->isPrivate() && $this->extensionIsEnabled('flarum-approval')) {
+            /** @phpstan-ignore-next-line */
             $event->discussion->is_approved = true;
         }
 

src/Listeners/QueueNotificationJobs.php

@@ -11,6 +11,7 @@
 
 namespace FoF\Byobu\Listeners;
 
+use Flarum\Post\CommentPost;
 use Flarum\Post\Event\Saving;
 use FoF\Byobu\Events\Created;
 use FoF\Byobu\Events\DiscussionMadePublic;
@@ -45,7 +46,7 @@ public function discussionMadePrivate(Created $event)
     public function postMadeInPrivateDiscussion(Saving $event)
     {
         // stop the notification from firing when events such as flarum/likes or fof/reactions re-save the post.
-        if ($event->post->exists) {
+        if ($event->post->exists || !$event->post instanceof CommentPost) {
             return;
         }
 

src/Listeners/SaveUserPreferences.php

@@ -26,7 +26,7 @@ public function handle(Saving $event)
         if ($blocksPd !== null) {
             $actor->assertPermission($actor->id === $user->id);
 
-            $user->blocks_byobu_pd = (bool) ($blocksPd ?? $user->blocks_byobu_pd);
+            $user->blocks_byobu_pd = (bool) $user->blocks_byobu_pd;
         }
     }
 }