Skip to content

Commit

Permalink
Merge pull request #215 from PHPCSStandards/parentheses/recognize-mor…
Browse files Browse the repository at this point in the history 
…e-constructs-as-owners

Parentheses: recognize more parentheses owners
  • Loading branch information
@jrfnl
jrfnl committed Sep 20, 2020
2 parents 8c488a1 + 00c1206 commit ee7baa5
Show file tree
Hide file tree
Showing 3 changed files with 254 additions and 18 deletions.
50 changes: 35 additions & 15 deletions PHPCSUtils/Utils/Parentheses.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,47 @@

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\BackCompat\Helper;
use PHPCSUtils\Tokens\Collections;
use PHPCSUtils\Utils\FunctionDeclarations;

/**
* Utility functions for use when examining parenthesis tokens and arbitrary tokens wrapped in
* parentheses.
* Utility functions for use when examining parenthesis tokens and arbitrary tokens wrapped
* in parentheses.
*
* In contrast to PHPCS natively, `isset()`, `unset()`, `empty()`, `exit()`, `die()` and `eval()`
* will be considered parentheses owners by the functions in this class.
*
* @since 1.0.0
* @since 1.0.0-alpha4 Added support for `isset()`, `unset()`, `empty()`, `exit()`, `die()`
* and `eval()`` as parentheses owners to all applicable functions.
*/
class Parentheses
{

/**
* Extra tokens which should be considered parentheses owners.
*
* - `T_LIST` and `T_ANON_CLASS` only became parentheses owners in PHPCS 3.5.0.
* - `T_ISSET`, `T_UNSET`, `T_EMPTY`, `T_EXIT` and `T_EVAL` are not PHPCS native parentheses,
* owners, but are considered such for the purposes of this class.
* Also {@see https://github.com/squizlabs/PHP_CodeSniffer/issues/3118}.
*
* @since 1.0.0-alpha4
*
* @var array <int|string> => <int|string>
*/
private static $extraParenthesesOwners = [
\T_LIST => \T_LIST,
\T_ISSET => \T_ISSET,
\T_UNSET => \T_UNSET,
\T_EMPTY => \T_EMPTY,
\T_EXIT => \T_EXIT,
\T_EVAL => \T_EVAL,
\T_ANON_CLASS => \T_ANON_CLASS,
// Work-around: anon classes were, in certain circumstances, tokenized as T_CLASS prior to PHPCS 3.4.0.
\T_CLASS => \T_CLASS,
];

/**
* Get the pointer to the parentheses owner of an open/close parenthesis.
*
Expand All @@ -47,18 +75,13 @@ public static function getOwner(File $phpcsFile, $stackPtr)
}

/*
* `T_LIST` and `T_ANON_CLASS` only became parentheses owners in PHPCS 3.5.0.
* `T_FN` was only backfilled in PHPCS 3.5.3/4/5.
* - On PHP 7.4 with PHPCS < 3.5.3, T_FN will not yet be a parentheses owner.
* - On PHP < 7.4 with PHPCS < 3.5.3, T_FN will be tokenized as T_STRING and not yet be a parentheses owner.
* - `T_FN` was only backfilled in PHPCS 3.5.3/4/5.
* - On PHP 7.4 with PHPCS < 3.5.3, T_FN will not yet be a parentheses owner.
* - On PHP < 7.4 with PHPCS < 3.5.3, T_FN will be tokenized as T_STRING and not yet be a parentheses owner.
*
* {@internal As the 'parenthesis_owner' index is only set on parentheses, we didn't need to do any
* input validation before, but now we do.}
*/
if (\version_compare(Helper::getVersion(), '3.5.4', '>=') === true) {
return false;
}

if (isset($tokens[$stackPtr]) === false
|| ($tokens[$stackPtr]['code'] !== \T_OPEN_PARENTHESIS
&& $tokens[$stackPtr]['code'] !== \T_CLOSE_PARENTHESIS)
Expand All @@ -75,10 +98,7 @@ public static function getOwner(File $phpcsFile, $stackPtr)

$prevNonEmpty = $phpcsFile->findPrevious($skip, ($stackPtr - 1), null, true);
if ($prevNonEmpty !== false
&& ($tokens[$prevNonEmpty]['code'] === \T_LIST
|| $tokens[$prevNonEmpty]['code'] === \T_ANON_CLASS
// Work-around: anon classes were, in certain circumstances, tokenized as T_CLASS prior to PHPCS 3.4.0.
|| $tokens[$prevNonEmpty]['code'] === \T_CLASS
&& (isset(self::$extraParenthesesOwners[$tokens[$prevNonEmpty]['code']])
// Possibly an arrow function.
|| FunctionDeclarations::isArrowFunction($phpcsFile, $prevNonEmpty) === true)
) {
Expand Down
12 changes: 12 additions & 0 deletions Tests/Utils/Parentheses/ParenthesesTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ $value = $obj->fn(true);
/* testArrowFunctionByReference */
$fn = fn &($x) => $x;

/* testIfWithIssetEmpty */
if ( isset( $a, $b ) && ! empty ($c) ) {
/* testUnset */
unset($a[1], $b->prop);
}

/* testEval */
eval($a . $b . $c );

/* testExit */
if (defined('FOO') || die('message')) {}

// Intentional parse error. This has to be the last test in the file.
/* testParseError */
declare(ticks=1
210 changes: 207 additions & 3 deletions Tests/Utils/Parentheses/ParenthesesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,37 @@ class ParenthesesTest extends UtilityMethodTestCase
'code' => \T_VARIABLE,
'content' => '$x',
],
'testIfIsset-$b' => [
'marker' => '/* testIfWithIssetEmpty */',
'code' => \T_VARIABLE,
'content' => '$b',
],
'testIfEmpty-$c' => [
'marker' => '/* testIfWithIssetEmpty */',
'code' => \T_VARIABLE,
'content' => '$c',
],
'testUnset-->' => [
'marker' => '/* testUnset */',
'code' => \T_OBJECT_OPERATOR,
],
'testUnsetParenthesis' => [
'marker' => '/* testUnset */',
'code' => \T_OPEN_PARENTHESIS,
],
'testEval-concat' => [
'marker' => '/* testEval */',
'code' => \T_STRING_CONCAT,
],
'testIfExitDie-boolean-or' => [
'marker' => '/* testExit */',
'code' => \T_BOOLEAN_OR,
],
'testIfExitDie-message' => [
'marker' => '/* testExit */',
'code' => \T_CONSTANT_ENCAPSED_STRING,
'content' => "'message'",
],
'testParseError-1' => [
'marker' => '/* testParseError */',
'code' => \T_LNUMBER,
Expand All @@ -174,7 +205,9 @@ class ParenthesesTest extends UtilityMethodTestCase
* This array is merged with expected result arrays for various unit tests
* to make sure all possible parentheses owners are tested.
*
* This array should be kept in sync with the Tokens::$parenthesisOpeners array.
* This array should be kept in sync with the Tokens::$parenthesisOpeners array
* + the extra tokens the Parentheses class allows for.
*
* This array isn't auto-generated based on the array in Tokens as for these
* tests we want to have access to the token constant names, not just their values.
*
Expand All @@ -195,6 +228,13 @@ class ParenthesesTest extends UtilityMethodTestCase
'T_CATCH' => false,
'T_DECLARE' => false,
'T_FN' => false,

// Extra tokens.
'T_ISSET' => false,
'T_UNSET' => false,
'T_EMPTY' => false,
'T_EXIT' => false,
'T_EVAL' => false,
];

/**
Expand Down Expand Up @@ -348,7 +388,7 @@ public function testPassingParenthesisTokenToMethodsWhichExpectParenthesisClose(
*
* @return void
*/
public function testPassingParenthesisCloseHandlinginBCLayer()
public function testPassingParenthesisCloseHandlingInBCLayer()
{
$stackPtr = $this->getTargetToken('/* testListOnCloseParens */', \T_CLOSE_PARENTHESIS);

Expand Down Expand Up @@ -756,6 +796,125 @@ public function dataWalkParentheses()
'lastIfElseOwner' => false,
],
],
'testIfIsset-$b' => [
'testIfIsset-$b',
[
'firstOpener' => -8,
'firstCloser' => 14,
'firstOwner' => -10,
'firstScopeOwnerOpener' => -8,
'firstScopeOwnerCloser' => 14,
'firstScopeOwnerOwner' => -10,
'lastOpener' => -5,
'lastCloser' => 2,
'lastOwner' => -6,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => -10,
],
],
'testIfEmpty-$c' => [
'testIfEmpty-$c',
[
'firstOpener' => -19,
'firstCloser' => 3,
'firstOwner' => -21,
'firstScopeOwnerOpener' => -19,
'firstScopeOwnerCloser' => 3,
'firstScopeOwnerOwner' => -21,
'lastOpener' => -1,
'lastCloser' => 1,
'lastOwner' => -3,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => -21,
],
],
'testUnset-->' => [
'testUnset-->',
[
'firstOpener' => -8,
'firstCloser' => 2,
'firstOwner' => -9,
'firstScopeOwnerOpener' => false,
'firstScopeOwnerCloser' => false,
'firstScopeOwnerOwner' => false,
'lastOpener' => -8,
'lastCloser' => 2,
'lastOwner' => -9,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => false,
],
],
'testUnsetParenthesis' => [
'testUnsetParenthesis',
[
'firstOpener' => false,
'firstCloser' => false,
'firstOwner' => false,
'firstScopeOwnerOpener' => false,
'firstScopeOwnerCloser' => false,
'firstScopeOwnerOwner' => false,
'lastOpener' => false,
'lastCloser' => false,
'lastOwner' => false,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => false,
],
],
'testEval-concat' => [
'testEval-concat',
[
'firstOpener' => -3,
'firstCloser' => 8,
'firstOwner' => -4,
'firstScopeOwnerOpener' => false,
'firstScopeOwnerCloser' => false,
'firstScopeOwnerOwner' => false,
'lastOpener' => -3,
'lastCloser' => 8,
'lastOwner' => -4,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => false,
],
],
'testIfExitDie-boolean-or' => [
'testIfExitDie-boolean-or',
[
'firstOpener' => -6,
'firstCloser' => 6,
'firstOwner' => -8,
'firstScopeOwnerOpener' => -6,
'firstScopeOwnerCloser' => 6,
'firstScopeOwnerOwner' => -8,
'lastOpener' => -6,
'lastCloser' => 6,
'lastOwner' => -8,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => -8,
],
],
'testIfExitDie-message' => [
'testIfExitDie-message',
[
'firstOpener' => -10,
'firstCloser' => 2,
'firstOwner' => -12,
'firstScopeOwnerOpener' => -10,
'firstScopeOwnerCloser' => 2,
'firstScopeOwnerOwner' => -12,
'lastOpener' => -1,
'lastCloser' => 1,
'lastOwner' => -2,
'lastArrayOpener' => false,
'lastFunctionCloser' => false,
'lastIfElseOwner' => -12,
],
],
'testParseError-1' => [
'testParseError-1',
[
Expand Down Expand Up @@ -953,8 +1112,43 @@ public function dataHasOwner()
],
'testArrowFunctionReturnByRef' => [
'testArrowFunctionReturnByRef',
['T_FN' => true],
],
'testIfIsset-$b' => [
'testIfIsset-$b',
[
'T_IF' => true,
'T_ISSET' => true,
],
],
'testIfEmpty-$c' => [
'testIfEmpty-$c',
[
'T_IF' => true,
'T_EMPTY' => true,
],
],
'testUnset-->' => [
'testUnset-->',
['T_UNSET' => true],
],
'testUnsetParenthesis' => [
'testUnsetParenthesis',
[],
],
'testEval-concat' => [
'testEval-concat',
['T_EVAL' => true],
],
'testIfExitDie-boolean-or' => [
'testIfExitDie-boolean-or',
['T_IF' => true],
],
'testIfExitDie-message' => [
'testIfExitDie-message',
[
'T_FN' => true,
'T_IF' => true,
'T_EXIT' => true,
],
],
'testParseError-1' => [
Expand Down Expand Up @@ -1193,6 +1387,16 @@ public function dataLastOwnerIn()
$arrowFunctionOwners,
-4,
],
'testIfEmpty-$c-unset' => [
'testIfEmpty-$c',
[\T_UNSET],
false,
],
'testIfEmpty-$c-isset-empty' => [
'testIfEmpty-$c',
[\T_ISSET, \T_EMPTY],
-3,
],
];
}
}

0 comments on commit ee7baa5

Please sign in to comment.