Skip to content

Commit b2a78dd

Browse files
committed
Squiz/FunctionDeclarationArgumentSpacing: bug fix / SpaceBeforeEquals + SpaceAfterEquals vs comments
When there is a comment between the parameter variable and the equal sign or the equal sign and the default value, the fixer would previously unintentionally and incorrectly remove most of these comments. For the tests added in this commit, the fixer would previously result in the following - note that three out of the four comments have been removed.. ```php function newlineBeforeAndAfterEqualsSignFixedRespectsComments( $param=true ) {} function commentBeforeAndAfterEqualsSignFixedRespectsComments( $param=/*comment*/ true ) {} ``` In this commit, I'm changing the sniff to still allow for flagging the spacing issue when there are comments, but to no longer auto-fix these to prevent the sniff removing the comments. Includes tests.
1 parent 0e271eb commit b2a78dd

File tree

4 files changed

+58
-14
lines changed

4 files changed

+58
-14
lines changed

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

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,18 @@ public function processBracket($phpcsFile, $openBracket)
215215
$spacesBefore,
216216
];
217217

218-
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
219-
if ($fix === true) {
220-
$padding = str_repeat(' ', $this->equalsSpacing);
221-
if ($spacesBefore === 0) {
222-
$phpcsFile->fixer->addContentBefore($equalToken, $padding);
223-
} else {
224-
$phpcsFile->fixer->replaceToken(($equalToken - 1), $padding);
218+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($param['token'] + 1), $equalToken, true);
219+
if ($nextNonWhitespace !== false) {
220+
$phpcsFile->addError($error, $equalToken, 'SpaceBeforeEquals', $data);
221+
} else {
222+
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
223+
if ($fix === true) {
224+
$padding = str_repeat(' ', $this->equalsSpacing);
225+
if ($spacesBefore === 0) {
226+
$phpcsFile->fixer->addContentBefore($equalToken, $padding);
227+
} else {
228+
$phpcsFile->fixer->replaceToken(($equalToken - 1), $padding);
229+
}
225230
}
226231
}
227232
}//end if
@@ -240,13 +245,18 @@ public function processBracket($phpcsFile, $openBracket)
240245
$spacesAfter,
241246
];
242247

243-
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
244-
if ($fix === true) {
245-
$padding = str_repeat(' ', $this->equalsSpacing);
246-
if ($spacesAfter === 0) {
247-
$phpcsFile->fixer->addContent($equalToken, $padding);
248-
} else {
249-
$phpcsFile->fixer->replaceToken(($equalToken + 1), $padding);
248+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($equalToken + 1), $param['default_token'], true);
249+
if ($nextNonWhitespace !== false) {
250+
$phpcsFile->addError($error, $equalToken, 'SpaceAfterEquals', $data);
251+
} else {
252+
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
253+
if ($fix === true) {
254+
$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+
}
250260
}
251261
}
252262
}//end if

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,19 @@ function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0(
150150

151151
true
152152
) {}
153+
154+
function commentBeforeOrAfterEqualsSignShouldBeFlaggedNotFixed(
155+
$param /*comment*/ = /*comment*/ true
156+
) {}
157+
158+
function newlineAndCommentBeforeAndAfterEqualsSignShouldBeFlaggedNotFixed(
159+
$param
160+
161+
//comment
162+
163+
=
164+
165+
//comment
166+
167+
true
168+
) {}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,19 @@ function newlineAfterVariadicFixerRespectsComment(
135135
function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0(
136136
$param=true
137137
) {}
138+
139+
function commentBeforeOrAfterEqualsSignShouldBeFlaggedNotFixed(
140+
$param /*comment*/ = /*comment*/ true
141+
) {}
142+
143+
function newlineAndCommentBeforeAndAfterEqualsSignShouldBeFlaggedNotFixed(
144+
$param
145+
146+
//comment
147+
148+
=
149+
150+
//comment
151+
152+
true
153+
) {}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ public function getErrorList()
7575
135 => 1,
7676
141 => 1,
7777
149 => 2,
78+
155 => 2,
79+
163 => 2,
7880
];
7981

8082
}//end getErrorList()

0 commit comments

Comments
 (0)