Skip to content

Commit 13c861e

Browse files
jrfnljrfnl
committed
Tokenizer/PHP: fix context sensitive keywords as goto labels
Previously quite some work has already been done to ensure that keywords which can be used in a non-keyword context would be tokenized as `T_STRING` in those situations. The `goto` context was, however, not taken into account. It should be noted that none of the official PHP keywords are allowed to be used as `goto` labels, so all cases covered by the `ContextSensitiveKeywordsGotoTest` are in actual fact parse errors. Having said that, those keywords being tokenized as the keyword, instead of as `T_STRING`, could seriously confuse sniffs, which is why, IMO, they should be (re-)tokenized to `T_STRING`. For the record: the "other" context sensitive keywords - `parent`, `self`, `true`, `false` and `null` are actually allowed as labels for `goto` statements, even though this makes for some pretty confusing code. Example: https://3v4l.org/dcbmm Either way, fixed now (for the most part). Note: This fix does not include fixing the (re-)tokenization of keywords to `T_GOTO_LABEL` for the "goto target statement". I'm not sure this can be done reliably without breaking other things, so that's considered outside the scope of this PR. Includes plenty of tests to safeguard the change. Related to 185
1 parent f22e90b commit 13c861e

File tree

5 files changed

+333
-6
lines changed

5 files changed

+333
-6
lines changed

‎src/Tokenizers/PHP.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ class PHP extends Tokenizer
494494
T_USE => true,
495495
T_NAMESPACE => true,
496496
T_PAAMAYIM_NEKUDOTAYIM => true,
497+
T_GOTO => true,
497498
];
498499

499500
/**

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php
2+
3+
// Intentional parse errors!
4+
// Using a keyword as a "goto" label is a parse error, but should still be tokenized correctly as T_STRING to not confuse sniffs.
5+
6+
function gotoLabels() {
7+
goto /* testAbstract */ abstract;
8+
goto /* testArray */ array;
9+
goto /* testAs */ as;
10+
goto /* testBreak */ break;
11+
goto /* testCallable */ callable;
12+
goto /* testCase */ case;
13+
goto /* testCatch */ catch;
14+
goto /* testClass */ class;
15+
goto /* testClone */ clone;
16+
goto /* testConst */ const;
17+
goto /* testContinue */ continue;
18+
goto /* testDeclare */ declare;
19+
goto /* testDefault */ default;
20+
goto /* testDo */ do;
21+
goto /* testEcho */ echo;
22+
goto /* testElse */ else;
23+
goto /* testElseIf */ elseif;
24+
goto /* testEmpty */ empty;
25+
goto /* testEndDeclare */ enddeclare;
26+
goto /* testEndFor */ endfor;
27+
goto /* testEndForeach */ endforeach;
28+
goto /* testEndIf */ endif;
29+
goto /* testEndSwitch */ endswitch;
30+
goto /* testEndWhile */ endwhile;
31+
goto /* testEnum */ enum;
32+
goto /* testEval */ eval;
33+
goto /* testExit */ exit;
34+
goto /* testExtends */ extends;
35+
goto /* testFinal */ final;
36+
goto /* testFinally */ finally;
37+
goto /* testFn */ fn;
38+
goto /* testFor */ for;
39+
goto /* testForeach */ ForEach;
40+
goto /* testFunction */ function;
41+
goto /* testGlobal */ global;
42+
goto /* testGoto */ goto;
43+
goto /* testIf */ if;
44+
goto /* testImplements */ implements;
45+
goto /* testInclude */ include;
46+
goto /* testIncludeOnce */ include_once;
47+
goto /* testInstanceOf */ instanceof;
48+
goto /* testInsteadOf */ insteadof;
49+
goto /* testInterface */ interface;
50+
goto /* testIsset */ isset;
51+
goto /* testList */ list;
52+
goto /* testMatch */ match;
53+
goto /* testNamespace */ namespace;
54+
goto /* testNew */ new;
55+
goto /* testPrint */ print;
56+
goto /* testPrivate */ private;
57+
goto /* testProtected */ protected;
58+
goto /* testPublic */ public;
59+
goto /* testReadonly */ readonly;
60+
goto /* testRequire */ require;
61+
goto /* testRequireOnce */ require_once;
62+
goto /* testReturn */ return;
63+
goto /* testStatic */ static;
64+
goto /* testSwitch */ switch;
65+
goto /* testThrows */ throw;
66+
goto /* testTrait */ trait;
67+
goto /* testTry */ try;
68+
goto /* testUnset */ unset;
69+
goto /* testUse */ use;
70+
goto /* testVar */ var;
71+
goto /* testWhile */ while;
72+
goto /* testYield */ yield;
73+
goto /* testYieldFrom */ yield_from;
74+
75+
goto /* testAnd */ and;
76+
goto /* testOr */ or;
77+
goto /* testXor */ xor;
78+
}

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

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
/**
3+
* Tests the conversion of PHP native context sensitive keywords to T_STRING when used in a "goto" statement.
4+
*
5+
* @copyright 2025 PHPCSStandards and contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
7+
*/
8+
9+
namespace PHP_CodeSniffer\Tests\Core\Tokenizers\PHP;
10+
11+
use PHP_CodeSniffer\Tests\Core\Tokenizers\AbstractTokenizerTestCase;
12+
use PHP_CodeSniffer\Util\Tokens;
13+
14+
/**
15+
* Tests the conversion of PHP native context sensitive keywords to T_STRING when used in a "goto" statement.
16+
*
17+
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
18+
*/
19+
final class ContextSensitiveKeywordsGotoTest extends AbstractTokenizerTestCase
20+
{
21+
22+
23+
/**
24+
* Test that context sensitive keyword is tokenized as string when used in a "goto" statement.
25+
*
26+
* @param string $testMarker The comment which prefaces the target token in the test file.
27+
*
28+
* @dataProvider dataStrings
29+
*
30+
* @return void
31+
*/
32+
public function testStrings($testMarker)
33+
{
34+
$tokens = $this->phpcsFile->getTokens();
35+
$target = $this->getTargetToken($testMarker, (Tokens::$contextSensitiveKeywords + [T_STRING]));
36+
$tokenArray = $tokens[$target];
37+
38+
$this->assertSame(T_STRING, $tokenArray['code'], 'Token tokenized as '.$tokenArray['type'].', not T_STRING (code)');
39+
$this->assertSame('T_STRING', $tokenArray['type'], 'Token tokenized as '.$tokenArray['type'].', not T_STRING (type)');
40+
41+
}//end testStrings()
42+
43+
44+
/**
45+
* Data provider.
46+
*
47+
* @see testStrings()
48+
*
49+
* @return array<string, array<string>>
50+
*/
51+
public static function dataStrings()
52+
{
53+
return [
54+
'abstract' => ['/* testAbstract */'],
55+
'array' => ['/* testArray */'],
56+
'as' => ['/* testAs */'],
57+
'break' => ['/* testBreak */'],
58+
'callable' => ['/* testCallable */'],
59+
'case' => ['/* testCase */'],
60+
'catch' => ['/* testCatch */'],
61+
'class' => ['/* testClass */'],
62+
'clone' => ['/* testClone */'],
63+
'const' => ['/* testConst */'],
64+
'continue' => ['/* testContinue */'],
65+
'declare' => ['/* testDeclare */'],
66+
'default' => ['/* testDefault */'],
67+
'do' => ['/* testDo */'],
68+
'echo' => ['/* testEcho */'],
69+
'else' => ['/* testElse */'],
70+
'elseif' => ['/* testElseIf */'],
71+
'empty' => ['/* testEmpty */'],
72+
'enddeclare' => ['/* testEndDeclare */'],
73+
'endfor' => ['/* testEndFor */'],
74+
'endforeach' => ['/* testEndForeach */'],
75+
'endif' => ['/* testEndIf */'],
76+
'endswitch' => ['/* testEndSwitch */'],
77+
'endwhile' => ['/* testEndWhile */'],
78+
'enum' => ['/* testEnum */'],
79+
'eval' => ['/* testEval */'],
80+
'exit' => ['/* testExit */'],
81+
'extends' => ['/* testExtends */'],
82+
'final' => ['/* testFinal */'],
83+
'finally' => ['/* testFinally */'],
84+
'fn' => ['/* testFn */'],
85+
'for' => ['/* testFor */'],
86+
'foreach' => ['/* testForeach */'],
87+
'function' => ['/* testFunction */'],
88+
'global' => ['/* testGlobal */'],
89+
'goto' => ['/* testGoto */'],
90+
'if' => ['/* testIf */'],
91+
'implements' => ['/* testImplements */'],
92+
'include' => ['/* testInclude */'],
93+
'include_once' => ['/* testIncludeOnce */'],
94+
'instanceof' => ['/* testInstanceOf */'],
95+
'insteadof' => ['/* testInsteadOf */'],
96+
'interface' => ['/* testInterface */'],
97+
'isset' => ['/* testIsset */'],
98+
'list' => ['/* testList */'],
99+
'match' => ['/* testMatch */'],
100+
'namespace' => ['/* testNamespace */'],
101+
'new' => ['/* testNew */'],
102+
'print' => ['/* testPrint */'],
103+
'private' => ['/* testPrivate */'],
104+
'protected' => ['/* testProtected */'],
105+
'public' => ['/* testPublic */'],
106+
'readonly' => ['/* testReadonly */'],
107+
'require' => ['/* testRequire */'],
108+
'require_once' => ['/* testRequireOnce */'],
109+
'return' => ['/* testReturn */'],
110+
'static' => ['/* testStatic */'],
111+
'switch' => ['/* testSwitch */'],
112+
'throws' => ['/* testThrows */'],
113+
'trait' => ['/* testTrait */'],
114+
'try' => ['/* testTry */'],
115+
'unset' => ['/* testUnset */'],
116+
'use' => ['/* testUse */'],
117+
'var' => ['/* testVar */'],
118+
'while' => ['/* testWhile */'],
119+
'yield' => ['/* testYield */'],
120+
'yield_from' => ['/* testYieldFrom */'],
121+
'and' => ['/* testAnd */'],
122+
'or' => ['/* testOr */'],
123+
'xor' => ['/* testXor */'],
124+
];
125+
126+
}//end dataStrings()
127+
128+
129+
}//end class

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,39 @@ function hasGoto() {
5252
do_something();
5353
}
5454

55+
// The "other" keywords are perfectly valid to be used as goto labels (just really confusing).
56+
function gotoLabels() {
57+
/* testParentAsGotoTargetShouldBeString */
58+
goto parent;
59+
/* testParentAsGotoLabelShouldBeGotoLabel */
60+
parent:
61+
doSomething();
62+
63+
/* testSelfAsGotoTargetShouldBeString */
64+
goto self;
65+
/* testSelfAsGotoLabelShouldBeGotoLabel */
66+
self:
67+
doSomething();
68+
69+
/* testTrueAsGotoTargetShouldBeString */
70+
goto true;
71+
/* testTrueAsGotoLabelShouldBeGotoLabel */
72+
true:
73+
doSomething();
74+
75+
/* testFalseAsGotoTargetShouldBeString */
76+
goto false;
77+
/* testFalseAsGotoLabelShouldBeGotoLabel */
78+
false:
79+
doSomething();
80+
81+
/* testNullAsGotoTargetShouldBeString */
82+
goto null;
83+
/* testNullAsGotoLabelShouldBeGotoLabel */
84+
null:
85+
doSomething();
86+
}
87+
5588
switch ($x) {
5689
/* testNotGotoDeclarationGlobalConstant */
5790
case CONSTANT:
@@ -72,6 +105,21 @@ switch ($x) {
72105
case $obj->property:
73106
// Do something.
74107
break;
108+
109+
/* testNotGotoDeclarationTrueInCase */
110+
case true:
111+
// Do something.
112+
break;
113+
114+
/* testNotGotoDeclarationFalseInCase */
115+
case false:
116+
// Do something.
117+
break;
118+
119+
/* testNotGotoDeclarationNullInCase */
120+
case null:
121+
// Do something.
122+
break;
75123
}
76124

77125
switch (true) {

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

Lines changed: 77 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,27 @@ public static function dataGotoStatement()
6969
'testMarker' => '/* testGotoStatementInFunction */',
7070
'testContent' => 'label',
7171
],
72+
73+
'goto parent' => [
74+
'testMarker' => '/* testParentAsGotoTargetShouldBeString */',
75+
'testContent' => 'parent',
76+
],
77+
'goto self' => [
78+
'testMarker' => '/* testSelfAsGotoTargetShouldBeString */',
79+
'testContent' => 'self',
80+
],
81+
'goto true' => [
82+
'testMarker' => '/* testTrueAsGotoTargetShouldBeString */',
83+
'testContent' => 'true',
84+
],
85+
'goto false' => [
86+
'testMarker' => '/* testFalseAsGotoTargetShouldBeString */',
87+
'testContent' => 'false',
88+
],
89+
'goto null' => [
90+
'testMarker' => '/* testNullAsGotoTargetShouldBeString */',
91+
'testContent' => 'null',
92+
],
7293
];
7394

7495
}//end dataGotoStatement()
@@ -130,6 +151,27 @@ public static function dataGotoDeclaration()
130151
'testMarker' => '/* testGotoDeclarationInFunction */',
131152
'testContent' => 'label',
132153
],
154+
155+
'label in goto declaration - parent' => [
156+
'testMarker' => '/* testParentAsGotoLabelShouldBeGotoLabel */',
157+
'testContent' => 'parent',
158+
],
159+
'label in goto declaration - self' => [
160+
'testMarker' => '/* testSelfAsGotoLabelShouldBeGotoLabel */',
161+
'testContent' => 'self',
162+
],
163+
'label in goto declaration - true' => [
164+
'testMarker' => '/* testTrueAsGotoLabelShouldBeGotoLabel */',
165+
'testContent' => 'true',
166+
],
167+
'label in goto declaration - false' => [
168+
'testMarker' => '/* testFalseAsGotoLabelShouldBeGotoLabel */',
169+
'testContent' => 'false',
170+
],
171+
'label in goto declaration - null' => [
172+
'testMarker' => '/* testNullAsGotoLabelShouldBeGotoLabel */',
173+
'testContent' => 'null',
174+
],
133175
];
134176

135177
}//end dataGotoDeclaration()
@@ -138,21 +180,35 @@ public static function dataGotoDeclaration()
138180
/**
139181
* Verify that the constant used in a switch - case statement is not confused with a goto label.
140182
*
141-
* @param string $testMarker The comment prefacing the target token.
142-
* @param string $testContent The token content to expect.
183+
* @param string $testMarker The comment prefacing the target token.
184+
* @param string $testContent The token content to expect.
185+
* @param string $expectedType Optional. The token type which is expected (not T_GOTO_LABEL).
186+
* Defaults to `T_STRING`.
143187
*
144188
* @dataProvider dataNotAGotoDeclaration
145189
*
146190
* @return void
147191
*/
148-
public function testNotAGotoDeclaration($testMarker, $testContent)
192+
public function testNotAGotoDeclaration($testMarker, $testContent, $expectedType='T_STRING')
149193
{
194+
$targetTypes = [
195+
T_STRING => T_STRING,
196+
T_GOTO_LABEL => T_GOTO_LABEL,
197+
];
198+
$expectedCode = T_STRING;
199+
200+
if ($expectedType !== 'T_STRING') {
201+
$expectedCode = constant($expectedType);
202+
$targetTypes[$expectedCode] = $expectedCode;
203+
}
204+
205+
$target = $this->getTargetToken($testMarker, $targetTypes, $testContent);
206+
150207
$tokens = $this->phpcsFile->getTokens();
151-
$target = $this->getTargetToken($testMarker, [T_GOTO_LABEL, T_STRING], $testContent);
152208
$tokenArray = $tokens[$target];
153209

154-
$this->assertSame(T_STRING, $tokenArray['code'], 'Token tokenized as '.$tokenArray['type'].', not T_STRING (code)');
155-
$this->assertSame('T_STRING', $tokenArray['type'], 'Token tokenized as '.$tokenArray['type'].', not T_STRING (type)');
210+
$this->assertSame($expectedCode, $tokenArray['code'], 'Token tokenized as '.$tokenArray['type'].', not '.$expectedType.' (code)');
211+
$this->assertSame($expectedType, $tokenArray['type'], 'Token tokenized as '.$tokenArray['type'].', not '.$expectedType.' (type)');
156212

157213
}//end testNotAGotoDeclaration()
158214

@@ -183,6 +239,21 @@ public static function dataNotAGotoDeclaration()
183239
'testMarker' => '/* testNotGotoDeclarationClassProperty */',
184240
'testContent' => 'property',
185241
],
242+
'not goto label - true followed by switch-case colon' => [
243+
'testMarker' => '/* testNotGotoDeclarationTrueInCase */',
244+
'testContent' => 'true',
245+
'expectedType' => 'T_TRUE',
246+
],
247+
'not goto label - false followed by switch-case colon' => [
248+
'testMarker' => '/* testNotGotoDeclarationFalseInCase */',
249+
'testContent' => 'false',
250+
'expectedType' => 'T_FALSE',
251+
],
252+
'not goto label - null followed by switch-case colon' => [
253+
'testMarker' => '/* testNotGotoDeclarationNullInCase */',
254+
'testContent' => 'null',
255+
'expectedType' => 'T_NULL',
256+
],
186257
'not goto label - global constant followed by ternary else' => [
187258
'testMarker' => '/* testNotGotoDeclarationGlobalConstantInTernary */',
188259
'testContent' => 'CONST_A',

0 commit comments

Comments
 (0)