diff --git a/src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php b/src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php index 11cdba8d2e..2621a43615 100644 --- a/src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php @@ -131,25 +131,30 @@ 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'], @@ -157,20 +162,25 @@ public function processBracket($phpcsFile, $openBracket) ]; $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'], @@ -178,16 +188,23 @@ public function processBracket($phpcsFile, $openBracket) ]; $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']; } @@ -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']; } @@ -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 @@ -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; @@ -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'; diff --git a/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc similarity index 69% rename from src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc rename to src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc index 07a6d452f5..764e47331d 100644 --- a/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc +++ b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc @@ -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, @@ -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' +) {} diff --git a/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc.fixed similarity index 70% rename from src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc.fixed rename to src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc.fixed index cb4265057c..1de259a75d 100644 --- a/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc.fixed @@ -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, @@ -111,3 +111,63 @@ $a = function ($var1, $var2=false) use ( 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' +) {} diff --git a/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.2.inc b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.2.inc new file mode 100644 index 0000000000..1b572336b2 --- /dev/null +++ b/src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.2.inc @@ -0,0 +1,6 @@ + */ - public function getErrorList() + public function getErrorList($testFile='') { - return [ - 3 => 1, - 5 => 2, - 7 => 2, - 8 => 2, - 9 => 2, - 11 => 2, - 13 => 7, - 14 => 2, - 15 => 2, - 16 => 4, - 18 => 2, - 35 => 2, - 36 => 2, - 44 => 2, - 45 => 1, - 46 => 1, - 51 => 2, - 53 => 2, - 55 => 1, - 56 => 1, - 58 => 1, - 73 => 7, - 76 => 1, - 77 => 1, - 81 => 1, - 89 => 2, - 92 => 1, - 93 => 1, - 94 => 1, - 95 => 1, - 99 => 11, - 100 => 2, - 101 => 2, - 102 => 2, - 106 => 1, - 107 => 2, - 111 => 3, - 113 => 1, - ]; + switch ($testFile) { + case 'FunctionDeclarationArgumentSpacingUnitTest.1.inc': + return [ + 3 => 1, + 5 => 2, + 7 => 2, + 8 => 2, + 9 => 2, + 11 => 2, + 13 => 7, + 14 => 2, + 15 => 2, + 16 => 4, + 18 => 2, + 35 => 2, + 36 => 2, + 44 => 2, + 45 => 1, + 46 => 1, + 51 => 2, + 53 => 2, + 55 => 1, + 56 => 1, + 58 => 1, + 73 => 7, + 76 => 1, + 77 => 1, + 81 => 1, + 89 => 2, + 92 => 1, + 93 => 1, + 94 => 1, + 95 => 1, + 99 => 11, + 100 => 2, + 101 => 2, + 102 => 2, + 103 => 1, + 106 => 1, + 107 => 2, + 111 => 3, + 113 => 1, + 117 => 1, + 123 => 1, + 129 => 1, + 135 => 1, + 141 => 1, + 149 => 2, + 155 => 2, + 163 => 2, + 174 => 2, + 182 => 1, + 185 => 1, + 191 => 1, + 193 => 1, + 195 => 1, + 196 => 1, + ]; + + default: + return []; + }//end switch }//end getErrorList()