phpstan / phpstan-strict-rules

Extra strict and opinionated rules for PHPStan
MIT License
605 stars 48 forks source link

False positive with OverwriteVariablesWithForeachRule rules #164

Closed VincentLanglet closed 2 years ago

VincentLanglet commented 2 years ago

I have some issues with the rule https://github.com/phpstan/phpstan-strict-rules/blob/master/src/Rules/ForeachLoop/OverwriteVariablesWithForeachRule.php

The error line 40 shouldn't be reported https://phpstan.org/r/91bd9f91-779d-414b-8a1e-61408e047f17 It's weird but removing three line in the previous foreach solve the issue https://phpstan.org/r/582420e9-d427-4dcb-9128-677379acb316

I already got an issue with https://phpstan.org/r/8de1e2ac-40ed-46d8-9dd8-2b714e5526d0 vs https://phpstan.org/r/c6e30a68-2d27-4d8c-8f8a-d6a2b7e34efe

According to me, the error shouldn't be reported, but if you consider it should be @ondrejmirtes, it should be for all the exemple I found. Anyway, it seems like the rule or the way scope are handled should be updated.

I may be wrong but this doesn't seems easy to fix.

ondrejmirtes commented 2 years ago

This behaviour is correct. The variable $data created on line 40 has the same name as on line 11. And PHPStan know this variable always exist when the line 40 is exdecuted.

VincentLanglet commented 2 years ago

This behaviour is correct. The variable $data created on line 40 has the same name as on line 11. And PHPStan know this variable always exist when the line 40 is exdecuted.

But phpstan report

foreach ($array as $key => $value) {
    $newArray[$key] = [$value];
}   
foreach ($newArray as $newValue) {
    foreach ($newValue as $key => $value) {
    }
}

but not

foreach ($array as $key => $value) {
    $newArray[$key] = [$value];
}

foreach ($newArray as $key => $value) {
}

This lacks of consistency.

Also, since using $value (or $data in the first example) outside of the foreach is reported by phpstan as an error (You should not use key/value of foreach outside of a foreach), so the variable name should be free to be re-used.

github-actions[bot] commented 2 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.