Skip to content

Commit e8e123e

Browse files
jrfnljrfnl
authored
Merge pull request #783 from PHPCSStandards/feature/squiz-functiondeclarationspacing-various-fixes
Squiz/FunctionDeclarationArgumentSpacing: various bug fixes and prevent fixer conflicts
2 parents aebd84b + d8924e3 commit e8e123e

9 files changed

+344
-79
lines changed

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

Lines changed: 105 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -131,63 +131,80 @@ public function processBracket($phpcsFile, $openBracket)
131131
$data = [$found];
132132
$fix = $phpcsFile->addFixableError($error, $openBracket, 'SpacingBetween', $data);
133133
if ($fix === true) {
134-
$phpcsFile->fixer->replaceToken(($openBracket + 1), '');
134+
$phpcsFile->fixer->beginChangeset();
135+
for ($i = ($openBracket + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
136+
$phpcsFile->fixer->replaceToken($i, '');
137+
}
138+
139+
$phpcsFile->fixer->endChangeset();
135140
}
136141
}
137142

138143
// No params, so we don't check normal spacing rules.
139144
return;
140-
}
145+
}//end if
141146
}//end if
142147

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

147-
$gap = 0;
148152
if ($tokens[($refToken + 1)]['code'] === T_WHITESPACE) {
149153
$gap = $tokens[($refToken + 1)]['length'];
150-
}
154+
if ($tokens[$refToken]['line'] !== $tokens[($refToken + 2)]['line']) {
155+
$gap = 'newline';
156+
}
151157

152-
if ($gap !== 0) {
153158
$error = 'Expected 0 spaces after reference operator for argument "%s"; %s found';
154159
$data = [
155160
$param['name'],
156161
$gap,
157162
];
158163
$fix = $phpcsFile->addFixableError($error, $refToken, 'SpacingAfterReference', $data);
159164
if ($fix === true) {
160-
$phpcsFile->fixer->replaceToken(($refToken + 1), '');
165+
$phpcsFile->fixer->beginChangeset();
166+
for ($i = ($refToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
167+
$phpcsFile->fixer->replaceToken($i, '');
168+
}
169+
170+
$phpcsFile->fixer->endChangeset();
161171
}
162-
}
172+
}//end if
163173
}//end if
164174

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

168-
$gap = 0;
169178
if ($tokens[($variadicToken + 1)]['code'] === T_WHITESPACE) {
170179
$gap = $tokens[($variadicToken + 1)]['length'];
171-
}
180+
if ($tokens[$variadicToken]['line'] !== $tokens[($variadicToken + 2)]['line']) {
181+
$gap = 'newline';
182+
}
172183

173-
if ($gap !== 0) {
174184
$error = 'Expected 0 spaces after variadic operator for argument "%s"; %s found';
175185
$data = [
176186
$param['name'],
177187
$gap,
178188
];
179189
$fix = $phpcsFile->addFixableError($error, $variadicToken, 'SpacingAfterVariadic', $data);
180190
if ($fix === true) {
181-
$phpcsFile->fixer->replaceToken(($variadicToken + 1), '');
191+
$phpcsFile->fixer->beginChangeset();
192+
for ($i = ($variadicToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
193+
$phpcsFile->fixer->replaceToken($i, '');
194+
}
195+
196+
$phpcsFile->fixer->endChangeset();
182197
}
183-
}
198+
}//end if
184199
}//end if
185200

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

189204
$spacesBefore = 0;
190-
if (($equalToken - $param['token']) > 1) {
205+
if ($tokens[$param['token']]['line'] !== $tokens[$equalToken]['line']) {
206+
$spacesBefore = 'newline';
207+
} else if ($tokens[($param['token'] + 1)]['code'] === T_WHITESPACE) {
191208
$spacesBefore = $tokens[($param['token'] + 1)]['length'];
192209
}
193210

@@ -198,19 +215,30 @@ public function processBracket($phpcsFile, $openBracket)
198215
$spacesBefore,
199216
];
200217

201-
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceBeforeEquals', $data);
202-
if ($fix === true) {
203-
$padding = str_repeat(' ', $this->equalsSpacing);
204-
if ($spacesBefore === 0) {
205-
$phpcsFile->fixer->addContentBefore($equalToken, $padding);
206-
} else {
207-
$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+
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, '');
231+
}
232+
233+
$phpcsFile->fixer->endChangeset();
208234
}
209235
}
210236
}//end if
211237

212238
$spacesAfter = 0;
213-
if ($tokens[($equalToken + 1)]['code'] === T_WHITESPACE) {
239+
if ($tokens[$equalToken]['line'] !== $tokens[$param['default_token']]['line']) {
240+
$spacesAfter = 'newline';
241+
} else if ($tokens[($equalToken + 1)]['code'] === T_WHITESPACE) {
214242
$spacesAfter = $tokens[($equalToken + 1)]['length'];
215243
}
216244

@@ -221,13 +249,22 @@ public function processBracket($phpcsFile, $openBracket)
221249
$spacesAfter,
222250
];
223251

224-
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
225-
if ($fix === true) {
226-
$padding = str_repeat(' ', $this->equalsSpacing);
227-
if ($spacesAfter === 0) {
252+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($equalToken + 1), $param['default_token'], true);
253+
if ($nextNonWhitespace !== false) {
254+
$phpcsFile->addError($error, $equalToken, 'SpaceAfterEquals', $data);
255+
} else {
256+
$fix = $phpcsFile->addFixableError($error, $equalToken, 'SpaceAfterEquals', $data);
257+
if ($fix === true) {
258+
$padding = str_repeat(' ', $this->equalsSpacing);
259+
260+
$phpcsFile->fixer->beginChangeset();
228261
$phpcsFile->fixer->addContent($equalToken, $padding);
229-
} else {
230-
$phpcsFile->fixer->replaceToken(($equalToken + 1), $padding);
262+
263+
for ($i = ($equalToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
264+
$phpcsFile->fixer->replaceToken($i, '');
265+
}
266+
267+
$phpcsFile->fixer->endChangeset();
231268
}
232269
}
233270
}//end if
@@ -275,18 +312,53 @@ public function processBracket($phpcsFile, $openBracket)
275312
}
276313

277314
if ($commaToken !== false) {
278-
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) {
279325
$error = 'Expected 0 spaces between argument "%s" and comma; %s found';
280326
$data = [
281327
$params[($paramNumber - 1)]['name'],
282-
$tokens[($commaToken - 1)]['length'],
328+
$spaceBeforeComma,
283329
];
284330

285331
$fix = $phpcsFile->addFixableError($error, $commaToken, 'SpaceBeforeComma', $data);
286332
if ($fix === true) {
287-
$phpcsFile->fixer->replaceToken(($commaToken - 1), '');
288-
}
289-
}
333+
$startOfCurrentParam = $phpcsFile->findNext(Tokens::$emptyTokens, ($commaToken + 1), null, true);
334+
335+
$phpcsFile->fixer->beginChangeset();
336+
$phpcsFile->fixer->addContent($endOfPreviousParam, ',');
337+
$phpcsFile->fixer->replaceToken($commaToken, '');
338+
339+
if ($tokens[$commaToken]['line'] === $tokens[$startOfCurrentParam]['line']) {
340+
for ($i = ($commaToken + 1); $tokens[$i]['code'] === T_WHITESPACE; $i++) {
341+
$phpcsFile->fixer->replaceToken($i, '');
342+
}
343+
} else {
344+
for ($i = ($commaToken - 1);
345+
$tokens[$i]['code'] === T_WHITESPACE && $tokens[$endOfPreviousParam]['line'] !== $tokens[$i]['line'];
346+
$i--
347+
) {
348+
$phpcsFile->fixer->replaceToken($i, '');
349+
}
350+
351+
for ($i = ($commaToken + 1);
352+
$tokens[$i]['code'] === T_WHITESPACE && $tokens[$commaToken]['line'] === $tokens[$i]['line'];
353+
$i++
354+
) {
355+
$phpcsFile->fixer->replaceToken($i, '');
356+
}
357+
}
358+
359+
$phpcsFile->fixer->endChangeset();
360+
}//end if
361+
}//end if
290362

291363
// Don't check spacing after the comma if it is the last content on the line.
292364
$checkComma = true;
@@ -324,10 +396,7 @@ public function processBracket($phpcsFile, $openBracket)
324396
}
325397
}//end if
326398
} else {
327-
$hint = $phpcsFile->getTokensAsString($param['type_hint_token'], (($param['type_hint_end_token'] - $param['type_hint_token']) + 1));
328-
if ($param['nullable_type'] === true) {
329-
$hint = '?'.$hint;
330-
}
399+
$hint = $param['type_hint'];
331400

332401
if ($tokens[($commaToken + 1)]['code'] !== T_WHITESPACE) {
333402
$error = 'Expected 1 space between comma and type hint "%s"; 0 found';

src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.inc renamed to src/Standards/Squiz/Tests/Functions/FunctionDeclarationArgumentSpacingUnitTest.1.inc

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ function functionName( ?string $arg1 = 'foo' , ?int & $arg2 , $arg3 ) {}
100100
function functionName(string $arg1, ... $arg2) {}
101101
function functionName(string $arg1, int ... $arg2) {}
102102
function functionName(string $arg1, & ... $arg2) {}
103-
103+
function functionName(string $arg1,int $arg2) {}
104104

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

115115
$number) {}
116+
117+
function spacingBetweenParenthesesShouldBeFixedInOneGo(
118+
119+
120+
) {}
121+
122+
function newlineAfterReferenceShouldBeFlaggedAndFixed(
123+
&
124+
125+
$param
126+
) {}
127+
128+
function newlineAfterReferenceFixerRespectsComment(
129+
&
130+
// comment
131+
$param
132+
) {}
133+
134+
function newlineAfterVariadicShouldBeFlaggedAndFixed(
135+
...
136+
137+
$param
138+
) {}
139+
140+
function newlineAfterVariadicFixerRespectsComment(
141+
...
142+
//comment
143+
$param
144+
) {}
145+
146+
function newlineBeforeAndAfterEqualsSignShouldBeFixedForSpacing0(
147+
$param
148+
149+
=
150+
151+
true
152+
) {}
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+
) {}
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
179+
180+
function newlineBeforeCommaShouldBeFixedInOneGo(
181+
$paramA
182+
,
183+
$paramB
184+
185+
,
186+
$paramC
187+
) {}
188+
189+
function newlineBeforeCommaFixerRespectsComments(
190+
$paramA // comment
191+
,
192+
$paramB=10 /* comment */
193+
,
194+
$paramC=20 # comment
195+
, $paramC=30
196+
, string $paramC='foo'
197+
) {}

0 commit comments

Comments
 (0)