Skip to content

Commit

Permalink
Squiz/FunctionSpacing: improve handling with comments before first/af…
Browse files Browse the repository at this point in the history
…ter 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.
  • Loading branch information
jrfnl committed Oct 17, 2019
1 parent 82cd0f8 commit f362965
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
45 changes: 45 additions & 0 deletions src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ public function getErrorList($testFile='')
479 => 1,
483 => 2,
495 => 1,
529 => 1,
539 => 1,
];

case 'FunctionSpacingUnitTest.2.inc':
Expand Down

0 comments on commit f362965

Please sign in to comment.