Skip to content

PHP 8.1: Added T_ENUM_CASE #3483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 7, 2022
Merged

PHP 8.1: Added T_ENUM_CASE #3483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4115be
PHP 8.1: Added T_ENUM_CASE
kukulich Nov 20, 2021
9fd9b01
PHP 8.1: Enum case name should be always tokenized as T_STRING
kukulich Nov 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<file baseinstalldir="" name="DefaultKeywordTest.php" role="test" />
<file baseinstalldir="" name="DoubleArrowTest.inc" role="test" />
<file baseinstalldir="" name="DoubleArrowTest.php" role="test" />
<file baseinstalldir="" name="EnumCaseTest.inc" role="test" />
<file baseinstalldir="" name="EnumCaseTest.php" role="test" />
<file baseinstalldir="" name="FinallyTest.inc" role="test" />
<file baseinstalldir="" name="FinallyTest.php" role="test" />
<file baseinstalldir="" name="GotoLabelTest.inc" role="test" />
Expand Down Expand Up @@ -2127,6 +2129,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/Tokenizer/DefaultKeywordTest.inc" name="tests/Core/Tokenizer/DefaultKeywordTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/DoubleArrowTest.php" name="tests/Core/Tokenizer/DoubleArrowTest.php" />
<install as="CodeSniffer/Core/Tokenizer/DoubleArrowTest.inc" name="tests/Core/Tokenizer/DoubleArrowTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/EnumCaseTest.php" name="tests/Core/Tokenizer/EnumCaseTest.php" />
<install as="CodeSniffer/Core/Tokenizer/EnumCaseTest.inc" name="tests/Core/Tokenizer/EnumCaseTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/FinallyTest.php" name="tests/Core/Tokenizer/FinallyTest.php" />
<install as="CodeSniffer/Core/Tokenizer/FinallyTest.inc" name="tests/Core/Tokenizer/FinallyTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/GotoLabelTest.php" name="tests/Core/Tokenizer/GotoLabelTest.php" />
Expand Down Expand Up @@ -2225,6 +2229,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
<install as="CodeSniffer/Core/Tokenizer/DefaultKeywordTest.inc" name="tests/Core/Tokenizer/DefaultKeywordTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/DoubleArrowTest.php" name="tests/Core/Tokenizer/DoubleArrowTest.php" />
<install as="CodeSniffer/Core/Tokenizer/DoubleArrowTest.inc" name="tests/Core/Tokenizer/DoubleArrowTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/EnumCaseTest.php" name="tests/Core/Tokenizer/EnumCaseTest.php" />
<install as="CodeSniffer/Core/Tokenizer/EnumCaseTest.inc" name="tests/Core/Tokenizer/EnumCaseTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/FinallyTest.php" name="tests/Core/Tokenizer/FinallyTest.php" />
<install as="CodeSniffer/Core/Tokenizer/FinallyTest.inc" name="tests/Core/Tokenizer/FinallyTest.inc" />
<install as="CodeSniffer/Core/Tokenizer/GotoLabelTest.php" name="tests/Core/Tokenizer/GotoLabelTest.php" />
Expand Down
64 changes: 64 additions & 0 deletions src/Tokenizers/PHP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ class PHP extends Tokenizer
T_ENDSWITCH => 9,
T_ENDWHILE => 8,
T_ENUM => 4,
T_ENUM_CASE => 4,
T_EVAL => 4,
T_EXTENDS => 7,
T_FILE => 8,
Expand Down Expand Up @@ -476,6 +477,7 @@ class PHP extends Tokenizer
T_INTERFACE => true,
T_TRAIT => true,
T_ENUM => true,
T_ENUM_CASE => true,
T_EXTENDS => true,
T_IMPLEMENTS => true,
T_ATTRIBUTE => true,
Expand Down Expand Up @@ -982,6 +984,9 @@ protected function tokenize($string)
&& is_array($tokens[$i]) === true
&& $tokens[$i][0] === T_STRING
) {
// Modify $tokens directly so we can use it later when converting enum "case".
$tokens[$stackPtr][0] = T_ENUM;

$newToken = [];
$newToken['code'] = T_ENUM;
$newToken['type'] = 'T_ENUM';
Expand All @@ -997,6 +1002,65 @@ protected function tokenize($string)
}
}//end if

/*
Convert enum "case" to T_ENUM_CASE
*/

if ($tokenIsArray === true
Copy link
Contributor

@jrfnl jrfnl Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels very inefficient. This basically means that for any T_CASE token, the tokenizer will do a lot of token walking to get this right, which for long switch statements with lots of cases will cause a lot of extra processing.

At the same time, I understand it makes sense to have a separate token for this as the switch T_CASE token will get scope, while this token shouldn't (similar to the problem we faced with match structures and the default keyword), which makes it neigh impossible to move the retokenization to processAdditional() as the scope setting would get skewed in that case.

I'm going to have a further think about this one.

Copy link
Contributor Author

@kukulich kukulich Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels very inefficient.

Yes, but it took me couple of hours to create this "final" version that solve all situations I found.

I've just figured out one small optimisation for enum case.

Copy link
Member

@gsherwood gsherwood Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out on some very long SWITCH statements and it doesn't seem to add too much time, but that's going to change based on the complexity of each CASE block. I'm not super worried though.

An alternative place to attempt this change would be when the scope map is being created. I haven't dug into it because it's not what it's normally used for, but it should be aware we're in an ENUM and then the T_ENUM_CASE logic could be selectively applied.

The token itself could be changed here, but it might be cleaner to use this function to skip the scope map logic for T_ENUM_CASE and then use processAdditional() to switch over the tokens properly.

I'm running short on time at the moment, but I will give this a go when I get some.

Copy link
Member

@gsherwood gsherwood Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing and it's definitely faster doing the work in Tokenizer::recurseScopeMap(), and a less code as well. It does require adding T_CASE to the tstringContexts member var otherwise the CASE values are not converted to T_STRING using the new method.

&& $token[0] === T_CASE
&& isset($this->tstringContexts[$finalTokens[$lastNotEmptyToken]['code']]) === false
) {
$isEnumCase = false;
$scope = 1;

for ($i = ($stackPtr - 1); $i > 0; $i--) {
if ($tokens[$i] === '}') {
$scope++;
continue;
}

if ($tokens[$i] === '{') {
$scope--;
continue;
}

if (is_array($tokens[$i]) === false) {
continue;
}

if ($scope !== 0) {
continue;
}

if ($tokens[$i][0] === T_SWITCH) {
break;
}

if ($tokens[$i][0] === T_ENUM || $tokens[$i][0] === T_ENUM_CASE) {
$isEnumCase = true;
break;
}
}//end for

if ($isEnumCase === true) {
// Modify $tokens directly so we can use it as optimisation for other enum "case".
$tokens[$stackPtr][0] = T_ENUM_CASE;

$newToken = [];
$newToken['code'] = T_ENUM_CASE;
$newToken['type'] = 'T_ENUM_CASE';
$newToken['content'] = $token[1];
$finalTokens[$newStackPtr] = $newToken;

if (PHP_CODESNIFFER_VERBOSITY > 1) {
echo "\t\t* token $stackPtr changed from T_CASE to T_ENUM_CASE".PHP_EOL;
}

$newStackPtr++;
continue;
}
}//end if

/*
As of PHP 8.0 fully qualified, partially qualified and namespace relative
identifier names are tokenized differently.
Expand Down
1 change: 1 addition & 0 deletions src/Util/Tokens.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
define('T_MATCH_ARROW', 'PHPCS_T_MATCH_ARROW');
define('T_MATCH_DEFAULT', 'PHPCS_T_MATCH_DEFAULT');
define('T_ATTRIBUTE_END', 'PHPCS_T_ATTRIBUTE_END');
define('T_ENUM_CASE', 'PHPCS_T_ENUM_CASE');

// Some PHP 5.5 tokens, replicated for lower versions.
if (defined('T_FINALLY') === false) {
Expand Down
95 changes: 95 additions & 0 deletions tests/Core/Tokenizer/EnumCaseTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<?php

enum Foo
{
/* testPureEnumCase */
case SOME_CASE;
}

enum Boo: int {
/* testBackingIntegerEnumCase */
case ONE = 1;
}

enum Hoo: string
{
/* testBackingStringEnumCase */
case ONE = 'one';
}

enum ComplexEnum: int implements SomeInterface
{
use SomeTrait {
traitMethod as enumMethod;
}

const SOME_CONSTANT = true;

/* testEnumCaseInComplexEnum */
case ONE = 1;

/* testEnumCaseIsCaseInsensitive */
CaSe TWO = 2;

public function someMethod(): bool
{
switch (true) {
/* testCaseWithSemicolonIsNotEnumCase */
case CONSTANT;
}
}

/* testEnumCaseAfterSwitch */
case THREE = 3;

public function someOtherMethod(): bool
{
switch (true):
case false:
endswitch;
}

/* testEnumCaseAfterSwitchWithEndSwitch */
case FOUR = 4;
}

switch (true) {
/* testCaseWithConstantIsNotEnumCase */
case CONSTANT:
/* testCaseWithConstantAndIdenticalIsNotEnumCase */
case CONSTANT === 1:
/* testCaseWithAssigmentToConstantIsNotEnumCase */
case CONSTANT = 1:
/* testIsNotEnumCaseIsCaseInsensitive */
cAsE CONSTANT:
}
Copy link
Contributor

@jrfnl jrfnl Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend adding far more extensive tests for varying combinations of enum and switch, as well as covering as many situations as one can think of where case can be used and should be tokenized as T_STRING instead of T_CASE or T_ENUM_CASE.

See the test cases for the DefaultKeywordTest for inspiration.


switch ($x) {
/* testCaseInSwitchWhenCreatingEnumInSwitch1 */
case 'a': {
enum Foo {}
break;
}

/* testCaseInSwitchWhenCreatingEnumInSwitch2 */
case 'b';
enum Bar {}
break;
}

enum Foo: string {
/* testKeywordAsEnumCaseNameShouldBeString1 */
case INTERFACE = 'interface';
/* testKeywordAsEnumCaseNameShouldBeString2 */
case TRAIT = 'trait';
/* testKeywordAsEnumCaseNameShouldBeString3 */
case ENUM = 'enum';
/* testKeywordAsEnumCaseNameShouldBeString4 */
case FUNCTION = 'function';
/* testKeywordAsEnumCaseNameShouldBeString5 */
case FALSE = 'false';
/* testKeywordAsEnumCaseNameShouldBeString6 */
case DEFAULT = 'default';
/* testKeywordAsEnumCaseNameShouldBeString7 */
case ARRAY = 'array';
}
157 changes: 157 additions & 0 deletions tests/Core/Tokenizer/EnumCaseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
<?php
/**
* Tests converting enum "case" to T_ENUM_CASE.
*
* @author Jaroslav Hanslík <[email protected]>
* @copyright 2021 Squiz Pty Ltd (ABN 77 084 670 600)
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Tests\Core\Tokenizer;

use PHP_CodeSniffer\Tests\Core\AbstractMethodUnitTest;

class EnumCaseTest extends AbstractMethodUnitTest
{


/**
* Test that the enum "case" is converted to T_ENUM_CASE.
*
* @param string $testMarker The comment which prefaces the target token in the test file.
*
* @dataProvider dataEnumCases
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::recurseScopeMap
*
* @return void
*/
public function testEnumCases($testMarker)
{
$tokens = self::$phpcsFile->getTokens();

$enumCase = $this->getTargetToken($testMarker, [T_ENUM_CASE, T_CASE]);

$this->assertSame(T_ENUM_CASE, $tokens[$enumCase]['code']);
$this->assertSame('T_ENUM_CASE', $tokens[$enumCase]['type']);

$this->assertArrayNotHasKey('scope_condition', $tokens[$enumCase], 'Scope condition is set');
$this->assertArrayNotHasKey('scope_opener', $tokens[$enumCase], 'Scope opener is set');
$this->assertArrayNotHasKey('scope_closer', $tokens[$enumCase], 'Scope closer is set');

}//end testEnumCases()


/**
* Data provider.
*
* @see testEnumCases()
*
* @return array
*/
public function dataEnumCases()
{
return [
['/* testPureEnumCase */'],
['/* testBackingIntegerEnumCase */'],
['/* testBackingStringEnumCase */'],
['/* testEnumCaseInComplexEnum */'],
['/* testEnumCaseIsCaseInsensitive */'],
['/* testEnumCaseAfterSwitch */'],
['/* testEnumCaseAfterSwitchWithEndSwitch */'],
];

}//end dataEnumCases()


/**
* Test that "case" that is not enum case is still tokenized as `T_CASE`.
*
* @param string $testMarker The comment which prefaces the target token in the test file.
*
* @dataProvider dataNotEnumCases
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
* @covers PHP_CodeSniffer\Tokenizers\Tokenizer::recurseScopeMap
*
* @return void
*/
public function testNotEnumCases($testMarker)
{
$tokens = self::$phpcsFile->getTokens();

$case = $this->getTargetToken($testMarker, [T_ENUM_CASE, T_CASE]);

$this->assertSame(T_CASE, $tokens[$case]['code']);
$this->assertSame('T_CASE', $tokens[$case]['type']);

$this->assertArrayHasKey('scope_condition', $tokens[$case], 'Scope condition is not set');
$this->assertArrayHasKey('scope_opener', $tokens[$case], 'Scope opener is not set');
$this->assertArrayHasKey('scope_closer', $tokens[$case], 'Scope closer is not set');

}//end testNotEnumCases()


/**
* Data provider.
*
* @see testNotEnumCases()
*
* @return array
*/
public function dataNotEnumCases()
{
return [
['/* testCaseWithSemicolonIsNotEnumCase */'],
['/* testCaseWithConstantIsNotEnumCase */'],
['/* testCaseWithConstantAndIdenticalIsNotEnumCase */'],
['/* testCaseWithAssigmentToConstantIsNotEnumCase */'],
['/* testIsNotEnumCaseIsCaseInsensitive */'],
['/* testCaseInSwitchWhenCreatingEnumInSwitch1 */'],
['/* testCaseInSwitchWhenCreatingEnumInSwitch2 */'],
];

}//end dataNotEnumCases()


/**
* Test that "case" that is not enum case is still tokenized as `T_CASE`.
*
* @param string $testMarker The comment which prefaces the target token in the test file.
*
* @dataProvider dataKeywordAsEnumCaseNameShouldBeString
* @covers PHP_CodeSniffer\Tokenizers\PHP::tokenize
*
* @return void
*/
public function testKeywordAsEnumCaseNameShouldBeString($testMarker)
{
$tokens = self::$phpcsFile->getTokens();

$enumCaseName = $this->getTargetToken($testMarker, [T_STRING, T_INTERFACE, T_TRAIT, T_ENUM, T_FUNCTION, T_FALSE, T_DEFAULT, T_ARRAY]);

$this->assertSame(T_STRING, $tokens[$enumCaseName]['code']);
$this->assertSame('T_STRING', $tokens[$enumCaseName]['type']);

}//end testKeywordAsEnumCaseNameShouldBeString()


/**
* Data provider.
*
* @see testKeywordAsEnumCaseNameShouldBeString()
*
* @return array
*/
public function dataKeywordAsEnumCaseNameShouldBeString()
{
return [
['/* testKeywordAsEnumCaseNameShouldBeString1 */'],
['/* testKeywordAsEnumCaseNameShouldBeString2 */'],
['/* testKeywordAsEnumCaseNameShouldBeString3 */'],
['/* testKeywordAsEnumCaseNameShouldBeString4 */'],
];

}//end dataKeywordAsEnumCaseNameShouldBeString()


}//end class