Skip to content

Commit aa71bb7

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 `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 aa71bb7

File tree

6 files changed

+84
-5
lines changed

6 files changed

+84
-5
lines changed

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,23 @@ 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.
83+
if ($tokens[$stackPtr]['code'] !== T_DO) {
84+
// This could be from a DO WHILE, which doesn't have an opening brace, or an
85+
// else/elseif/if/for/foreach/while without body.
8586
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
86-
$afterParensCloser = $phpcsFile->findNext(Tokens::$emptyTokens, ($tokens[$stackPtr]['parenthesis_closer'] + 1), null, true);
87-
if ($afterParensCloser === false) {
87+
$nextTokenIndex = ($tokens[$stackPtr]['parenthesis_closer'] + 1);
88+
} else if ($tokens[$stackPtr]['code'] === T_ELSE) {
89+
$nextTokenIndex = ($stackPtr + 1);
90+
}
91+
92+
if (isset($nextTokenIndex) === true) {
93+
$afterElseOrParensCloser = $phpcsFile->findNext(Tokens::$emptyTokens, $nextTokenIndex, null, true);
94+
if ($afterElseOrParensCloser === false) {
8895
// Live coding.
8996
return;
9097
}
9198

92-
if ($tokens[$afterParensCloser]['code'] === T_SEMICOLON) {
99+
if ($tokens[$afterElseOrParensCloser]['code'] === T_SEMICOLON) {
93100
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'no');
94101
return;
95102
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,22 @@ 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 */;

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,3 +314,23 @@ 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 */;

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,18 @@ 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--);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,19 @@ 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--);

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

Lines changed: 2 additions & 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
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)