From 5f27b94b66f70298789f101bed17b8c9355c99c0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 9 Aug 2023 19:27:05 +0200 Subject: [PATCH 1/2] Generic/OpeningFunctionBraceKernighanRitchie: check spacing before brace for empty functions As things were, when an empty function was detected, the sniff would bow out and not execute the "SpaceBeforeBrace" check. Fixed now. Includes tests. --- CHANGELOG.md | 2 ++ ...eningFunctionBraceKernighanRitchieSniff.php | 18 ++++++++---------- ...ngFunctionBraceKernighanRitchieUnitTest.inc | 4 ++++ ...tionBraceKernighanRitchieUnitTest.inc.fixed | 4 ++++ ...ngFunctionBraceKernighanRitchieUnitTest.php | 2 ++ 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5899e1e894..14029138ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,8 @@ The file documents changes to the PHP_CodeSniffer project. - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Sniff error messages are now more informative to help bugs get reported to the correct project - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch +- Generic.Functions.OpeningFunctionBraceKernighanRitchie will now check the spacing before the opening brace for empty functions + - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - PSR2.Classes.PropertyDeclaration now enforces that the readonly modifier comes after the visibility modifier - PSR2 and PSR12 do not have documented rules for this as they pre-date the readonly modifier - PSR-PER has been used to confirm the order of this keyword so it can be applied to PSR2 and PSR12 correctly diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php index ca26fcd561..ca259854e7 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceKernighanRitchieSniff.php @@ -130,17 +130,15 @@ public function process(File $phpcsFile, $stackPtr) $ignore[] = T_WHITESPACE; $next = $phpcsFile->findNext($ignore, ($openingBrace + 1), null, true); if ($tokens[$next]['line'] === $tokens[$openingBrace]['line']) { - if ($next === $tokens[$stackPtr]['scope_closer'] - || $tokens[$next]['code'] === T_CLOSE_TAG + // Only throw this error when this is not an empty function. + if ($next !== $tokens[$stackPtr]['scope_closer'] + && $tokens[$next]['code'] !== T_CLOSE_TAG ) { - // Ignore empty functions. - return; - } - - $error = 'Opening brace must be the last content on the line'; - $fix = $phpcsFile->addFixableError($error, $openingBrace, 'ContentAfterBrace'); - if ($fix === true) { - $phpcsFile->fixer->addNewline($openingBrace); + $error = 'Opening brace must be the last content on the line'; + $fix = $phpcsFile->addFixableError($error, $openingBrace, 'ContentAfterBrace'); + if ($fix === true) { + $phpcsFile->fixer->addNewline($openingBrace); + } } } diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc index f7b3c922e8..6c937a823b 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc @@ -208,3 +208,7 @@ function myFunction($a, $lot, $of, $params) : array { // phpcs:ignore Standard.Category.Sniff -- for reasons. return null; } + +function myFunction() {} +function myFunction() {} // Too many spaces with an empty function. +function myFunction() {} // Too many spaces (tab) with an empty function. diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed index 533fb567d2..bfb2283844 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.inc.fixed @@ -196,3 +196,7 @@ function myFunction($a, $lot, $of, $params) : array { // phpcs:ignore Standard.Category.Sniff -- for reasons. return null; } + +function myFunction() {} +function myFunction() {} // Too many spaces with an empty function. +function myFunction() {} // Too many spaces (tab) with an empty function. diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php index 7cd60a3023..a2b32a8587 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceKernighanRitchieUnitTest.php @@ -52,6 +52,8 @@ public function getErrorList() 191 => 1, 197 => 1, 203 => 1, + 213 => 1, + 214 => 1, ]; }//end getErrorList() From 1c10cba46dc2e790ab52541e29db4901b7c1f3b5 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 9 Aug 2023 19:32:42 +0200 Subject: [PATCH 2/2] Generic/OpeningFunctionBraceBsdAllman: check spacing before brace for empty functions As things were, when an empty function was detected, the sniff would bow out and not execute the "BraceIndent" check. Fixed now. Includes tests. --- CHANGELOG.md | 2 ++ .../OpeningFunctionBraceBsdAllmanSniff.php | 16 +++++++--------- .../OpeningFunctionBraceBsdAllmanUnitTest.inc | 7 +++++++ ...eningFunctionBraceBsdAllmanUnitTest.inc.fixed | 7 +++++++ .../OpeningFunctionBraceBsdAllmanUnitTest.php | 2 ++ 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14029138ec..302fa327fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,8 @@ The file documents changes to the PHP_CodeSniffer project. - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Sniff error messages are now more informative to help bugs get reported to the correct project - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch +- Generic.Functions.OpeningFunctionBraceBsdAllman will now check the brace indent before the opening brace for empty functions + - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Generic.Functions.OpeningFunctionBraceKernighanRitchie will now check the spacing before the opening brace for empty functions - Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - PSR2.Classes.PropertyDeclaration now enforces that the readonly modifier comes after the visibility modifier diff --git a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php index de2c083175..d60ab61e7b 100644 --- a/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php +++ b/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php @@ -170,15 +170,13 @@ public function process(File $phpcsFile, $stackPtr) $ignore[] = T_WHITESPACE; $next = $phpcsFile->findNext($ignore, ($openingBrace + 1), null, true); if ($tokens[$next]['line'] === $tokens[$openingBrace]['line']) { - if ($next === $tokens[$stackPtr]['scope_closer']) { - // Ignore empty functions. - return; - } - - $error = 'Opening brace must be the last content on the line'; - $fix = $phpcsFile->addFixableError($error, $openingBrace, 'ContentAfterBrace'); - if ($fix === true) { - $phpcsFile->fixer->addNewline($openingBrace); + // Only throw this error when this is not an empty function. + if ($next !== $tokens[$stackPtr]['scope_closer']) { + $error = 'Opening brace must be the last content on the line'; + $fix = $phpcsFile->addFixableError($error, $openingBrace, 'ContentAfterBrace'); + if ($fix === true) { + $phpcsFile->fixer->addNewline($openingBrace); + } } } diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc index 3ae3b1ed0b..146f9cf6dd 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc @@ -261,3 +261,10 @@ class Issue3357WithComment // code here. } } + + function myFunction() + {} + function myFunction() + {} // Too many spaces indent with an empty function. + function myFunction() +{} // Too little spaces indent with an empty function. diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc.fixed b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc.fixed index f164c4934e..ac4929d7d5 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.inc.fixed @@ -278,3 +278,10 @@ class Issue3357WithComment // code here. } } + + function myFunction() + {} + function myFunction() + {} // Too many spaces indent with an empty function. + function myFunction() + {} // Too little spaces indent with an empty function. diff --git a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.php b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.php index 6d4abebaad..c8f218dbdc 100644 --- a/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.php +++ b/src/Standards/Generic/Tests/Functions/OpeningFunctionBraceBsdAllmanUnitTest.php @@ -61,6 +61,8 @@ public function getErrorList() 244 => 1, 252 => 1, 260 => 1, + 268 => 1, + 270 => 1, ]; }//end getErrorList()