Github Actions: Add Phpstan (#105)

.github/workflows/php.yml

@@ -31,16 +31,23 @@ jobs:
           tools: phpcs
 
       - name: Setup dependencies
-        run: composer require -n --no-progress overtrue/phplint
+        run: composer require -n --no-progress overtrue/phplint phpstan/phpstan
+          && sudo git clone --depth 1 https://github.com/Icinga/icingaweb2.git /icingaweb2
+          && sudo git clone --depth 1 -b snapshot/nightly https://github.com/Icinga/icinga-php-library.git /usr/share/icinga-php/ipl
+          && sudo git clone --depth 1 -b snapshot/nightly https://github.com/Icinga/icinga-php-thirdparty.git /usr/share/icinga-php/vendor
 
       - name: PHP Lint
-        if: success() || matrix.allow_failure
+        if: ${{ ! cancelled() }}
         run: ./vendor/bin/phplint -n --exclude={^vendor/.*} -- .
 
       - name: PHP CodeSniffer
-        if: success() || matrix.allow_failure
+        if: ${{ ! cancelled() }}
         run: phpcs -wps --colors
 
+      - name: PHPStan
+        if: ${{ ! cancelled() }}
+        run: ./vendor/bin/phpstan analyse
+
   test:
     name: Unit tests with php ${{ matrix.php }} on ${{ matrix.os }}
     runs-on: ${{ matrix.os }}

application/controllers/ContactController.php

@@ -8,6 +8,7 @@
 use Icinga\Module\Notifications\Model\Contact;
 use Icinga\Module\Notifications\Web\Form\ContactForm;
 use Icinga\Web\Notification;
+use ipl\Html\FormElement\FieldsetElement;
 use ipl\Sql\Connection;
 use ipl\Stdlib\Filter;
 use ipl\Web\Compat\CompatController;
@@ -39,19 +40,21 @@ public function indexAction()
             ->populate($contact)
             ->on(ContactForm::ON_SUCCESS, function (ContactForm $form) {
                 $form->addOrUpdateContact();
-
+                /** @var FieldsetElement $contactElement */
+                $contactElement = $form->getElement('contact');
                 Notification::success(sprintf(
                     t('Contact "%s" has successfully been saved'),
-                    $form->getElement('contact')->getValue('full_name')
+                    $contactElement->getValue('full_name')
                 ));
 
                 $this->redirectNow('__CLOSE__');
             })->on(ContactForm::ON_REMOVE, function (ContactForm $form) {
                 $form->removeContact();
-
+                /** @var FieldsetElement $contactElement */
+                $contactElement = $form->getElement('contact');
                 Notification::success(sprintf(
                     t('Deleted contact "%s" successfully'),
-                    $form->getElement('contact')->getValue('full_name')
+                    $contactElement->getValue()
                 ));
 
                 $this->redirectNow('__CLOSE__');

application/controllers/EventRuleController.php

@@ -225,20 +225,22 @@ public function searchEditorAction(): void
         $this->setTitle($this->translate('Adjust Filter'));
     }
 
-    public function editAction()
+    public function editAction(): void
     {
-        $ruleId = (int) $this->params->getRequired('id');
+        /** @var string $ruleId */
+        $ruleId = $this->params->getRequired('id');
+        /** @var ?array<string, mixed> $cache */
         $cache = $this->sessionNamespace->get($ruleId);
 
         if ($this->params->has('clearCache')) {
             $this->sessionNamespace->delete($ruleId);
             $cache = [];
         }
 
-        if (isset($cache) || $ruleId === -1) {
+        if (isset($cache) || $ruleId === '-1') {
             $config = $cache ?? [];
         } else {
-            $config = $this->fromDb($ruleId);
+            $config = $this->fromDb((int) $ruleId);
         }
 
         $eventRuleForm = (new EventRuleForm())
@@ -248,13 +250,13 @@ public function editAction()
                 $config['name'] = $form->getValue('name');
                 $config['is_active'] = $form->getValue('is_active');
 
-                if ($cache || $ruleId === -1) {
+                if ($cache || $ruleId === '-1') {
                     $this->sessionNamespace->set($ruleId, $config);
                 } else {
-                    (new SaveEventRuleForm())->editRule($ruleId, $config);
+                    (new SaveEventRuleForm())->editRule((int) $ruleId, $config);
                 }
 
-                if ($ruleId === -1) {
+                if ($ruleId === '-1') {
                     $redirectUrl = Url::fromPath('notifications/event-rules/add', [
                         'use_cache' => true
                     ]);
@@ -269,7 +271,7 @@ public function editAction()
                 $this->redirectNow($redirectUrl);
             })->handleRequest($this->getServerRequest());
 
-        if ($ruleId === -1) {
+        if ($ruleId === '-1') {
             $this->setTitle($this->translate('New Event Rule'));
         } else {
             $this->setTitle($this->translate('Edit Event Rule'));

application/controllers/IncidentsController.php

@@ -9,16 +9,21 @@
 use Icinga\Module\Notifications\Web\Control\SearchBar\ObjectSuggestions;
 use Icinga\Module\Notifications\Model\Incident;
 use Icinga\Module\Notifications\Widget\ItemList\IncidentList;
+use ipl\Stdlib\Filter;
 use ipl\Web\Compat\CompatController;
 use ipl\Web\Compat\SearchControls;
 use ipl\Web\Control\LimitControl;
 use ipl\Web\Control\SortControl;
+use ipl\Web\Filter\QueryString;
 
 class IncidentsController extends CompatController
 {
     use Auth;
     use SearchControls;
 
+    /** @var Filter\Rule Filter from query string parameters */
+    private $filter;
+
     public function indexAction(): void
     {
         $this->addTitleTab(t('Incidents'));
@@ -92,4 +97,18 @@ protected function getPageSize($default)
     {
         return parent::getPageSize($default ?? 50);
     }
+
+    /**
+     * Get the filter created from query string parameters
+     *
+     * @return Filter\Rule
+     */
+    public function getFilter(): Filter\Rule
+    {
+        if ($this->filter === null) {
+            $this->filter = QueryString::parse((string) $this->params);
+        }
+
+        return $this->filter;
+    }
 }

application/forms/EscalationConditionForm.php

@@ -241,8 +241,8 @@ protected function handleRemove(): void
 
         if ($button && $button->getName() !== 'add') {
             [$name, $toRemove] = explode('_', $button->getName(), 2);
-
-            $this->removedOptionNumber = (int) $toRemove;
+            $toRemove = (int) $toRemove;
+            $this->removedOptionNumber = $toRemove;
             $optionCount = count($this->options);
 
             for ($i = $toRemove; $i < $optionCount; $i++) {

application/forms/EscalationRecipientForm.php

@@ -154,8 +154,10 @@ public function getValues()
 
     public function populate($values)
     {
+        /** @var int $key */
         foreach ($values as $key => $condition) {
             if (is_array($condition)) {
+                $count = 0;
                 foreach ($condition as $elementName => $elementValue) {
                     if ($elementValue === null) {
                         continue;
@@ -207,8 +209,8 @@ protected function handleRemove(): void
 
         if ($button && $button->getName() !== 'add') {
             [$name, $toRemove] = explode('_', $button->getName(), 2);
-
-            $this->removedOptionNumber = (int) $toRemove;
+            $toRemove = (int) $toRemove;
+            $this->removedOptionNumber = $toRemove;
             $optionCount = count($this->options);
 
             for ($i = $toRemove; $i < $optionCount; $i++) {

application/forms/RemoveEscalationForm.php

@@ -57,7 +57,7 @@ protected function assemble()
     /**
      * Method to set disabled state of remove button
      *
-     * @param bool $disable
+     * @param bool $state
      *
      * @return $this
      */

library/Notifications/Web/Control/SearchBar/ObjectSuggestions.php

@@ -170,7 +170,6 @@ public static function collectFilterColumns(Model $model, Resolver $resolver): T
         self::collectRelations($resolver, $model, $models, []);
 
         foreach ($models as $path => $targetModel) {
-            /** @var Model $targetModel */
             foreach ($resolver->getColumnDefinitions($targetModel) as $columnName => $definition) {
                 yield $path . '.' . $columnName => $definition->getLabel();
             }
@@ -195,7 +194,7 @@ protected static function collectRelations(Resolver $resolver, Model $subject, a
             $isHasOne = $relation instanceof HasOne;
             if (empty($path)) {
                 $relationPath = [$name];
-                if ($isHasOne && empty($path)) {
+                if ($isHasOne) {
                     array_unshift($relationPath, $subject->getTableName());
                 }
 

library/Notifications/Widget/Detail/IncidentQuickActions.php

@@ -227,7 +227,6 @@ protected function removeEntry(IncidentContact $incidentContact, string $roleNam
      * Update the incident history
      *
      * @param IncidentContact $incidentContact
-     * @param string $msg
      * @param string|null $newRole
      *
      * @return void

library/Notifications/Widget/EventRuleConfig.php

@@ -48,10 +48,10 @@ class EventRuleConfig extends BaseHtmlElement
     /** @var Url The url to open the SearchEditor at */
     protected $searchEditorUrl;
 
-    /** @var array */
+    /** @var array<int, array<int, EscalationConditionForm|EscalationRecipientForm>> */
     private $escalationForms = [];
 
-    /** @var RemoveEscalationForm[] */
+    /** @var array<int, RemoveEscalationForm> */
     private $removeEscalationForms;
 
     /** @var int */
@@ -110,6 +110,7 @@ protected function createForms(): void
         ];
 
         foreach ($escalations as $position => $escalation) {
+            /** @var int $position */
             $values = explode('|', $escalation['condition'] ?? '');
             $escalationCondition = $this->createConditionForm($position, $values);
 
@@ -375,23 +376,27 @@ private function createRemoveEscalationForm(int $position): RemoveEscalationForm
                 }
 
                 if (! empty($this->removeEscalationForms)) {
-                    $this->removeEscalationForms = array_combine(
+                    /** @var array<int, RemoveEscalationForm> $removeEscalationForms */
+                    $removeEscalationForms = array_combine(
                         range(
                             1,
                             count($this->removeEscalationForms)
                         ),
                         array_values($this->removeEscalationForms)
                     );
+                    $this->removeEscalationForms = $removeEscalationForms;
                 }
 
                 if (! empty($this->escalationForms)) {
-                    $this->escalationForms = array_combine(
+                    /** @var array<int, array<int, EscalationConditionForm|EscalationRecipientForm>> $escalationForms */
+                    $escalationForms = array_combine(
                         range(
                             1,
                             count($this->escalationForms)
                         ),
                         array_values($this->escalationForms)
                     );
+                    $this->escalationForms = $escalationForms;
                 }
 
                 $numEscalation = count($this->escalationForms);

phpstan-baseline-7x.neon

@@ -0,0 +1,26 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Parameter \\#2 \\$assoc of function json_decode expects bool\\|null, int given\\.$#"
+			count: 1
+			path: application/forms/ChannelForm.php
+
+		-
+			message: "#^Parameter \\#2 \\.\\.\\.\\$args of function array_merge expects array, mixed given\\.$#"
+			count: 1
+			path: application/forms/ChannelForm.php
+
+		-
+			message: "#^Parameter \\#2 \\$str of function explode expects string, mixed given\\.$#"
+			count: 2
+			path: application/forms/EntryForm.php
+
+		-
+			message: "#^Parameter \\#2 \\$str of function explode expects string, mixed given\\.$#"
+			count: 2
+			path: application/forms/EscalationRecipientForm.php
+
+		-
+			message: "#^Parameter \\#1 \\$time of function strtotime expects string, mixed given\\.$#"
+			count: 1
+			path: library/Notifications/Widget/Calendar.php

phpstan-baseline-8x.neon

@@ -0,0 +1,26 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Parameter \\#2 \\$associative of function json_decode expects bool\\|null, int given\\.$#"
+			count: 1
+			path: application/forms/ChannelForm.php
+
+		-
+			message: "#^Parameter \\#2 \\.\\.\\.\\$arrays of function array_merge expects array, mixed given\\.$#"
+			count: 1
+			path: application/forms/ChannelForm.php
+
+		-
+			message: "#^Parameter \\#2 \\$string of function explode expects string, mixed given\\.$#"
+			count: 2
+			path: application/forms/EntryForm.php
+
+		-
+			message: "#^Parameter \\#2 \\$string of function explode expects string, mixed given\\.$#"
+			count: 2
+			path: application/forms/EscalationRecipientForm.php
+
+		-
+			message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, mixed given\\.$#"
+			count: 1
+			path: library/Notifications/Widget/Calendar.php

phpstan-baseline-by-php-version.php

@@ -0,0 +1,12 @@
+<?php
+
+$includes = [];
+if (PHP_VERSION_ID >= 80000) {
+    $includes[] = __DIR__ . '/phpstan-baseline-8x.neon';
+} else {
+    $includes[] = __DIR__ . '/phpstan-baseline-7x.neon';
+}
+
+return [
+    'includes' => $includes
+];