diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index a1b12b0a..92dd3518 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -3,7 +3,6 @@ namespace VariableAnalysis\Lib; use PHP_CodeSniffer\Files\File; -use VariableAnalysis\Lib\VariableInfo; class Helpers { public static function findContainingOpeningSquareBracket(File $phpcsFile, $stackPtr) { @@ -84,6 +83,12 @@ public static function findPreviousFunctionPtr(File $phpcsFile, $openPtr) { return $phpcsFile->findPrevious($functionPtrTypes, $openPtr - 1, null, true, null, true); } + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return int|false + */ public static function findFunctionCall(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); @@ -198,6 +203,12 @@ public static function findFunctionPrototype(File $phpcsFile, $stackPtr) { return false; } + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return int|false + */ public static function findVariableScope(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); $token = $tokens[$stackPtr]; diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index c9180990..0034908e 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -123,11 +123,29 @@ public function process(File $phpcsFile, $stackPtr) { if (($token['code'] === T_STRING) && ($token['content'] === 'compact')) { return $this->processCompact($phpcsFile, $stackPtr); } + if ($this->isGetDefinedVars($phpcsFile, $stackPtr)) { + Helpers::debug('get_defined_vars is being called'); + return $this->markAllVariablesRead($phpcsFile, $stackPtr); + } if (($token['code'] === T_CLOSE_CURLY_BRACKET) && isset($token['scope_condition'])) { return $this->processScopeClose($phpcsFile, $token['scope_condition']); } } + protected function isGetDefinedVars(File $phpcsFile, $stackPtr) { + $tokens = $phpcsFile->getTokens(); + $token = $tokens[$stackPtr]; + if (! $token || $token['content'] !== 'get_defined_vars') { + return false; + } + // Make sure this is a function call + $parenPointer = $phpcsFile->findNext(T_OPEN_PARENTHESIS, $stackPtr, $stackPtr + 2); + if (! $parenPointer) { + return false; + } + return true; + } + protected function getScopeKey($currScope) { if ($currScope === false) { $currScope = 'file'; @@ -135,11 +153,21 @@ protected function getScopeKey($currScope) { return ($this->currentFile ? $this->currentFile->getFilename() : 'unknown file') . ':' . $currScope; } + /** + * @param int $currScope + * + * @return ?ScopeInfo + */ 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])) { @@ -241,6 +269,13 @@ protected function markVariableDeclaration( $varInfo->firstDeclared = $stackPtr; } + /** + * @param string $varName + * @param int $stackPtr + * @param int $currScope + * + * @return void + */ protected function markVariableRead($varName, $stackPtr, $currScope) { $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); if (isset($varInfo->firstRead) && ($varInfo->firstRead <= $stackPtr)) { @@ -284,6 +319,25 @@ protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $sta } } + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return void + */ + protected function markAllVariablesRead(File $phpcsFile, $stackPtr) { + $currScope = Helpers::findVariableScope($phpcsFile, $stackPtr); + if (! $currScope) { + return false; + } + $scopeInfo = $this->getOrCreateScopeInfo($currScope); + $count = count($scopeInfo->variables); + Helpers::debug("marking all $count variables in scope as read"); + foreach ($scopeInfo->variables as $varInfo) { + $this->markVariableRead($varInfo->name, $stackPtr, $scopeInfo->owner); + } + } + protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varName, $currScope) { $tokens = $phpcsFile->getTokens(); $token = $tokens[$stackPtr]; @@ -723,7 +777,7 @@ protected function checkForPassByReferenceFunctionCall(File $phpcsFile, $stackPt // Are we pass-by-reference to known pass-by-reference function? $functionPtr = Helpers::findFunctionCall($phpcsFile, $stackPtr); - if ($functionPtr === false) { + if ($functionPtr === false || ! isset($tokens[$functionPtr])) { return false; } diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 0f8e1a79..b44ea5dc 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -895,4 +895,18 @@ public function testUnusedForeachVariablesAreIgnoredIfSet() { ]; $this->assertEquals($expectedWarnings, $lines); } + + public function testGetDefinedVarsCountsAsRead() { + $fixtureFile = $this->getFixture('GetDefinedVarsFixture.php'); + $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + $expectedWarnings = [ + 6, + 18, + 22, + 29, + ]; + $this->assertEquals($expectedWarnings, $lines); + } } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/GetDefinedVarsFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/GetDefinedVarsFixture.php new file mode 100644 index 00000000..89e762fd --- /dev/null +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/GetDefinedVarsFixture.php @@ -0,0 +1,31 @@ +transform_data( $data ); + $object->continue_things( get_defined_vars() ); +} + +function send_vars_to_method_with_global( $object, $data ) { + global $global_data; + $new_data = $object->transform_data( $data ); + $object->continue_things( get_defined_vars() ); +} + +function send_vars_to_method_with_scope_import( $object, $data ) { + $unused_data = 42; // should be a warning + $imported_data = 76; + return array_map( function( $datum ) use ( $object, $imported_data ) { + $new_data = $object->transform_data( $datum ); + echo $undefined_data; // should be a warning + $object->continue_things( get_defined_vars() ); + }, $data ); +} + +function send_var_with_var_named_get_defined_vars( $object, $data ) { + $get_defined_vars = 'hi'; + $new_data = $object->transform_data( $data ); // should be a warning + $object->continue_things( $get_defined_vars ); +}