Skip to content

Commit 6327f77

Browse files
jrfnljrfnl
committed
Tokenizer/PHP: PHP close tag should be regarded as scope opener for switch case/default statements
In PHP, if the statement before a PHP close tag is "unclosed", PHP will automagically insert a semi-colon before the PHP close tag when compiling the file. As `case` and `default` condition statements in a `switch` structure can be "closed" with a semi-colon, this will not result in a parse error, but will be perfectly valid code. However, the practice of "closing" `case`/`default` statements with a semi-colon is deprecated as of PHP 8.5 and while looking into that deprecation, I realized that the deprecation also applies to the "implied"/automagically inserted semi-colon when a `case`/`default` statement ends on a PHP close tag. Digging deeper, it turned out that a PHP close tag was not supported in PHPCS as a scope opener for `case`/`default` statements, which in turn meant that if that type of code was seen by sniffs, all sorts of false positives/negatives and potential `Undefined array key "scope_opener/closer"` notices would start to get thrown. This commit fixes the tokenizer part of the issue and adds tests with a variety of scope openers for `case`/`default` statements to a number of Tokenizer related test files.
1 parent 9397930 commit 6327f77

9 files changed

+158
-16
lines changed

‎src/Tokenizers/PHP.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ class PHP extends Tokenizer
259259
'start' => [
260260
T_COLON => T_COLON,
261261
T_SEMICOLON => T_SEMICOLON,
262+
T_CLOSE_TAG => T_CLOSE_TAG,
262263
],
263264
'end' => [
264265
T_BREAK => T_BREAK,
@@ -280,6 +281,7 @@ class PHP extends Tokenizer
280281
'start' => [
281282
T_COLON => T_COLON,
282283
T_SEMICOLON => T_SEMICOLON,
284+
T_CLOSE_TAG => T_CLOSE_TAG,
283285
],
284286
'end' => [
285287
T_BREAK => T_BREAK,

‎tests/Core/Tokenizers/PHP/DefaultKeywordTest.inc‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,47 @@ function switchWithDefaultAndCurlies($i) {
3434
}
3535
}
3636

37+
function switchWithDefaultAndSemicolon($i) {
38+
switch ($i) {
39+
case 1;
40+
return 1;
41+
case 2;
42+
return 2;
43+
/* testSimpleSwitchDefaultAndSemicolon */
44+
default;
45+
return 'default';
46+
}
47+
}
48+
49+
function switchWithDefaultSemicolonAndCurlies($i) {
50+
switch ($i) {
51+
case 1; {
52+
return 1;
53+
}
54+
case 2 ; {
55+
return 2;
56+
}
57+
/* testSimpleSwitchDefaultSemicolonAndCurlies */
58+
default; {
59+
return 'default';
60+
}
61+
}
62+
}
63+
64+
function switchWithDefaultAndCloseTag($i) {
65+
switch ($i) {
66+
case 1:
67+
return 1;
68+
case 2 ?>
69+
<?php
70+
return 2;
71+
/* testSimpleSwitchDefaultAndCloseTag */
72+
default ?>
73+
<div>default</div>
74+
<?php
75+
}
76+
}
77+
3778
function matchWithDefaultInSwitch() {
3879
switch ($something) {
3980
case 'foo':

‎tests/Core/Tokenizers/PHP/DefaultKeywordTest.php‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,19 +132,28 @@ public function testSwitchDefault($testMarker, $testContent = 'default')
132132
public static function dataSwitchDefault()
133133
{
134134
return [
135-
'simple_switch_default' => [
135+
'simple_switch_default' => [
136136
'testMarker' => '/* testSimpleSwitchDefault */',
137137
],
138-
'simple_switch_default_with_curlies' => [
138+
'simple_switch_default_with_curlies' => [
139139
'testMarker' => '/* testSimpleSwitchDefaultWithCurlies */',
140140
],
141-
'switch_default_toplevel' => [
141+
'simple_switch_default_with_semicolon' => [
142+
'testMarker' => '/* testSimpleSwitchDefaultAndSemicolon */',
143+
],
144+
'simple_switch_default_with_semicolon_and_curlies' => [
145+
'testMarker' => '/* testSimpleSwitchDefaultSemicolonAndCurlies */',
146+
],
147+
'simple_switch_default_with_php_close_tag' => [
148+
'testMarker' => '/* testSimpleSwitchDefaultAndCloseTag */',
149+
],
150+
'switch_default_toplevel' => [
142151
'testMarker' => '/* testSwitchDefault */',
143152
],
144-
'switch_default_nested_in_match_case' => [
153+
'switch_default_nested_in_match_case' => [
145154
'testMarker' => '/* testSwitchDefaultNestedInMatchCase */',
146155
],
147-
'switch_default_nested_in_match_default' => [
156+
'switch_default_nested_in_match_default' => [
148157
'testMarker' => '/* testSwitchDefaultNestedInMatchDefault */',
149158
],
150159
];

‎tests/Core/Tokenizers/PHP/EnumCaseTest.inc‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,22 @@ switch (true) {
5858
case CONSTANT:
5959
/* testCaseWithConstantAndIdenticalIsNotEnumCase */
6060
case CONSTANT === 1:
61-
/* testCaseWithAssigmentToConstantIsNotEnumCase */
62-
case CONSTANT = 1:
61+
/* testCaseWithAssigmentIsNotEnumCase */
62+
case $var = 1:
6363
/* testIsNotEnumCaseIsCaseInsensitive */
6464
cAsE CONSTANT:
65+
/* testIsNotEnumCaseWithCurlies */
66+
case CONSTANT: {
67+
break;
68+
}
69+
/* testIsNotEnumCaseWithSemicolonAndCurlies */
70+
case CONSTANT ; {
71+
break;
72+
}
73+
/* testIsNotEnumCaseWithCloseTag */
74+
case CONSTANT?>
75+
<div>something</div>
76+
<?php
6577
}
6678

6779
switch ($x) {

‎tests/Core/Tokenizers/PHP/EnumCaseTest.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,16 @@ public function testNotEnumCases($testMarker)
8989
public static function dataNotEnumCases()
9090
{
9191
return [
92-
'switch case with constant, semicolon condition end' => ['/* testCaseWithSemicolonIsNotEnumCase */'],
93-
'switch case with constant, colon condition end' => ['/* testCaseWithConstantIsNotEnumCase */'],
94-
'switch case with constant, comparison' => ['/* testCaseWithConstantAndIdenticalIsNotEnumCase */'],
95-
'switch case with constant, assignment' => ['/* testCaseWithAssigmentToConstantIsNotEnumCase */'],
96-
'switch case with constant, keyword in mixed case' => ['/* testIsNotEnumCaseIsCaseInsensitive */'],
97-
'switch case, body in curlies declares enum' => ['/* testCaseInSwitchWhenCreatingEnumInSwitch1 */'],
98-
'switch case, body after semicolon declares enum' => ['/* testCaseInSwitchWhenCreatingEnumInSwitch2 */'],
92+
'switch case with constant, semicolon condition end' => ['/* testCaseWithSemicolonIsNotEnumCase */'],
93+
'switch case with constant, colon condition end' => ['/* testCaseWithConstantIsNotEnumCase */'],
94+
'switch case with constant, comparison' => ['/* testCaseWithConstantAndIdenticalIsNotEnumCase */'],
95+
'switch case, assignment' => ['/* testCaseWithAssigmentIsNotEnumCase */'],
96+
'switch case with constant, keyword in mixed case' => ['/* testIsNotEnumCaseIsCaseInsensitive */'],
97+
'switch case with constant, curlies condition end' => ['/* testIsNotEnumCaseWithCurlies */'],
98+
'switch case with constant, semicolon + curlies' => ['/* testIsNotEnumCaseWithSemicolonAndCurlies */'],
99+
'switch case with constant, php close tag condition end' => ['/* testIsNotEnumCaseWithCloseTag */'],
100+
'switch case, body in curlies declares enum' => ['/* testCaseInSwitchWhenCreatingEnumInSwitch1 */'],
101+
'switch case, body after semicolon declares enum' => ['/* testCaseInSwitchWhenCreatingEnumInSwitch2 */'],
99102
];
100103
}
101104

‎tests/Core/Tokenizers/Tokenizer/RecurseScopeMapCaseKeywordConditionsTest.inc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,19 @@ switch ($num) {
132132
);
133133
break;
134134
}
135+
136+
// Case and default bodies can also be started with a PHP close tag, which has an implied semicolon.
137+
switch ($foo):
138+
/* testSwitchCaseStartsWithColon */
139+
case "foo":
140+
echo '<div>Some html</div>';
141+
break;
142+
/* testSwitchCaseStartsWithSemicolon */
143+
case "bar";
144+
echo '<div>Some html</div>';
145+
return;
146+
/* testSwitchCaseStartsWithPHPCloseTag */
147+
case "baz" ?>
148+
<div>Some html</div>
149+
<?php goto FOO;
150+
endswitch;

‎tests/Core/Tokenizers/Tokenizer/RecurseScopeMapCaseKeywordConditionsTest.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,27 @@ public static function dataNotEnumCases()
213213
'scope_closer' => T_BREAK,
214214
],
215215
],
216+
'switch case, body starts with colon' => [
217+
'testMarker' => '/* testSwitchCaseStartsWithColon */',
218+
'expectedTokens' => [
219+
'scope_opener' => T_COLON,
220+
'scope_closer' => T_BREAK,
221+
],
222+
],
223+
'switch case, body starts with semicolon' => [
224+
'testMarker' => '/* testSwitchCaseStartsWithSemicolon */',
225+
'expectedTokens' => [
226+
'scope_opener' => T_SEMICOLON,
227+
'scope_closer' => T_RETURN,
228+
],
229+
],
230+
'switch case, body starts with PHP close tag' => [
231+
'testMarker' => '/* testSwitchCaseStartsWithPHPCloseTag */',
232+
'expectedTokens' => [
233+
'scope_opener' => T_CLOSE_TAG,
234+
'scope_closer' => T_GOTO,
235+
],
236+
],
216237
];
217238
}
218239

‎tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.inc‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,25 @@ class Foo {
229229
/* testMethodDeclaration */
230230
public function default() {}
231231
}
232+
233+
// Case and default bodies can also be started with a PHP close tag, which has an implied semicolon.
234+
switch ($foo) {
235+
/* testSwitchDefaultStartsWithColon */
236+
default:
237+
echo '<div>Some html</div>';
238+
break;
239+
}
240+
241+
switch ($foo) {
242+
/* testSwitchDefaultStartsWithSemicolon */
243+
default;
244+
echo '<div>Some html</div>';
245+
return;
246+
}
247+
248+
switch ($foo):
249+
/* testSwitchDefaultStartsWithPHPCloseTag */
250+
default ?>
251+
<div>Some html</div>
252+
<?php goto FOO;
253+
endswitch;

‎tests/Core/Tokenizers/Tokenizer/RecurseScopeMapDefaultKeywordConditionsTest.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ public function testSwitchDefault($testMarker, $openerMarker, $closerMarker, $co
159159

160160
$token = $this->getTargetToken($testMarker, [T_MATCH_DEFAULT, T_DEFAULT, T_STRING], $testContent);
161161
$tokenArray = $tokens[$token];
162-
$expectedScopeOpener = $this->getTargetToken($openerMarker, [T_COLON, T_OPEN_CURLY_BRACKET, T_SEMICOLON]);
163-
$expectedScopeCloser = $this->getTargetToken($closerMarker, [T_BREAK, T_CLOSE_CURLY_BRACKET, T_RETURN, T_ENDSWITCH]);
162+
$expectedScopeOpener = $this->getTargetToken($openerMarker, [T_COLON, T_OPEN_CURLY_BRACKET, T_SEMICOLON, T_CLOSE_TAG]);
163+
$expectedScopeCloser = $this->getTargetToken($closerMarker, [T_BREAK, T_CLOSE_CURLY_BRACKET, T_RETURN, T_GOTO, T_ENDSWITCH]);
164164

165165
// Make sure we're looking at the right token.
166166
$this->assertSame(
@@ -338,6 +338,22 @@ public static function dataSwitchDefault()
338338
'openerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBraces */',
339339
'closerMarker' => '/* testSwitchDefaultNestedIfWithAndWithoutBracesScopeCloser */',
340340
],
341+
342+
'switch_default_starts_with_colon' => [
343+
'testMarker' => '/* testSwitchDefaultStartsWithColon */',
344+
'openerMarker' => '/* testSwitchDefaultStartsWithColon */',
345+
'closerMarker' => '/* testSwitchDefaultStartsWithColon */',
346+
],
347+
'switch_default_starts_with_semicolon' => [
348+
'testMarker' => '/* testSwitchDefaultStartsWithSemicolon */',
349+
'openerMarker' => '/* testSwitchDefaultStartsWithSemicolon */',
350+
'closerMarker' => '/* testSwitchDefaultStartsWithSemicolon */',
351+
],
352+
'switch_default_starts_with_PHP_close_tag' => [
353+
'testMarker' => '/* testSwitchDefaultStartsWithPHPCloseTag */',
354+
'openerMarker' => '/* testSwitchDefaultStartsWithPHPCloseTag */',
355+
'closerMarker' => '/* testSwitchDefaultStartsWithPHPCloseTag */',
356+
],
341357
];
342358
}
343359

0 commit comments

Comments
 (0)