Skip to content

Commit

Permalink
Arrays::isShortArray()/Lists::isShortList(): consolidate the caching [2]
Browse files Browse the repository at this point in the history
This is step 2 in a refactor to improve short list/short array determination.

While caching had previously already been implemented, as things were, there could be four caches which would each refer to the same result, i.e.
1. A cache for an opener token passed to `Arrays::isShortArray()`.
2. A cache for an closer token of the same set of brackets passed to `Arrays::isShortArray()`.
3. A cache for an opener token of the same set of brackets passed to `Lists::isShortList()`.
4. A cache for an closer token of the same set of brackets passed to `Lists::isShortList()`.

This also meant that, depending on how the methods were called, the underlying `IsShortArrayOrList` class could be called up to four times, while the result would already be known after the first time and is not subject to change.

This commit introduces a wrapper class around the `IsShortArrayOrList` class which will handle the caching and will only create one cache entry for all four potential starting points, making the caching far more efficient and effective.

As this new `IsShortArrayOrListWithCache` class will now be the entry point to the `IsShortArrayOrList` class, we can rely upon the token being passed to the `IsShortArrayOrList` always being an _opener_ for the brackets and can remove some redundancy from the `IsShortArrayOrList` class.

Includes adding a set of dedicated tests to cover the functionality in the `IsShortArrayOrListWithCache` class, replacing the previous caching tests in the `IsShortArrayOrListTest` class.

Includes updating the `IsShortArrayOrList` tests to no longer pass closer tokens to the `IsShortArrayOrList` class.
  • Loading branch information
jrfnl committed Oct 21, 2022
1 parent d92f659 commit 13bd7ba
Show file tree
Hide file tree
Showing 12 changed files with 878 additions and 133 deletions.
20 changes: 2 additions & 18 deletions PHPCSUtils/Internal/IsShortArrayOrList.php
Expand Up @@ -72,15 +72,6 @@ final class IsShortArrayOrList
*/
private $phpcsFile;

/**
* The current stack pointer.
*
* @since 1.0.0-alpha4
*
* @var int
*/
private $stackPtr;

/**
* The token stack from the current file.
*
Expand Down Expand Up @@ -141,7 +132,7 @@ final class IsShortArrayOrList
* @since 1.0.0-alpha4
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the short array bracket token.
* @param int $stackPtr The position of the short array opener token.
*
* @return void
*/
Expand All @@ -155,15 +146,8 @@ public function __construct(File $phpcsFile, $stackPtr)
}

$this->phpcsFile = $phpcsFile;
$this->stackPtr = $stackPtr;
$this->tokens = $tokens;

$this->opener = $stackPtr;
if (isset($this->tokens[$stackPtr]['bracket_opener'])
&& $stackPtr !== $this->tokens[$stackPtr]['bracket_opener']
) {
$this->opener = $this->tokens[$stackPtr]['bracket_opener'];
}
$this->opener = $stackPtr;

$this->closer = $stackPtr;
if (isset($this->tokens[$stackPtr]['bracket_closer'])
Expand Down
272 changes: 272 additions & 0 deletions PHPCSUtils/Internal/IsShortArrayOrListWithCache.php
@@ -0,0 +1,272 @@
<?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\Internal;

use PHP_CodeSniffer\Files\File;
use PHPCSUtils\Internal\Cache;
use PHPCSUtils\Internal\IsShortArrayOrList;
use PHPCSUtils\Tokens\Collections;

/**
* Determination of short array vs short list vs square brackets.
*
* Uses caching of previous results to mitigate performance issues.
*
* ---------------------------------------------------------------------------------------------
* This class is only intended for internal use by PHPCSUtils and is not part of the public API.
* This also means that it has no promise of backward compatibility.
*
* End-users should use the {@see \PHPCSUtils\Utils\Arrays::isShortArray()}
* or the {@see \PHPCSUtils\Utils\Lists::isShortList()} method instead.
* ---------------------------------------------------------------------------------------------
*
* @internal
*
* @since 1.0.0-alpha4
*/
final class IsShortArrayOrListWithCache
{

/**
* The PHPCS file in which the current stackPtr was found.
*
* @since 1.0.0-alpha4
*
* @var \PHP_CodeSniffer\Files\File
*/
private $phpcsFile;

/**
* The token stack from the current file.
*
* @since 1.0.0-alpha4
*
* @var array
*/
private $tokens;

/**
* The current stack pointer.
*
* @since 1.0.0-alpha4
*
* @var int
*/
private $stackPtr;

/**
* Stack pointer to the open bracket.
*
* @since 1.0.0-alpha4
*
* @var int
*/
private $opener;

/**
* Valid return values from the IsShortArrayOrList class.
*
* @since 1.0.0-alpha4
*
* @var string[]
*/
private $validTypes = [
IsShortArrayOrList::SHORT_ARRAY => IsShortArrayOrList::SHORT_ARRAY,
IsShortArrayOrList::SHORT_LIST => IsShortArrayOrList::SHORT_LIST,
IsShortArrayOrList::SQUARE_BRACKETS => IsShortArrayOrList::SQUARE_BRACKETS,
];

/**
* Determine whether a T_OPEN/CLOSE_SHORT_ARRAY token is a short array construct
* and not a short list.
*
* This method also accepts `T_OPEN/CLOSE_SQUARE_BRACKET` tokens to allow it to be
* PHPCS cross-version compatible as the short array tokenizing has been plagued by
* a number of bugs over time.
*
* @since 1.0.0-alpha4
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the short array bracket token.
*
* @return bool `TRUE` if the token passed is the open/close bracket of a short array.
* `FALSE` if the token is a short list bracket, a plain square bracket
* or not one of the accepted tokens.
*/
public static function isShortArray(File $phpcsFile, $stackPtr)
{
return (self::getType($phpcsFile, $stackPtr) === IsShortArrayOrList::SHORT_ARRAY);
}

/**
* Determine whether a T_OPEN/CLOSE_SHORT_ARRAY token is a short list construct
* in contrast to a short array.
*
* This method also accepts `T_OPEN/CLOSE_SQUARE_BRACKET` tokens to allow it to be
* PHPCS cross-version compatible as the short array tokenizing has been plagued by
* a number of bugs over time, which affects the short list determination.
*
* @since 1.0.0-alpha4
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the short array bracket token.
*
* @return bool `TRUE` if the token passed is the open/close bracket of a short list.
* `FALSE` if the token is a short array bracket or plain square bracket
* or not one of the accepted tokens.
*/
public static function isShortList(File $phpcsFile, $stackPtr)
{
return (self::getType($phpcsFile, $stackPtr) === IsShortArrayOrList::SHORT_LIST);
}

/**
* Determine whether a T_OPEN/CLOSE_SHORT_ARRAY token is a short array or short list construct.
*
* This method also accepts `T_OPEN/CLOSE_SQUARE_BRACKET` tokens to allow it to be
* PHPCS cross-version compatible as the short array tokenizing has been plagued by
* a number of bugs over time, which affects the short list determination.
*
* @since 1.0.0-alpha4
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the short array bracket token.
*
* @return string|false The type of construct this bracket was determined to be.
* Either 'short array', 'short list' or 'square brackets'.
* Or FALSE is this was not a bracket token.
*/
public static function getType(File $phpcsFile, $stackPtr)
{
return (new self($phpcsFile, $stackPtr))->process();
}

/**
* Constructor.
*
* @since 1.0.0-alpha4
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the short array bracket token.
*
* @return void
*/
private function __construct(File $phpcsFile, $stackPtr)
{
$this->phpcsFile = $phpcsFile;
$this->tokens = $phpcsFile->getTokens();
$this->stackPtr = $stackPtr;
}

/**
* Determine whether a T_[OPEN|CLOSE}_[SHORT_ARRAY|SQUARE_BRACKET] token is a short array
* or short list construct using previously cached results whenever possible.
*
* @since 1.0.0-alpha4
*
* @return string|false The type of construct this bracket was determined to be.
* Either 'short array', 'short list' or 'square brackets'.
* Or FALSE is this was not a bracket token.
*/
private function process()
{
if ($this->isValidStackPtr() === false) {
return false;
}

$this->opener = $this->getOpener();

/*
* Check the cache in case we've seen this token before.
*/
$type = $this->getFromCache();
if ($type !== false) {
return $type;
}

/*
* If we've not seen the token before, try and solve it and cache the results.
*/
$solver = new IsShortArrayOrList($this->phpcsFile, $this->opener);
$type = $solver->solve();

$this->updateCache($type);

return $type;
}

/**
* Verify the passed token could potentially be a short array or short list token.
*
* @since 1.0.0-alpha4
*
* @return bool
*/
private function isValidStackPtr()
{
return (isset($this->tokens[$this->stackPtr]) === true
&& isset(Collections::shortArrayTokensBC()[$this->tokens[$this->stackPtr]['code']]) === true);
}

/**
* Get the stack pointer to the short array/list opener.
*
* @since 1.0.0-alpha4
*
* @return int
*/
private function getOpener()
{
$opener = $this->stackPtr;
if (isset($this->tokens[$this->stackPtr]['bracket_opener'])) {
$opener = $this->tokens[$this->stackPtr]['bracket_opener'];
}

return $opener;
}

/**
* Retrieve the bracket "type" of a token from the cache.
*
* @since 1.0.0-alpha4
*
* @return string|false The previously determined type (which could be an empty string)
* or FALSE if no cache entry was found for this token.
*/
private function getFromCache()
{
if (Cache::isCached($this->phpcsFile, __CLASS__, $this->opener) === true) {
return Cache::get($this->phpcsFile, __CLASS__, $this->opener);
}

return false;
}

/**
* Update the cache with information about a particular bracket token.
*
* @since 1.0.0-alpha4
*
* @param string $type The type this bracket has been determined to be.
* Either 'short array', 'short list' or 'square brackets'.
*
* @return void
*/
private function updateCache($type)
{
$result = false;
if (isset($this->validTypes[$type]) === true) {
$result = $type;
}

Cache::set($this->phpcsFile, __CLASS__, $this->opener, $result);
}
}
22 changes: 2 additions & 20 deletions PHPCSUtils/Utils/Arrays.php
Expand Up @@ -13,7 +13,7 @@
use PHP_CodeSniffer\Exceptions\RuntimeException;
use PHP_CodeSniffer\Files\File;
use PHPCSUtils\Internal\Cache;
use PHPCSUtils\Internal\IsShortArrayOrList;
use PHPCSUtils\Internal\IsShortArrayOrListWithCache;
use PHPCSUtils\Tokens\Collections;

/**
Expand Down Expand Up @@ -64,25 +64,7 @@ final class Arrays
*/
public static function isShortArray(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

// Is this one of the tokens this function handles ?
if (isset($tokens[$stackPtr]) === false
|| isset(Collections::shortArrayTokensBC()[$tokens[$stackPtr]['code']]) === false
) {
return false;
}

if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) {
return Cache::get($phpcsFile, __METHOD__, $stackPtr);
}

$solver = new IsShortArrayOrList($phpcsFile, $stackPtr);
$type = $solver->solve();
$returnValue = ($type === IsShortArrayOrList::SHORT_ARRAY);

Cache::set($phpcsFile, __METHOD__, $stackPtr, $returnValue);
return $returnValue;
return IsShortArrayOrListWithCache::isShortArray($phpcsFile, $stackPtr);
}

/**
Expand Down
22 changes: 2 additions & 20 deletions PHPCSUtils/Utils/Lists.php
Expand Up @@ -14,7 +14,7 @@
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Internal\Cache;
use PHPCSUtils\Internal\IsShortArrayOrList;
use PHPCSUtils\Internal\IsShortArrayOrListWithCache;
use PHPCSUtils\Tokens\Collections;
use PHPCSUtils\Utils\GetTokensAsString;

Expand Down Expand Up @@ -66,25 +66,7 @@ final class Lists
*/
public static function isShortList(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

// Is this one of the tokens this function handles ?
if (isset($tokens[$stackPtr]) === false
|| isset(Collections::shortListTokensBC()[$tokens[$stackPtr]['code']]) === false
) {
return false;
}

if (Cache::isCached($phpcsFile, __METHOD__, $stackPtr) === true) {
return Cache::get($phpcsFile, __METHOD__, $stackPtr);
}

$solver = new IsShortArrayOrList($phpcsFile, $stackPtr);
$type = $solver->solve();
$returnValue = ($type === IsShortArrayOrList::SHORT_LIST);

Cache::set($phpcsFile, __METHOD__, $stackPtr, $returnValue);
return $returnValue;
return IsShortArrayOrListWithCache::isShortList($phpcsFile, $stackPtr);
}

/**
Expand Down
Expand Up @@ -64,7 +64,7 @@ echo 'just here to prevent the below test running into a tokenizer issue tested
/* testShortList */
[$b] = $c;

/* testShortListDetectOnCloseBracket */
/* testShortListMultiItem */
[$a, $b] = $c;

/* testShortListWithKeys */
Expand Down

0 comments on commit 13bd7ba

Please sign in to comment.