Merge branch 'feature/2857-3592-squiz-nonexecutable-code-allow-for-php8-throw-expressions' of https://github.com/jrfnl/PHP_CodeSniffer

Author: gsherwood

src/Standards/Squiz/Sniffs/PHP/NonExecutableCodeSniff.php

@@ -16,6 +16,22 @@
 class NonExecutableCodeSniff implements Sniff
 {
 
+    /**
+     * Tokens for terminating expressions, which can be used inline.
+     *
+     * This is in contrast to terminating statements, which cannot be used inline
+     * and would result in a parse error (which is not the concern of this sniff).
+     *
+     * `throw` can be used as an expression since PHP 8.0.
+     * {@link https://wiki.php.net/rfc/throw_expression}
+     *
+     * @var array
+     */
+    private $expressionTokens = [
+        T_EXIT  => T_EXIT,
+        T_THROW => T_THROW,
+    ];
+
 
     /**
      * Returns an array of tokens this test wants to listen for.
@@ -49,12 +65,34 @@ public function process(File $phpcsFile, $stackPtr)
     {
         $tokens = $phpcsFile->getTokens();
 
-        // If this token is preceded with an "or", it only relates to one line
-        // and should be ignored. For example: fopen() or die().
         $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
-        if ($tokens[$prev]['code'] === T_LOGICAL_OR || $tokens[$prev]['code'] === T_BOOLEAN_OR) {
-            return;
-        }
+
+        // Tokens which can be used in inline expressions need special handling.
+        if (isset($this->expressionTokens[$tokens[$stackPtr]['code']]) === true) {
+            // If this token is preceded by a logical operator, it only relates to one line
+            // and should be ignored. For example: fopen() or die().
+            // Note: There is one exception: throw expressions can not be used with xor.
+            if (isset(Tokens::$booleanOperators[$tokens[$prev]['code']]) === true
+                && ($tokens[$stackPtr]['code'] === T_THROW && $tokens[$prev]['code'] === T_LOGICAL_XOR) === false
+            ) {
+                return;
+            }
+
+            // Expressions are allowed in the `else` clause of ternaries.
+            if ($tokens[$prev]['code'] === T_INLINE_THEN || $tokens[$prev]['code'] === T_INLINE_ELSE) {
+                return;
+            }
+
+            // Expressions are allowed with PHP 7.0+ null coalesce and PHP 7.4+ null coalesce equals.
+            if ($tokens[$prev]['code'] === T_COALESCE || $tokens[$prev]['code'] === T_COALESCE_EQUAL) {
+                return;
+            }
+
+            // Expressions are allowed in arrow functions.
+            if ($tokens[$prev]['code'] === T_FN_ARROW) {
+                return;
+            }
+        }//end if
 
         // Check if this token is actually part of a one-line IF or ELSE statement.
         for ($i = ($stackPtr - 1); $i > 0; $i--) {
@@ -139,7 +177,6 @@ public function process(File $phpcsFile, $stackPtr)
         // This token may be part of an inline condition.
         // If we find a closing parenthesis that belongs to a condition
         // we should ignore this token.
-        $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
         if (isset($tokens[$prev]['parenthesis_owner']) === true) {
             $owner  = $tokens[$prev]['parenthesis_owner'];
             $ignore = [

src/Standards/Squiz/Tests/PHP/NonExecutableCodeUnitTest.1.inc

@@ -297,5 +297,104 @@ class TestAlternativeControlStructures {
 $var_after_class_in_global_space = 1;
 do_something_else();
 
+// These are parse errors, but that's not the concern of the sniff.
+function parseError1() {
+    defined('FOO') or return 'foo';
+    echo 'unreachable';
+}
+
+function parseError2() {
+    defined('FOO') || continue;
+    echo 'unreachable';
+}
+
+// All logical operators are allowed with inline expressions (but this was not correctly handled by the sniff).
+function exitExpressionsWithLogicalOperators() {
+    $condition = false;
+    $condition || exit();
+    $condition or die();
+
+    $condition = true;
+    $condition && die();
+    $condition and exit;
+
+    $condition xor die();
+
+    echo 'still executable as exit, in all of the above cases, is used as part of an expression';
+}
+
+// Inline expressions are allowed in ternaries.
+function exitExpressionsInTernary() {
+    $value = $myValue ? $myValue : exit();
+    $value = $myValue ?: exit();
+    $value = $var == 'foo' ? 'bar' : die( 'world' );
+
+    $value = (!$myValue ) ? exit() : $myValue;
+    $value = $var != 'foo' ? die( 'world' ) : 'bar';
+
+    echo 'still executable';
+}
+
+// Inline expressions are allowed with null coalesce and null coalesce equals.
+function exitExpressionsWithNullCoalesce() {
+    $value = $nullableValue ?? exit();
+    $value ??= die();
+    echo 'still executable';
+}
+
+// Inline expressions are allowed in arrow functions.
+function exitExpressionsInArrowFunction() {
+    $callable = fn() => die();
+    echo 'still executable';
+}
+
+// PHP 8.0+: throw expressions which don't stop execution.
+function nonStoppingThrowExpressions() {
+    $callable = fn() => throw new Exception();
+
+    $value = $myValue ? 'something' : throw new Exception();
+    $value = $myValue ?: throw new Exception();
+    $value = $myValue ? throw new Exception() : 'something';
+
+    $value = $nullableValue ?? throw new Exception();
+    $value ??= throw new Exception();
+
+    $condition && throw new Exception();
+    $condition || throw new Exception();
+    $condition and throw new Exception();
+    $condition or throw new Exception();
+
+    echo 'still executable as throw, in all of the above cases, is used as part of an expression';
+
+    throw new Exception();
+    echo 'non-executable';
+}
+
+// PHP 8.0+: throw expressions which do stop execution.
+function executionStoppingThrowExpressionsA() {
+    $condition xor throw new Exception();
+    echo 'non-executable';
+}
+
+function executionStoppingThrowExpressionsB() {
+    throw $userIsAuthorized ? new ForbiddenException() : new UnauthorizedException();
+    echo 'non-executable';
+}
+
+function executionStoppingThrowExpressionsC() {
+    throw $condition1 && $condition2 ? new Exception1() : new Exception2();
+    echo 'non-executable';
+}
+
+function executionStoppingThrowExpressionsD() {
+    throw $exception ??= new Exception();
+    echo 'non-executable';
+}
+
+function executionStoppingThrowExpressionsE() {
+    throw $maybeNullException ?? new Exception();
+    echo 'non-executable';
+}
+
 // Intentional syntax error.
 return array_map(

src/Standards/Squiz/Tests/PHP/NonExecutableCodeUnitTest.php

@@ -74,6 +74,14 @@ public function getWarningList($testFile='')
                 252 => 1,
                 253 => 1,
                 254 => 2,
+                303 => 1,
+                308 => 1,
+                370 => 1,
+                376 => 1,
+                381 => 1,
+                386 => 1,
+                391 => 1,
+                396 => 1,
             ];
             break;
         case 'NonExecutableCodeUnitTest.2.inc':