From ab4bae20b4c45fcd6be4c0980ae54f67f1d96cf6 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 25 Apr 2024 14:01:48 -0300 Subject: [PATCH 1/4] Generic/EmptyPHPStatement: remove some unreachable code * Removed two unreachable conditions. `$prevNonEmpty` will never be `false` as there will always be at least a PHP open tag token before the semicolon token. * Removed unreachable default case `$tokens[$stackPtr]['type']` will always be either `T_SEMICOLON` or `T_CLOSE_TAG` as those are the two tokens that this sniff listens for. So, the default case will never be reached. --- .../Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php index 4ecf630378..eb7c50ce85 100644 --- 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() From 498ecdaa1e066eb14de82631f65a6be837f1e3c6 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 20 Nov 2024 17:20:21 -0300 Subject: [PATCH 2/4] Generic/EmptyPHPStatement: rename test case file Doing this to be able to create another test file to test the sniff with the short open tag. --- ...st.inc => EmptyPHPStatementUnitTest.1.inc} | 0 ... => EmptyPHPStatementUnitTest.1.inc.fixed} | 0 .../EmptyPHPStatementUnitTest.php | 51 +++++++++++-------- 3 files changed, 29 insertions(+), 22 deletions(-) rename src/Standards/Generic/Tests/CodeAnalysis/{EmptyPHPStatementUnitTest.inc => EmptyPHPStatementUnitTest.1.inc} (100%) rename src/Standards/Generic/Tests/CodeAnalysis/{EmptyPHPStatementUnitTest.inc.fixed => EmptyPHPStatementUnitTest.1.inc.fixed} (100%) diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.inc b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.inc rename to src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.inc.fixed b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed similarity index 100% rename from src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.inc.fixed rename to src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php index 1b31b5f288..66db633e52 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php @@ -41,31 +41,38 @@ 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 */ - 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 => 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, + ]; + default: + return []; + }//end switch }//end getWarningList() From 6105bf7d66d65b34a4fb6ba8bd0df0a03d9304a1 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 20 Nov 2024 17:55:05 -0300 Subject: [PATCH 3/4] Generic/EmptyPHPStatement: add tests for the short open tag --- .../EmptyPHPStatementUnitTest.2.inc | 27 +++++++++++++++++ .../EmptyPHPStatementUnitTest.2.inc.fixed | 23 ++++++++++++++ .../EmptyPHPStatementUnitTest.php | 30 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc new file mode 100644 index 0000000000..a9f895d71b --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc @@ -0,0 +1,27 @@ + + + + + +/* + * Test empty statement: no code between PHP open and close tag. + */ + + + + + + + + + + + + diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed new file mode 100644 index 0000000000..fd27d0a25c --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed @@ -0,0 +1,23 @@ + + + + + +/* + * Test empty statement: no code between PHP open and close tag. + */ + + + + + + + + + + + diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php index 66db633e52..1e59c0a398 100644 --- 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. * @@ -70,6 +91,15 @@ public function getWarningList($testFile='') 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 From eb7c3a9a355ba9c010b9cc46e6fbf7ccaa9b0aea Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 25 Nov 2024 15:50:51 -0300 Subject: [PATCH 4/4] Generic/EmptyPHPStatement: minor tweaks to test case file * Updated tests to have more examples with more than one superfluous semicolon. * Minor test comment punctuation tweaks. --- .../Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc | 8 ++++---- .../CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed | 4 ++-- .../Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc index 080dda9382..83e071bf42 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc @@ -19,7 +19,7 @@ function_call(); ?> - + /* * Test empty statement: no code between PHP open and close tag. @@ -42,7 +42,7 @@ function_call(); --> - + @@ -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/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed index 7f16742288..6bf8b21261 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.1.inc.fixed +++ b/src/Standards/Generic/Tests/CodeAnalysis/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.php b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php index 1e59c0a398..77542acbaa 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php @@ -76,11 +76,11 @@ public function getWarningList($testFile='') 15 => 1, 18 => 1, 21 => 1, - 22 => 1, + 22 => 2, 31 => 1, 33 => 1, 43 => 1, - 45 => 1, + 45 => 2, 49 => 1, 50 => 1, 57 => 1,