From e50784d443b4966109319b495b91fb2fa4f82572 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 3 Jan 2024 11:39:29 -0300 Subject: [PATCH 1/3] Generic/ForLoopShouldBeWhileLoop: rename test case file Doing this to be able to create a test with a syntax error on a separate file. --- ...c => ForLoopShouldBeWhileLoopUnitTest.1.inc} | 0 .../ForLoopShouldBeWhileLoopUnitTest.php | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) rename src/Standards/Generic/Tests/CodeAnalysis/{ForLoopShouldBeWhileLoopUnitTest.inc => ForLoopShouldBeWhileLoopUnitTest.1.inc} (100%) diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.inc rename to src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.php index 2fcfffb173..662f9f2a68 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.php +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.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 [ - 6 => 1, - 10 => 1, - ]; + switch ($testFile) { + case 'ForLoopShouldBeWhileLoopUnitTest.1.inc': + return [ + 6 => 1, + 10 => 1, + ]; + default: + return []; + } }//end getWarningList() From 9c4b38c76d48dbffbf53ac580767e90923296cfc Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 3 Jan 2024 11:49:48 -0300 Subject: [PATCH 2/3] Generic/ForLoopShouldBeWhileLoop: improve test coverage This commit adds the following groups of tests: - A few tests that do not trigger the sniff but should help to protect against regressions in the future. Including tests using the for loop alternative syntax. - A test to exercise code in the sniff to bail early if the `for` keyword is found without a opening parenthesis. This part was not covered before. --- .../ForLoopShouldBeWhileLoopUnitTest.1.inc | 26 ++++++++++++++++++- .../ForLoopShouldBeWhileLoopUnitTest.2.inc | 4 +++ .../ForLoopShouldBeWhileLoopUnitTest.php | 1 + 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.2.inc diff --git a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc index cc69c67963..3b47942a9a 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.1.inc @@ -10,4 +10,28 @@ for (; $it->valid();) { for (;(($it1->valid() && $foo) || (!$it2->value && ($bar || false)));/*Could be ignored*/) { $it1->next(); $it2->next(); -} \ No newline at end of file +} + +for ($i = 0, $j = 10; $i < $j; $i++, $j--) { + echo "i: $i, j: $j\n"; +} + +for (;;) { + if ($i >= 10) { + break; + } + echo $i++; +} + +for ($i = 0; $i < 10; $i++): ?> +

+ 1, 10 => 1, + 34 => 1, ]; default: return []; From 1ebc09edd54c3193ba3842781d271d06f56c6131 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 4 Jan 2024 11:32:56 -0300 Subject: [PATCH 3/3] Generic/ForLoopShouldBeWhileLoop: 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/ForLoopShouldBeWhileLoopSniff.php:65 ``` 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/ForLoopShouldBeWhileLoopSniff.php#L74). 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. --- .../Sniffs/CodeAnalysis/ForLoopShouldBeWhileLoopSniff.php | 2 +- .../CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.3.inc | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.3.inc diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopShouldBeWhileLoopSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopShouldBeWhileLoopSniff.php index 286ce62704..726b32c552 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopShouldBeWhileLoopSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/ForLoopShouldBeWhileLoopSniff.php @@ -57,7 +57,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/ForLoopShouldBeWhileLoopUnitTest.3.inc b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.3.inc new file mode 100644 index 0000000000..5502b61438 --- /dev/null +++ b/src/Standards/Generic/Tests/CodeAnalysis/ForLoopShouldBeWhileLoopUnitTest.3.inc @@ -0,0 +1,6 @@ +