From e3dcefeb8037ffe90e637c9fdddf32b4c4b917ac Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 4 Jan 2024 16:32:14 -0300 Subject: [PATCH 1/4] Generic/ForLoopWithTestFunctionCall: rename test case file Doing this to create more test case files in subsequent commits. --- ...> ForLoopWithTestFunctionCallUnitTest.1.inc} | 0 .../ForLoopWithTestFunctionCallUnitTest.php | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) rename src/Standards/Generic/Tests/CodeAnalysis/{ForLoopWithTestFunctionCallUnitTest.inc => ForLoopWithTestFunctionCallUnitTest.1.inc} (100%) diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.inc rename to src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.php index a82dcfeb27..06c99c682f 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.php +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.php @@ -41,14 +41,21 @@ 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 test file being tested. + * * @return array */ - public function getWarningList() + public function getWarningList($testFile='') { - return [ - 4 => 1, - 13 => 1, - ]; + switch ($testFile) { + case 'ForLoopWithTestFunctionCallUnitTest.1.inc': + return [ + 4 => 1, + 13 => 1, + ]; + default: + return []; + } }//end getWarningList() From 6e61c8520f648dadbe901d658dcdd156b6a8e46e Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 4 Jan 2024 16:46:55 -0300 Subject: [PATCH 2/4] Generic/ForLoopWithTestFunctionCall: improve test coverage This commit adds more test cases related to the Generic/ForLoopWithTestFunctionCall sniff including test cases for the alternative for syntax, more types of function calls in the test part of the for loop and a test case to check defensive code when the for loop doesn't have a opening parenthesis. --- .../ForLoopWithTestFunctionCallUnitTest.1.inc | 82 ++++++++++++++++++- .../ForLoopWithTestFunctionCallUnitTest.2.inc | 5 ++ .../ForLoopWithTestFunctionCallUnitTest.php | 14 +++- 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.2.inc diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc index ef51b5a1ed..31f1a64199 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.1.inc @@ -12,4 +12,84 @@ for ($i = 0, $c = sizeof($a); $i < $c; ++$i) { $it = new ArrayIterator($a); for ($it->rewind(); $it->valid(); $it->next()) { echo $it->current(); -} \ No newline at end of file +} + +for ($i = 0; MyClass::staticMethod($value); $i++) { + echo $i; +} + +for ($i = 0; $countFunction($value); $i++) { + echo $i; +} + +$a = array(1, 2, 3, 4); +for ($i = 0; $i < count($a); $i++): + $a[$i] *= $i; +endfor; + +for ($i = 0, $c = sizeof($a); $i < $c; ++$i): + $a[$i] *= $i; +endfor; + +$it = new ArrayIterator($a); +for ($it->rewind(); $it->valid(); $it->next()): + echo $it->current(); +endfor; + +for ($i = 0; MyClass::staticMethod($value); $i++) : + echo $i; +endfor; + +for ($i = 0; $countFunction($value); $i++): + echo $i; +endfor; + +for ($i = 0; (new MyClass)->method(); $i++) { +} + +for (; $i < 10; ++$i) {} + +for (; count($a); ++$i) {} + +for ($i = 0;; ++$i) {} + +for ($i = 0; $i < 10;) {} + +for ($i = 0; count($a);) {} + +for (;; $i++) {} + +for ($i = 0;;) {} + +for (;;) {} + +for ($i = 0; (new MyClass)->method(); $i++): +endfor; + +for (; $i < 10; ++$i) : +endfor; + +for (; count($a); ++$i) : +endfor; + +for ($i = 0;; ++$i) : +endfor; + +for ($i = 0; $i < 10;) : +endfor; + +for ($i = 0; count($a);) : +endfor; + +for (;; $i++) : +endfor; + +for ($i = 0;;) : +endfor; + +for (;;) : +endfor; + +for ($i = 0; $i < 10; $i = increment($i)) {} + +for ($i = initialValue(); $i < 10; $i = increment($i)) {} diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.2.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.2.inc new file mode 100644 index 0000000000..54ef0a522b --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.2.inc @@ -0,0 +1,5 @@ + 1, 13 => 1, + 17 => 1, + 21 => 1, + 26 => 1, + 35 => 1, + 39 => 1, + 43 => 1, + 47 => 1, + 52 => 1, + 58 => 1, + 66 => 1, + 72 => 1, + 81 => 1, ]; default: return []; - } + }//end switch }//end getWarningList() From 837e602af87641c2ba7685b7fea942bfad57bb80 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 5 Jan 2024 10:51:14 -0300 Subject: [PATCH 3/4] Generic/ForLoopWithTestFunctionCall: fix E_DEPRECATED error This commit fixes an issue in the sniff that could result in the following E_DEPRECATED error when running PHP 8.3: ``` Decrement on type null has no effect, this will change in the next major version of PHP src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCall.php:69 ``` This sniff relies on finding the position of the open and closing parentheses for a given `for` loop. However, the problem was that there was no defensive code for cases when the closing parenthesis is missing. The sniff would still work when running PHP >= 8.2, but on PHP 8.3 it would throw the deprecated message above. This would happen because since there is no closing parenthesis `$end` is set to null, and $next <= $end always evaluates to false (https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/84acf4e56f110db8e75cb9a575c5727df637643c/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php#L73). The issue was fixed by bailing early if the closing parenthesis is missing. A test with a `for` loop without the closing parenthesis was added. --- .../CodeAnalysis/ForLoopWithTestFunctionCallSniff.php | 2 +- .../CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.3.inc | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.3.inc diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php index 45b4f7306f..84ed101745 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php @@ -61,7 +61,7 @@ public function process(File $phpcsFile, $stackPtr) $token = $tokens[$stackPtr]; // Skip invalid statement. - if (isset($token['parenthesis_opener']) === false) { + if (isset($token['parenthesis_opener'], $token['parenthesis_closer']) === false) { return; } diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.3.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.3.inc new file mode 100644 index 0000000000..8bc6904e3d --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopWithTestFunctionCallUnitTest.3.inc @@ -0,0 +1,6 @@ + Date: Wed, 17 Jan 2024 20:18:53 +0100 Subject: [PATCH 4/4] Generic/ForLoopWithTestFunctionCall: minor doc fix --- .../Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php index 84ed101745..0374a8f757 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopWithTestFunctionCallSniff.php @@ -84,7 +84,7 @@ public function process(File $phpcsFile, $stackPtr) continue; } - // Find next non empty token, if it is a open curly brace we have a + // Find next non empty token, if it is a open parenthesis we have a // function call. $index = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true);