Skip to content

Commit 93b7e2d

Browse files
committed
Squiz/FunctionDeclarationArgumentSpacing: bug fix / unclear SpaceBeforeComma message
When there would be a new line between the end of the previous parameter and the comma, the error message was not always descriptive enough. For example, for the test added in this commit, the messages would be: ``` 182 | ERROR | [x] Expected 0 spaces between argument "$paramA" and comma; 0 found | | (Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeComma) 185 | ERROR | [x] Expected 0 spaces between argument "$paramB" and comma; 4 found | | (Squiz.Functions.FunctionDeclarationArgumentSpacing.SpaceBeforeComma) ``` In particular, take not of the "Expected 0 spaces... 0 found". This commit improves the messaging to take new lines into account. Includes tests.
1 parent 40a0f6b commit 93b7e2d

File tree

4 files changed

+28
-2
lines changed

4 files changed

+28
-2
lines changed

src/Standards/Squiz/Sniffs/Functions/FunctionDeclarationArgumentSpacingSniff.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,20 @@ public function processBracket($phpcsFile, $openBracket)
312312
}
313313

314314
if ($commaToken !== false) {
315-
if ($tokens[($commaToken - 1)]['code'] === T_WHITESPACE) {
315+
$endOfPreviousParam = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($commaToken - 1), null, true);
316+
317+
$spaceBeforeComma = 0;
318+
if ($tokens[$endOfPreviousParam]['line'] !== $tokens[$commaToken]['line']) {
319+
$spaceBeforeComma = 'newline';
320+
} else if ($tokens[($commaToken - 1)]['code'] === T_WHITESPACE) {
321+
$spaceBeforeComma = $tokens[($commaToken - 1)]['length'];
322+
}
323+
324+
if ($spaceBeforeComma !== 0) {
316325
$error = 'Expected 0 spaces between argument "%s" and comma; %s found';
317326
$data = [
318327
$params[($paramNumber - 1)]['name'],
319-
$tokens[($commaToken - 1)]['length'],
328+
$spaceBeforeComma,
320329
];
321330

322331
$fix = $phpcsFile->addFixableError($error, $commaToken, 'SpaceBeforeComma', $data);

src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,12 @@ function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1(
176176
true
177177
) {}
178178
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 0
179+
180+
function newlineBeforeCommaShouldBeFixedInOneGo(
181+
$paramA
182+
,
183+
$paramB
184+
185+
,
186+
$paramC
187+
) {}

src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,9 @@ function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1(
157157
$param = true
158158
) {}
159159
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 0
160+
161+
function newlineBeforeCommaShouldBeFixedInOneGo(
162+
$paramA,
163+
$paramB,
164+
$paramC
165+
) {}

src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ public function getErrorList()
7878
155 => 2,
7979
163 => 2,
8080
174 => 2,
81+
182 => 1,
82+
185 => 1,
8183
];
8284

8385
}//end getErrorList()

0 commit comments

Comments
 (0)