Merge pull request #160 from PHPCSStandards/universal/disallowalterna…

…tivesyntax-bug-fix-1 Universal/DisallowAlternativeSyntax: bug fix - ignore inline HTML in nested closed scopes
PHPCSStandards · Nov 14, 2022 · 6e4697f · 6e4697f
2 parents 538b90b + e4cfa4f
commit 6e4697f
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 8 deletions.
diff --git a/Universal/Sniffs/ControlStructures/DisallowAlternativeSyntaxSniff.php b/Universal/Sniffs/ControlStructures/DisallowAlternativeSyntaxSniff.php
@@ -37,6 +37,9 @@ final class DisallowAlternativeSyntaxSniff implements Sniff
      * Whether to allow the alternative syntax when it is wrapped around
      * inline HTML, as is often seen in views.
      *
+     * Note: inline HTML within "closed scopes" - like function declarations -,
+     * within the control structure body will not be taken into account.
+     *
      * @since 1.0.0
      *
      * @var bool
@@ -108,19 +111,31 @@ public function process(File $phpcsFile, $stackPtr)
             return;
         }
 
-        $hasInlineHTML = $phpcsFile->findNext(
-            \T_INLINE_HTML,
-            $opener,
-            $closer
-        );
+        $closedScopes         = Collections::closedScopes();
+        $find                 = $closedScopes;
+        $find[\T_INLINE_HTML] = \T_INLINE_HTML;
+        $inlineHTMLPtr        = $opener;
+        $hasInlineHTML        = false;
+
+        do {
+            $inlineHTMLPtr = $phpcsFile->findNext($find, ($inlineHTMLPtr + 1), $closer);
+            if ($tokens[$inlineHTMLPtr]['code'] === \T_INLINE_HTML) {
+                $hasInlineHTML = true;
+                break;
+            }
+
+            if (isset($closedScopes[$tokens[$inlineHTMLPtr]['code']], $tokens[$inlineHTMLPtr]['scope_closer'])) {
+                $inlineHTMLPtr = $tokens[$inlineHTMLPtr]['scope_closer'];
+            }
+        } while ($inlineHTMLPtr !== false && $inlineHTMLPtr < $closer);
 
-        if ($hasInlineHTML !== false) {
+        if ($hasInlineHTML === true) {
             $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'alternative syntax with inline HTML');
         } else {
             $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'alternative syntax');
         }
 
-        if ($hasInlineHTML !== false && $this->allowWithInlineHTML === true) {
+        if ($hasInlineHTML === true && $this->allowWithInlineHTML === true) {
             return;
         }
 
@@ -131,7 +146,7 @@ public function process(File $phpcsFile, $stackPtr)
         $error .= '. Found: %1$s(): ... end%1$s;';
 
         $code = 'Found' . \ucfirst($tokens[$stackPtr]['content']);
-        if ($hasInlineHTML !== false) {
+        if ($hasInlineHTML === true) {
             $code .= 'WithInlineHTML';
         }
 

diff --git a/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.inc b/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.inc
@@ -222,6 +222,45 @@ A is something else
 <?php enddeclare; ?>
 
 <?php
+/*
+ * Tests specifically for the "has inline HTML ?" determination to make sure inline HTML
+ * in nested closed scopes is ignored.
+ */
+if (true):
+    echo 'no inline HTML';
+    ?><?php
+    echo 'between close and open tag';
+endif;
+
+if (!class_exists('Foo')) :
+    class Foo {
+        // Long piece of code
+        function bar() {
+            ?>
+            Inline HTML in closed scopes should be disregarded.
+            <?php
+        }
+    }
+endif;
+
+switch ($foo) :
+    case 1:
+        $closure = function () {
+            ?>
+            Inline HTML in closed scopes should be disregarded.
+            <?php
+        };
+        break;
+endswitch;
+
+declare (ticks = 1):
+    function showTicks() {
+        ?>
+        Inline HTML in closed scopes should be disregarded.
+        <?php
+    }
+enddeclare;
+
 // phpcs:set Universal.ControlStructures.DisallowAlternativeSyntax allowWithInlineHTML false
 
 // Live coding.

diff --git a/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.inc.fixed b/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.inc.fixed
@@ -222,6 +222,45 @@ A is something else
 <?php enddeclare; ?>
 
 <?php
+/*
+ * Tests specifically for the "has inline HTML ?" determination to make sure inline HTML
+ * in nested closed scopes is ignored.
+ */
+if (true){
+    echo 'no inline HTML';
+    ?><?php
+    echo 'between close and open tag';
+}
+
+if (!class_exists('Foo')) {
+    class Foo {
+        // Long piece of code
+        function bar() {
+            ?>
+            Inline HTML in closed scopes should be disregarded.
+            <?php
+        }
+    }
+}
+
+switch ($foo) {
+    case 1:
+        $closure = function () {
+            ?>
+            Inline HTML in closed scopes should be disregarded.
+            <?php
+        };
+        break;
+}
+
+declare (ticks = 1){
+    function showTicks() {
+        ?>
+        Inline HTML in closed scopes should be disregarded.
+        <?php
+    }
+}
+
 // phpcs:set Universal.ControlStructures.DisallowAlternativeSyntax allowWithInlineHTML false
 
 // Live coding.

diff --git a/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.php b/Universal/Tests/ControlStructures/DisallowAlternativeSyntaxUnitTest.php
@@ -54,6 +54,10 @@ public function getErrorList()
             172 => 1,
             176 => 1,
             185 => 1,
+            229 => 1,
+            235 => 1,
+            246 => 1,
+            256 => 1,
         ];
     }