From 274711644a647844ea19c5ff7f6daa5321aacd04 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 25 Jul 2022 13:19:42 -0400 Subject: [PATCH] Refactor to move scope management to ScopeManager class (#265) * Fix some minor type errors and improve code comments * Remove unnecessary scopeEndIndexCache * Add more function comments * If no scope closer is found, use the end of the file * Add ScopeManager * Improve index of ScopeManager using scopeStart * Replace scope list with ScopeManager * Move getScopesClosedBy to ScopeManager * Linting fixes * Support php 5.5 by not passing expression to empty) * Remove unused imports --- VariableAnalysis/Lib/ScopeManager.php | 108 +++++++++++++ .../CodeAnalysis/VariableAnalysisSniff.php | 142 +++--------------- 2 files changed, 130 insertions(+), 120 deletions(-) create mode 100644 VariableAnalysis/Lib/ScopeManager.php diff --git a/VariableAnalysis/Lib/ScopeManager.php b/VariableAnalysis/Lib/ScopeManager.php new file mode 100644 index 0000000..ae3b029 --- /dev/null +++ b/VariableAnalysis/Lib/ScopeManager.php @@ -0,0 +1,108 @@ +> + */ + private $scopes = []; + + /** + * Add a scope's start and end index to our record for the file. + * + * @param File $phpcsFile + * @param int $scopeStartIndex + * + * @return ScopeInfo + */ + public function recordScopeStartAndEnd(File $phpcsFile, $scopeStartIndex) + { + $scopeEndIndex = Helpers::getScopeCloseForScopeOpen($phpcsFile, $scopeStartIndex); + $filename = $phpcsFile->getFilename(); + if (! isset($this->scopes[$filename])) { + $this->scopes[$filename] = []; + } + Helpers::debug('recording scope for file', $filename, 'start/end', $scopeStartIndex, $scopeEndIndex); + $scope = new ScopeInfo($scopeStartIndex, $scopeEndIndex); + $this->scopes[$filename][$scopeStartIndex] = $scope; + return $scope; + } + + /** + * Return the scopes for a file. + * + * @param string $filename + * + * @return ScopeInfo[] + */ + public function getScopesForFilename($filename) + { + if (empty($this->scopes[$filename])) { + return []; + } + return array_values($this->scopes[$filename]); + } + + /** + * Return the scope for a scope start index. + * + * @param string $filename + * @param int $scopeStartIndex + * + * @return ScopeInfo|null + */ + public function getScopeForScopeStart($filename, $scopeStartIndex) + { + if (empty($this->scopes[$filename][$scopeStartIndex])) { + return null; + } + return $this->scopes[$filename][$scopeStartIndex]; + } + + /** + * Find scopes closed by a scope close index. + * + * @param string $filename + * @param int $scopeEndIndex + * + * @return ScopeInfo[] + */ + public function getScopesForScopeEnd($filename, $scopeEndIndex) + { + $scopePairsForFile = $this->getScopesForFilename($filename); + $scopeIndicesThisCloses = array_reduce( + $scopePairsForFile, + /** + * @param ScopeInfo[] $found + * @param ScopeInfo $scope + * + * @return ScopeInfo[] + */ + function ($found, $scope) use ($scopeEndIndex) { + if (! is_int($scope->scopeEndIndex)) { + Helpers::debug('No scope closer found for scope start', $scope->scopeStartIndex); + return $found; + } + + if ($scopeEndIndex === $scope->scopeEndIndex) { + $found[] = $scope; + } + return $found; + }, + [] + ); + return $scopeIndicesThisCloses; + } +} diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 3125663..6b5e2e5 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -7,6 +7,7 @@ use VariableAnalysis\Lib\VariableInfo; use VariableAnalysis\Lib\Constants; use VariableAnalysis\Lib\Helpers; +use VariableAnalysis\Lib\ScopeManager; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Util\Tokens; @@ -21,36 +22,9 @@ class VariableAnalysisSniff implements Sniff protected $currentFile = null; /** - * An associative array of scopes for variables encountered so far and the - * variables within them. - * - * Each scope is keyed by a string of the form `filename:scopeStartIndex` - * (see `getScopeKey`). - * - * @var array - */ - private $scopes = []; - - /** - * An associative array of a list of token index pairs which start and end - * scopes and will be used to check for unused variables. - * - * Each array of scopes is keyed by a string containing the filename (see - * `getFilename`). - * - * Unlike the `ScopeInfo` objects stored in `$this->scopes`, these objects do - * not track variables themselves, only the position of the scope boundaries. - * - * @var array + * @var ScopeManager */ - private $scopeStartEndPairs = []; - - /** - * A cache of scope end indices in the current file to improve performance. - * - * @var int[] - */ - private $scopeEndIndexCache = []; + private $scopeManager; /** * A list of custom functions which pass in variables to be initialized by @@ -165,6 +139,11 @@ class VariableAnalysisSniff implements Sniff */ public $allowUnusedVariablesBeforeRequire = false; + public function __construct() + { + $this->scopeManager = new ScopeManager(); + } + /** * Decide which tokens to scan. * @@ -237,13 +216,12 @@ public function process(File $phpcsFile, $stackPtr) // easily accessed in other places which aren't passed the object. if ($this->currentFile !== $phpcsFile) { $this->currentFile = $phpcsFile; - // Reset the scope end cache when the File changes since it is per-file. - $this->scopeEndIndexCache = []; } // Add the global scope for the current file to our scope indexes. - if (empty($this->scopeStartEndPairs[$this->getFilename()])) { - $this->recordScopeStartAndEnd($phpcsFile, 0); + $scopesForFilename = $this->scopeManager->getScopesForFilename($phpcsFile->getFilename()); + if (empty($scopesForFilename)) { + $this->scopeManager->recordScopeStartAndEnd($phpcsFile, 0); } // Report variables defined but not used in the current scope as unused @@ -282,69 +260,11 @@ public function process(File $phpcsFile, $stackPtr) Helpers::isArrowFunction($phpcsFile, $stackPtr) ) { Helpers::debug('found scope condition', $token); - $this->recordScopeStartAndEnd($phpcsFile, $stackPtr); + $this->scopeManager->recordScopeStartAndEnd($phpcsFile, $stackPtr); return; } } - /** - * Add a scope's start and end index to our record for the file. - * - * @param File $phpcsFile - * @param int $scopeStartIndex - * - * @return void - */ - private function recordScopeStartAndEnd($phpcsFile, $scopeStartIndex) - { - $scopeEndIndex = Helpers::getScopeCloseForScopeOpen($phpcsFile, $scopeStartIndex); - $filename = $this->getFilename(); - if (! isset($this->scopeStartEndPairs[$filename])) { - $this->scopeStartEndPairs[$filename] = []; - } - Helpers::debug('recording scope for file', $filename, 'start/end', $scopeStartIndex, $scopeEndIndex); - $this->scopeStartEndPairs[$filename][] = new ScopeInfo($scopeStartIndex, $scopeEndIndex); - $this->scopeEndIndexCache[] = $scopeEndIndex; - } - - /** - * Find scopes closed by a token. - * - * @param File $phpcsFile - * @param int $stackPtr - * - * @return ScopeInfo[] - */ - private function getScopesClosedBy($phpcsFile, $stackPtr) - { - if (! in_array($stackPtr, $this->scopeEndIndexCache, true)) { - return []; - } - $scopePairsForFile = isset($this->scopeStartEndPairs[$this->getFilename()]) ? $this->scopeStartEndPairs[$this->getFilename()] : []; - $scopeIndicesThisCloses = array_reduce( - $scopePairsForFile, - /** - * @param ScopeInfo[] $found - * @param ScopeInfo $scope - * - * @return ScopeInfo[] - */ - function ($found, $scope) use ($stackPtr) { - if (! is_int($scope->scopeEndIndex)) { - Helpers::debug('No scope closer found for scope start', $scope->scopeStartIndex); - return $found; - } - - if ($stackPtr === $scope->scopeEndIndex) { - $found[] = $scope; - } - return $found; - }, - [] - ); - return $scopeIndicesThisCloses; - } - /** * Find scopes closed by a token and process their variables. * @@ -357,7 +277,7 @@ function ($found, $scope) use ($stackPtr) { */ private function searchForAndProcessClosingScopesAt($phpcsFile, $stackPtr) { - $scopeIndicesThisCloses = $this->getScopesClosedBy($phpcsFile, $stackPtr); + $scopeIndicesThisCloses = $this->scopeManager->getScopesForScopeEnd($phpcsFile->getFilename(), $stackPtr); foreach ($scopeIndicesThisCloses as $scopeIndexThisCloses) { Helpers::debug('found closing scope at index', $stackPtr, 'for scopes starting at:', $scopeIndexThisCloses); @@ -388,16 +308,6 @@ protected function isGetDefinedVars(File $phpcsFile, $stackPtr) return true; } - /** - * @param int $currScope - * - * @return string - */ - protected function getScopeKey($currScope) - { - return $this->getFilename() . ':' . $currScope; - } - /** * @return string */ @@ -406,17 +316,6 @@ protected function getFilename() return $this->currentFile ? $this->currentFile->getFilename() : 'unknown file'; } - /** - * @param int $currScope - * - * @return ScopeInfo|null - */ - protected function getScopeInfo($currScope) - { - $scopeKey = $this->getScopeKey($currScope); - return isset($this->scopes[$scopeKey]) ? $this->scopes[$scopeKey] : null; - } - /** * @param int $currScope * @@ -424,11 +323,14 @@ protected function getScopeInfo($currScope) */ protected function getOrCreateScopeInfo($currScope) { - $scopeKey = $this->getScopeKey($currScope); - if (!isset($this->scopes[$scopeKey])) { - $this->scopes[$scopeKey] = new ScopeInfo($currScope); + $scope = $this->scopeManager->getScopeForScopeStart($this->getFilename(), $currScope); + if (! $scope) { + if (! $this->currentFile) { + throw new \Exception('Cannot create scope info; current file is not set.'); + } + $scope = $this->scopeManager->recordScopeStartAndEnd($this->currentFile, $currScope); } - return $this->scopes[$scopeKey]; + return $scope; } /** @@ -439,7 +341,7 @@ protected function getOrCreateScopeInfo($currScope) */ protected function getVariableInfo($varName, $currScope) { - $scopeInfo = $this->getScopeInfo($currScope); + $scopeInfo = $this->scopeManager->getScopeForScopeStart($this->getFilename(), $currScope); return ($scopeInfo && isset($scopeInfo->variables[$varName])) ? $scopeInfo->variables[$varName] : null; } @@ -1873,7 +1775,7 @@ protected function processCompact(File $phpcsFile, $stackPtr) */ protected function processScopeClose(File $phpcsFile, $stackPtr) { - $scopeInfo = $this->getScopeInfo($stackPtr); + $scopeInfo = $this->scopeManager->getScopeForScopeStart($phpcsFile->getFilename(), $stackPtr); if (is_null($scopeInfo)) { return; }