The description of this task has to begin with a confession; take it as a user story, if you want. If I find code like the following (source):
if ( !$attribs['rc_patrolled'] && ( $useRCPatrol || $attribs['rc_type'] == RC_NEW && $useNPPatrol ) // <-- relevant line ) { $flag .= '!'; }
I don't know what it does. I just do not remember what the operator precedence is; maybe I never even learned it. Is it || before && or the other way around? Whenever I find such code, I generally do one of two things: 1) go check the manual to see what the order is (and promptly forget it afterwards), or 2) trust the author of the code to know operator precedence better than me, and hope that the code does what it looks like it was meant to do (which is probably the case if I'm looking at code written in 2004). Clearly, this is subpar: I have to spend a decent amount of time parsing the code in question. But then I think, maybe it's just me. After all, there are so many things that I have to re-learn often because I forget them immediately.
But now that PHPCS 3.9.0 is out and it introduces a new Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence sniff, I think it's time for me to actually ask this and see if it's just me--or actually, ask: would it be a problem if we enabled this new rule to forbid mixing boolean operators without parentheses? To clarify, the code above would be flagged, and the accepted way of rewriting it (no autofix available) would be:
if ( !$attribs['rc_patrolled'] && ( $useRCPatrol || ( $attribs['rc_type'] == RC_NEW && $useNPPatrol ) ) ) { $flag .= '!'; }
where the precedence is made very clear by the parentheses. I think the extra parentheses are well worth the clarity, as well as the reassurance that the code is doing exactly what we want it to do. Still, there are a few occurrences of this pattern (at least in MW core, see patch attached to this task) and we don't have a sniff for it, so I'm wondering if I'm missing some potential downsides.