Merge pull request #647 from PHPCSStandards/feature/529-tokenizer-yie…

…ld-from-tokenization-review PHP 8.3 | Tokenizer/PHP: support "yield from" with comments
PHPCSStandards · Nov 2, 2024 · c2b6235 · c2b6235
2 parents db1ebe1 + fc6d98a
commit c2b6235
Show file tree

Hide file tree

Showing 29 changed files with 682 additions and 84 deletions.
diff --git a/src/Standards/Generic/Sniffs/WhiteSpace/DisallowTabIndentSniff.php b/src/Standards/Generic/Sniffs/WhiteSpace/DisallowTabIndentSniff.php
@@ -78,6 +78,7 @@ public function process(File $phpcsFile, $stackPtr)
             T_COMMENT                => true,
             T_END_HEREDOC            => true,
             T_END_NOWDOC             => true,
+            T_YIELD_FROM             => true,
         ];
 
         for ($i = 0; $i < $phpcsFile->numTokens; $i++) {

diff --git a/src/Standards/Generic/Sniffs/WhiteSpace/LanguageConstructSpacingSniff.php b/src/Standards/Generic/Sniffs/WhiteSpace/LanguageConstructSpacingSniff.php
@@ -80,42 +80,57 @@ public function process(File $phpcsFile, $stackPtr)
         if ($tokens[$stackPtr]['code'] === T_YIELD_FROM
             && strtolower($content) !== 'yield from'
         ) {
-            if ($tokens[($stackPtr - 1)]['code'] === T_YIELD_FROM) {
-                // A multi-line statement that has already been processed.
-                return;
-            }
+            $found        = $content;
+            $hasComment   = false;
+            $yieldFromEnd = $stackPtr;
+
+            // Handle potentially multi-line/multi-token "yield from" expressions.
+            if (preg_match('`yield\s+from`i', $content) !== 1) {
+                for ($i = ($stackPtr + 1); $i < $phpcsFile->numTokens; $i++) {
+                    if (isset(Tokens::$emptyTokens[$tokens[$i]['code']]) === false
+                        && $tokens[$i]['code'] !== T_YIELD_FROM
+                    ) {
+                        break;
+                    }
+
+                    if (isset(Tokens::$commentTokens[$tokens[$i]['code']]) === true) {
+                        $hasComment = true;
+                    }
 
-            $found = $content;
-            if ($tokens[($stackPtr + 1)]['code'] === T_YIELD_FROM) {
-                // This yield from statement is split over multiple lines.
-                $i = ($stackPtr + 1);
-                do {
                     $found .= $tokens[$i]['content'];
-                    $i++;
-                } while ($tokens[$i]['code'] === T_YIELD_FROM);
-            }
+
+                    if ($tokens[$i]['code'] === T_YIELD_FROM
+                        && strtolower(trim($tokens[$i]['content'])) === 'from'
+                    ) {
+                        break;
+                    }
+                }
+
+                $yieldFromEnd = $i;
+            }//end if
 
             $error = 'Language constructs must be followed by a single space; expected 1 space between YIELD FROM found "%s"';
             $data  = [Common::prepareForOutput($found)];
-            $fix   = $phpcsFile->addFixableError($error, $stackPtr, 'IncorrectYieldFrom', $data);
-            if ($fix === true) {
-                preg_match('/yield/i', $found, $yield);
-                preg_match('/from/i', $found, $from);
-                $phpcsFile->fixer->beginChangeset();
-                $phpcsFile->fixer->replaceToken($stackPtr, $yield[0].' '.$from[0]);
-
-                if ($tokens[($stackPtr + 1)]['code'] === T_YIELD_FROM) {
-                    $i = ($stackPtr + 1);
-                    do {
+
+            if ($hasComment === true) {
+                $phpcsFile->addError($error, $stackPtr, 'IncorrectYieldFromWithComment', $data);
+            } else {
+                $fix = $phpcsFile->addFixableError($error, $stackPtr, 'IncorrectYieldFrom', $data);
+                if ($fix === true) {
+                    preg_match('/yield/i', $found, $yield);
+                    preg_match('/from/i', $found, $from);
+                    $phpcsFile->fixer->beginChangeset();
+                    $phpcsFile->fixer->replaceToken($stackPtr, $yield[0].' '.$from[0]);
+
+                    for ($i = ($stackPtr + 1); $i <= $yieldFromEnd; $i++) {
                         $phpcsFile->fixer->replaceToken($i, '');
-                        $i++;
-                    } while ($tokens[$i]['code'] === T_YIELD_FROM);
-                }
+                    }
 
-                $phpcsFile->fixer->endChangeset();
-            }
+                    $phpcsFile->fixer->endChangeset();
+                }
+            }//end if
 
-            return;
+            return ($yieldFromEnd + 1);
         }//end if
 
         if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {

diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.inc b/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.inc
@@ -53,5 +53,12 @@ if (ISSET($a) && !Empty($a)) { UnSeT($a); }
 eval('foo');
 eVaL('foo');
 
+$c = function() {
+    Yield /*comment*/ From fun();
+    YIELD
+    /*comment*/
+    FROM fun();
+}
+
 __HALT_COMPILER(); // An exception due to phar support.
 function
diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.inc.fixed b/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.inc.fixed
@@ -53,5 +53,12 @@ if (isset($a) && !empty($a)) { unset($a); }
 eval('foo');
 eval('foo');
 
+$c = function() {
+    yield /*comment*/ from fun();
+    yield
+    /*comment*/
+    from fun();
+}
+
 __HALT_COMPILER(); // An exception due to phar support.
 function
diff --git a/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.php b/src/Standards/Generic/Tests/PHP/LowerCaseKeywordUnitTest.php
@@ -52,6 +52,9 @@ public function getErrorList()
             48 => 1,
             52 => 3,
             54 => 1,
+            57 => 2,
+            58 => 1,
+            60 => 1,
         ];
 
     }//end getErrorList()

diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.1.inc
@@ -116,3 +116,10 @@ $x = 1;
 
      Another line.
      */
+
+// A `yield from` can be multiline and may contain spaces in the indentation whitespace between the keywords.
+function myGenerator() {
+	yield
+    from
+	gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.1.inc.fixed
@@ -116,3 +116,10 @@ $x = 1;
 
 	 Another line.
 	 */
+
+// A `yield from` can be multiline and may contain spaces in the indentation whitespace between the keywords.
+function myGenerator() {
+	yield
+	from
+	gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.2.inc b/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.2.inc
@@ -116,3 +116,10 @@ $x = 1;
 
      Another line.
      */
+
+// A `yield from` can be multiline and may contain spaces in the indentation whitespace between the keywords.
+function myGenerator() {
+	yield
+    from
+	gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.2.inc.fixed
@@ -116,3 +116,10 @@ $x = 1;
 
 	 Another line.
 	 */
+
+// A `yield from` can be multiline and may contain spaces in the indentation whitespace between the keywords.
+function myGenerator() {
+	yield
+	from
+	gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.php
@@ -89,6 +89,7 @@ public function getErrorList($testFile='')
                 115 => 1,
                 117 => 1,
                 118 => 1,
+                123 => 1,
             ];
 
         case 'DisallowSpaceIndentUnitTest.3.inc':

diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.1.inc
@@ -91,3 +91,12 @@ $var = "$hello	$there";
 
 	 Another line.
 	 */
+
+// A `yield from` can be single-line and multiline and may contain a tab in the whitespace between the keywords.
+function myGenerator() {
+    yield		from gen1();
+
+    yield
+	from
+    gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.1.inc.fixed
@@ -91,3 +91,12 @@ $var = "$hello	$there";
 
      Another line.
      */
+
+// A `yield from` can be single-line and multiline and may contain a tab in the whitespace between the keywords.
+function myGenerator() {
+    yield       from gen1();
+
+    yield
+    from
+    gen2();
+}
diff --git a/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/DisallowTabIndentUnitTest.php
@@ -50,43 +50,45 @@ public function getErrorList($testFile='')
         switch ($testFile) {
         case 'DisallowTabIndentUnitTest.1.inc':
             return [
-                5  => 2,
-                9  => 1,
-                15 => 1,
-                20 => 2,
-                21 => 1,
-                22 => 2,
-                23 => 1,
-                24 => 2,
-                31 => 1,
-                32 => 2,
-                33 => 2,
-                41 => 1,
-                42 => 1,
-                43 => 1,
-                44 => 1,
-                45 => 1,
-                46 => 1,
-                47 => 1,
-                48 => 1,
-                54 => 1,
-                55 => 1,
-                56 => 1,
-                57 => 1,
-                58 => 1,
-                59 => 1,
-                79 => 1,
-                80 => 1,
-                81 => 1,
-                82 => 1,
-                83 => 1,
-                85 => 1,
-                86 => 1,
-                87 => 1,
-                89 => 1,
-                90 => 1,
-                92 => 1,
-                93 => 1,
+                5   => 2,
+                9   => 1,
+                15  => 1,
+                20  => 2,
+                21  => 1,
+                22  => 2,
+                23  => 1,
+                24  => 2,
+                31  => 1,
+                32  => 2,
+                33  => 2,
+                41  => 1,
+                42  => 1,
+                43  => 1,
+                44  => 1,
+                45  => 1,
+                46  => 1,
+                47  => 1,
+                48  => 1,
+                54  => 1,
+                55  => 1,
+                56  => 1,
+                57  => 1,
+                58  => 1,
+                59  => 1,
+                79  => 1,
+                80  => 1,
+                81  => 1,
+                82  => 1,
+                83  => 1,
+                85  => 1,
+                86  => 1,
+                87  => 1,
+                89  => 1,
+                90  => 1,
+                92  => 1,
+                93  => 1,
+                97  => 1,
+                100 => 1,
             ];
 
         case 'DisallowTabIndentUnitTest.2.inc':

diff --git a/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.1.inc
@@ -89,3 +89,12 @@ $newLine;
 // The following line must have a single space at the end (after return)
 return 
 $spaceAndNewLine;
+
+// Related to issue #529. These should not be auto-fixed as we don't know what to do with the comment.
+yield    /*comment*/    from $test();
+yield
+    # comment
+    from $test();
+yield
+    // phpcs:ignore Stnd.Category.SniffName
+    from $test();
diff --git a/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.1.inc.fixed
@@ -83,3 +83,12 @@ return $newLine;
 
 // The following line must have a single space at the end (after return)
 return $spaceAndNewLine;
+
+// Related to issue #529. These should not be auto-fixed as we don't know what to do with the comment.
+yield    /*comment*/    from $test();
+yield
+    # comment
+    from $test();
+yield
+    // phpcs:ignore Stnd.Category.SniffName
+    from $test();
diff --git a/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/LanguageConstructSpacingUnitTest.php
@@ -71,6 +71,9 @@ public function getErrorList($testFile='')
                 85 => 1,
                 86 => 1,
                 90 => 1,
+                94 => 1,
+                95 => 1,
+                98 => 1,
             ];
 
         default:

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc
@@ -1614,6 +1614,12 @@ match (true) {
     ]
 };
 
+// Issue squizlabs/PHP_CodeSniffer#3808
+function test() {
+    yield
+        from [ 3, 4 ];
+}
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed
@@ -1614,6 +1614,12 @@ match (true) {
     ]
 };
 
+// Issue squizlabs/PHP_CodeSniffer#3808
+function test() {
+    yield
+        from [ 3, 4 ];
+}
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc
@@ -1614,6 +1614,12 @@ match (true) {
 	]
 };
 
+// Issue squizlabs/PHP_CodeSniffer#3808
+function test() {
+	yield
+		from [ 3, 4 ];
+}
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed
@@ -1614,6 +1614,12 @@ match (true) {
 	]
 };
 
+// Issue squizlabs/PHP_CodeSniffer#3808
+function test() {
+	yield
+		from [ 3, 4 ];
+}
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.3.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.3.inc
@@ -1,6 +1,6 @@
-phpcs:set Generic.WhiteSpace.ScopeIndent tabIndent false
-phpcs:set Generic.WhiteSpace.ScopeIndent exact true
 <?php
+// phpcs:set Generic.WhiteSpace.ScopeIndent tabIndent false
+// phpcs:set Generic.WhiteSpace.ScopeIndent exact true
 function test()
 {
      echo 'test';
@@ -26,3 +26,9 @@ if ($foo) {
 $this->foo()
     ->bar()
     ->baz();
+
+// Issue squizlabs/PHP_CodeSniffer#3808
+function test() {
+    yield
+        from [ 3, 4 ];
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -52,6 +52,9 @@ public function getErrorList() @@
 => 1,
 => 3,
 => 1,
+=> 2,
+=> 1,
+=> 1,
             ];
         }//end getErrorList()
@@ Expand Down @@