Skip to content

Commit 702cae9

Browse files
authored
feat(ControlSignature): Check finally keyword spacing (#3559471)
1 parent e0a63c5 commit 702cae9

File tree

5 files changed

+193
-88
lines changed

5 files changed

+193
-88
lines changed

.github/workflows/testing.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,4 @@ jobs:
100100
sed -i 's/Drupal.Methods.MethodDeclaration/PSR2.Methods.MethodDeclaration/g' phpcs.xml.dist
101101
sed -i '/<rule ref="Drupal.WhiteSpace.Namespace"\/>/d' phpcs.xml.dist
102102
sed -i 's/Drupal.Classes.InterfaceName/Generic.NamingConventions.InterfaceNameSuffix/g' phpcs.xml.dist
103-
../../vendor/bin/phpcs -p -s --parallel=$(nproc) --ignore=lib/Drupal/Core/Command/GenerateTheme.php,modules/mysql/tests/src/Kernel/mysql/Console/DbDumpCommandTest.php,modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php,tests/fixtures/plugins/CustomPlugin.php,modules/package_manager/tests/modules/package_manager_test_api/src/ApiController.php,modules/views/tests/src/Kernel/Plugin/StyleGridTest.php
103+
../../vendor/bin/phpcs -p -s --parallel=$(nproc) --exclude=Drupal.ControlStructures.ControlSignature --ignore=lib/Drupal/Core/Command/GenerateTheme.php,modules/mysql/tests/src/Kernel/mysql/Console/DbDumpCommandTest.php,modules/big_pipe/tests/modules/big_pipe_test/src/BigPipePlaceholderTestCases.php,tests/fixtures/plugins/CustomPlugin.php,modules/package_manager/tests/modules/package_manager_test_api/src/ApiController.php,modules/views/tests/src/Kernel/Plugin/StyleGridTest.php

coder_sniffer/Drupal/Sniffs/ControlStructures/ControlSignatureSniff.php

Lines changed: 161 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,25 @@
2727
class ControlSignatureSniff implements Sniff
2828
{
2929

30+
/**
31+
* How many spaces should precede the colon if using alternative syntax.
32+
*
33+
* @var integer
34+
*/
35+
public $requiredSpacesBeforeColon = 0;
36+
3037

3138
/**
3239
* Returns an array of tokens this test wants to listen for.
3340
*
34-
* @return int[]
41+
* @return array<int|string>
3542
*/
3643
public function register()
3744
{
3845
return [
3946
T_TRY,
4047
T_CATCH,
48+
T_FINALLY,
4149
T_DO,
4250
T_WHILE,
4351
T_FOR,
@@ -46,6 +54,7 @@ public function register()
4654
T_ELSE,
4755
T_ELSEIF,
4856
T_SWITCH,
57+
T_MATCH,
4958
];
5059
}
5160

@@ -59,39 +68,62 @@ public function register()
5968
*
6069
* @return void
6170
*/
62-
public function process(File $phpcsFile, $stackPtr)
71+
public function process(File $phpcsFile, int $stackPtr)
6372
{
6473
$tokens = $phpcsFile->getTokens();
6574

66-
if (isset($tokens[($stackPtr + 1)]) === false) {
75+
$nextNonEmpty = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($stackPtr + 1), null, true);
76+
if ($nextNonEmpty === false) {
6777
return;
6878
}
6979

80+
$isAlternative = false;
81+
if (isset($tokens[$stackPtr]['scope_opener']) === true
82+
&& $tokens[$tokens[$stackPtr]['scope_opener']]['code'] === T_COLON
83+
) {
84+
$isAlternative = true;
85+
}
86+
7087
// Single space after the keyword.
88+
$expected = 1;
89+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === false && $isAlternative === true) {
90+
// Catching cases like:
91+
// if (condition) : ... else: ... endif
92+
// where there is no condition.
93+
$expected = (int) $this->requiredSpacesBeforeColon;
94+
}
95+
7196
$found = 1;
7297
if ($tokens[($stackPtr + 1)]['code'] !== T_WHITESPACE) {
7398
$found = 0;
7499
} elseif ($tokens[($stackPtr + 1)]['content'] !== ' ') {
75100
if (strpos($tokens[($stackPtr + 1)]['content'], $phpcsFile->eolChar) !== false) {
76101
$found = 'newline';
77102
} else {
78-
$found = strlen($tokens[($stackPtr + 1)]['content']);
103+
$found = $tokens[($stackPtr + 1)]['length'];
79104
}
80105
}
81106

82-
if ($found !== 1) {
83-
$error = 'Expected 1 space after %s keyword; %s found';
107+
if ($found !== $expected) {
108+
$pluralizeSpace = 's';
109+
if ($expected === 1) {
110+
$pluralizeSpace = '';
111+
}
112+
113+
$error = 'Expected %s space%s after %s keyword; %s found';
84114
$data = [
115+
$expected,
116+
$pluralizeSpace,
85117
strtoupper($tokens[$stackPtr]['content']),
86118
$found,
87119
];
88120

89121
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceAfterKeyword', $data);
90122
if ($fix === true) {
91123
if ($found === 0) {
92-
$phpcsFile->fixer->addContent($stackPtr, ' ');
124+
$phpcsFile->fixer->addContent($stackPtr, str_repeat(' ', $expected));
93125
} else {
94-
$phpcsFile->fixer->replaceToken(($stackPtr + 1), ' ');
126+
$phpcsFile->fixer->replaceToken(($stackPtr + 1), str_repeat(' ', $expected));
95127
}
96128
}
97129
}
@@ -100,41 +132,71 @@ public function process(File $phpcsFile, $stackPtr)
100132
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true
101133
&& isset($tokens[$stackPtr]['scope_opener']) === true
102134
) {
135+
$expected = 1;
136+
if ($isAlternative === true) {
137+
$expected = (int) $this->requiredSpacesBeforeColon;
138+
}
139+
103140
$closer = $tokens[$stackPtr]['parenthesis_closer'];
104141
$opener = $tokens[$stackPtr]['scope_opener'];
105142
$content = $phpcsFile->getTokensAsString(($closer + 1), ($opener - $closer - 1));
106143

107-
if ($content !== ' ') {
108-
$error = 'Expected 1 space after closing parenthesis; found %s';
109-
if (trim($content) === '') {
110-
$found = strlen($content);
144+
if (trim($content) === '') {
145+
if (strpos($content, $phpcsFile->eolChar) !== false) {
146+
$found = 'newline';
111147
} else {
112-
$found = '"' . str_replace($phpcsFile->eolChar, '\n', $content) . '"';
148+
$found = strlen($content);
149+
}
150+
} else {
151+
$found = '"' . str_replace($phpcsFile->eolChar, '\n', $content) . '"';
152+
}
153+
154+
if ($found !== $expected) {
155+
$pluralizeSpace = 's';
156+
if ($expected === 1) {
157+
$pluralizeSpace = '';
113158
}
114159

115-
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceAfterCloseParenthesis', [$found]);
160+
$error = 'Expected %s space%s after closing parenthesis; found %s';
161+
$data = [
162+
$expected,
163+
$pluralizeSpace,
164+
$found,
165+
];
166+
167+
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceAfterCloseParenthesis', $data);
116168
if ($fix === true) {
169+
$padding = str_repeat(' ', $expected);
117170
if ($closer === ($opener - 1)) {
118-
$phpcsFile->fixer->addContent($closer, ' ');
171+
$phpcsFile->fixer->addContent($closer, $padding);
119172
} else {
120173
$phpcsFile->fixer->beginChangeset();
121-
$phpcsFile->fixer->addContent($closer, ' ' . $tokens[$opener]['content']);
122-
$phpcsFile->fixer->replaceToken($opener, '');
123-
124-
if ($tokens[$opener]['line'] !== $tokens[$closer]['line']) {
125-
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
126-
if ($tokens[$next]['line'] !== $tokens[$opener]['line']) {
127-
for ($i = ($opener + 1); $i < $next; $i++) {
174+
if (trim($content) === '') {
175+
$phpcsFile->fixer->addContent($closer, $padding);
176+
if ($found !== 0) {
177+
for ($i = ($closer + 1); $i < $opener; $i++) {
128178
$phpcsFile->fixer->replaceToken($i, '');
129179
}
130180
}
181+
} else {
182+
$phpcsFile->fixer->addContent($closer, $padding . $tokens[$opener]['content']);
183+
$phpcsFile->fixer->replaceToken($opener, '');
184+
185+
if ($tokens[$opener]['line'] !== $tokens[$closer]['line']) {
186+
$next = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), null, true);
187+
if ($tokens[$next]['line'] !== $tokens[$opener]['line']) {
188+
for ($i = ($opener + 1); $i < $next; $i++) {
189+
$phpcsFile->fixer->replaceToken($i, '');
190+
}
191+
}
192+
}
131193
}
132194

133195
$phpcsFile->fixer->endChangeset();
134196
}
135197
}
136-
}//end if
137-
}//end if
198+
}
199+
}
138200

139201
// Single newline after opening brace.
140202
if (isset($tokens[$stackPtr]['scope_opener']) === true) {
@@ -160,7 +222,7 @@ public function process(File $phpcsFile, $stackPtr)
160222
// We found the first bit of a code, or a comment on the
161223
// following line.
162224
break;
163-
}//end for
225+
}
164226

165227
if ($tokens[$next]['line'] === $tokens[$opener]['line']) {
166228
$error = 'Newline required after opening brace';
@@ -179,92 +241,105 @@ public function process(File $phpcsFile, $stackPtr)
179241
$phpcsFile->fixer->addContent($opener, $phpcsFile->eolChar);
180242
$phpcsFile->fixer->endChangeset();
181243
}
182-
}//end if
244+
}
183245
} elseif ($tokens[$stackPtr]['code'] === T_WHILE) {
184-
// Zero spaces after parenthesis closer.
185-
$closer = $tokens[$stackPtr]['parenthesis_closer'];
186-
$found = 0;
187-
if ($tokens[($closer + 1)]['code'] === T_WHITESPACE) {
188-
if (strpos($tokens[($closer + 1)]['content'], $phpcsFile->eolChar) !== false) {
189-
$found = 'newline';
190-
} else {
191-
$found = strlen($tokens[($closer + 1)]['content']);
246+
// Zero spaces after parenthesis closer, but only if followed by a semicolon.
247+
$closer = $tokens[$stackPtr]['parenthesis_closer'];
248+
$nextNonEmpty = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($closer + 1), null, true);
249+
if ($nextNonEmpty !== false && $tokens[$nextNonEmpty]['code'] === T_SEMICOLON) {
250+
$found = 0;
251+
if ($tokens[($closer + 1)]['code'] === T_WHITESPACE) {
252+
if (strpos($tokens[($closer + 1)]['content'], $phpcsFile->eolChar) !== false) {
253+
$found = 'newline';
254+
} else {
255+
$found = $tokens[($closer + 1)]['length'];
256+
}
192257
}
193-
}
194258

195-
if ($found !== 0) {
196-
$error = 'Expected 0 spaces before semicolon; %s found';
197-
$data = [$found];
198-
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceBeforeSemicolon', $data);
199-
if ($fix === true) {
200-
$phpcsFile->fixer->replaceToken(($closer + 1), '');
259+
if ($found !== 0) {
260+
$error = 'Expected 0 spaces before semicolon; %s found';
261+
$data = [$found];
262+
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceBeforeSemicolon', $data);
263+
if ($fix === true) {
264+
$phpcsFile->fixer->replaceToken(($closer + 1), '');
265+
}
201266
}
202267
}
203-
}//end if
268+
}
204269

205270
// Only want to check multi-keyword structures from here on.
206-
if ($tokens[$stackPtr]['code'] === T_DO) {
207-
$closer = false;
208-
if (isset($tokens[$stackPtr]['scope_closer']) === true) {
209-
$closer = $tokens[$stackPtr]['scope_closer'];
271+
if ($tokens[$stackPtr]['code'] === T_WHILE) {
272+
if (isset($tokens[$stackPtr]['scope_closer']) !== false) {
273+
return;
274+
}
275+
276+
$closer = $phpcsFile->findPrevious(Tokens::EMPTY_TOKENS, ($stackPtr - 1), null, true);
277+
if ($closer === false
278+
|| $tokens[$closer]['code'] !== T_CLOSE_CURLY_BRACKET
279+
|| $tokens[$tokens[$closer]['scope_condition']]['code'] !== T_DO
280+
) {
281+
return;
210282
}
211283
} elseif ($tokens[$stackPtr]['code'] === T_ELSE
212284
|| $tokens[$stackPtr]['code'] === T_ELSEIF
213285
|| $tokens[$stackPtr]['code'] === T_CATCH
286+
|| $tokens[$stackPtr]['code'] === T_FINALLY
214287
) {
288+
if (isset($tokens[$stackPtr]['scope_opener']) === true
289+
&& $tokens[$tokens[$stackPtr]['scope_opener']]['code'] === T_COLON
290+
) {
291+
// Special case for alternate syntax, where this token is actually
292+
// the closer for the previous block, so there is no spacing to check.
293+
return;
294+
}
295+
215296
$closer = $phpcsFile->findPrevious(Tokens::EMPTY_TOKENS, ($stackPtr - 1), null, true);
216297
if ($closer === false || $tokens[$closer]['code'] !== T_CLOSE_CURLY_BRACKET) {
217298
return;
218299
}
219300
} else {
220301
return;
221-
}//end if
302+
}
222303

223-
if ($tokens[$stackPtr]['code'] === T_DO) {
224-
// Single space after closing brace.
225-
$found = 1;
226-
if ($tokens[($closer + 1)]['code'] !== T_WHITESPACE) {
227-
$found = 0;
228-
} elseif ($tokens[($closer + 1)]['content'] !== ' ') {
229-
if (strpos($tokens[($closer + 1)]['content'], $phpcsFile->eolChar) !== false) {
230-
$found = 'newline';
231-
} else {
232-
$found = strlen($tokens[($closer + 1)]['content']);
233-
}
304+
// Single space after closing brace.
305+
$found = 1;
306+
if ($tokens[($closer + 1)]['code'] !== T_WHITESPACE) {
307+
$found = 0;
308+
} elseif ($tokens[$closer]['line'] !== $tokens[$stackPtr]['line']) {
309+
$found = 'newline';
310+
} elseif ($tokens[($closer + 1)]['content'] !== ' ') {
311+
$found = $tokens[($closer + 1)]['length'];
312+
}
313+
314+
if ($tokens[$stackPtr]['code'] === T_WHILE && $found !== 1) {
315+
$error = 'Expected 1 space after closing brace; %s found';
316+
$data = [$found];
317+
318+
if ($phpcsFile->findNext(Tokens::COMMENT_TOKENS, ($closer + 1), $stackPtr) !== false) {
319+
// Comment found between closing brace and keyword, don't auto-fix.
320+
$phpcsFile->addError($error, $closer, 'SpaceAfterCloseBrace', $data);
321+
return;
234322
}
235323

236-
if ($found !== 1) {
237-
$error = 'Expected 1 space after closing brace; %s found';
238-
$data = [$found];
239-
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceAfterCloseBrace', $data);
240-
if ($fix === true) {
241-
if ($found === 0) {
242-
$phpcsFile->fixer->addContent($closer, ' ');
243-
} else {
244-
$phpcsFile->fixer->replaceToken(($closer + 1), ' ');
245-
}
324+
$fix = $phpcsFile->addFixableError($error, $closer, 'SpaceAfterCloseBrace', $data);
325+
if ($fix === true) {
326+
if ($found === 0) {
327+
$phpcsFile->fixer->addContent($closer, ' ');
328+
} else {
329+
$phpcsFile->fixer->replaceToken(($closer + 1), ' ');
246330
}
247331
}
248-
} else {
332+
} elseif ($tokens[$stackPtr]['code'] !== T_WHILE && $found !== 'newline') {
249333
// New line after closing brace.
250-
$found = 'newline';
251-
if ($tokens[($closer + 1)]['code'] !== T_WHITESPACE) {
252-
$found = 'none';
253-
} elseif (strpos($tokens[($closer + 1)]['content'], "\n") === false) {
254-
$found = 'spaces';
255-
}
256-
257-
if ($found !== 'newline') {
258-
$error = 'Expected newline after closing brace';
259-
$fix = $phpcsFile->addFixableError($error, $closer, 'NewlineAfterCloseBrace');
260-
if ($fix === true) {
261-
if ($found === 'none') {
262-
$phpcsFile->fixer->addContent($closer, "\n");
263-
} else {
264-
$phpcsFile->fixer->replaceToken(($closer + 1), "\n");
265-
}
334+
$error = 'Expected newline after closing brace';
335+
$fix = $phpcsFile->addFixableError($error, $closer, 'NewlineAfterCloseBrace');
336+
if ($fix === true) {
337+
if ($found === 0) {
338+
$phpcsFile->fixer->addContent($closer, "\n");
339+
} else {
340+
$phpcsFile->fixer->replaceToken(($closer + 1), "\n");
266341
}
267342
}
268-
}//end if
343+
}
269344
}
270345
}

tests/Drupal/bad/BadUnitTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,12 @@ protected function getErrorList(string $testFile): array
389389
16 => 1,
390390
31 => 1,
391391
];
392+
case 'FinallySpacingUnitTest.inc':
393+
return [
394+
1 => 1,
395+
5 => 1,
396+
7 => 1,
397+
];
392398
case 'NamespaceUnitTest.inc':
393399
return [
394400
1 => 1,
@@ -540,4 +546,4 @@ protected function shouldSkipTest()
540546
}
541547

542548

543-
}
549+
}

0 commit comments

Comments
 (0)