Unlock phpstan level 5 #1924
Unlock phpstan level 5 #1924
Conversation
(+ minor adjustments in case of "a && b always evaluates to...")
Here is why: The expected parameter is a UserInterface (enforced via UserProviderInterface) but only FrontendUser|BackendUser should be allowed. This type checking, however, is dynamically, so static analyzers cannot resolve it (= it must be annotated). The test is effectively validating the dynamic type check and therefore must be allowed to ignore the annotation.
m-vo
force-pushed
the
bugfix/unlock-phpstan-level-5
branch
from
e274a24
to
d990f9a
Compare
| @@ -79,7 +79,7 @@ static function (string $type, string $buffer) use ($event): void { | |||
| ); | |||
|
|
|||
| if (!$process->isSuccessful()) { | |||
| throw new \RuntimeException(sprintf('An error occurred while executing the "%s" command.', $cmd)); | |||
| throw new \RuntimeException(sprintf('An error occurred while executing the "%s" command.', implode(' ', $cmd))); | |||
There was a problem hiding this comment.
(This was introduced with #1917 and therefore must not be backported.)
Description ----------- Because they will no longer trigger `type MockObject is not within the annotated types` warnings in PhpStorm (see #1924 (comment)). Commits ------- 2584d3d Remove @param Something&MockObject annotations
Description ----------- Because they will no longer trigger `type MockObject is not within the annotated types` warnings in PhpStorm (see contao/contao#1924 (comment)). Commits ------- 2584d3d0 Remove @param Something&MockObject annotations
| @@ -491,11 +490,12 @@ private function findPages(array $candidates): array | |||
| return []; | |||
| } | |||
|
|
|||
| /** @var array<PageModel> */ | |||
There was a problem hiding this comment.
Shouldn't we add @mixin<T> and @return array<T> (or some other generics magic) to the Model class so PhpStan knows what we are returning here?
There was a problem hiding this comment.
That's a tough one. We would imo need to add generics to the Collection, the Adapter and the Model for this to work. And then we will end with a lot of @phpstan-param, @phpstan-return, @phpstan-var... declarations so that our IDEs are still able to guess the type.
Everything would look a bit like this I guess: https://phpstan.org/r/15377a79-1be1-4f2d-999f-f1ae9dbe37f0
There was a problem hiding this comment.
Challenge accepted. Are you ok with all those tags?
There was a problem hiding this comment.
Yes, I am. We are already using them in the Adapter class anyway.
There was a problem hiding this comment.
I had a deep dive into the topic.
Most of the things could theoretically be solved by having a Collection<T of Model> + static/Collection<static> return type annotations in the model base class and forced return overwrites for all of the findBy* methods because of the dynamic Model vs Collection vs Array return type nature of find().
Unfortunately, in reality this doesn't work as expected because we're unfortunately using __callStatic and PHPStan then is not able to resolve the types.
It would possibly work to create our own extension, but well...
There was a problem hiding this comment.
Feel free to modify https://github.com/contao/phpstan 😎
There was a problem hiding this comment.
Feel free to modify https://github.com/contao/phpstan sunglasses
If I only knew how. 😄
There was a problem hiding this comment.
I have created a follow-up issue here: #1940
|
Thank you @m-vo. |
Description ----------- Because they will no longer trigger `type MockObject is not within the annotated types` warnings in PhpStorm (see contao#1924 (comment)). Commits ------- 2584d3d Remove @param Something&MockObject annotations
Description ----------- | Q | A | -----------------| --- | Fixed issues | - | Docs PR or issue | - I was annoyed by an invalid type annotation. But while fixing it I thought we could as well raise our phpstan level to 5. 😄 The commit messages should explain the motivation behind each set of changes. Commits ------- cecc5da upgrade phpstan level to 5 1b376cb report unmatched errors 8a9c0da drop unneeded rule exceptions (+ minor adjustments in case of "a && b always evaluates to...") b32fbb8 fix invalid array arguments 013cbab remove/rewrite implicit type casting 40272d1 fix parameter type annotations 135e0d0 drop unnecessary (+ invalid) param type annotations from tests baf6417 drop tests that test for invalid but annotated types b49947d add exception for ContaoUserProviderTest testing with an invalid class Here is why: The expected parameter is a UserInterface (enforced via UserProviderInterface) but only FrontendUser|BackendUser should be allowed. This type checking, however, is dynamically, so static analyzers cannot resolve it (= it must be annotated). The test is effectively validating the dynamic type check and therefore must be allowed to ignore the annotation. d990f9a fix tests 1eb47e9 revert preventing errors via \constant() 63da7dd define BE_USER_LOGGED_IN as a dynamic constant 3c5beba Revert dropping tests with invalid types / widening Image:set* types This reverts commit baf6417 + partly reverts 40272d1 daf0e04 ignore tests intentionally calling legacy code with other types than annotated a07ffbf remove verbose 'null' from 'mixed' type annotation e3b80e0 re-add annotation f18f3e5 Merge branch 'master' into bugfix/unlock-phpstan-level-5 ef6603e Use @phpstan-ignore-next-line instead of adding ignoreErrors for single issues c7a20c4 Fix the phpDoc of the ImageFactory::create() method af4cf5d Fix the coding style 77e2a4d revert strict check on $port
I was annoyed by an invalid type annotation. But while fixing it I thought we could as well raise our phpstan level to 5. 😄
The commit messages should explain the motivation behind each set of changes.