Skip to content

Commit

Permalink
Arrays::isShortArray()/Lists::isShortList(): bugfix for tokenizer iss…
Browse files Browse the repository at this point in the history
…ue icw short open tag

The tokenizer issue as tested for in the BC1 and BC2 test case files and described in squizlabs/php_codesniffer 1971, where a short array bracket is tokenized as a square bracket, also happens when the PHP short open echo tag is used to open PHP.

This commit accounts for this tokenizer bug in the `Arrays::isShortArray()` and the `Lists::isShortList()` methods and adds additional tests to safeguard the correct handling of this in the `Arrays::isShortArray()` and the `Lists::isShortList()` methods.
  • Loading branch information
jrfnl committed May 20, 2021
1 parent 0cf9a85 commit f22f8e0
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 2 deletions.
4 changes: 3 additions & 1 deletion PHPCSUtils/Utils/Arrays.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ public static function isShortArray(File $phpcsFile, $stackPtr)
*
* @link https://github.com/squizlabs/PHP_CodeSniffer/issues/1971
*/
if ($prevNonEmpty !== 0 || $tokens[$prevNonEmpty]['code'] !== \T_OPEN_TAG) {
if ($prevNonEmpty !== 0
|| isset(Collections::phpOpenTags()[$tokens[$prevNonEmpty]['code']]) === false
) {
return false;
}
}
Expand Down
3 changes: 2 additions & 1 deletion PHPCSUtils/Utils/Lists.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public static function isShortList(File $phpcsFile, $stackPtr)
}

$prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), null, true);
if ((($prevNonEmpty === 0 && $tokens[$prevNonEmpty]['code'] === \T_OPEN_TAG) // Bug #1971.
if ((($prevNonEmpty === 0
&& isset(Collections::phpOpenTags()[$tokens[$prevNonEmpty]['code']]) === true) // Bug #1971.
|| ($tokens[$prevNonEmpty]['code'] === \T_CLOSE_CURLY_BRACKET
&& isset($tokens[$prevNonEmpty]['scope_condition']))) // Bug #1284.
) {
Expand Down
5 changes: 5 additions & 0 deletions Tests/Utils/Lists/IsShortArrayOrListTokenizerBC3Test.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?=

/* testTokenizerIssue1971PHPCSlt330gt271E */
// Valid PHP, though not useful as the implied echo will break on the array.
[1, 2, /* testTokenizerIssue1971PHPCSlt330gt271F */ [3], 4];
121 changes: 121 additions & 0 deletions Tests/Utils/Lists/IsShortArrayOrListTokenizerBC3Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<?php
/**
* PHPCSUtils, utility functions and classes for PHP_CodeSniffer sniff developers.
*
* @package PHPCSUtils
* @copyright 2019-2020 PHPCSUtils Contributors
* @license https://opensource.org/licenses/LGPL-3.0 LGPL3
* @link https://github.com/PHPCSStandards/PHPCSUtils
*/

namespace PHPCSUtils\Tests\Utils\Lists;

use PHPCSUtils\TestUtils\UtilityMethodTestCase;
use PHPCSUtils\Utils\Arrays;
use PHPCSUtils\Utils\Lists;

/**
* Tests for specific PHPCS tokenizer issues which can affect the \PHPCSUtils\Utils\Arrays::isShortArray()
* and the \PHPCSUtils\Utils\Lists::isShortList() methods.
*
* @group arrays
* @group lists
*
* @since 1.0.0
*/
class IsShortArrayOrListTokenizerBC3Test extends UtilityMethodTestCase
{

/**
* Data integrity test. Verify that the data provider is consistent.
*
* Something is either a short array or a short list or neither. It can never be both at the same time.
*
* @dataProvider dataIsShortArrayOrList
* @coversNothing
*
* @param string $ignore Unused.
* @param bool[] $data The expected boolean return value for list and array.
*
* @return void
*/
public function testValidDataProvider($ignore, $data)
{
$forbidden = [
'list' => true,
'array' => true,
];

$this->assertNotSame($forbidden, $data);
}

/**
* Test correctly determining whether a short array open token is a short array,
* even when the token is incorrectly tokenized.
*
* @dataProvider dataIsShortArrayOrList
* @covers \PHPCSUtils\Internal\IsShortArrayOrList
* @covers \PHPCSUtils\Utils\Arrays::isShortArray
*
* @param string $testMarker The comment which prefaces the target token in the test file.
* @param bool[] $expected The expected boolean return value for list and array.
*
* @return void
*/
public function testIsShortArray($testMarker, $expected)
{
$stackPtr = $this->getTargetToken($testMarker, [\T_OPEN_SHORT_ARRAY, \T_OPEN_SQUARE_BRACKET]);
$result = Arrays::isShortArray(self::$phpcsFile, $stackPtr);

$this->assertSame($expected['array'], $result);
}

/**
* Test correctly determining whether a short array open token is a short array or a short list,
* even when the token is incorrectly tokenized.
*
* @dataProvider dataIsShortArrayOrList
* @covers \PHPCSUtils\Internal\IsShortArrayOrList
* @covers \PHPCSUtils\Utils\Lists::isShortList
*
* @param string $testMarker The comment which prefaces the target token in the test file.
* @param bool[] $expected The expected boolean return value for list and array.
*
* @return void
*/
public function testIsShortList($testMarker, $expected)
{
$stackPtr = $this->getTargetToken($testMarker, [\T_OPEN_SHORT_ARRAY, \T_OPEN_SQUARE_BRACKET]);
$result = Lists::isShortList(self::$phpcsFile, $stackPtr);

$this->assertSame($expected['list'], $result);
}

/**
* Data provider.
*
* @see testIsShortArray() For the array format.
* @see testIsShortList() For the array format.
*
* @return array
*/
public function dataIsShortArrayOrList()
{
return [
'issue-1971-short-array-first-in-file' => [
'/* testTokenizerIssue1971PHPCSlt330gt271E */',
[
'array' => true,
'list' => false,
],
],
'issue-1971-short-array-first-in-file-nested' => [
'/* testTokenizerIssue1971PHPCSlt330gt271F */',
[
'array' => true,
'list' => false,
],
],
];
}
}

0 comments on commit f22f8e0

Please sign in to comment.