Skip to content

Commit 40a0f6b

Browse files
committed
Squiz/FunctionDeclarationArgumentSpacing: fix fixer conflict / improve SpaceBeforeEquals + SpaceAfterEquals fixers
The fixers for the `SpaceBeforeEquals` and `SpaceAfterEquals` error codes would only handle one whitespace token at a time, which is inefficient. While this worked correctly when `$equalsSpacing` was set to `0`, as covered by the `newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0()` test, it would take four loops instead of two to make the fixes. However, when `$equalsSpacing` was set to `1` AND there were new lines to deal with, the fixer would get into a fixer conflict with itself as it would keep trying to add a space before the equal sign. ``` Squiz.Functions.FunctionDeclarationArgumentSpacing:227 replaced token 31 (T_WHITESPACE on line 13) " =" => " =" ``` This commit changes both fixers to handle all whitespace tokens in one go. This makes the fixers more efficient, but also fixes the fixer conflict which was identified. Tested via the pre-existing `newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0()` test case, as well as the new `newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1()` test case.
1 parent b2a78dd commit 40a0f6b

File tree

4 files changed

+33
-8
lines changed

4 files changed

+33
-8
lines changed

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

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,15 @@ public function processBracket($phpcsFile, $openBracket)
222222
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
223223
if ($fix === true) {
224224
$padding = str_repeat(' ', $this->equalsSpacing);
225-
if ($spacesBefore === 0) {
226-
$phpcsFile->fixer->addContentBefore($equalToken, $padding);
227-
} else {
228-
$phpcsFile->fixer->replaceToken(($equalToken - 1), $padding);
225+
226+
$phpcsFile->fixer->beginChangeset();
227+
$phpcsFile->fixer->addContent($param['token'], $padding);
228+
229+
for ($i = ($param['token'] + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
230+
$phpcsFile->fixer->replaceToken($i, '');
229231
}
232+
233+
$phpcsFile->fixer->endChangeset();
230234
}
231235
}
232236
}//end if
@@ -252,11 +256,15 @@ public function processBracket($phpcsFile, $openBracket)
252256
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
253257
if ($fix === true) {
254258
$padding = str_repeat(' ', $this->equalsSpacing);
255-
if ($spacesAfter === 0) {
256-
$phpcsFile->fixer->addContent($equalToken, $padding);
257-
} else {
258-
$phpcsFile->fixer->replaceToken(($equalToken + 1), $padding);
259+
260+
$phpcsFile->fixer->beginChangeset();
261+
$phpcsFile->fixer->addContent($equalToken, $padding);
262+
263+
for ($i = ($equalToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
264+
$phpcsFile->fixer->replaceToken($i, '');
259265
}
266+
267+
$phpcsFile->fixer->endChangeset();
260268
}
261269
}
262270
}//end if

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,13 @@ function newlineAndCommentBeforeAndAfterEqualsSignShouldBeFlaggedNotFixed(
166166

167167
true
168168
) {}
169+
170+
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 1
171+
function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1(
172+
$param
173+
174+
=
175+
176+
true
177+
) {}
178+
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 0

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,9 @@ function newlineAndCommentBeforeAndAfterEqualsSignShouldBeFlaggedNotFixed(
151151

152152
true
153153
) {}
154+
155+
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 1
156+
function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing1(
157+
$param = true
158+
) {}
159+
// phpcs:set Squiz.Functions.FunctionDeclarationArgumentSpacing equalsSpacing 0

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public function getErrorList()
7777
149 => 2,
7878
155 => 2,
7979
163 => 2,
80+
174 => 2,
8081
];
8182

8283
}//end getErrorList()

0 commit comments

Comments
 (0)