Merge pull request #699 from rodrigoprimo/test-coverage-empty-php-sta…

…tement Generic/EmptyPHPStatement: improve code coverage
PHPCSStandards · Nov 26, 2024 · c88c241 · c88c241
2 parents 609ac10 + eb7c3a9
commit c88c241
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 39 deletions.
diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php
@@ -53,10 +53,6 @@ public function process(File $phpcsFile, $stackPtr)
         case 'T_SEMICOLON':
             $prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
 
-            if ($prevNonEmpty === false) {
-                return;
-            }
-
             if ($tokens[$prevNonEmpty]['code'] !== T_SEMICOLON
                 && $tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG
                 && $tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG_WITH_ECHO
@@ -128,9 +124,8 @@ public function process(File $phpcsFile, $stackPtr)
         case 'T_CLOSE_TAG':
             $prevNonEmpty = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
 
-            if ($prevNonEmpty === false
-                || ($tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG
-                && $tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG_WITH_ECHO)
+            if ($tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG
+                && $tokens[$prevNonEmpty]['code'] !== T_OPEN_TAG_WITH_ECHO
             ) {
                 return;
             }
@@ -150,10 +145,6 @@ public function process(File $phpcsFile, $stackPtr)
                 $phpcsFile->fixer->endChangeset();
             }
             break;
-
-        default:
-            // Deliberately left empty.
-            break;
         }//end switch
 
     }//end process()

diff --git a/...odeAnalysis/EmptyPHPStatementUnitTest.inc → ...eAnalysis/EmptyPHPStatementUnitTest.1.inc b/...odeAnalysis/EmptyPHPStatementUnitTest.inc → ...eAnalysis/EmptyPHPStatementUnitTest.1.inc
@@ -19,7 +19,7 @@ function_call();
 
 ?>
 <input name="<?php ; something_else(); ?>" />
-<input name="<?php something_else(); ; ?>" />
+<input name="<?php something_else(); ; ; ?>" />
 
 /*
  * Test empty statement: no code between PHP open and close tag.
@@ -42,7 +42,7 @@ function_call();
 -->
 <?php ; ?>
 
-<input name="<?php ; ?>" /> <!-- Bad. -->
+<input name="<?php ;; ?>" /> <!-- Bad. -->
 
 <!-- Tests with short open echo tag. -->
 <input name="<?= 'some text' ?>" /> <!-- OK. -->
@@ -53,7 +53,7 @@ function_call();
 // Guard against false positives for two consecutive semicolons in a for statement.
 for ( $i = 0; ; $i++ ) {}
 
-// Test for useless semicolons
+// Test for useless semicolons.
 for ( $i = 0; ; $i++ ) {};
 
 if (true) {};
@@ -80,7 +80,7 @@ if ($foo) {
 ;
 }
 
-// Do not remove semicolon after match
+// Do not remove semicolon after match.
 $c = match ($a) {
     1 => true,
 };
diff --git a/...lysis/EmptyPHPStatementUnitTest.inc.fixed → ...sis/EmptyPHPStatementUnitTest.1.inc.fixed b/...lysis/EmptyPHPStatementUnitTest.inc.fixed → ...sis/EmptyPHPStatementUnitTest.1.inc.fixed
@@ -48,7 +48,7 @@ function_call();
 // Guard against false positives for two consecutive semicolons in a for statement.
 for ( $i = 0; ; $i++ ) {}
 
-// Test for useless semicolons
+// Test for useless semicolons.
 for ( $i = 0; ; $i++ ) {}
 
 if (true) {}
@@ -74,7 +74,7 @@ echo $a{0};
 if ($foo) {
 }
 
-// Do not remove semicolon after match
+// Do not remove semicolon after match.
 $c = match ($a) {
     1 => true,
 };
diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc
@@ -0,0 +1,27 @@
+<!-- Tests with short open tag. -->
+
+<input name="<? ; something_else(); ?>" />
+<input name="<? something_else(); ; ?>" />
+
+/*
+ * Test empty statement: no code between PHP open and close tag.
+ */
+<input name="<? something_else() ?>" /> <!-- OK. -->
+<input name="<? something_else(); ?>" /> <!-- OK. -->
+<input name="<? /* comment */ ?>" /> <!-- OK. -->
+
+<input name="<? ?>" /> <!-- Bad. -->
+
+<input name="<?
+
+
+?>" /> <!-- Bad. -->
+
+<!--
+/*
+ * Test detecting & fixing a combination of the two checks.
+ */
+-->
+<? ; ?>
+
+<input name="<? ; ?>" /> <!-- Bad. -->
diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed
@@ -0,0 +1,23 @@
+<!-- Tests with short open tag. -->
+
+<input name="<?something_else(); ?>" />
+<input name="<? something_else(); ?>" />
+
+/*
+ * Test empty statement: no code between PHP open and close tag.
+ */
+<input name="<? something_else() ?>" /> <!-- OK. -->
+<input name="<? something_else(); ?>" /> <!-- OK. -->
+<input name="<? /* comment */ ?>" /> <!-- OK. -->
+
+<input name="" /> <!-- Bad. -->
+
+<input name="" /> <!-- Bad. -->
+
+<!--
+/*
+ * Test detecting & fixing a combination of the two checks.
+ */
+-->
+
+<input name="" /> <!-- Bad. -->
diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php
@@ -20,6 +20,27 @@ final class EmptyPHPStatementUnitTest extends AbstractSniffUnitTest
 {
 
 
+    /**
+     * Get a list of all test files to check.
+     *
+     * @param string $testFileBase The base path that the unit tests files will have.
+     *
+     * @return string[]
+     */
+    protected function getTestFiles($testFileBase)
+    {
+        $testFiles = [$testFileBase.'1.inc'];
+
+        $option = (bool) ini_get('short_open_tag');
+        if ($option === true) {
+            $testFiles[] = $testFileBase.'2.inc';
+        }
+
+        return $testFiles;
+
+    }//end getTestFiles()
+
+
     /**
      * Returns the lines where errors should occur.
      *
@@ -41,31 +62,47 @@ public function getErrorList()
      * 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.
      *
+     * @param string $testFile The name of the file being tested.
+     *
      * @return array<int, int>
      */
-    public function getWarningList()
+    public function getWarningList($testFile='')
     {
-        return [
-            9  => 1,
-            12 => 1,
-            15 => 1,
-            18 => 1,
-            21 => 1,
-            22 => 1,
-            31 => 1,
-            33 => 1,
-            43 => 1,
-            45 => 1,
-            49 => 1,
-            50 => 1,
-            57 => 1,
-            59 => 1,
-            61 => 1,
-            63 => 2,
-            71 => 1,
-            72 => 1,
-            80 => 1,
-        ];
+        switch ($testFile) {
+        case 'EmptyPHPStatementUnitTest.1.inc':
+            return [
+                9  => 1,
+                12 => 1,
+                15 => 1,
+                18 => 1,
+                21 => 1,
+                22 => 2,
+                31 => 1,
+                33 => 1,
+                43 => 1,
+                45 => 2,
+                49 => 1,
+                50 => 1,
+                57 => 1,
+                59 => 1,
+                61 => 1,
+                63 => 2,
+                71 => 1,
+                72 => 1,
+                80 => 1,
+            ];
+        case 'EmptyPHPStatementUnitTest.2.inc':
+            return [
+                3  => 1,
+                4  => 1,
+                13 => 1,
+                15 => 1,
+                25 => 1,
+                27 => 1,
+            ];
+        default:
+            return [];
+        }//end switch
 
     }//end getWarningList()