-
Notifications
You must be signed in to change notification settings - Fork 51
[WIP] Disallow complex conditional expressions #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Disallow complex conditional expressions #21
Conversation
@Majkl578 you added this to the 3.0 milestone - is there a sniff we can use or is it just considered a guideline? |
I can prepare the sniff. However there are already some new useful sniffs so I do a release of Slevomat CS so everyone can test them and report bugs. It's also possible to merge some PR here and test them in |
Deferred for now |
Can someone please make a better explanation what this sniff should do? Examples would be perfect :) |
Maybe doctrine/dbal#2980 (comment) and #18 (comment) is something to start but is basically split long conditions (maybe a Sniffers configurable with how many checks are inside it?) |
@carusogabriel @kukulich it might be quite tricky to implement the automatic fix fo this though... |
I'm removing the I don't think this is worth adding an auto-fixer, btw. |
Should we give up on this one? |
The question is what does "long" mean here. Does it mean to disallow boolean operators like |
IMO long means longer than 120 chars. I don't understand what's the proposed replacement, a code sample would help, because the related issues shows something quite simple, that doesn't seem applicable to if statements containing big blocks of code. |
if ($foo === 1 && $bar === 2 && $baz === 3 && $tar === 4) { // too long - 16 paths if ($foo === 1 && $bar === 2 && $baz === 3) { // acceptable - 8 paths |
A better wording might be "complex" instead of "long" then :P |
@greg0ire adapted title 👍 |
Related to doctrine/dbal#2980 (comment)