Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workflows: Add Phpstan #378

Merged
merged 38 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
661bff8
workflows: Add phpstan
sukhwinder33445 Jun 19, 2023
9e67b56
RenderedProcessActionBar: Remove unused constructor param `$auth`
sukhwinder33445 Jul 12, 2023
e638fad
Renderer: Fix that `!empty($elements)` is always true
sukhwinder33445 Jul 12, 2023
92e982b
Remove unused class `Form` and `FormStateOverrides`
sukhwinder33445 Jul 12, 2023
35d151b
Sort Trait: Make `$sort` nullable and adjust code accordingly
sukhwinder33445 Aug 10, 2023
7346cd1
HostServiceTermValidator:isValid(): Add missing `return` statement
sukhwinder33445 Aug 11, 2023
21933d1
NodeAction::create(): Avoid unnecessary variable initialization
sukhwinder33445 Aug 11, 2023
6306b53
Define variable type when method returns a class object
sukhwinder33445 Aug 14, 2023
5899aa9
ProcessCommand: Define `$name` in method scope
sukhwinder33445 Aug 14, 2023
3fffecc
DeleteNodeForm: Make property `$parentNode` nullable
sukhwinder33445 Aug 14, 2023
3643c79
SimulationForm: Property `$simulatedNode` can be nullable
sukhwinder33445 Aug 14, 2023
ae8e711
BpConfig::hasChanges(): Fix incorrect return type
sukhwinder33445 Aug 14, 2023
598e9a1
BpConfig::getNode(): Define exact return types in PhpDoc
sukhwinder33445 Aug 14, 2023
541ec5d
BpNode: Property `$children` can be null
sukhwinder33445 Aug 14, 2023
f960d09
BpNode: Remove unreachable `break` statements
sukhwinder33445 Aug 14, 2023
ad3cb81
NodeAction: `json_decode()` $param#2 expects bool
sukhwinder33445 Aug 14, 2023
6934016
Node: `var_export()` param#2 expects bool
sukhwinder33445 Aug 14, 2023
3544c21
Node: Property `$state` can be null
sukhwinder33445 Aug 14, 2023
ab8fe2a
(ServiceDetail/DetailView)Extension: Property `$storage` can be null
sukhwinder33445 Aug 14, 2023
26b0d92
Renderer::renderStateBadges(): Return type can be null
sukhwinder33445 Aug 14, 2023
6f81fed
NodeTile: Fix incorrect phpDoc
sukhwinder33445 Aug 14, 2023
7605d5e
LegacyConfigParser: Property `$prevKey` can be null
sukhwinder33445 Aug 14, 2023
43b3918
LegacyConfigRenderer: Don't use dynamic property
sukhwinder33445 Aug 14, 2023
d0df785
BaseTestCase: Param `$subDir` can be null
sukhwinder33445 Aug 14, 2023
95776d3
Controller::storage(): Only return an instance of `LegacyStorage`
sukhwinder33445 Aug 14, 2023
9550413
FormLoader: Define var `$file` in method scope
sukhwinder33445 Aug 14, 2023
2efe54d
QuickBaseForm: Property `$icingaModule` can be null
sukhwinder33445 Aug 14, 2023
52732dc
QuickForm: Make property `$successUrl` nullable and add phpDoc to `se…
sukhwinder33445 Aug 14, 2023
74c3440
ProcessProblemsBadge: Fix `variable $count is probably undefined`
sukhwinder33445 Aug 14, 2023
c497c73
Url::getRequest(): Add missing return type
sukhwinder33445 Aug 14, 2023
f74a054
ProcessChanges: Fix incorrect phpDoc
sukhwinder33445 Aug 14, 2023
9aabeb1
Fix call an undefined method `Filterable::fetchRow()` error
yhabteab Aug 22, 2023
b2c25d3
Node: Property $icon can be null
sukhwinder33445 Aug 24, 2023
1c6ad87
Add variable type hint & fix argument type hints
yhabteab Aug 22, 2023
1f3514c
phpstan: Set level to max and add `baseline` config
yhabteab Aug 22, 2023
b1025c6
phpstan: Don't report errors that we do not acknowledge as errors
sukhwinder33445 Aug 23, 2023
8e5c3d3
phpstan: Analyze tests is not necessary
sukhwinder33445 Aug 24, 2023
36cd13a
Github Action: Do not cancel further tests if one fails
sukhwinder33445 Aug 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions .github/workflows/php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php: ['7.2', '7.3', '7.4', '8.0', '8.1']
php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2']
os: ['ubuntu-latest']

steps:
Expand All @@ -31,16 +31,26 @@ jobs:
tools: phpcs

- name: Setup dependencies
run: composer require -n --no-progress overtrue/phplint
run: |
composer require -n --no-progress overtrue/phplint
git clone --depth 1 https://github.com/Icinga/icingaweb2.git vendor/icingaweb2
git clone --depth 1 https://github.com/Icinga/icingadb-web.git vendor/icingadb-web
git clone --depth 1 https://github.com/Icinga/icingaweb2-module-director.git vendor/icingaweb2-module-director
git clone --depth 1 -b snapshot/nightly https://github.com/Icinga/icinga-php-library.git vendor/icinga-php-library
git clone --depth 1 -b snapshot/nightly https://github.com/Icinga/icinga-php-thirdparty.git vendor/icinga-php-thirdparty

- 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

- name: PHPStan
if: ${{ ! cancelled() }}
uses: php-actions/phpstan@v3

test:
name: Unit tests with php ${{ matrix.php }} on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
Expand Down
4 changes: 2 additions & 2 deletions application/clicommands/ProcessCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ public function checkAction()
exit(1);
}

$name = $this->params->get('config');
try {
$name = $this->params->get('config');
if ($name === null) {
$name = $this->getFirstProcessName();
}
Expand All @@ -132,8 +132,8 @@ public function checkAction()
}
}

/** @var BpNode $node */
try {
/** @var BpNode $node */
$node = $bp->getNode($nodeName);
if (Module::exists('icingadb')
&& (! $bp->hasBackendName() && IcingadbSupport::useIcingaDbAsBackend())
Expand Down
4 changes: 3 additions & 1 deletion application/controllers/HostController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Icinga\Module\Businessprocess\ProvidedHook\Icingadb\IcingadbSupport;
use Icinga\Module\Icingadb\Model\Host;
use Icinga\Module\Monitoring\Controller;
use Icinga\Module\Monitoring\DataView\DataView;
use Icinga\Web\Url;
use ipl\Stdlib\Filter;

Expand Down Expand Up @@ -54,7 +55,8 @@ public function showAction()
->from('hoststatus', array('host_name'))
->where('host_name', $hostName);

if ($this->applyRestriction('monitoring/filter/objects', $query)->fetchRow() !== false) {
$this->applyRestriction('monitoring/filter/objects', $query);
if ($query->fetchRow() !== false) {
$this->redirectNow(Url::fromPath('monitoring/host/show')->setParams($this->params));
}
}
Expand Down
7 changes: 5 additions & 2 deletions application/controllers/ProcessController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use ipl\Html\TemplateString;
use ipl\Html\Text;
use ipl\Web\Control\SortControl;
use ipl\Web\FormElement\TermInput;
use ipl\Web\Widget\Link;
use ipl\Web\Widget\Icon;

Expand Down Expand Up @@ -199,7 +200,7 @@ protected function prepareControls($bp, $renderer)
$controls->add(Breadcrumb::create(clone $renderer));
if (! $this->showFullscreen && ! $this->view->compact) {
$controls->add(
new RenderedProcessActionBar($bp, $renderer, $this->Auth(), $this->url())
new RenderedProcessActionBar($bp, $renderer, $this->url())
);
}

Expand Down Expand Up @@ -282,7 +283,9 @@ protected function loadActionForm(BpConfig $bp, Node $node = null)
->handleRequest($this->getServerRequest());

if ($form->hasElement('children')) {
foreach ($form->getElement('children')->prepareMultipartUpdate($this->getServerRequest()) as $update) {
/** @var TermInput $childrenElement */
$childrenElement = $form->getElement('children');
foreach ($childrenElement->prepareMultipartUpdate($this->getServerRequest()) as $update) {
if (! is_array($update)) {
$update = [$update];
}
Expand Down
4 changes: 3 additions & 1 deletion application/controllers/ServiceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Icinga\Module\Businessprocess\ProvidedHook\Icingadb\IcingadbSupport;
use Icinga\Module\Icingadb\Model\Service;
use Icinga\Module\Monitoring\Controller;
use Icinga\Module\Monitoring\DataView\DataView;
use Icinga\Web\Url;
use ipl\Stdlib\Filter;

Expand Down Expand Up @@ -61,7 +62,8 @@ public function showAction()
->where('host_name', $hostName)
->where('service_description', $serviceName);

if ($this->applyRestriction('monitoring/filter/objects', $query)->fetchRow() !== false) {
$this->applyRestriction('monitoring/filter/objects', $query);
if ($query->fetchRow() !== false) {
$this->redirectNow(Url::fromPath('monitoring/service/show')->setParams($this->params));
}
}
Expand Down
4 changes: 3 additions & 1 deletion application/forms/AddNodeForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,11 @@ protected function onSuccess()

$changes->createNode(BpConfig::escapeName($this->getValue('name')), $properties);
} else {
/** @var TermInput $term */
$term = $this->getElement('children');
$children = array_unique(array_map(function ($term) {
return $term->getSearchValue();
}, $this->getElement('children')->getTerms()));
}, $term->getTerms()));

if ($nodeType === 'host' || $nodeType === 'service') {
$stateOverrides = $this->getValue('stateOverrides');
Expand Down
2 changes: 1 addition & 1 deletion application/forms/BpUploadForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ protected function getTempDir()

protected function processUploadedSource()
{
/** @var \Zend_Form_Element_File $el */
/** @var ?\Zend_Form_Element_File $el */
$el = $this->getElement('uploaded_file');

if ($el && $this->hasBeenSent()) {
Expand Down
5 changes: 4 additions & 1 deletion application/forms/DeleteNodeForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@
use Icinga\Module\Businessprocess\Modification\ProcessChanges;
use Icinga\Module\Businessprocess\Node;
use Icinga\Module\Businessprocess\Web\Form\BpConfigBaseForm;
use Icinga\Web\View;

class DeleteNodeForm extends BpConfigBaseForm
{
/** @var Node */
protected $node;

/** @var BpNode */
/** @var ?BpNode */
protected $parentNode;

public function setup()
{
$node = $this->node;
$nodeName = $node->getAlias() ?? $node->getName();

/** @var View $view */
$view = $this->getView();
$this->addHtml(
'<h2>' . $view->escape(
Expand Down
5 changes: 4 additions & 1 deletion application/forms/MoveNodeForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Icinga\Module\Businessprocess\Forms;

use Icinga\Application\Icinga;
use Icinga\Application\Web;
use Icinga\Exception\Http\HttpException;
use Icinga\Module\Businessprocess\BpConfig;
use Icinga\Module\Businessprocess\BpNode;
Expand Down Expand Up @@ -136,7 +137,9 @@ public function onSuccess()
);
} catch (ModificationError $e) {
$this->notifyError($e->getMessage());
Icinga::app()->getResponse()
/** @var Web $app */
$app = Icinga::app();
$app->getResponse()
// Web 2's JS forces a content update for non-200s. Our own JS
// can't prevent this, hence we're not making this a 400 :(
//->setHttpResponseCode(400)
Expand Down
6 changes: 5 additions & 1 deletion application/forms/ProcessForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Icinga\Module\Businessprocess\Node;
use Icinga\Module\Businessprocess\Web\Form\BpConfigBaseForm;
use Icinga\Web\Notification;
use Icinga\Web\View;

class ProcessForm extends BpConfigBaseForm
{
Expand All @@ -16,8 +17,11 @@ class ProcessForm extends BpConfigBaseForm
public function setup()
{
if ($this->node !== null) {
/** @var View $view */
$view = $this->getView();

$this->addHtml(
'<h2>' . $this->getView()->escape(
'<h2>' . $view->escape(
sprintf($this->translate('Modify "%s"'), $this->node->getAlias())
) . '</h2>'
);
Expand Down
4 changes: 3 additions & 1 deletion application/forms/SimulationForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
use Icinga\Module\Businessprocess\MonitoredNode;
use Icinga\Module\Businessprocess\Simulation;
use Icinga\Module\Businessprocess\Web\Form\BpConfigBaseForm;
use Icinga\Web\View;

class SimulationForm extends BpConfigBaseForm
{
/** @var MonitoredNode */
protected $node;

/** @var MonitoredNode */
/** @var ?MonitoredNode */
protected $simulatedNode;

/** @var Simulation */
Expand All @@ -36,6 +37,7 @@ public function setup()
$node = $this->node;
}

/** @var View $view */
$view = $this->getView();
if ($hasSimulation) {
$title = $this->translate('Modify simulation for %s');
Expand Down
40 changes: 0 additions & 40 deletions application/views/helpers/FormStateOverrides.php

This file was deleted.

4 changes: 2 additions & 2 deletions library/Businessprocess/BpConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public function countChanges()
/**
* Whether changes have been applied to this configuration
*
* @return int
* @return bool
*/
public function hasChanges()
{
Expand Down Expand Up @@ -631,7 +631,7 @@ protected function storage()

/**
* @param string $name
* @return Node
* @return MonitoredNode|BpNode
* @throws Exception
*/
public function getNode($name)
Expand Down
6 changes: 1 addition & 5 deletions library/Businessprocess/BpNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class BpNode extends Node

protected $display = 0;

/** @var Node[] */
/** @var ?Node[] */
protected $children;

/** @var array */
Expand Down Expand Up @@ -623,18 +623,14 @@ public function operatorHtml()
switch ($this->getOperator()) {
case self::OP_AND:
return 'AND';
break;
case self::OP_OR:
return 'OR';
case self::OP_XOR:
return 'XOR';
break;
case self::OP_NOT:
return 'NOT';
break;
case self::OP_DEGRADED:
return 'DEG';
break;
default:
// MIN
$this->assertNumericOperator();
Expand Down
12 changes: 8 additions & 4 deletions library/Businessprocess/Common/Sort.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@

trait Sort
{
/** @var string Current sort specification */
/** @var ?string Current sort specification */
protected $sort;

/** @var callable Actual sorting function */
/** @var ?callable Actual sorting function */
protected $sortFn;

/**
Expand All @@ -29,14 +29,18 @@ public function getSort(): ?string
/**
* Set the sort specification
*
* @param string $sort
* @param ?string $sort
*
* @return $this
*
* @throws InvalidArgumentException When sorting according to the specified specification is not possible
*/
public function setSort(string $sort): self
public function setSort(?string $sort): self
{
if (empty($sort)) {
return $this;
}

list($sortBy, $direction) = Str::symmetricSplit($sort, ' ', 2, 'asc');

switch ($sortBy) {
Expand Down
36 changes: 0 additions & 36 deletions library/Businessprocess/Form.php

This file was deleted.

Loading