Skip to content

Commit 2e875e7

Browse files
committed
Generic/InlineControlStructure: bail early for control structures without body
The sniff now consistently handles all supported control structures without a body, in both PHP and JS, by bailing early. Extending the existing behavior for `while` and `for` to also include `do while`, `else`, `elseif`, `if`, and `foreach` (PHP only). Previously, the sniff would incorrectly flag these empty control structures as inline control structures that needed curly braces.
1 parent 9feb4e5 commit 2e875e7

6 files changed

+97
-13
lines changed

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

+17-13
Original file line numberDiff line numberDiff line change
@@ -80,21 +80,25 @@ public function process(File $phpcsFile, $stackPtr)
8080
}
8181
}
8282

83-
if ($tokens[$stackPtr]['code'] === T_WHILE || $tokens[$stackPtr]['code'] === T_FOR) {
84-
// This could be from a DO WHILE, which doesn't have an opening brace or a while/for without body.
85-
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
86-
$afterParensCloser = $phpcsFile->findNext(Tokens::$emptyTokens, ($tokens[$stackPtr]['parenthesis_closer'] + 1), null, true);
87-
if ($afterParensCloser === false) {
88-
// Live coding.
89-
return;
90-
}
83+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
84+
$nextTokenIndex = ($tokens[$stackPtr]['parenthesis_closer'] + 1);
85+
} else if (in_array($tokens[$stackPtr]['code'], [T_ELSE, T_DO], true) === true) {
86+
$nextTokenIndex = ($stackPtr + 1);
87+
}
9188

92-
if ($tokens[$afterParensCloser]['code'] === T_SEMICOLON) {
93-
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'no');
94-
return;
95-
}
89+
if (isset($nextTokenIndex) === true) {
90+
$firstNonEmptyToken = $phpcsFile->findNext(Tokens::$emptyTokens, $nextTokenIndex, null, true);
91+
if ($firstNonEmptyToken === false) {
92+
// Live coding.
93+
return;
9694
}
97-
}//end if
95+
96+
if ($tokens[$firstNonEmptyToken]['code'] === T_SEMICOLON) {
97+
// This is a control structure without a body. Bow out.
98+
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'no');
99+
return;
100+
}
101+
}
98102

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

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

+21
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

+22
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.1.js

+17
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,20 @@ if ($("#myid").rotationDegrees()=='90')
3333
if (something) {
3434
alert('hello');
3535
} else /* comment */ if (somethingElse) alert('hi');
36+
37+
if (sniffShouldBailEarly);
38+
39+
if (false) {
40+
} else if (sniffShouldBailEarly);
41+
42+
if (false) {
43+
} else (sniffShouldGenerateError);
44+
45+
if (false) {
46+
} else; // Sniff should bail early.
47+
48+
while (sniffShouldBailEarly);
49+
50+
for (sniffShouldBailEarly; sniffShouldBailEarly > 0; sniffShouldBailEarly--);
51+
52+
do ; while ($sniffShouldBailEarly > 5);

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

+18
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,21 @@ if (something) {
4242
alert('hello');
4343
} else /* comment */ if (somethingElse) { alert('hi');
4444
}
45+
46+
if (sniffShouldBailEarly);
47+
48+
if (false) {
49+
} else if (sniffShouldBailEarly);
50+
51+
if (false) {
52+
} else { (sniffShouldGenerateError);
53+
}
54+
55+
if (false) {
56+
} else; // Sniff should bail early.
57+
58+
while (sniffShouldBailEarly);
59+
60+
for (sniffShouldBailEarly; sniffShouldBailEarly > 0; sniffShouldBailEarly--);
61+
62+
do ; while ($sniffShouldBailEarly > 5);

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

+2
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
case 'InlineControlStructureUnitTest.1.js':
@@ -94,6 +95,7 @@ public function getErrorList($testFile='')
9495
27 => 1,
9596
30 => 1,
9697
35 => 1,
98+
43 => 1,
9799
];
98100

99101
default:

0 commit comments

Comments
 (0)