Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider get_defined_vars() to be a read #92

Merged
merged 6 commits into from
Jun 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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];
Expand Down
56 changes: 55 additions & 1 deletion VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +123,51 @@ 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';
}
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])) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
}

Expand Down
14 changes: 14 additions & 0 deletions VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

function send_vars_to_method( $object, $data ) {
$some_number = 4;
$some_string = 'hello';
echo $undefined_data; // should be a warning
$new_data = $object->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 );
}