From f3629656d6984e8216488500304e45b31086ffd8 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Oct 2019 21:21:52 +0200 Subject: [PATCH] Squiz/FunctionSpacing: improve handling with comments before first/after last If there would be comments (or commented out code) before the first method or after the last method in an OO structure, the fixer as it was, would - depending on the various `$spacing` settings, potentially remove the space between the method and the comment which would easily conflict with other comment rules. By allowing for comments before the first method/after the last method, this is prevented. The fix now implemented means that if there is anything between the class opener/trait import `use` and the first method, other than a function docblock/comment, the method will be treated as any method and won't get the special "first method" treatment. Similarly, when there is anything between the last method and the class closer, other than a trailing comment, the method will be treated as any method and won't get the special "last" method treatment. Includes unit test. --- .../WhiteSpace/FunctionSpacingSniff.php | 14 +++++- .../WhiteSpace/FunctionSpacingUnitTest.1.inc | 45 ++++++++++++++++++ .../FunctionSpacingUnitTest.1.inc.fixed | 47 +++++++++++++++++++ .../WhiteSpace/FunctionSpacingUnitTest.php | 2 + 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php index 264dc99552..dcc45fb36b 100644 --- a/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php @@ -112,14 +112,26 @@ public function process(File $phpcsFile, $stackPtr) $isFirst = false; $isLast = false; - $ignore = (Tokens::$emptyTokens + Tokens::$methodPrefixes); + $ignore = ([T_WHITESPACE => T_WHITESPACE] + Tokens::$methodPrefixes); $prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true); + if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) { + // Skip past function docblocks. + $prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['comment_opener'] - 1), null, true); + } + if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) { $isFirst = true; } $next = $phpcsFile->findNext($ignore, ($closer + 1), null, true); + if (isset(Tokens::$emptyTokens[$tokens[$next]['code']]) === true + && $tokens[$next]['line'] === $tokens[$closer]['line'] + ) { + // Skip past "end" comments. + $next = $phpcsFile->findNext($ignore, ($next + 1), null, true); + } + if ($tokens[$next]['code'] === T_CLOSE_CURLY_BRACKET) { $isLast = true; } diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc index c49e98f338..2e058050d8 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc @@ -495,6 +495,51 @@ interface OneBlankLineBeforeFirstFunctionClassInterface public function interfaceMethod(); } +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 1 +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 0 +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0 + +class MyClass { + + // phpcs:disable Stnd.Cat.Sniff -- For reasons. + + /** + * Description. + */ + function a(){} + + /** + * Description. + */ + function b(){} + + /** + * Description. + */ + function c(){} + + // phpcs:enable +} + +class MyClass { + // Some unrelated comment + /** + * Description. + */ + function a(){} + + /** + * Description. + */ + function b(){} + + /** + * Description. + */ + function c(){} + // function d() {} +} + // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 2 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2 diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed index 74f085d868..eec26c6913 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc.fixed @@ -577,6 +577,53 @@ interface OneBlankLineBeforeFirstFunctionClassInterface public function interfaceMethod(); } +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 1 +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 0 +// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0 + +class MyClass { + + // phpcs:disable Stnd.Cat.Sniff -- For reasons. + + /** + * Description. + */ + function a(){} + + /** + * Description. + */ + function b(){} + + /** + * Description. + */ + function c(){} + + // phpcs:enable +} + +class MyClass { + // Some unrelated comment + + /** + * Description. + */ + function a(){} + + /** + * Description. + */ + function b(){} + + /** + * Description. + */ + function c(){} + + // function d() {} +} + // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacing 2 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2 // phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2 diff --git a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php index 7251319974..9f202888a9 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php +++ b/src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.php @@ -88,6 +88,8 @@ public function getErrorList($testFile='') 479 => 1, 483 => 2, 495 => 1, + 529 => 1, + 539 => 1, ]; case 'FunctionSpacingUnitTest.2.inc':