Skip to content

Commit

Permalink
Merge pull request #783 from PHPCSStandards/feature/squiz-functiondec…
Browse files Browse the repository at this point in the history 
…larationspacing-various-fixes

Squiz/FunctionDeclarationArgumentSpacing: various bug fixes and prevent fixer conflicts
  • Loading branch information
@jrfnl
jrfnl authored Jan 23, 2025
2 parents aebd84b + d8924e3 commit e8e123e
Show file tree
Hide file tree
Showing 9 changed files with 344 additions and 79 deletions.
141 changes: 105 additions & 36 deletions src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,63 +131,80 @@ public function processBracket($phpcsFile, $openBracket)
$data = [$found];
$fix = $phpcsFile->addFixableError($error, $openBracket, 'SpacingBetween', $data);
if ($fix === true) {
$phpcsFile->fixer->replaceToken(($openBracket + 1), '');
$phpcsFile->fixer->beginChangeset();
for ($i = ($openBracket + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->endChangeset();
}
}

// No params, so we don't check normal spacing rules.
return;
}
}//end if
}//end if

foreach ($params as $paramNumber => $param) {
if ($param['pass_by_reference'] === true) {
$refToken = $param['reference_token'];

$gap = 0;
if ($tokens[($refToken + 1)]['code'] === T_WHITESPACE) {
$gap = $tokens[($refToken + 1)]['length'];
}
if ($tokens[$refToken]['line'] !== $tokens[($refToken + 2)]['line']) {
$gap = 'newline';
}

if ($gap !== 0) {
$error = 'Expected 0 spaces after reference operator for argument "%s"; %s found';
$data = [
$param['name'],
$gap,
];
$fix = $phpcsFile->addFixableError($error, $refToken, 'SpacingAfterReference', $data);
if ($fix === true) {
$phpcsFile->fixer->replaceToken(($refToken + 1), '');
$phpcsFile->fixer->beginChangeset();
for ($i = ($refToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->endChangeset();
}
}
}//end if
}//end if

if ($param['variable_length'] === true) {
$variadicToken = $param['variadic_token'];

$gap = 0;
if ($tokens[($variadicToken + 1)]['code'] === T_WHITESPACE) {
$gap = $tokens[($variadicToken + 1)]['length'];
}
if ($tokens[$variadicToken]['line'] !== $tokens[($variadicToken + 2)]['line']) {
$gap = 'newline';
}

if ($gap !== 0) {
$error = 'Expected 0 spaces after variadic operator for argument "%s"; %s found';
$data = [
$param['name'],
$gap,
];
$fix = $phpcsFile->addFixableError($error, $variadicToken, 'SpacingAfterVariadic', $data);
if ($fix === true) {
$phpcsFile->fixer->replaceToken(($variadicToken + 1), '');
$phpcsFile->fixer->beginChangeset();
for ($i = ($variadicToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->endChangeset();
}
}
}//end if
}//end if

if (isset($param['default_equal_token']) === true) {
$equalToken = $param['default_equal_token'];

$spacesBefore = 0;
if (($equalToken - $param['token']) > 1) {
if ($tokens[$param['token']]['line'] !== $tokens[$equalToken]['line']) {
$spacesBefore = 'newline';
} else if ($tokens[($param['token'] + 1)]['code'] === T_WHITESPACE) {
$spacesBefore = $tokens[($param['token'] + 1)]['length'];
}

Expand All @@ -198,19 +215,30 @@ public function processBracket($phpcsFile, $openBracket)
$spacesBefore,
];

$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
if ($fix === true) {
$padding = str_repeat(' ', $this->equalsSpacing);
if ($spacesBefore === 0) {
$phpcsFile->fixer->addContentBefore($equalToken, $padding);
} else {
$phpcsFile->fixer->replaceToken(($equalToken - 1), $padding);
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($param['token'] + 1), $equalToken, true);
if ($nextNonWhitespace !== false) {
$phpcsFile->addError($error, $equalToken, 'SpaceBeforeEquals', $data);
} else {
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
if ($fix === true) {
$padding = str_repeat(' ', $this->equalsSpacing);

$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($param['token'], $padding);

for ($i = ($param['token'] + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->endChangeset();
}
}
}//end if

$spacesAfter = 0;
if ($tokens[($equalToken + 1)]['code'] === T_WHITESPACE) {
if ($tokens[$equalToken]['line'] !== $tokens[$param['default_token']]['line']) {
$spacesAfter = 'newline';
} else if ($tokens[($equalToken + 1)]['code'] === T_WHITESPACE) {
$spacesAfter = $tokens[($equalToken + 1)]['length'];
}

Expand All @@ -221,13 +249,22 @@ public function processBracket($phpcsFile, $openBracket)
$spacesAfter,
];

$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
if ($fix === true) {
$padding = str_repeat(' ', $this->equalsSpacing);
if ($spacesAfter === 0) {
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($equalToken + 1), $param['default_token'], true);
if ($nextNonWhitespace !== false) {
$phpcsFile->addError($error, $equalToken, 'SpaceAfterEquals', $data);
} else {
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
if ($fix === true) {
$padding = str_repeat(' ', $this->equalsSpacing);

$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($equalToken, $padding);
} else {
$phpcsFile->fixer->replaceToken(($equalToken + 1), $padding);

for ($i = ($equalToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}

$phpcsFile->fixer->endChangeset();
}
}
}//end if
Expand Down Expand Up @@ -275,18 +312,53 @@ public function processBracket($phpcsFile, $openBracket)
}

if ($commaToken !== false) {
if ($tokens[($commaToken - 1)]['code'] === T_WHITESPACE) {
$endOfPreviousParam = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($commaToken - 1), null, true);

$spaceBeforeComma = 0;
if ($tokens[$endOfPreviousParam]['line'] !== $tokens[$commaToken]['line']) {
$spaceBeforeComma = 'newline';
} else if ($tokens[($commaToken - 1)]['code'] === T_WHITESPACE) {
$spaceBeforeComma = $tokens[($commaToken - 1)]['length'];
}

if ($spaceBeforeComma !== 0) {
$error = 'Expected 0 spaces between argument "%s" and comma; %s found';
$data = [
$params[($paramNumber - 1)]['name'],
$tokens[($commaToken - 1)]['length'],
$spaceBeforeComma,
];

$fix = $phpcsFile->addFixableError($error, $commaToken, 'SpaceBeforeComma', $data);
if ($fix === true) {
$phpcsFile->fixer->replaceToken(($commaToken - 1), '');
}
}
$startOfCurrentParam = $phpcsFile->findNext(Tokens::$emptyTokens, ($commaToken + 1), null, true);

$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent($endOfPreviousParam, ',');
$phpcsFile->fixer->replaceToken($commaToken, '');

if ($tokens[$commaToken]['line'] === $tokens[$startOfCurrentParam]['line']) {
for ($i = ($commaToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
$phpcsFile->fixer->replaceToken($i, '');
}
} else {
for ($i = ($commaToken - 1);
$tokens[$i]['code'] === T_WHITESPACE && $tokens[$endOfPreviousParam]['line'] !== $tokens[$i]['line'];
$i--
) {
$phpcsFile->fixer->replaceToken($i, '');
}

for ($i = ($commaToken + 1);
$tokens[$i]['code'] === T_WHITESPACE && $tokens[$commaToken]['line'] === $tokens[$i]['line'];
$i++
) {
$phpcsFile->fixer->replaceToken($i, '');
}
}

$phpcsFile->fixer->endChangeset();
}//end if
}//end if

// Don't check spacing after the comma if it is the last content on the line.
$checkComma = true;
Expand Down Expand Up @@ -324,10 +396,7 @@ public function processBracket($phpcsFile, $openBracket)
}
}//end if
} else {
$hint = $phpcsFile->getTokensAsString($param['type_hint_token'], (($param['type_hint_end_token'] - $param['type_hint_token']) + 1));
if ($param['nullable_type'] === true) {
$hint = '?'.$hint;
}
$hint = $param['type_hint'];

if ($tokens[($commaToken + 1)]['code'] !== T_WHITESPACE) {
$error = 'Expected 1 space between comma and type hint "%s"; 0 found';
Expand Down
84 changes: 83 additions & 1 deletion ...ionDeclarationArgumentSpacingUnitTest.inc → ...nDeclarationArgumentSpacingUnitTest.1.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function functionName( ?string $arg1 = 'foo' , ?int & $arg2 , $arg3 ) {}
function functionName(string $arg1, ... $arg2) {}
function functionName(string $arg1, int ... $arg2) {}
function functionName(string $arg1, & ... $arg2) {}

function functionName(string $arg1,int $arg2) {}

$a = function ($var1, $var2=false) use (
$longVar1, & $longerVar1,
Expand All @@ -113,3 +113,85 @@ fn ($a,$b = null) => $a($b);
function multipleWhitespaceTokensAfterType(int

$number) {}

function spacingBetweenParenthesesShouldBeFixedInOneGo(


) {}

function newlineAfterReferenceShouldBeFlaggedAndFixed(
&

$param
) {}

function newlineAfterReferenceFixerRespectsComment(
&
// comment
$param
) {}

function newlineAfterVariadicShouldBeFlaggedAndFixed(
...

$param
) {}

function newlineAfterVariadicFixerRespectsComment(
...
//comment
$param
) {}

function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0(
$param

=

true
) {}

function commentBeforeOrAfterEqualsSignShouldBeFlaggedNotFixed(
$param /*comment*/ = /*comment*/ true
) {}

function newlineAndCommentBeforeAndAfterEqualsSignShouldBeFlaggedNotFixed(
$param

//comment

=

//comment

true
) {}

// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 1
function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1(
$param

=

true
) {}
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 0

function newlineBeforeCommaShouldBeFixedInOneGo(
$paramA
,
$paramB

,
$paramC
) {}

function newlineBeforeCommaFixerRespectsComments(
$paramA // comment
,
$paramB=10 /* comment */
,
$paramC=20 # comment
, $paramC=30
, string $paramC='foo'
) {}
Loading

0 comments on commit e8e123e

Please sign in to comment.