Skip to content

Commit

Permalink
Create special case for T_BAD_CHARACTER constant (#1586)
Browse files Browse the repository at this point in the history 
The `T_BAD_CHARACTER` constant was removed in PHP 7.0 and re-added in PHP 7.4.

This led to confusing error messages from the PHPCompatibility `NewConstants` and `RemovedConstants` sniffs, which would report the constant as removed, but also as not available yet (new).

This PR fixes this by special casing the handling of the `T_BAD_CHARACTER` constant in both the `NewConstants` as well as the `RemovedConstants` sniffs with a special error message which is only shown when PHP 7.0 - 7.3 needs to be support and not when only PHP < 7.0 or PHP 7.4+ needs to be supported.

Includes dedicated unit tests to verify the special casing works as expected.
  • Loading branch information
@fredden
fredden committed Feb 12, 2024
1 parent 8a46895 commit e5cd2e2
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 4 deletions.
16 changes: 15 additions & 1 deletion PHPCompatibility/Sniffs/Constants/NewConstantsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class NewConstantsSniff extends Sniff
* A list of new PHP Constants, not present in older versions.
*
* The array lists : version number with false (not present) or true (present).
* If's sufficient to list the first version where the constant appears.
* It's sufficient to list the first version where the constant appears.
*
* Note: PHP constants are case-sensitive!
*
Expand Down Expand Up @@ -6802,6 +6802,7 @@ class NewConstantsSniff extends Sniff
'7.3' => false,
'7.4' => true,
],
// Note: this constant has special casing in the handleFeature() method as it was also present in PHP < 7.0.
'T_BAD_CHARACTER' => [
'7.3' => false,
'7.4' => true,
Expand Down Expand Up @@ -8091,6 +8092,19 @@ protected function handleFeature(File $phpcsFile, $stackPtr, array $itemInfo)
return;
}

if ($itemInfo['name'] === 'T_BAD_CHARACTER') {
// T_BAD_CHARACTER is a special case. It was removed in 7.0.0 and re-added in 7.4.0
// See also PHPCompatibility.Constants.RemovedConstants
if (ScannedCode::shouldRunOnOrAbove('7.0')) {
$message = 'The constant "T_BAD_CHARACTER" is not present in PHP versions 7.0 through 7.3';
$msgInfo = $this->getMessageInfo($itemInfo['name'], $itemInfo['name'], $versionInfo);

$phpcsFile->addError($message, $stackPtr, $msgInfo['errorcode'], $msgInfo['data']);
}

return;
}

$this->addError($phpcsFile, $stackPtr, $itemInfo, $versionInfo);
}

Expand Down
16 changes: 15 additions & 1 deletion PHPCompatibility/Sniffs/Constants/RemovedConstantsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class RemovedConstantsSniff extends Sniff
* A list of removed PHP Constants.
*
* The array lists : version number with false (deprecated) or true (removed).
* If's sufficient to list the first version where the constant was deprecated/removed.
* It's sufficient to list the first version where the constant was deprecated/removed.
*
* Optional, the array can contain an `alternative` key listing an alternative constant
* to be used instead.
Expand Down Expand Up @@ -1932,6 +1932,7 @@ class RemovedConstantsSniff extends Sniff
'7.0' => true,
'extension' => 'tokenizer',
],
// Note: this constant has special casing in the handleFeature() method as it is also present in PHP >= 7.4.
'T_BAD_CHARACTER' => [
'7.0' => true,
'extension' => 'tokenizer',
Expand Down Expand Up @@ -2735,6 +2736,19 @@ protected function handleFeature(File $phpcsFile, $stackPtr, array $itemInfo)
return;
}

if ($itemInfo['name'] === 'T_BAD_CHARACTER') {
// T_BAD_CHARACTER is a special case. It was removed in 7.0.0 and re-added in 7.4.0
// See also PHPCompatibility.Constants.NewConstants
if (ScannedCode::shouldRunOnOrBelow('7.3')) {
$message = 'The constant "T_BAD_CHARACTER" is not present in PHP versions 7.0 through 7.3';
$msgInfo = $this->getMessageInfo($itemInfo['name'], $itemInfo['name'], $versionInfo);

$phpcsFile->addError($message, $stackPtr, $msgInfo['errorcode'], $msgInfo['data']);
}

return;
}

$this->addMessage($phpcsFile, $stackPtr, $isError, $itemInfo, $versionInfo);
}

Expand Down
59 changes: 58 additions & 1 deletion PHPCompatibility/Tests/Constants/NewConstantsUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1433,7 +1433,6 @@ public static function dataNewConstant()
['PASSWORD_ARGON2_PROVIDER', '7.3', 864, '7.4'],
['PHP_WINDOWS_EVENT_CTRL_C', '7.3', 830, '7.4'],
['PHP_WINDOWS_EVENT_CTRL_BREAK', '7.3', 831, '7.4'],
['T_BAD_CHARACTER', '7.3', 862, '7.4'],
['T_COALESCE_EQUAL', '7.3', 1048, '7.4'],
['T_FN', '7.3', 1049, '7.4'],
['TIDY_TAG_ARTICLE', '7.3', 832, '7.4'],
Expand Down Expand Up @@ -1725,4 +1724,62 @@ public function testNoViolationsInFileOnValidVersion()
$file = $this->sniffFile(__FILE__, '99.0'); // High version beyond newest addition.
$this->assertNoViolation($file);
}

/**
* The T_BAD_CHARACTER constant is a special case
*
* @dataProvider dataTBadCharacter
*
* @param string $phpVersion PHP version (or range) tot test with.
* @param bool $shouldError If we expect to get an error or not from the sniff
*
* @return void
*/
public function testTBadCharacter($phpVersion, $shouldError)
{
$line = 862;
$message = 'The constant "T_BAD_CHARACTER" is not present in PHP versions 7.0 through 7.3';
$file = $this->sniffFile(__FILE__, $phpVersion);

if ($shouldError) {
$this->assertError($file, $line, $message);
} else {
$this->assertNoViolation($file, $line);
}
}

/**
* Data provider
*
* @see testTBadCharacter
*
* @return array<array<string|bool>>
*/
public static function dataTBadCharacter()
{
// This could be more elegantly written with a generator, but this project supports PHP v5.4 which is before generators were introduced (in PHP 5.5).
return [
['5.6', false], // Last version before removal
['7.0', true], // Removed
['7.1', true], // Removed
['7.2', true], // Removed
['7.3', true], // Removed
['7.4', false], // Added again

['-5.6', false], // Before
['-7.2', true], // Inside
['-8.2', true], // After

['5.4-', true], // Before
['7.2-', true], // Inside
['7.4-', false], // After

['5.0-5.6', false], // Before
['7.4-8.3', false], // After
['5.0-8.3', true], // Inside and both sides
['5.0-7.2', true], // Inside and before
['7.0-7.3', true], // Inside only
['7.2-8.1', true], // Inside and after
];
}
}
59 changes: 58 additions & 1 deletion PHPCompatibility/Tests/Constants/RemovedConstantsUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,6 @@ public static function dataRemovedConstant()

['PGSQL_ATTR_DISABLE_NATIVE_PREPARED_STATEMENT', '7.0', 15, '5.6'],
['T_CHARACTER', '7.0', 139, '5.6'],
['T_BAD_CHARACTER', '7.0', 140, '5.6'],
['MSSQL_ASSOC', '7.0', 557, '5.6'],
['MSSQL_NUM', '7.0', 558, '5.6'],
['MSSQL_BOTH', '7.0', 559, '5.6'],
Expand Down Expand Up @@ -928,4 +927,62 @@ public function testNoViolationsInFileOnValidVersion()
$file = $this->sniffFile(__FILE__, '5.0'); // Low version below the first deprecation.
$this->assertNoViolation($file);
}

/**
* The T_BAD_CHARACTER constant is a special case
*
* @dataProvider dataTBadCharacter
*
* @param string $phpVersion PHP version (or range) tot test with.
* @param bool $shouldError If we expect to get an error or not from the sniff
*
* @return void
*/
public function testTBadCharacter($phpVersion, $shouldError)
{
$line = 140;
$message = 'The constant "T_BAD_CHARACTER" is not present in PHP versions 7.0 through 7.3';
$file = $this->sniffFile(__FILE__, $phpVersion);

if ($shouldError) {
$this->assertError($file, $line, $message);
} else {
$this->assertNoViolation($file, $line);
}
}

/**
* Data provider
*
* @see testTBadCharacter
*
* @return array<array<string|bool>>
*/
public static function dataTBadCharacter()
{
// This could be more elegantly written with a generator, but this project supports PHP v5.4 which is before generators were introduced (in PHP 5.5).
return [
['5.6', false], // Last version before removal
['7.0', true], // Removed
['7.1', true], // Removed
['7.2', true], // Removed
['7.3', true], // Removed
['7.4', false], // Added again

['-5.6', false], // Before
['-7.2', true], // Inside
['-8.2', true], // After

['5.4-', true], // Before
['7.2-', true], // Inside
['7.4-', false], // After

['5.0-5.6', false], // Before
['7.4-8.3', false], // After
['5.0-8.3', true], // Inside and both sides
['5.0-7.2', true], // Inside and before
['7.0-7.3', true], // Inside only
['7.2-8.1', true], // Inside and after
];
}
}

0 comments on commit e5cd2e2

Please sign in to comment.