Skip to content

Commit b498dbe

Browse files
gsherwoodgsherwood
committed
Fixed bug PHPCSStandards#879 : Generic InlineControlStructureSniff can create parse error when case/if/elseif/else have mixed brace and braceless definitions
1 parent 9a70ae2 commit b498dbe

File tree

6 files changed

+126
-83
lines changed

6 files changed

+126
-83
lines changed

‎CodeSniffer/File.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1948,6 +1948,20 @@ private static function _recurseScopeMap(
19481948
return $i;
19491949
}
19501950

1951+
if ($opener === null
1952+
&& $ignore === 0
1953+
&& $tokenType === T_CLOSE_CURLY_BRACKET
1954+
&& isset($tokenizer->scopeOpeners[$currType]['end'][$tokenType]) === true
1955+
) {
1956+
if (PHP_CODESNIFFER_VERBOSITY > 1) {
1957+
$type = $tokens[$stackPtr]['type'];
1958+
echo str_repeat("\t", $depth);
1959+
echo "=> Found curly brace closer before scope opener for $stackPtr:$type, bailing".PHP_EOL;
1960+
}
1961+
1962+
return ($i - 1);
1963+
}
1964+
19511965
if ($opener !== null
19521966
&& (isset($tokens[$i]['scope_opener']) === false
19531967
|| $tokenizer->scopeOpeners[$tokens[$stackPtr]['code']]['shared'] === true)
@@ -2116,7 +2130,7 @@ private static function _recurseScopeMap(
21162130
echo "=> Found new opening condition before scope opener for $stackPtr:$type, bailing".PHP_EOL;
21172131
}
21182132

2119-
return $stackPtr;
2133+
return ($i - 1);
21202134
}//end if
21212135

21222136
if (PHP_CODESNIFFER_VERBOSITY > 1) {

‎CodeSniffer/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php‎

Lines changed: 85 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -134,107 +134,110 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
134134

135135
$phpcsFile->recordMetric($stackPtr, 'Control structure defined inline', 'yes');
136136

137-
if ($fix === true) {
138-
$phpcsFile->fixer->beginChangeset();
139-
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
140-
$closer = $tokens[$stackPtr]['parenthesis_closer'];
141-
} else {
142-
$closer = $stackPtr;
137+
// Stop here if we are not fixing the error.
138+
if ($fix !== true) {
139+
return;
140+
}
141+
142+
$phpcsFile->fixer->beginChangeset();
143+
if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
144+
$closer = $tokens[$stackPtr]['parenthesis_closer'];
145+
} else {
146+
$closer = $stackPtr;
147+
}
148+
149+
if ($tokens[($closer + 1)]['code'] === T_WHITESPACE
150+
|| $tokens[($closer + 1)]['code'] === T_SEMICOLON
151+
) {
152+
$phpcsFile->fixer->addContent($closer, ' {');
153+
} else {
154+
$phpcsFile->fixer->addContent($closer, ' { ');
155+
}
156+
157+
$lastNonEmpty = $closer;
158+
for ($end = ($closer + 1); $end < $phpcsFile->numTokens; $end++) {
159+
if ($tokens[$end]['code'] === T_SEMICOLON) {
160+
break;
143161
}
144162

145-
if ($tokens[($closer + 1)]['code'] === T_WHITESPACE
146-
|| $tokens[($closer + 1)]['code'] === T_SEMICOLON
147-
) {
148-
$phpcsFile->fixer->addContent($closer, ' {');
149-
} else {
150-
$phpcsFile->fixer->addContent($closer, ' { ');
163+
if ($tokens[$end]['code'] === T_CLOSE_TAG) {
164+
$end = $lastNonEmpty;
165+
break;
151166
}
152167

153-
$lastNonEmpty = $closer;
154-
for ($end = ($closer + 1); $end < $phpcsFile->numTokens; $end++) {
155-
if ($tokens[$end]['code'] === T_SEMICOLON) {
156-
break;
157-
}
168+
if (isset($tokens[$end]['scope_opener']) === true) {
169+
$type = $tokens[$end]['code'];
170+
$end = $tokens[$end]['scope_closer'];
171+
if ($type === T_DO || $type === T_IF || $type === T_ELSEIF) {
172+
$next = $phpcsFile->findNext(PHP_CodeSniffer_Tokens::$emptyTokens, ($end + 1), null, true);
173+
if ($next === false) {
174+
break;
175+
}
158176

159-
if ($tokens[$end]['code'] === T_CLOSE_TAG) {
160-
$end = $lastNonEmpty;
161-
break;
162-
}
177+
$nextType = $tokens[$next]['code'];
163178

164-
if (isset($tokens[$end]['scope_opener']) === true) {
165-
$type = $tokens[$end]['code'];
166-
$end = $tokens[$end]['scope_closer'];
167-
if ($type === T_DO || $type === T_IF || $type === T_ELSEIF) {
168-
$next = $phpcsFile->findNext(PHP_CodeSniffer_Tokens::$emptyTokens, ($end + 1), null, true);
169-
if ($next === false) {
170-
break;
171-
}
172-
173-
$nextType = $tokens[$next]['code'];
174-
175-
// Let additional conditions loop and find their ending.
176-
if (($type === T_IF
177-
|| $type === T_ELSEIF)
178-
&& ($nextType === T_ELSEIF
179-
|| $nextType === T_ELSE)
180-
) {
181-
continue;
182-
}
183-
184-
// Account for DO... WHILE conditions.
185-
if ($type === T_DO && $nextType === T_WHILE) {
186-
$end = $phpcsFile->findNext(T_SEMICOLON, ($next + 1));
187-
}
188-
}//end if
189-
190-
break;
191-
}//end if
179+
// Let additional conditions loop and find their ending.
180+
if (($type === T_IF
181+
|| $type === T_ELSEIF)
182+
&& ($nextType === T_ELSEIF
183+
|| $nextType === T_ELSE)
184+
) {
185+
continue;
186+
}
192187

193-
if ($tokens[$end]['code'] !== T_WHITESPACE) {
194-
$lastNonEmpty = $end;
195-
}
196-
}//end for
188+
// Account for DO... WHILE conditions.
189+
if ($type === T_DO && $nextType === T_WHILE) {
190+
$end = $phpcsFile->findNext(T_SEMICOLON, ($next + 1));
191+
}
192+
}//end if
197193

198-
$next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), ($end + 1), true);
194+
break;
195+
}//end if
199196

200-
// Account for a comment on the end of the line.
201-
for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
202-
if (isset($tokens[($endLine + 1)]) === false
203-
|| $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
204-
) {
205-
break;
206-
}
197+
if ($tokens[$end]['code'] !== T_WHITESPACE) {
198+
$lastNonEmpty = $end;
207199
}
200+
}//end for
208201

209-
if ($tokens[$endLine]['code'] !== T_COMMENT) {
210-
$endLine = $end;
202+
$next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), ($end + 1), true);
203+
204+
// Account for a comment on the end of the line.
205+
for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
206+
if (isset($tokens[($endLine + 1)]) === false
207+
|| $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
208+
) {
209+
break;
211210
}
211+
}
212212

213-
if ($next !== $end) {
214-
if ($endLine !== $end) {
215-
$phpcsFile->fixer->addContent($endLine, '}');
216-
} else {
217-
if ($tokens[$end]['code'] !== T_SEMICOLON
218-
&& $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
219-
) {
220-
$phpcsFile->fixer->addContent($end, ';');
221-
}
213+
if ($tokens[$endLine]['code'] !== T_COMMENT) {
214+
$endLine = $end;
215+
}
222216

223-
$phpcsFile->fixer->addContent($end, ' }');
224-
}
217+
if ($next !== $end) {
218+
if ($endLine !== $end) {
219+
$phpcsFile->fixer->addContent($endLine, '}');
225220
} else {
226-
if ($endLine !== $end) {
227-
$phpcsFile->fixer->replaceToken($end, '');
228-
$phpcsFile->fixer->addNewlineBefore($endLine);
229-
$phpcsFile->fixer->addContent($endLine, '}');
230-
} else {
231-
$phpcsFile->fixer->replaceToken($end, '}');
221+
if ($tokens[$end]['code'] !== T_SEMICOLON
222+
&& $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
223+
) {
224+
$phpcsFile->fixer->addContent($end, ';');
232225
}
233-
}//end if
234226

235-
$phpcsFile->fixer->endChangeset();
227+
$phpcsFile->fixer->addContent($end, ' }');
228+
}
229+
} else {
230+
if ($endLine !== $end) {
231+
$phpcsFile->fixer->replaceToken($end, '');
232+
$phpcsFile->fixer->addNewlineBefore($endLine);
233+
$phpcsFile->fixer->addContent($endLine, '}');
234+
} else {
235+
$phpcsFile->fixer->replaceToken($end, '}');
236+
}
236237
}//end if
237238

239+
$phpcsFile->fixer->endChangeset();
240+
238241
}//end process()
239242

240243

‎CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,15 @@ foreach($stuff as $num)
142142
if ($foo) echo 'foo';
143143
elseif ($bar) echo 'bar';
144144
else echo 'baz';
145+
146+
switch ($type) {
147+
case 1:
148+
if ($foo) {
149+
return true;
150+
} elseif ($baz)
151+
return true;
152+
else {
153+
echo 'else';
154+
}
155+
break;
156+
}

‎CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc.fixed‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,15 @@ foreach($stuff as $num) {
143143
if ($foo) { echo 'foo'; }
144144
elseif ($bar) { echo 'bar'; }
145145
else { echo 'baz'; }
146+
147+
switch ($type) {
148+
case 1:
149+
if ($foo) {
150+
return true;
151+
} elseif ($baz) {
152+
return true; }
153+
else {
154+
echo 'else';
155+
}
156+
break;
157+
}

‎CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public function getErrorList($testFile='InlineControlStructureUnitTest.inc')
6868
142 => 1,
6969
143 => 1,
7070
144 => 1,
71+
150 => 1,
7172
);
7273
break;
7374
case 'InlineControlStructureUnitTest.js':

‎package.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
5353
- Squiz FunctionCommentThrowTagSniff now ignores non-docblock comments
5454
- Squiz ComparisonOperatorUsageSniff now allows conditions like while(true)
5555
- Fixed bug #872 : Incorrect detection of blank lines between CSS class names
56+
- Fixed bug #879 : Generic InlineControlStructureSniff can create parse error when case/if/elseif/else have mixed brace and braceless definitions
5657
</notes>
5758
<contents>
5859
<dir name="/">

0 commit comments

Comments
 (0)