Skip to content

Commit

Permalink
Refactor to move scope management to ScopeManager class (#265)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
sirbrillig authored Jul 25, 2022
1 parent e1b89d1 commit 2747116
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 120 deletions.
108 changes: 108 additions & 0 deletions VariableAnalysis/Lib/ScopeManager.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php

namespace VariableAnalysis\Lib;

use VariableAnalysis\Lib\ScopeInfo;
use VariableAnalysis\Lib\Helpers;
use PHP_CodeSniffer\Files\File;

class ScopeManager
{
/**
* An associative array of a list of token index pairs which start and end
* scopes and will be used to check for unused variables.
*
* The outer array of scopes is keyed by a string containing the filename.
* The inner array of scopes in keyed by the scope start token index.
*
* @var array<string, array<int, ScopeInfo>>
*/
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;
}
}
142 changes: 22 additions & 120 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<string, ScopeInfo>
*/
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<string, ScopeInfo[]>
* @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
Expand Down Expand Up @@ -165,6 +139,11 @@ class VariableAnalysisSniff implements Sniff
*/
public $allowUnusedVariablesBeforeRequire = false;

public function __construct()
{
$this->scopeManager = new ScopeManager();
}

/**
* Decide which tokens to scan.
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*
Expand All @@ -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);
Expand Down Expand Up @@ -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
*/
Expand All @@ -406,29 +316,21 @@ 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
*
* @return ScopeInfo
*/
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;
}

/**
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 2747116

Please sign in to comment.