From 739aa751d11025da18a10dddf0904d5dd2d46337 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 18 Oct 2024 16:13:30 -0300 Subject: [PATCH] Generic/RequireExplicitBooleanOperatorPrecedence: remove unreachable condition This commit reverts the changes to the `Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence` sniff introduced in https://github.com/PHPCSStandards/PHPCSExtra/pull/271/commits/7b38efb98c3383b84819a79f85828d517919f17d. 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 https://github.com/PHPCSStandards/PHPCSExtra/pull/271#pullrequestreview -1634348864 and https://github.com/PHPCSStandards/PHPCSExtra/pull/271#issuecomment-1729104556 ). 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 https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/502/commits/b82438f2e1199fb29f4825782dad686168f70352 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. --- .../RequireExplicitBooleanOperatorPrecedenceSniff.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceSniff.php index 93afe2e35a..41922efc6d 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceSniff.php @@ -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; @@ -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.';