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

Enable phpstan bleading edge and strict rules #1832

Merged
merged 9 commits into from
Aug 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"phpstan/extension-installer": "^1.1",
"phpstan/phpstan": "^1.0",
"phpstan/phpstan-deprecation-rules": "^1.0",
"phpstan/phpstan-strict-rules": "^1.3",
"phpunit/phpunit": "^9.5.5",
"symfony/process": "^4.4.30 || ^5.3.7"
},
Expand Down
1 change: 1 addition & 0 deletions demos/basic/button.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public function __construct(int $n)
{
Icon::addTo(Button::addTo($this, ['Forks', 'class.blue' => true]), ['fork']);
Label::addTo($this, [number_format($n), 'class.basic blue left pointing' => true]);

parent::__construct(['class.labeled' => true]);
}
});
Expand Down
2 changes: 1 addition & 1 deletion demos/collection/multitable.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function setModel(Model $model, array $route = []): void

$selections = explode(',', $_GET[$this->name] ?? '');

if (!empty($selections[0])) {
if ($selections[0]) {
$table->js(true)->find('tr[data-id=' . $selections[0] . ']')->addClass('active');
}

Expand Down
3 changes: 1 addition & 2 deletions demos/form/form.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@

$form->onSubmit(function (Form $form) {
$errors = [];

foreach ($form->model->getFields() as $name => $ff) {
if ($name === 'id') {
continue;
Expand All @@ -243,5 +242,5 @@
}
}

return $errors ?: $form->success('No more errors', 'so we have saved everything into the database');
return $errors !== [] ? $errors : $form->success('No more errors', 'so we have saved everything into the database');
});
2 changes: 1 addition & 1 deletion demos/form/form3.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,5 @@
}
}

return $errors ?: 'No fields were changed';
return $errors !== [] ? $errors : 'No fields were changed';
});
2 changes: 2 additions & 0 deletions demos/interactive/modal.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@

$session->memorize('success', true);
$session->memorize('name', $form->model->get('name'));

$js = [];
$js[] = $form->success('Thank you, ' . $form->model->get('name') . ' you can go on!');
$js[] = $nextAction->js()->removeClass('disabled');

Expand Down
4 changes: 2 additions & 2 deletions demos/interactive/paginator.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
Header::addTo($app, ['Local Sticky Usage']);
$seg = View::addTo($app, ['ui' => 'blue segment']);

$month = $seg->stickyGet('month') ?: 1;
$day = $seg->stickyGet('day') ?: 1;
$month = $seg->stickyGet('month') ?? 1;
$day = $seg->stickyGet('day') ?? 1;

// we intentionally left 31 days here and do not calculate number of days in particular month to keep example simple
$monthPaginator = Paginator::addTo($seg, ['total' => 12, 'range' => 3, 'urlTrigger' => 'month']);
Expand Down
3 changes: 1 addition & 2 deletions demos/layout/layouts_admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,11 @@

$form->onSubmit(function (Form $form) {
$errors = [];

foreach (['first_name', 'last_name', 'address'] as $field) {
if (!$form->model->get($field)) {
$errors[] = $form->error($field, 'Field ' . $field . ' is mandatory');
}
}

return $errors ?: $form->success('No more errors', 'so we have saved everything into the database');
return $errors !== [] ? $errors : $form->success('No more errors', 'so we have saved everything into the database');
});
2 changes: 1 addition & 1 deletion demos/obsolete/notify.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
$form->addControl('name', [], ['caption' => 'Add your name']);

$form->onSubmit(function (Form $form) use ($modal) {
if (empty($form->model->get('name'))) {
if (!$form->model->get('name')) {
return $form->error('name', 'Please add a name!');
}

Expand Down
2 changes: 1 addition & 1 deletion docs/app.rst
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ Utilities by App
App provides various utilities that are used by other components.

.. php:method:: getTag()
.. php:method:: encodeAttribute()
.. php:method:: encodeHtmlAttribute()
.. php:method:: encodeHtml()

Apart from basic utility, App class provides several mechanisms that are helpful for components.
Expand Down
4 changes: 2 additions & 2 deletions docs/form-control.rst
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ As default, Dropdown will use the `$model->idField` as value, and `$model->title
If you want to customize how a record is displayed and/or add an icon, Dropdown has the :php:meth:`Form::renderRowFunction()` to do this.
This function is called with each model record and needs to return an array::

$dropdown->renderRowFunction = function ($record) {
$dropdown->renderRowFunction = function (Model $record) {
return [
'value' => $record->idField,
'title' => $record->getTitle() . ' (' . $record->get('subtitle') . ')',
Expand All @@ -347,7 +347,7 @@ This function is called with each model record and needs to return an array::

You can also use this function to add an Icon to a record::

$dropdown->renderRowFunction = function ($record) {
$dropdown->renderRowFunction = function (Model $record) {
return [
'value' => $record->idField,
'title' => $record->getTitle() . ' (' . $record->get('subtitle') . ')',
Expand Down
2 changes: 1 addition & 1 deletion docs/form.rst
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ ATK Data has two field flags - "nullable" and "required". Because ATK Data works
values, the values are defined like this:

- nullable = value of the field can be null.
- required = value of the field must not be empty (see `empty()`), null is empty too.
- required = value of the field must not be empty/false/zero, null is empty too.

Form changes things slightly, because it does not allow user to enter NULL values. For
example - string (or unspecified type) fields will contain empty string if are not
Expand Down
1 change: 1 addition & 0 deletions docs/view.rst
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ Here is a best practice for using custom template::
protected function renderView(): void
{
parent::renderView();

$this->template->set('title', $this->title);
}
}
Expand Down
24 changes: 12 additions & 12 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
includes:
- phar://phpstan.phar/conf/bleedingEdge.neon

parameters:
level: 6
paths:
Expand All @@ -6,19 +9,16 @@ parameters:
- vendor/
- js/

# some extra rules
checkAlwaysTrueCheckTypeFunctionCall: true
checkAlwaysTrueInstanceof: true
checkAlwaysTrueStrictComparison: true
checkExplicitMixedMissingReturn: true
checkFunctionNameCase: true
# TODO checkMissingClosureNativeReturnTypehintRule: true
reportMaybesInMethodSignatures: true
reportStaticMethodSignatures: true
checkTooWideReturnTypesInProtectedAndPublicMethods: true
checkMissingIterableValueType: false # TODO

ignoreErrors:
- '~^(Property .+ has|Method .+\(\) (has parameter \$.+ with|return type has)) no value type specified in iterable type .+\.~'

# relax strict rules
- '~^Only booleans are allowed in .+, .+ given( on the (left|right) side)?\.~'
- '~^Variable (static )?(property access|method call) on .+\.~'

# https://github.com/phpstan/phpstan/issues/7839
- '~PHPDoc type .+ of property Atk4\\Ui\\.+ is not the same as PHPDoc type .+ of overridden property .+\.~'

# TODO these rules are generated, this ignores should be fixed in the code
# for level = 2
-
Expand Down
48 changes: 24 additions & 24 deletions src/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ class App
/** @var App\SessionManager */
public $session;

/** @var string[] Extra HTTP headers to send on exit. */
protected $responseHeaders = [
/** @var array<string, string> Extra HTTP headers to send on exit. */
protected array $responseHeaders = [
self::HEADER_STATUS_CODE => '200',
'cache-control' => 'no-store', // disable caching by default
];

/** @var View[] Modal view that need to be rendered using json output. */
/** @var array<string, View> Modal view that need to be rendered using json output. */
private $portals = [];

/**
Expand All @@ -136,7 +136,7 @@ class App
public $page;

/** @var array global sticky arguments */
protected $stickyGetArguments = [
protected array $stickyGetArguments = [
'__atk_json' => false,
'__atk_tab' => false,
];
Expand Down Expand Up @@ -186,7 +186,7 @@ static function (int $severity, string $msg, string $file, int $line): bool {
},
\E_ALL
);
$this->outputResponseUnsafe('', [self::HEADER_STATUS_CODE => 500]);
$this->outputResponseUnsafe('', [self::HEADER_STATUS_CODE => '500']);
}

// Always run app on shutdown
Expand Down Expand Up @@ -219,7 +219,7 @@ public function registerPortals($portal): void
// TODO in https://github.com/atk4/ui/pull/1771 it has been discovered this method causes DOM code duplication,
// for some reasons, it seems even not needed, at least all Unit & Behat tests pass
// must be investigated
// $this->portals[$portal->shortName] = $portal;
// $this->portals[$portal->name] = $portal;
}

public function setExecutorFactory(ExecutorFactory $factory): void
Expand Down Expand Up @@ -315,9 +315,9 @@ public function caughtException(\Throwable $exception): void
/**
* Normalize HTTP headers to associative array with LC keys.
*
* @param string[] $headers
* @param array<string, string> $headers
*
* @return string[]
* @return array<string, string>
*/
protected function normalizeHeaders(array $headers): array
{
Expand Down Expand Up @@ -367,17 +367,17 @@ public function setResponseHeader(string $name, string $value): self
* directly, instead call it form Callback, JsCallback or similar
* other classes.
*
* @param string|array $output Array type is supported only for JSON response
* @param string[] $headers content-type header must be always set or consider using App::terminateHtml() or App::terminateJson() methods
* @param string|array $output Array type is supported only for JSON response
* @param array<string, string> $headers content-type header must be always set or consider using App::terminateHtml() or App::terminateJson() methods
*
* @return never
*/
public function terminate($output = '', array $headers = []): void
{
$headers = $this->normalizeHeaders($headers);
if (empty($headers['content-type'])) {
if (!isset($headers['content-type'])) {
$this->responseHeaders = $this->normalizeHeaders($this->responseHeaders);
if (empty($this->responseHeaders['content-type'])) {
if (!isset($this->responseHeaders['content-type'])) {
throw new Exception('Content type must be always set');
}

Expand Down Expand Up @@ -941,7 +941,7 @@ public function getTag(string $tag = null, $attr = null, $value = null): string
} elseif ($key === 0) {
$tag = $val;
} else {
$tmp[] = $key . '="' . $this->encodeAttribute($val) . '"';
$tmp[] = $key . '="' . $this->encodeHtmlAttribute((string) $val) . '"';
}
}

Expand All @@ -950,14 +950,10 @@ public function getTag(string $tag = null, $attr = null, $value = null): string

/**
* Encodes string - removes HTML special chars.
*
* @param string $val
*
* @return string
*/
public function encodeAttribute($val)
public function encodeHtmlAttribute(string $val): string
{
return htmlspecialchars((string) $val);
return htmlspecialchars($val);
}

/**
Expand Down Expand Up @@ -1046,6 +1042,8 @@ function () {
/**
* This can be overridden for future PSR-7 implementation.
*
* @param array<string, string> $headersNew
*
* @internal should be called only from self::outputResponse()
*/
protected function outputResponseUnsafe(string $data, array $headersNew): void
Expand All @@ -1071,11 +1069,13 @@ protected function outputResponseUnsafe(string $data, array $headersNew): void
echo $data;
}

/** @var string[] */
private static $_sentHeaders = [];
/** @var array<string, string> */
private static array $_sentHeaders = [];

/**
* Output Response to the client.
*
* @param array<string, string> $headers
*/
protected function outputResponse(string $data, array $headers): void
{
Expand Down Expand Up @@ -1141,7 +1141,7 @@ protected function outputLateOutputError(LateOutputError $exception): void
/**
* Output HTML response to the client.
*
* @param string[] $headers
* @param array<string, string> $headers
*/
private function outputResponseHtml(string $data, array $headers = []): void
{
Expand All @@ -1154,8 +1154,8 @@ private function outputResponseHtml(string $data, array $headers = []): void
/**
* Output JSON response to the client.
*
* @param string|array $data
* @param string[] $headers
* @param string|array $data
* @param array<string, string> $headers
*/
private function outputResponseJson($data, array $headers = []): void
{
Expand Down
2 changes: 1 addition & 1 deletion src/Behat/MinkSeleniumDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class MinkSeleniumDriver extends \Behat\Mink\Driver\Selenium2Driver
{
use WarnDynamicPropertyTrait;

public function __construct(\Behat\Mink\Driver\Selenium2Driver $driver)
public function __construct(\Behat\Mink\Driver\Selenium2Driver $driver) // @phpstan-ignore-line
{
$class = self::class;
while (($class = get_parent_class($class)) !== false) {
Expand Down
2 changes: 1 addition & 1 deletion src/Breadcrumb.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function addCrumb($section = null, $link = null)
public function popTitle()
{
$title = array_pop($this->path);
$this->set($title['section'] ?: '');
$this->set($title['section'] ?? '');

return $this;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Callback.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected function init(): void

public function setUrlTrigger(string $trigger = null): void
{
$this->urlTrigger = $trigger ?: $this->name;
$this->urlTrigger = $trigger ?? $this->name;

$this->getOwner()->stickyGet(self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger);
}
Expand Down Expand Up @@ -124,7 +124,7 @@ public function canTerminate(): bool
*/
public function canTrigger(): bool
{
return $this->triggerOnReload || empty($_GET['__atk_reload']);
return $this->triggerOnReload || !isset($_GET['__atk_reload']);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/CardDeck.php
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ protected function getNotifier(string $msg = null, Model\UserAction $action = nu
*/
protected function jsModelReturn(Model\UserAction $action = null, string $msg = 'Done!'): array
{
$js = [];
$js[] = $this->getNotifier($msg, $action);
if ($action->getModel()->isLoaded() && $card = $this->findCard($action->getModel())) {
$js[] = $card->jsReload($this->getReloadArgs());
Expand Down Expand Up @@ -390,6 +391,7 @@ protected function renderView(): void
if ($this->container->name === ($_GET['__atk_reload'] ?? null)) {
$this->applyReload();
}

parent::renderView();
}

Expand Down
2 changes: 1 addition & 1 deletion src/CardSection.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CardSection extends View
/** @var string */
public $glue = ': ';

/** @var string[] */
/** @var array<int, string> */
public $tableClass = ['ui', 'fixed', 'small'];

protected function init(): void
Expand Down
4 changes: 2 additions & 2 deletions src/Columns.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Columns extends View
* Explicitly specify the width of all columns. Normally that's 16, but
* semantic-ui allows you to override with 5 => "ui five column grid".
*
* @var int
* @var int|null
*/
public $width;

Expand Down Expand Up @@ -75,7 +75,7 @@ public function addRow($width = null)

protected function renderView(): void
{
$width = $this->width ?: $this->calculatedWidth;
$width = $this->width ?? $this->calculatedWidth;
if ($this->content) {
$this->addClass($this->content);
$this->content = null;
Expand Down
Loading