Skip to content

Commit 29d0be3

Browse files
committed
Generic/InlineControlStructure: bail early for control structures without body
The sniff now consistently handles all supported control structures without a body by bailing early. Extending the existing behavior for `while` and `for` to also include `do while`, `else`, `elseif`, `if`, and `foreach`. Previously, the sniff would incorrectly flag these empty control structures as inline control structures that needed curly braces. For `else`, `elseif`, `if`, and `foreach`, the fixer would remove the semicolon and add the curly braces. For `do while`, the fixer would add the curly braces and keep the semicolon in between the braces. In all the cases, the resulting code was syntactically correct. Consider the following example: ``` do ; while ($foo < 5); ``` Previously, PHPCS would flag this as an inline control structure and PHPCBF would fix it to: ``` do { ; } while ($foo < 5); ``` Now an empty `do while` is ignored by the sniff (no warnings and no fixes). Here is a link showing that control structures without a body are valid in PHP: https://3v4l.org/slnYL And here is a link showing that the way that they were being fixed by PHPCBF was resulting in valid code (`while` and `for` are not included below as they were already ignored before this commit): https://3v4l.org/8k1N3
1 parent ad852ee commit 29d0be3

File tree

4 files changed

+61
-13
lines changed

4 files changed

+61
-13
lines changed

src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,25 @@ public function process(File $phpcsFile, $stackPtr)
7070
}
7171
}
7272

73-
if ($tokens[$stackPtr]['code'] === T_WHILE || $tokens[$stackPtr]['code'] === T_FOR) {
74-
// This could be from a DO WHILE, which doesn't have an opening brace or a while/for without body.
75-
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
76-
$afterParensCloser = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($tokens[$stackPtr]['parenthesis_closer'] + 1), null, true);
77-
if ($afterParensCloser === false) {
78-
// Live coding.
79-
return;
80-
}
73+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
74+
$nextTokenIndex = ($tokens[$stackPtr]['parenthesis_closer'] + 1);
75+
} else if (in_array($tokens[$stackPtr]['code'], [T_ELSE, T_DO], true) === true) {
76+
$nextTokenIndex = ($stackPtr + 1);
77+
}
8178

82-
if ($tokens[$afterParensCloser]['code'] === T_SEMICOLON) {
83-
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'no');
84-
return;
85-
}
79+
if (isset($nextTokenIndex) === true) {
80+
$firstNonEmptyToken = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, $nextTokenIndex, null, true);
81+
if ($firstNonEmptyToken === false) {
82+
// Live coding.
83+
return;
8684
}
87-
}//end if
85+
86+
if ($tokens[$firstNonEmptyToken]['code'] === T_SEMICOLON) {
87+
// This is a control structure without a body. Bow out.
88+
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'no');
89+
return;
90+
}
91+
}
8892

8993
if (isset($tokens[$stackPtr]['parenthesis_opener'], $tokens[$stackPtr]['parenthesis_closer']) === false
9094
&& $tokens[$stackPtr]['code'] !== T_ELSE

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,24 @@ function testFinally()
276276
if ($something) {
277277
echo 'hello';
278278
} else /* comment */ if ($somethingElse) echo 'hi';
279+
280+
if ($sniffShouldBailEarly);
281+
282+
if (false) {
283+
} elseif ($sniffShouldBailEarly);
284+
285+
if (false) {
286+
} else if ($sniffShouldBailEarly);
287+
288+
if (false) {
289+
} else ($sniffShouldGenerateError);
290+
291+
if (false) {
292+
} else; // Sniff should bail early.
293+
294+
foreach ($array as $sniffShouldBailEarly);
295+
296+
foreach ($array as $sniffShouldBailEarly)
297+
/* some comment */;
298+
299+
do ; while ($sniffShouldBailEarly > 5);

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,3 +314,25 @@ if ($something) {
314314
echo 'hello';
315315
} else /* comment */ if ($somethingElse) { echo 'hi';
316316
}
317+
318+
if ($sniffShouldBailEarly);
319+
320+
if (false) {
321+
} elseif ($sniffShouldBailEarly);
322+
323+
if (false) {
324+
} else if ($sniffShouldBailEarly);
325+
326+
if (false) {
327+
} else { ($sniffShouldGenerateError);
328+
}
329+
330+
if (false) {
331+
} else; // Sniff should bail early.
332+
333+
foreach ($array as $sniffShouldBailEarly);
334+
335+
foreach ($array as $sniffShouldBailEarly)
336+
/* some comment */;
337+
338+
do ; while ($sniffShouldBailEarly > 5);

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ public function getErrorList($testFile='')
8181
260 => 1,
8282
269 => 1,
8383
278 => 1,
84+
289 => 1,
8485
];
8586

8687
default:

0 commit comments

Comments
 (0)