Skip to content

Commit

Permalink
PHPStan level 10 part II (#446)
Browse files Browse the repository at this point in the history
_A few weeks and +10% test coverage later..._

Bring the rest of the code to PHPStan level 10 (follow-up to #443). The level is bumped back to `max` in this PR.

Several bugs were found and taken care of during the effort, mostly because I've added more tests, but without the effort of going to level 10, I would have written no additional tests because why 😅
  • Loading branch information
spaze authored Dec 19, 2024
2 parents 7a8b801 + 86db85b commit f13af02
Show file tree
Hide file tree
Showing 48 changed files with 1,770 additions and 540 deletions.
3 changes: 2 additions & 1 deletion app/config/tests.neon
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ parameters:
services:
- MichalSpacekCz\Test\Application\ApplicationPresenter
articles: MichalSpacekCz\Test\Articles\ArticlesMock
localeLinkGenerator: MichalSpacekCz\Test\Application\LocaleLinkGeneratorMock
localeLinkGenerator: MichalSpacekCz\Test\Application\LocaleLinkGeneratorMock(languages: %locales.languages%)
database.default.explorer: MichalSpacekCz\Test\Database\Database
database.upcKeys.explorer: @database.default.explorer
database.pulse.explorer: @database.default.explorer
Expand All @@ -54,6 +54,7 @@ services:
security.userStorage: MichalSpacekCz\Test\Security\NullUserStorage
- MichalSpacekCz\Test\TestCaseRunner
trainingFilesStorage: MichalSpacekCz\Test\Training\TrainingFilesNullStorage
- MichalSpacekCz\Test\Training\TrainingTestDataFactory
cache.storage: Nette\Caching\Storages\DevNullStorage
netteHttpResponse: # Needed for User\Manager because https://github.com/nette/http/issues/200
create: Nette\Http\Response
Expand Down
4 changes: 3 additions & 1 deletion app/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ parameters:
fileExtensions:
- php
- phpt
level: 9
level: max
checkMissingOverrideMethodAttribute: true
stubFiles:
- stubs/Nette/Http/FileUpload.phpstub

includes:
- phar://phpstan.phar/conf/bleedingEdge.neon
Expand Down
128 changes: 0 additions & 128 deletions app/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,134 +5,6 @@
<code><![CDATA[$input->getValue()]]></code>
</MixedArgument>
</file>
<file src="src/Form/ChangePasswordFormFactory.php">
<MixedArgument>
<code><![CDATA[$values->newPassword]]></code>
<code><![CDATA[$values->password]]></code>
</MixedArgument>
</file>
<file src="src/Form/PostFormFactory.php">
<MixedArgument>
<code><![CDATA[$values->lead === '' ? null : $values->lead]]></code>
<code><![CDATA[$values->locale]]></code>
<code><![CDATA[$values->locale]]></code>
<code><![CDATA[$values->ogImage === '' ? null : $values->ogImage]]></code>
<code><![CDATA[$values->originally === '' ? null : $values->originally]]></code>
<code><![CDATA[$values->previewKey === '' ? null : $values->previewKey]]></code>
<code><![CDATA[$values->published]]></code>
<code><![CDATA[$values->recommended]]></code>
<code><![CDATA[$values->tags]]></code>
<code><![CDATA[$values->text]]></code>
<code><![CDATA[$values->translationGroup === '' ? null : $values->translationGroup]]></code>
<code><![CDATA[empty($values->editSummary) ? null : $values->editSummary]]></code>
</MixedArgument>
</file>
<file src="src/Form/Pulse/PasswordsStorageAlgorithmFormFactory.php">
<MixedArgument>
<code><![CDATA[$values->algo->new->algo]]></code>
<code><![CDATA[$values->company->id]]></code>
<code><![CDATA[$values->company->new->name]]></code>
<code><![CDATA[$values->site->new->url]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingApplicationAdminFormFactory.php">
<MixedArgument>
<code><![CDATA[$dateId]]></code>
<code><![CDATA[$values->citySet ? $values->city : null]]></code>
<code><![CDATA[$values->companyIdSet ? $values->companyId : null]]></code>
<code><![CDATA[$values->companySet ? $values->company : null]]></code>
<code><![CDATA[$values->companyTaxIdSet ? $values->companyTaxId : null]]></code>
<code><![CDATA[$values->countrySet ? $values->country : null]]></code>
<code><![CDATA[$values->discount]]></code>
<code><![CDATA[$values->emailSet ? $values->email : null]]></code>
<code><![CDATA[$values->familiar]]></code>
<code><![CDATA[$values->invoiceId]]></code>
<code><![CDATA[$values->nameSet ? $values->name : null]]></code>
<code><![CDATA[$values->noteSet ? $values->note : null]]></code>
<code><![CDATA[$values->paid]]></code>
<code><![CDATA[$values->source]]></code>
<code><![CDATA[$values->streetSet ? $values->street : null]]></code>
<code><![CDATA[$values->vatRate]]></code>
<code><![CDATA[$values->zipSet ? $values->zip : null]]></code>
<code><![CDATA[trim($values->vatRate) !== '' ? $values->vatRate / 100 : null]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingApplicationMultipleFormFactory.php">
<MixedArgument>
<code><![CDATA[$application->city]]></code>
<code><![CDATA[$application->company]]></code>
<code><![CDATA[$application->companyId]]></code>
<code><![CDATA[$application->companyTaxId]]></code>
<code><![CDATA[$application->email]]></code>
<code><![CDATA[$application->name]]></code>
<code><![CDATA[$application->note]]></code>
<code><![CDATA[$application->street]]></code>
<code><![CDATA[$application->zip]]></code>
<code><![CDATA[$values->country]]></code>
<code><![CDATA[$values->date]]></code>
<code><![CDATA[$values->source]]></code>
<code><![CDATA[$values->status]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingApplicationStatusesFormFactory.php">
<MixedArgument>
<code><![CDATA[$id]]></code>
<code><![CDATA[$status]]></code>
<code><![CDATA[$values->date]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingDateFormFactory.php">
<MixedArgument>
<code><![CDATA[$values->cooperation]]></code>
<code><![CDATA[$values->cooperation]]></code>
<code><![CDATA[$values->end]]></code>
<code><![CDATA[$values->end]]></code>
<code><![CDATA[$values->feedbackHref]]></code>
<code><![CDATA[$values->feedbackHref]]></code>
<code><![CDATA[$values->label]]></code>
<code><![CDATA[$values->label]]></code>
<code><![CDATA[$values->note]]></code>
<code><![CDATA[$values->note]]></code>
<code><![CDATA[$values->public]]></code>
<code><![CDATA[$values->public]]></code>
<code><![CDATA[$values->remote]]></code>
<code><![CDATA[$values->remote]]></code>
<code><![CDATA[$values->remoteNotes]]></code>
<code><![CDATA[$values->remoteNotes]]></code>
<code><![CDATA[$values->remoteUrl]]></code>
<code><![CDATA[$values->remoteUrl]]></code>
<code><![CDATA[$values->start]]></code>
<code><![CDATA[$values->start]]></code>
<code><![CDATA[$values->status]]></code>
<code><![CDATA[$values->status]]></code>
<code><![CDATA[$values->training]]></code>
<code><![CDATA[$values->training]]></code>
<code><![CDATA[$values->training]]></code>
<code><![CDATA[$values->venue]]></code>
<code><![CDATA[$values->venue]]></code>
<code><![CDATA[$values->videoHref]]></code>
<code><![CDATA[$values->videoHref]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingInvoiceFormFactory.php">
<MixedArgument>
<code><![CDATA[$values->invoice]]></code>
<code><![CDATA[$values->paid]]></code>
</MixedArgument>
</file>
<file src="src/Form/TrainingMailsOutboxFormFactory.php">
<MixedArgument>
<code><![CDATA[$data->additional]]></code>
<code><![CDATA[$data->cc ?: null]]></code>
<code><![CDATA[$data->feedbackRequest ?? false]]></code>
<code><![CDATA[$data->invoice]]></code>
<code><![CDATA[$data->invoiceId]]></code>
<code><![CDATA[$id]]></code>
<code><![CDATA[$id]]></code>
<code><![CDATA[$id]]></code>
<code><![CDATA[$id]]></code>
</MixedArgument>
</file>
<file src="src/Formatter/TexyFormatter.php">
<MixedArgument>
<code><![CDATA[$matches[2]]]></code>
Expand Down
2 changes: 1 addition & 1 deletion app/src/Admin/Presenters/EmailsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
class EmailsPresenter extends BasePresenter
{

/** @var list<TrainingApplication> */
/** @var array<int, TrainingApplication> */
private array $applications = [];


Expand Down
5 changes: 3 additions & 2 deletions app/src/Articles/Blog/BlogPosts.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

namespace MichalSpacekCz\Articles\Blog;

use DateTime;
use Exception;
use MichalSpacekCz\Articles\Blog\Exceptions\BlogPostDoesNotExistException;
use MichalSpacekCz\Articles\Blog\Exceptions\BlogPostWithoutIdException;
use MichalSpacekCz\Database\TypedDatabase;
use MichalSpacekCz\DateTime\DateTimeFactory;
use MichalSpacekCz\DateTime\Exceptions\InvalidTimezoneException;
use MichalSpacekCz\Tags\Tags;
use MichalSpacekCz\Utils\Exceptions\JsonItemNotStringException;
Expand All @@ -30,6 +30,7 @@ public function __construct(
private BlogPostFactory $factory,
private Cache $exportsCache,
private Tags $tags,
private DateTimeFactory $dateTimeFactory,
private int $updatedInfoThreshold,
) {
}
Expand Down Expand Up @@ -229,7 +230,7 @@ public function update(BlogPost $post, ?string $editSummary, array $previousSlug
],
$postId,
);
$editedAt = new DateTime();
$editedAt = $this->dateTimeFactory->create();
if ($editSummary !== null) {
$timeZone = $editedAt->getTimezone()->getName();
$this->database->query(
Expand Down
2 changes: 2 additions & 0 deletions app/src/Form/ChangePasswordFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public function create(callable $onSuccess): UiForm

$form->onSuccess[] = function (UiForm $form) use ($onSuccess): void {
$values = $form->getFormValues();
assert(is_string($values->password));
assert(is_string($values->newPassword));
$this->authenticator->changePassword($this->user, $values->password, $values->newPassword);
$onSuccess();
};
Expand Down
54 changes: 26 additions & 28 deletions app/src/Form/PostFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use MichalSpacekCz\DateTime\Exceptions\InvalidTimezoneException;
use MichalSpacekCz\Form\Controls\TrainingControlsFactory;
use MichalSpacekCz\Formatter\TexyFormatter;
use MichalSpacekCz\ShouldNotHappenException;
use MichalSpacekCz\Tags\Tags;
use MichalSpacekCz\Twitter\Exceptions\TwitterCardNotFoundException;
use MichalSpacekCz\Twitter\TwitterCards;
Expand Down Expand Up @@ -153,16 +152,15 @@ function () use ($form, $post): BlogPost {
$newPost = $this->buildPost($values, $post?->getId());
try {
if ($post) {
$this->blogPosts->update($newPost, empty($values->editSummary) ? null : $values->editSummary, $post->getSlugTags());
assert(is_string($values->editSummary));
$this->blogPosts->update($newPost, $values->editSummary === '' ? null : $values->editSummary, $post->getSlugTags());
$onSuccessEdit($newPost);
} else {
$onSuccessAdd($this->blogPosts->add($newPost));
}
} catch (UniqueConstraintViolationException) {
$slug = $form->getComponent('slug');
if (!$slug instanceof TextInput) {
throw new ShouldNotHappenException(sprintf("The 'slug' component should be '%s' but it's a %s", TextInput::class, get_debug_type($slug)));
}
assert($slug instanceof TextInput);
$slug->addError($this->texyFormatter->translate('messages.blog.admin.duplicateslug'));
}
};
Expand All @@ -181,33 +179,33 @@ function () use ($form, $post): BlogPost {
*/
private function buildPost(stdClass $values, ?int $postId): BlogPost
{
$title = $values->title;
if (!is_string($title)) {
throw new ShouldNotHappenException("Title should be a string, but it's a " . get_debug_type($title));
}
$slug = $values->slug;
if (!is_string($slug)) {
throw new ShouldNotHappenException("Slug should be a string, but it's a " . get_debug_type($slug));
}
$twitterCard = $values->twitterCard;
if (!is_string($twitterCard)) {
throw new ShouldNotHappenException("Twitter card should be a string, but it's a " . get_debug_type($twitterCard));
}
assert(is_int($values->translationGroup) || $values->translationGroup === null);
assert(is_string($values->title));
assert(is_string($values->slug));
assert(is_int($values->locale));
assert(is_string($values->lead));
assert(is_string($values->text));
assert(is_string($values->published));
assert(is_string($values->previewKey));
assert(is_string($values->originally));
assert(is_string($values->ogImage));
assert(is_string($values->tags));
assert(is_string($values->recommended));
assert(is_string($values->twitterCard));
assert(is_array($values->cspSnippets) && array_is_list($values->cspSnippets));
assert(is_array($values->allowedTags) && array_is_list($values->allowedTags));
assert(is_bool($values->omitExports));
/** @var list<string> $cspSnippets */
$cspSnippets = $values->cspSnippets;
if (!is_array($cspSnippets) || !array_is_list($cspSnippets)) {
throw new ShouldNotHappenException("CSP snippets should be a list<string>, but it's a " . get_debug_type($cspSnippets));
}
/** @var list<string> $allowedTagsGroups */
$allowedTagsGroups = $values->allowedTags;
if (!is_array($allowedTagsGroups) || !array_is_list($allowedTagsGroups)) {
throw new ShouldNotHappenException("Allowed tags groups should be a list<string>, but it's a " . get_debug_type($allowedTagsGroups));
}
return $this->blogPostFactory->create(
$postId,
$slug === '' ? Strings::webalize($title) : $slug,
$values->slug === '' ? Strings::webalize($values->title) : $values->slug,
$values->locale,
$this->locales->getLocaleById($values->locale),
$values->translationGroup === '' ? null : $values->translationGroup,
$title,
$values->translationGroup,
$values->title,
$values->lead === '' ? null : $values->lead,
$values->text,
$values->published === '' ? null : new DateTime($values->published),
Expand All @@ -217,10 +215,10 @@ private function buildPost(stdClass $values, ?int $postId): BlogPost
$values->tags === '' ? [] : $this->tags->toArray($values->tags),
$values->tags === '' ? [] : $this->tags->toSlugArray($values->tags),
$values->recommended ? $this->recommendedLinks->getFromJson($values->recommended) : [],
$twitterCard === '' ? null : $this->twitterCards->getCard($twitterCard),
$values->twitterCard === '' ? null : $this->twitterCards->getCard($values->twitterCard),
$cspSnippets,
$allowedTagsGroups,
(bool)$values->omitExports,
$values->omitExports,
);
}

Expand Down
13 changes: 12 additions & 1 deletion app/src/Form/Pulse/PasswordsStorageAlgorithmFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,18 @@ public function create(callable $onSuccess, int $newDisclosures): UiForm
*/
private function validatePasswordsStorages(UiForm $form, ArrayHash $values): void
{
assert($values->company instanceof ArrayHash);
assert($values->company->new instanceof ArrayHash);
assert(is_string($values->company->new->name));
assert($values->site instanceof ArrayHash);
assert(is_int($values->site->id) || $values->site->id === Sites::ALL || $values->site->id === null);
assert($values->site->new instanceof ArrayHash);
assert(is_string($values->site->new->url));
assert($values->algo instanceof ArrayHash);
assert($values->algo->new instanceof ArrayHash);
assert(is_string($values->algo->new->algoName));
if (empty($values->company->new->name)) {
assert(is_int($values->company->id));
$storages = $this->passwords->getStoragesByCompanyId($values->company->id);
$specificSites = array_filter($storages->getSites(), function ($site) {
return $site instanceof StorageSpecificSite;
Expand All @@ -206,7 +217,7 @@ private function validatePasswordsStorages(UiForm $form, ArrayHash $values): voi
if (!empty($values->site->new->url) && $this->sites->getByUrl($values->site->new->url)) {
$form->addError('Can\'t add new site, duplicated URL');
}
if (!empty($values->algo->new->algo) && $this->hashingAlgorithms->getAlgorithmByName($values->algo->new->algo)) {
if (!empty($values->algo->new->algoName) && $this->hashingAlgorithms->getAlgorithmByName($values->algo->new->algoName)) {
$form->addError('Can\'t add new algorithm, duplicated name');
}
}
Expand Down
42 changes: 35 additions & 7 deletions app/src/Form/TrainingApplicationAdminFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,35 @@ public function create(callable $onSuccess, callable $onStatusHistoryDeleteSucce

$form->onSuccess[] = function (UiForm $form) use ($application, $onSuccess): void {
$values = $form->getFormValues();
$dateId = $values->date ?? null;
assert(is_bool($values->nameSet));
assert(is_string($values->name));
assert(is_bool($values->emailSet));
assert(is_string($values->email));
assert(is_bool($values->companySet));
assert(is_string($values->company));
assert(is_bool($values->streetSet));
assert(is_string($values->street));
assert(is_bool($values->citySet));
assert(is_string($values->city));
assert(is_bool($values->zipSet));
assert(is_string($values->zip));
assert(is_bool($values->countrySet));
assert(is_string($values->country));
assert(is_bool($values->companyIdSet));
assert(is_string($values->companyId));
assert(is_bool($values->companyTaxIdSet));
assert(is_string($values->companyTaxId));
assert(is_bool($values->noteSet));
assert(is_string($values->note));
assert(is_string($values->source));
assert(is_float($values->price) || $values->price === null);
assert(is_string($values->vatRate));
assert(is_float($values->priceVat) || $values->priceVat === null);
assert(is_string($values->discount));
assert(is_string($values->invoiceId) || $values->invoiceId === null);
assert(is_string($values->paid));
assert(is_bool($values->familiar));
assert(is_int($values->date) || $values->date === null);
$this->trainingApplicationStorage->updateApplicationData(
$application->getId(),
$values->nameSet ? $values->name : null,
Expand All @@ -105,16 +133,16 @@ public function create(callable $onSuccess, callable $onStatusHistoryDeleteSucce
$values->companyTaxIdSet ? $values->companyTaxId : null,
$values->noteSet ? $values->note : null,
$values->source,
(is_float($values->price) ? $values->price : null),
(trim($values->vatRate) !== '' ? $values->vatRate / 100 : null),
(is_float($values->priceVat) ? $values->priceVat : null),
(trim($values->discount) !== '' ? (int)$values->discount : null),
$values->price,
trim($values->vatRate) !== '' ? (float)$values->vatRate / 100 : null,
$values->priceVat,
trim($values->discount) !== '' ? (int)$values->discount : null,
$values->invoiceId,
$values->paid,
$values->familiar,
$dateId,
$values->date,
);
$onSuccess($dateId);
$onSuccess($values->date);
};
$this->setApplication($form, $application);
return $form;
Expand Down
Loading

0 comments on commit f13af02

Please sign in to comment.