From 70f8f2cdc619910b44079fb720fe02aa6ca4c702 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 21 Nov 2024 08:55:34 -0300 Subject: [PATCH 1/5] Generic/OpeningFunctionBraceKernighanRichie: remove repeated call to findPrevious() This commit removes a repeated call to findPrevious() to get the pointer to the end of the function declaration. The same `$prev` variable that was defined a few lines above can be reused and there is no need to define it again calling `findPrevious()` with exactly the same parameters. --- .../Functions/OpeningFunctionBraceKernighanRitchieSniff.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php index ca259854e7..3b6c8227b5 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php @@ -99,7 +99,6 @@ public function process(File $phpcsFile, $stackPtr) $error = 'Opening brace should be on the same line as the declaration'; $fix = $phpcsFile->addFixableError($error, $openingBrace, 'BraceOnNewLine'); if ($fix === true) { - $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($openingBrace - 1), $closeBracket, true); $phpcsFile->fixer->beginChangeset(); $phpcsFile->fixer->addContent($prev, ' {'); $phpcsFile->fixer->replaceToken($openingBrace, ''); From a375eeace776843826d768c872ab2610bc1472d4 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 21 Nov 2024 15:31:34 -0300 Subject: [PATCH 2/5] Generic/OpeningFunctionBraceKernighanRichie: improve handling of tabs in a fixer This commit makes a tiny performance improvement to the sniff when handling fixing code with tabs. Before this change, the sniff would need two passes of the fixer for code with one tab before the opening brace. First, it would add a space between the tab and the opening brace. Then, on a second pass, it would replace the tab and the space with just one space. Now, it replaces the tab with the space directly without the need for a second pass. --- .../Functions/OpeningFunctionBraceKernighanRitchieSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php index 3b6c8227b5..f8fa07582d 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php @@ -167,7 +167,7 @@ public function process(File $phpcsFile, $stackPtr) $data = [$length]; $fix = $phpcsFile->addFixableError($error, $openingBrace, 'SpaceBeforeBrace', $data); if ($fix === true) { - if ($length === 0 || $length === '\t') { + if ($length === 0) { $phpcsFile->fixer->addContentBefore($openingBrace, ' '); } else { $phpcsFile->fixer->replaceToken(($openingBrace - 1), ' '); From 111d8515795a703b26fce6d1f58fa12e281481f0 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 21 Nov 2024 07:50:56 -0300 Subject: [PATCH 3/5] Generic/OpeningFunctionBraceKernighanRichie: rename test case file Doing this to be able to create tests with syntax errors on separate files. --- ...nctionBraceKernighanRitchieUnitTest.1.inc} | 0 ...BraceKernighanRitchieUnitTest.1.inc.fixed} | 0 ...gFunctionBraceKernighanRitchieUnitTest.php | 69 ++++++++++--------- 3 files changed, 38 insertions(+), 31 deletions(-) rename src/Standards/Generic/Tests/Functions/{OpeningFunctionBraceKernighanRitchieUnitTest.inc => OpeningFunctionBraceKernighanRitchieUnitTest.1.inc} (100%) rename src/Standards/Generic/Tests/Functions/{OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed => OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed} (100%) diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc similarity index 100% rename from src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc rename to src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed similarity index 100% rename from src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed rename to src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php index f98430768f..30e74ce105 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php @@ -26,40 +26,47 @@ final class OpeningFunctionBraceKernighanRitchieUnitTest extends AbstractSniffUn * The key of the array should represent the line number and the value * should represent the number of errors that should occur on that line. * + * @param string $testFile The name of the test file to process. + * * @return array */ - public function getErrorList() + public function getErrorList($testFile='') { - return [ - 9 => 1, - 13 => 1, - 17 => 1, - 29 => 1, - 33 => 1, - 37 => 1, - 53 => 1, - 58 => 1, - 63 => 1, - 77 => 1, - 82 => 1, - 87 => 1, - 104 => 1, - 119 => 1, - 123 => 1, - 127 => 1, - 132 => 1, - 137 => 1, - 142 => 1, - 157 => 1, - 162 => 1, - 171 => 1, - 181 => 1, - 191 => 1, - 197 => 1, - 203 => 1, - 213 => 1, - 214 => 1, - ]; + switch ($testFile) { + case 'OpeningFunctionBraceKernighanRitchieUnitTest.1.inc': + return [ + 9 => 1, + 13 => 1, + 17 => 1, + 29 => 1, + 33 => 1, + 37 => 1, + 53 => 1, + 58 => 1, + 63 => 1, + 77 => 1, + 82 => 1, + 87 => 1, + 104 => 1, + 119 => 1, + 123 => 1, + 127 => 1, + 132 => 1, + 137 => 1, + 142 => 1, + 157 => 1, + 162 => 1, + 171 => 1, + 181 => 1, + 191 => 1, + 197 => 1, + 203 => 1, + 213 => 1, + 214 => 1, + ]; + default: + return []; + }//end switch }//end getErrorList() From 195346f36e4ad32b00584c63576f93b7053802ca Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 21 Nov 2024 08:51:33 -0300 Subject: [PATCH 4/5] Generic/OpeningFunctionBraceKernighanRichie: improve test coverage Includes removing unnecessary trailing spaces from empty lines in the test file. --- ...unctionBraceKernighanRitchieUnitTest.1.inc | 30 +++++++++++++---- ...nBraceKernighanRitchieUnitTest.1.inc.fixed | 32 +++++++++++++++---- ...unctionBraceKernighanRitchieUnitTest.2.inc | 11 +++++++ ...nBraceKernighanRitchieUnitTest.2.inc.fixed | 11 +++++++ ...unctionBraceKernighanRitchieUnitTest.3.inc | 7 ++++ ...gFunctionBraceKernighanRitchieUnitTest.php | 25 +++++++++++++++ 6 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc create mode 100644 src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc.fixed create mode 100644 src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.3.inc diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc index 6c937a823b..2e76d62155 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc @@ -23,16 +23,16 @@ class myClass // Good. function myFunction() { } - + // Brace should be on same line. function myFunction() { } - + // Too many spaces. function myFunction() { } - + // Uses tab. function myFunction() { } @@ -70,18 +70,18 @@ class myClass function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Brace should be on same line. function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Too many spaces. function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Uses tab. function myFunction($variable1, $variable2, $variable3, $variable4) { @@ -212,3 +212,21 @@ function myFunction($a, $lot, $of, $params) function myFunction() {} function myFunction() {} // Too many spaces with an empty function. function myFunction() {} // Too many spaces (tab) with an empty function. + +// phpcs:set Generic.Functions.OpeningFunctionBraceKernighanRitchie checkFunctions 0 +function shouldBeIgnored() +{} +// phpcs:set Generic.Functions.OpeningFunctionBraceKernighanRitchie checkFunctions 1 + +function dnfReturnType(): (Response&SuccessResponse)|AnotherResponse|string +{} + +function commentAfterOpeningBrace() { // Some comment. +} + +function variableAssignmentAfterOpeningBrace() { $a = 1; +} + +abstract class MyClass { + abstract public function abstractMethod(); +} diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed index bfb2283844..14e62eae90 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.1.inc.fixed @@ -22,15 +22,15 @@ class myClass // Good. function myFunction() { } - + // Brace should be on same line. function myFunction() { } - + // Too many spaces. function myFunction() { } - + // Uses tab. function myFunction() { } @@ -67,17 +67,17 @@ class myClass function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Brace should be on same line. function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Too many spaces. function myFunction($variable1, $variable2, $variable3, $variable4) { } - + // Uses tab. function myFunction($variable1, $variable2, $variable3, $variable4) { @@ -200,3 +200,23 @@ function myFunction($a, $lot, $of, $params) function myFunction() {} function myFunction() {} // Too many spaces with an empty function. function myFunction() {} // Too many spaces (tab) with an empty function. + +// phpcs:set Generic.Functions.OpeningFunctionBraceKernighanRitchie checkFunctions 0 +function shouldBeIgnored() +{} +// phpcs:set Generic.Functions.OpeningFunctionBraceKernighanRitchie checkFunctions 1 + +function dnfReturnType(): (Response&SuccessResponse)|AnotherResponse|string { +} + +function commentAfterOpeningBrace() { + // Some comment. +} + +function variableAssignmentAfterOpeningBrace() { + $a = 1; +} + +abstract class MyClass { + abstract public function abstractMethod(); +} diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc new file mode 100644 index 0000000000..a145bb9efe --- /dev/null +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.2.inc @@ -0,0 +1,11 @@ +tabWidth = 4; + } + + }//end setCliValues() + + /** * Returns the lines where errors should occur. * @@ -63,6 +80,14 @@ public function getErrorList($testFile='') 203 => 1, 213 => 1, 214 => 1, + 222 => 1, + 224 => 1, + 227 => 1, + ]; + case 'OpeningFunctionBraceKernighanRitchieUnitTest.2.inc': + return [ + 6 => 1, + 10 => 1, ]; default: return []; From 286cd814b8e105a472d64d68a9aba524862809a7 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 26 Nov 2024 16:54:28 -0300 Subject: [PATCH 5/5] Generic/OpeningFunctionBraceKernighanRitchie: simplify logic to find end of the function signature When finding the end of the function signature, the sniff had an unnecessary block of code to change the `$end` parameter passed to `findPrevious()` when handling a closure with a `use` statement. Since we are looking for the first non-empty token before the opening curly brace, it is not necessary to change the value of the `$end` parameter. Actually, we don't even need to limit the search and we can pass `null` (the default value). The returned first non-empty token will be the same anyway regardless if `$end` is the closing parenthesis before `use`, after `use` or `null`. --- .../OpeningFunctionBraceKernighanRitchieSniff.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php index f8fa07582d..1e2df37d86 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php @@ -72,17 +72,9 @@ public function process(File $phpcsFile, $stackPtr) } $openingBrace = $tokens[$stackPtr]['scope_opener']; - $closeBracket = $tokens[$stackPtr]['parenthesis_closer']; - if ($tokens[$stackPtr]['code'] === T_CLOSURE) { - $use = $phpcsFile->findNext(T_USE, ($closeBracket + 1), $tokens[$stackPtr]['scope_opener']); - if ($use !== false) { - $openBracket = $phpcsFile->findNext(T_OPEN_PARENTHESIS, ($use + 1)); - $closeBracket = $tokens[$openBracket]['parenthesis_closer']; - } - } // Find the end of the function declaration. - $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($openingBrace - 1), $closeBracket, true); + $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($openingBrace - 1), null, true); $functionLine = $tokens[$prev]['line']; $braceLine = $tokens[$openingBrace]['line'];