vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.54k stars 659 forks source link

Conditionalling calling `unset()` in a loop doesn't remove `non-empty` status for array #8866

Open NickMolloy opened 1 year ago

NickMolloy commented 1 year ago

Example: https://psalm.dev/r/bc65d88d33 Psalm incorrectly reports that the last empty() check is never falsy, despite the fact that it may be empty.

Compare that to an example where unset() is not called inside a foreach loop that correctly relaxes the non-empty status of the array: https://psalm.dev/r/93b6a3f313

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/bc65d88d33 ```php 1, 'bad' => 2, ]; } else if ($random < 8) { return [ 'bad' => 1 ]; } else { return []; } } function foo(): void { $data = get_data(); if (empty($data)) { return; } $remove = ['bad']; foreach ($remove as $r) { if (isset($data[$r])) { unset($data[$r]); } } if (empty($data)) { echo('Data is now empty'); } } foo(); ``` ``` Psalm output (using commit e4b64af): ERROR: TypeDoesNotContainType - 37:9 - Type non-empty-array for $data is never falsy ```
https://psalm.dev/r/93b6a3f313 ```php 1, 'bad' => 2, ]; } else if ($random < 8) { return [ 'bad' => 1 ]; } else { return []; } } function foo(): void { $data = get_data(); if (empty($data)) { return; } if (isset($data['bad'])) { unset($data['bad']); } if (empty($data)) { echo('Data is now empty'); } } foo(); ``` ``` Psalm output (using commit e4b64af): No issues! ```
weirdan commented 1 year ago

Simplified: https://psalm.dev/r/d81d7cf3d3

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/d81d7cf3d3 ```php INFO: Trace - 8:30 - $data: non-empty-array INFO: Trace - 10:26 - $data: non-empty-array ```