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.

Sign up for GitHub

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 2 commits into from
Mar 7, 2025
Merged

Squiz/FunctionSpacing: bug fix for attribute handling #826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3799727
Squiz/FunctionSpacing: bug fix for attribute handling
jrfnl Feb 15, 2025
04bf4c7
Squiz/FunctionSpacing: iterate on previous fix for attributes
jrfnl Feb 17, 2025
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
View file
Open in desktop
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
View file
Open in desktop
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