add a new rule to detect duplicate switch cases#5849
Conversation
|
I think dead code is reported level 4, but we can be similar to duplicate array keys... Also, it should close |
Oh wow, that's an old issue! Test cases I wrote are similar, but do you want me to add a new test case with the code snippet from that issue? I don't think it'll add much though. |
|
this test case looks usefull: phpstan/phpstan#712 (comment) |
We already have the |
| } | ||
| } | ||
|
|
||
| public function looseEqualityIsNotReported(mixed $m): void |
There was a problem hiding this comment.
Why do we not report loose comparisons?
There was a problem hiding this comment.
Cause switch does loose comparison. So they are not duplicates.
There was a problem hiding this comment.
Cause switch does loose comparison
hmm thats the reason why I think it is a duplicate tbh.
see https://3v4l.org/RVlEU#veol
the "1" case is actually dead code, because a previous loose-compared-equal case already consumed the input
|
should the error be prevented when the case-blocks are not |
| if ($firstSeen === null) { | ||
| $seenCases[] = [ | ||
| 'key' => $key, | ||
| 'printed' => $this->exprPrinter->printExpr($case->cond), |
There was a problem hiding this comment.
expr printing can be expensive. maybe only remember the $case->cond and print the condition only when building the error?
|
Hi, I'm sorry to crash your party here but I'd imagine this rule differently 😊 It'd make sense to report "always false" conditions by tracking the types in Scope and checking that. We already do that for With FNSR we don't even need those virtual nodes anymore, if we're okay the rule is only going to work when running PHPStan on PHP 8.1+. The rule can just ask for the type of Looking at Switch_ handling in NodeScopeResolver we might actually be pretty close to achieving that. Feel free to create an issue with this description and feed it to the bot (Claude) 😊 |
|
There's now #5854 - how do you like it when compared to this PR? |
4 participants
Hello 👋🏽
Added a new rule to detect duplicate
switchcases. Was debugging some stuff today and turns out we made a typo and the case was matching with another case before it 😅Added to level 0 with bleeding edge toggle. Let me know if that's not ok.
Closes phpstan/phpstan#712