RemoveAlwaysTrueIfConditionRector removes if-statement that is required for undefined array key check

[gruberroland]

Bug Report

Subject	Details
Rector version	0.19.0

The RemoveAlwaysTrueIfConditionRector tries to remove an if-statement that is needed to prevent access to undefined array keys.

Minimal PHP Code Causing Issue

https://getrector.com/demo/a1bd0aae-6e03-4fe7-b3c5-a6ccbaaff5bc

PHP Warning: Undefined array key "add" in rectorTest.php on line 22 PHP Fatal error: Uncaught TypeError: array_merge(): Argument #1 must be of type array, null given in rectorTest.php:22 Stack trace: #0 rectorTest.php(22): array_merge() #1 rectorTest.php(31): Test->test() #2 {main} thrown in rectorTest.php on line 22

Expected Behaviour

There should be no change as the "if" is required.

[samsonasik]

Could you create failing test case fixture PR? Thank you.

[gruberroland]

Here is a sample code. As "$changeSet['add']" might not be set the Rector fix will lead to PHP notice messages. Also, it is unclear why the two if-conditions are not treated the same way (only second is removed by Rector).

<?php

final class DemoFile
{
    public function run(bool $param)
    {
		$changes = [];
		foreach (toAdd() as $add) {
			$changes[$add['id']]['add'][] = doSomething($add);
		}
		foreach (toRem() as $del) {
			$changes[$add['id']]['del'][] = doSomething($del);
		}
   		foreach ($changes as $changeSet) {
			if (isset($changeSet['del'])) {
				doDel($changeSet['del']);
			}
			if (isset($changeSet['add'])) {
				doAdd($changeSet['add']);
			}
        }

    }
}

<?php

use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;
use Rector\Set\ValueObject\SetList;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->sets([
        SetList::DEAD_CODE,
    ]);
};

[samsonasik]

That's seems phpstan bug based on its type detected, I created issue on phpstan side, see https://github.com/phpstan/phpstan/issues/10640

[samsonasik]

Also, $changes[$add['id']]['del'][] seems should be $changes[$del['id']]['del'][]

-$changes[$add['id']]['del'][] = doSomething($del);
+$changes[$del['id']]['del'][] = doSomething($del);

on 2nd loop, otherwise, the $add may be undefined

[samsonasik]

I am closing it as it is phpstan bug and already reported there https://github.com/phpstan/phpstan/issues/10640

You can skip the rule until it got fixed on phpstan side ;)