New Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence s…

…niff (#197)

This PR adds a sniff that detects mixed boolean operators (`&&`, `||`) within a single expression without making precedence clear using parentheses.

src/Standards/Generic/Docs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceStandard.xml

@@ -0,0 +1,44 @@
+<documentation title="Require Explicit Boolean Operator Precedence"
+    >
+    <standard>
+    <![CDATA[
+    Forbids mixing different binary boolean operators (&&, ||, and, or, xor) within a single expression without making precedence clear using parentheses.
+    ]]>
+    </standard>
+    <code_comparison>
+        <code title="Valid: Making precedence clear with parentheses.">
+        <![CDATA[
+$one = false;
+$two = false;
+$three = true;
+
+$result = <em>($one && $two) || $three</em>;
+$result2 = <em>$one && ($two || $three)</em>;
+$result3 = <em>($one && !$two) xor $three</em>;
+$result4 = <em>$one && (!$two xor $three)</em>;
+
+if (
+    <em>($result && !$result3)
+    || (!$result && $result3)</em>
+) {}
+        ]]>
+        </code>
+        <code title="Invalid: Not using parentheses.">
+        <![CDATA[
+$one = false;
+$two = false;
+$three = true;
+
+$result = <em>$one && $two || $three</em>;
+
+$result3 = <em>$one && !$two xor $three</em>;
+
+
+if (
+    <em>$result && !$result3
+    || !$result && $result3</em>
+) {}
+        ]]>
+        </code>
+    </code_comparison>
+</documentation>

src/Standards/Generic/Sniffs/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceSniff.php

@@ -0,0 +1,119 @@
+<?php
+/**
+ * Forbid mixing different binary boolean operators within a single expression without making precedence
+ * clear using parentheses.
+ *
+ * <code>
+ * $one = false;
+ * $two = false;
+ * $three = true;
+ *
+ * $result = $one && $two || $three;
+ * $result3 = $one && !$two xor $three;
+ * </code>
+ *
+ * {@internal The unary `!` operator is not handled, because its high precedence matches its visuals of
+ * applying only to the sub-expression right next to it, making it unlikely that someone would
+ * misinterpret its precedence. Requiring parentheses around it would reduce the readability of
+ * expressions due to the additional characters, especially if multiple subexpressions / variables
+ * need to be negated.}
+ *
+ * Sister-sniff to the `Squiz.ControlStructures.InlineIfDeclaration` and
+ * `Squiz.Formatting.OperatorBracket.MissingBrackets` sniffs.
+ *
+ * @author    Tim Duesterhus <[email protected]>
+ * @copyright 2021-2023 WoltLab GmbH.
+ * @copyright 2024 PHPCSStandards and contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Tokens;
+
+class RequireExplicitBooleanOperatorPrecedenceSniff implements Sniff
+{
+
+    /**
+     * Array of tokens this test searches for to find either a boolean
+     * operator or the start of the current (sub-)expression. Used for
+     * performance optimization purposes.
+     *
+     * @var array<int|string>
+     */
+    private $searchTargets = [];
+
+
+    /**
+     * Returns an array of tokens this test wants to listen for.
+     *
+     * @return array<int|string>
+     */
+    public function register()
+    {
+        $this->searchTargets  = Tokens::$booleanOperators;
+        $this->searchTargets += Tokens::$blockOpeners;
+        $this->searchTargets[T_INLINE_THEN] = T_INLINE_THEN;
+        $this->searchTargets[T_INLINE_ELSE] = T_INLINE_ELSE;
+
+        return Tokens::$booleanOperators;
+
+    }//end register()
+
+
+    /**
+     * Processes this test, when one of its tokens is encountered.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @param int                         $stackPtr  The position of the current token
+     *                                               in the stack passed in $tokens.
+     *
+     * @return void
+     */
+    public function process(File $phpcsFile, $stackPtr)
+    {
+        $tokens = $phpcsFile->getTokens();
+
+        $start = $phpcsFile->findStartOfStatement($stackPtr);
+
+        $previous = $phpcsFile->findPrevious(
+            $this->searchTargets,
+            ($stackPtr - 1),
+            $start,
+            false,
+            null,
+            true
+        );
+
+        if ($previous === false) {
+            // No token found.
+            return;
+        }
+
+        if ($tokens[$previous]['code'] === $tokens[$stackPtr]['code']) {
+            // Identical operator found.
+            return;
+        }
+
+        if (in_array($tokens[$previous]['code'], [T_INLINE_THEN, T_INLINE_ELSE], true) === true) {
+            // Beginning of the expression found for the ternary conditional operator.
+            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.';
+        $phpcsFile->addError($error, $stackPtr, 'MissingParentheses');
+
+    }//end process()
+
+
+}//end class

src/Standards/Generic/Tests/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceUnitTest.inc

@@ -0,0 +1,131 @@
+<?php
+
+if (true && true || true); // Not OK.
+if ((true && true) || true);
+if (true && (true || true));
+
+$var = true && true || true; // Not OK.
+$var = (true && true) || true;
+$var = true && (true || true);
+
+$complex = true && (true || true) && true;
+$complex = true && (true || true) || true; // Not OK.
+
+if (
+    true
+    && true
+    || true // Not OK.
+);
+
+if (
+    true
+    && (
+        true
+        || true
+    )
+);
+
+if (true && foo(true || true));
+if (true && foo(true && true || true)); // Not OK.
+if (true && $foo[true || true]);
+if (true && $foo[true && true || true]); // Not OK.
+
+if (true && foo(true) || true); // Not OK.
+if (true && $foo[true] || true); // Not OK.
+if (true && foo($foo[true]) || true); // Not OK.
+
+$foo[] = true && true || false; // Not OK.
+
+foo([true && true || false]); // Not OK.
+
+if (true && true || true && true); // Not OK.
+
+$foo = false || true && (#[\Attr(true && true || true)] function (#[\SensitiveParameter] $p) { // Not OK.
+    echo true || true && true; // Not OK.
+
+    return true;
+})('dummy') || false; // Not OK.
+
+$foo = false || (true && (#[\Attr((true && true) || true)] function (#[\SensitiveParameter] $p) {
+    echo (true || true) && true;
+
+    return true;
+})('dummy')) || false;
+
+$foo = true || true || (#[\Attr(true && true && true)] function (#[\SensitiveParameter] $p) {
+    echo true && true && true;
+
+    return true;
+})('dummy') || false;
+
+if (true && [true, callMe(), ${true || true}] || true); // Not OK.
+if (true && [true, callMe(), ${true || true}] && true);
+
+for (true || true || true; true && true && true; true || true || true);
+for (true || true && true; true && true || true; true || true && true); // Not OK.
+
+for ($a = true || true || true, $b = true && true && true; $a; $b);
+for ($a = true || true && true, $b = true || true && true; $a; $b); // Not OK.
+
+$foo = true || true || true ? true && true && true : true || true || true;
+$foo = true && true || true // Not OK.
+    ? true || true && true // Not OK.
+    : true || true && true; // Not OK.
+
+for(true || true || true, true && true && true);
+for(true && true || true, true && true || true); // Not OK.
+
+(true && true and true); // Not OK.
+(true && true or true); // Not OK.
+(true and true or true); // Not OK.
+(true and true xor true and true); // Not OK.
+
+if (true || true && true && true && true); // Not OK.
+
+match (true) {
+    // OK.
+    $a || ($b && $c) => true,
+};
+
+match (true) {
+    // Not OK.
+    $a || $b && $c => true,
+};
+
+match (true) {
+    // OK.
+    $a || $b => true,
+    $a && $b => true,
+};
+
+match (true) {
+    // Debatable.
+    $a || $b, $a && $b => true,
+};
+
+// OK.
+$foo = fn ($a, $b, $c) => $a && ($b || $c);
+
+// Not OK.
+$foo = fn ($a, $b, $c) => $a && $b || $c;
+
+// OK.
+$foo = $a && (fn ($a, $b, $c) => $a || $b);
+
+// Debatable.
+$foo = $a && fn ($a, $b, $c) => $a || $b;
+
+// OK.
+\array_map(
+    fn ($a, $b, $c) => $a || $b,
+    $a && $b
+);
+
+match (true) {
+    // Not OK.
+    $a || ($b && $c) && $d => true,
+    // Not OK.
+    $b && $c['a'] || $d => true,
+    // Not OK.
+    $b && ${$var} || $d => true,
+};

src/Standards/Generic/Tests/CodeAnalysis/RequireExplicitBooleanOperatorPrecedenceUnitTest.php

@@ -0,0 +1,91 @@
+<?php
+/**
+ * Unit test class for the RequireExplicitBooleanOperatorPrecedence sniff.
+ *
+ * @author    Tim Duesterhus <[email protected]>
+ * @copyright 2021-2023 WoltLab GmbH.
+ * @copyright 2024 PHPCSStandards and contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Standards\Generic\Tests\CodeAnalysis;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+/**
+ * Unit test class for the RequireExplicitBooleanOperatorPrecedence sniff.
+ *
+ * @covers \PHP_CodeSniffer\Standards\Generic\Sniffs\CodeAnalysis\RequireExplicitBooleanOperatorPrecedenceSniff
+ */
+final class RequireExplicitBooleanOperatorPrecedenceUnitTest extends AbstractSniffUnitTest
+{
+
+
+    /**
+     * Returns the lines where errors should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of errors that should occur on that line.
+     *
+     * @return array<int, int>
+     */
+    public function getErrorList()
+    {
+        return [
+            3   => 1,
+            7   => 1,
+            12  => 1,
+            17  => 1,
+            29  => 1,
+            31  => 1,
+            33  => 1,
+            34  => 1,
+            35  => 1,
+            37  => 1,
+            39  => 1,
+            41  => 2,
+            43  => 2,
+            44  => 1,
+            47  => 1,
+            61  => 1,
+            65  => 3,
+            68  => 2,
+            71  => 1,
+            72  => 1,
+            73  => 1,
+            76  => 2,
+            78  => 1,
+            79  => 1,
+            80  => 1,
+            81  => 2,
+            83  => 1,
+            92  => 1,
+            110 => 1,
+            126 => 1,
+            128 => 1,
+            130 => 1,
+
+            // Debatable.
+            103 => 1,
+            116 => 1,
+        ];
+
+    }//end getErrorList()
+
+
+    /**
+     * Returns the lines where warnings should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of warnings that should occur on that line.
+     *
+     * @return array<int, int>
+     */
+    public function getWarningList()
+    {
+        return [];
+
+    }//end getWarningList()
+
+
+}//end class