Skip to content

Commit 604c25f

Browse files
committed
Squiz/FunctionDeclarationArgumentSpacing: improve SpacingBetween fixer
The fixer for the `SpacingBetween` error code would only handle one whitespace token at a time, which is inefficient and could also lead to more complex fixer conflicts. This commit changes the fixer to handle all whitespace between the parentheses in one go. For the code sample added as a test, this means the difference between the fixer needing 7 loops (including two skipped loops due to possible conflicts) versus 2 loops. <details> <summary>Old:</summary> ``` => Fixing file: 1/1 violations remaining Squiz.Functions.FunctionDeclarationArgumentSpacing:133 replaced token 6 (T_WHITESPACE on line 3) "\n\n" => "\n" => Fixing file: 1/1 violations remaining [made 1 pass]... * fixed 1 violations, starting loop 2 * Squiz.Functions.FunctionDeclarationArgumentSpacing:133 replaced token 6 (T_WHITESPACE on line 3) "\n\n" => "\n" => Fixing file: 1/1 violations remaining [made 2 passes]... * fixed 1 violations, starting loop 3 * **** Squiz.Functions.FunctionDeclarationArgumentSpacing:133 has possible conflict with another sniff on loop 1; caused by the following change **** **** replaced token 6 (T_WHITESPACE on line 3) "\n " => " " **** **** ignoring all changes until next loop **** => Fixing file: 0/1 violations remaining [made 3 passes]... * fixed 0 violations, starting loop 4 * Squiz.Functions.FunctionDeclarationArgumentSpacing:133 replaced token 6 (T_WHITESPACE on line 3) "\n " => " " => Fixing file: 1/1 violations remaining [made 4 passes]... * fixed 1 violations, starting loop 5 * **** Squiz.Functions.FunctionDeclarationArgumentSpacing:133 has possible conflict with another sniff on loop 3; caused by the following change **** **** replaced token 6 (T_WHITESPACE on line 3) " )" => ")" **** **** ignoring all changes until next loop **** => Fixing file: 0/1 violations remaining [made 5 passes]... * fixed 0 violations, starting loop 6 * Squiz.Functions.FunctionDeclarationArgumentSpacing:133 replaced token 6 (T_WHITESPACE on line 3) " )" => ")" => Fixing file: 1/1 violations remaining [made 6 passes]... * fixed 1 violations, starting loop 7 * => Fixing file: 0/1 violations remaining [made 7 passes]... ``` </details> <details> <summary>New:</summary> ``` => Fixing file: 1/1 violations remaining => Changeset started by Squiz.Functions.FunctionDeclarationArgumentSpacing:133 Q: Squiz.Functions.FunctionDeclarationArgumentSpacing:135 replaced token 6 (T_WHITESPACE on line 3) "\n\n" => "\n" Q: Squiz.Functions.FunctionDeclarationArgumentSpacing:135 replaced token 7 (T_WHITESPACE on line 4) "\n\n" => "\n" Q: Squiz.Functions.FunctionDeclarationArgumentSpacing:135 replaced token 8 (T_WHITESPACE on line 5) "\n " => " " Q: Squiz.Functions.FunctionDeclarationArgumentSpacing:135 replaced token 9 (T_WHITESPACE on line 6) " )" => ")" A: Squiz.Functions.FunctionDeclarationArgumentSpacing:137 replaced token 6 (T_WHITESPACE on line 3) "\n\n" => "\n" A: Squiz.Functions.FunctionDeclarationArgumentSpacing:137 replaced token 7 (T_WHITESPACE on line 4) "\n\n" => "\n" A: Squiz.Functions.FunctionDeclarationArgumentSpacing:137 replaced token 8 (T_WHITESPACE on line 5) "\n " => " " A: Squiz.Functions.FunctionDeclarationArgumentSpacing:137 replaced token 9 (T_WHITESPACE on line 6) " )" => ")" => Changeset ended: 4 changes applied => Fixing file: 4/1 violations remaining [made 1 pass]... * fixed 4 violations, starting loop 2 * => Fixing file: 0/1 violations remaining [made 2 passes]... ``` </details>
1 parent aebd84b commit 604c25f

File tree

4 files changed

+15
-2
lines changed

4 files changed

+15
-2
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,18 @@ 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) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,8 @@ fn ($a,$b = null) => $a($b);
113113
function multipleWhitespaceTokensAfterType(int
114114

115115
$number) {}
116+
117+
function spacingBetweenParenthesesShouldBeFixedInOneGo(
118+
119+
120+
) {}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,5 @@ $a = function ($var1, $var2=false) use (
111111
fn ($a, $b=null) => $a($b);
112112

113113
function multipleWhitespaceTokensAfterType(int $number) {}
114+
115+
function spacingBetweenParenthesesShouldBeFixedInOneGo() {}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public function getErrorList()
6969
107 => 2,
7070
111 => 3,
7171
113 => 1,
72+
117 => 1,
7273
];
7374

7475
}//end getErrorList()

0 commit comments

Comments
 (0)