[DeadCode] Removing useless if statements

[JanMikes]

Hey, i wave just made few changes in our code and making this change in like 60 places, wondering if we have already rector for it :-)

Diff

- /** @var ICache|null */
+ /** @var ICache */
private $cache;

- if ($this->cache) {
    $this->cache->save($x);
- }

PHPStan link: https://phpstan.org/r/1d1026c1-ab7c-46e4-8146-17c052eb8dae

To show how real this issue is, check this (just part of) phpstan report:

[TomasVotruba]

1 line is enough :D KISS

The code above can be null, so it's hard to say.

[TomasVotruba]

Also, text is better, because I can copy-paste it and find responsbile PHPStan rule - mimic its logic much faster. Again, unique 1 line only.

[JanMikes]

Yeah, i just wanted to show how useful can rector be (again) 😄

Well, the cache is filled from __construct() so it imho never can be null (well, before filling in constructor).

[TomasVotruba]

Could you update the snippet so it's full? Best with PHPStan website link

[JanMikes]

Could you update the snippet so it's full? Best with PHPStan website link

It took me while to understand what do you want from me 😄 i added link to phpstan.

It does not have tobe filled via constructor, it can use etc @inject annotation, should it somehow check whether it can be null or not?

[TomasVotruba]

It does not have tobe filled via constructor, it can use etc @Inject annotation, should it somehow check whether it can be null or not?

Not sure. What's you use case?

[TomasVotruba]

Probably related phpstan/phpstan@6771e62

[TomasVotruba]

PHPStan is buggy https://phpstan.org/r/a98649a0-345c-44c4-b38a-81dde2e8816c
We can't rely on this, but we can do it better.

Let's start with constructor.

[JanMikes]

We have both use cases, either with @inject annotation (in parent presenters, where __construct can not be used because of constructor hell) and filling property in __construct wherever the class does not have any children.

[JanMikes]

PHPStan is buggy https://phpstan.org/r/a98649a0-345c-44c4-b38a-81dde2e8816c

Well, it depends, because there is the annotation which says there is only instance of that object, missing |null in that, phpstan thinks it is fine - can rector do better? It should be developer's responsibility to keep annotations up to date, but as we all know, developers are lazy people and sometimes they forget to something, like updating the annotation 😄

[TomasVotruba]

It should be developer's responsibility to keep annotations up to date.

In a dreamland maybe :D