Skip to content

Commit

Permalink
Generic/RequireExplicitBooleanOperatorPrecedence: remove
Browse files Browse the repository at this point in the history
unreachable condition

This commit reverts the changes to the
`Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence` sniff
introduced in
PHPCSStandards/PHPCSExtra@7b38efb.
The sniff's tests remain relevant, so they were preserved.

The original commit was added to fix false positives that the sniff was
triggering when handling boolean operators inside a match (see
PHPCSStandards/PHPCSExtra#271 (review)
-1634348864 and
PHPCSStandards/PHPCSExtra#271 (comment)
). Example:

```php
match (true) {
    $a || ($b && $c) => true,
};
```

I believe the false positive was actually caused by a bug in
`File::findStartOfStatement()`. This bug was then fixed in
b82438f
which rendered the changes to the sniff itself unnecessary and the
removed condition unreachable.

Before this fix, when processing the code example above,
`File::findStartOfStatement()` returned the variable `$a` as the
start of the statement for the `&&` boolean operator. This meant that
`$previous` would be set to `||` and the removed condition would be
needed to ensure the sniff would bail instead of triggering an error.

After this fix, `File::findStartOfStatement()` returns `$b` as the start
of the statement and then `$previous` is set to `false` and the sniff
bails before reaching the removed condition.

Including `Tokens::$blockOpeners` in
`RequireExplicitBooleanOperatorPrecedenceSniff::$searchTargets` was
necessary only for the removed condition, so it was removed as well.
  • Loading branch information
rodrigoprimo authored and jrfnl committed Oct 28, 2024
1 parent 7e7e712 commit 739aa75
Showing 1 changed file with 1 addition and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ class RequireExplicitBooleanOperatorPrecedenceSniff implements Sniff
*/
public function register()
{
$this->searchTargets = Tokens::$booleanOperators;
$this->searchTargets += Tokens::$blockOpeners;
$this->searchTargets = Tokens::$booleanOperators;
$this->searchTargets[T_INLINE_THEN] = T_INLINE_THEN;
$this->searchTargets[T_INLINE_ELSE] = T_INLINE_ELSE;

Expand Down Expand Up @@ -102,12 +101,6 @@ public function process(File $phpcsFile, $stackPtr)
return;
}

if (isset(Tokens::$blockOpeners[$tokens[$previous]['code']]) === true) {
// Beginning of the expression found for a block opener. Needed to
// correctly handle match arms.
return;
}

// We found a mismatching operator, thus we must report the error.
$error = 'Mixing different binary boolean operators within an expression';
$error .= ' without using parentheses to clarify precedence is not allowed.';
Expand Down

0 comments on commit 739aa75

Please sign in to comment.