Skip to content

Squiz/FunctionSpacing: bug fix for attribute handling #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 52 additions & 51 deletions src/Standards/Squiz/Sniffs/WhiteSpace/FunctionSpacingSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,36 @@ public function process(File $phpcsFile, $stackPtr)

$prev = $phpcsFile->findPrevious($ignore, ($stackPtr - 1), null, true);

while ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
// Skip past function attributes.
$prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['attribute_opener'] - 1), null, true);
$startOfDeclarationLine = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);
for ($i = $startOfDeclarationLine; $i >= 0; $i--) {
if ($tokens[$i]['line'] === $tokens[$startOfDeclarationLine]['line']) {
$startOfDeclarationLine = $i;
continue;
}

break;
}

if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
// Skip past function docblocks.
$prev = $phpcsFile->findPrevious($ignore, ($tokens[$prev]['comment_opener'] - 1), null, true);
// Skip past function docblocks and attributes.
$prev = $startOfDeclarationLine;
if ($startOfDeclarationLine > 0) {
for ($prev = ($startOfDeclarationLine - 1); $prev > 0; $prev--) {
if ($tokens[$prev]['code'] === T_WHITESPACE) {
continue;
}

if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
$prev = $tokens[$prev]['comment_opener'];
continue;
}

if ($tokens[$prev]['code'] === T_ATTRIBUTE_END) {
$prev = $tokens[$prev]['attribute_opener'];
continue;
}

break;
}
}

if ($tokens[$prev]['code'] === T_OPEN_CURLY_BRACKET) {
Expand Down Expand Up @@ -224,9 +246,11 @@ public function process(File $phpcsFile, $stackPtr)
before the function.
*/

$startOfPreamble = $phpcsFile->findNext(T_WHITESPACE, ($prev + 1), null, true);

$prevLineToken = null;
for ($i = $stackPtr; $i >= 0; $i--) {
if ($tokens[$i]['line'] === $tokens[$stackPtr]['line']) {
for ($i = $startOfPreamble; $i >= 0; $i--) {
if ($tokens[$i]['line'] === $tokens[$startOfPreamble]['line']) {
continue;
}

Expand All @@ -241,33 +265,15 @@ public function process(File $phpcsFile, $stackPtr)
$prevContent = 0;
$prevLineToken = 0;
} else {
$currentLine = $tokens[$stackPtr]['line'];

$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, $prevLineToken, null, true);

if ($tokens[$prevContent]['code'] === T_COMMENT
|| isset(Tokens::$phpcsCommentTokens[$tokens[$prevContent]['code']]) === true
$firstBefore = $phpcsFile->findPrevious(T_WHITESPACE, ($startOfDeclarationLine - 1), null, true);
if ($tokens[$firstBefore]['code'] === T_COMMENT
|| isset(Tokens::$phpcsCommentTokens[$tokens[$firstBefore]['code']]) === true
) {
// Ignore comments as they can have different spacing rules, and this
// isn't a proper function comment anyway.
return;
}

while ($tokens[$prevContent]['code'] === T_ATTRIBUTE_END
&& $tokens[$prevContent]['line'] === ($currentLine - 1)
) {
// Account for function attributes.
$currentLine = $tokens[$tokens[$prevContent]['attribute_opener']]['line'];
$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['attribute_opener'] - 1), null, true);
}

if ($tokens[$prevContent]['code'] === T_DOC_COMMENT_CLOSE_TAG
&& $tokens[$prevContent]['line'] === ($currentLine - 1)
) {
// Account for function comments.
$prevContent = $phpcsFile->findPrevious(T_WHITESPACE, ($tokens[$prevContent]['comment_opener'] - 1), null, true);
}

// Before we throw an error, check that we are not throwing an error
// for another function. We don't want to error for no blank lines after
// the previous function and no blank lines before this one as well.
Expand All @@ -278,38 +284,33 @@ public function process(File $phpcsFile, $stackPtr)
$stopAt = array_pop($conditions);
}

$prevLineToken = $prevContent;
$prevLine = ($tokens[$prevContent]['line'] - 1);
$i = ($stackPtr - 1);
$foundLines = 0;
$currentLine = $tokens[$startOfPreamble]['line'];
$prevContent = $prev;
$prevLine = ($tokens[$prevContent]['line'] - 1);
$foundLines = ($currentLine - $tokens[$prevContent]['line'] - 1);

for ($i = $prevContent; $i > $stopAt; $i--) {
if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET) {
if (isset($tokens[$i]['scope_condition']) === true
&& $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
) {
// Found a previous function.
return;
} else {
break;
}
}

while ($currentLine !== $prevLine && $currentLine > 1 && $i > $stopAt) {
if ($tokens[$i]['code'] === T_FUNCTION) {
// Found another interface or abstract function.
return;
}

if ($tokens[$i]['code'] === T_CLOSE_CURLY_BRACKET
&& $tokens[$tokens[$i]['scope_condition']]['code'] === T_FUNCTION
) {
// Found a previous function.
return;
}

$currentLine = $tokens[$i]['line'];
if ($currentLine === $prevLine) {
break;
}

if ($tokens[($i - 1)]['line'] < $currentLine && $tokens[($i + 1)]['line'] > $currentLine) {
// This token is on a line by itself. If it is whitespace, the line is empty.
if ($tokens[$i]['code'] === T_WHITESPACE) {
$foundLines++;
}
}

$i--;
}//end while
}//end for
}//end if

$requiredSpacing = $this->spacing;
Expand Down
133 changes: 133 additions & 0 deletions src/Standards/Squiz/Tests/WhiteSpace/FunctionSpacingUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -593,3 +593,136 @@ function a() {
*/
function b() {
}


// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 1
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 0

class DocblockFollowedByAttributesCorrectSpacing {

/**
* No error.
*/
#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
function FirstFunction()
{
// Code
}
}

class DocblockFollowedByAttributesTooMuchSpacing {



/**
* Docblock.
*/
#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
function FirstFunction()
{
// Code
}
}

class DocblockFollowedByAttributesTooLittleSpacing {
/**
* Docblock.
*/
#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
function FirstFunction()
{
// Code
}
}

class DocblockPrecededByAttributesCorrectSpacing {

#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
/**
* No error.
*/
function FirstFunction()
{
// Code
}
}

class DocblockPrecededByAttributesTooMuchSpacing {


#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
/**
* Docblock.
*/
function FirstFunction()
{
// Code
}
}

class DocblockPrecededByAttributesTooLittleSpacing {
#[AttributesShouldBeJumpedOver]
#[
ASecondAttributeShouldBeJumpedOverToo
]#[AndAThirdAsWell]
/**
* Docblock.
*/
function FirstFunction()
{
// Code
}
}

// Reset properties to their default value.
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingBeforeFirst 2
// phpcs:set Squiz.WhiteSpace.FunctionSpacing spacingAfterLast 2

class SilenceBeforeErrorIfPreviousThingWasAFunctionBug
{
/**
* Docblock.
*/

#[ReturnTypeWillChange]





#[

AnotherAttribute

]#[AndAThirdAsWell]

public function blankLineDetectionA()
{

}//end blankLineDetectionA()

/**
* Docblock.
*/
#[ReturnTypeWillChange]

public function blankLineDetectionB()
{

}//end blankLineDetectionB()
}//end class
Loading