Skip to content

Refactor methods to have nullable return types #95

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 25, 2019
Merged

Refactor methods to have nullable return types #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
987ee80
Change findContaining methods to nullable return
sirbrillig Jun 25, 2019
66e7fc4
Change findFunctionCallArguments to array return
sirbrillig Jun 25, 2019
6886448
Change isNextThingAnAssign to nullable return
sirbrillig Jun 25, 2019
7e2bca5
Rename isNextThingAnAssign to getNextAssignPointer
sirbrillig Jun 25, 2019
e165c4a
Change findFunctionPrototype to nullable return
sirbrillig Jun 25, 2019
9940ee6
Change findVariableScope to nullable return
sirbrillig Jun 25, 2019
716b755
Remove 'file' default for getScopeKey
sirbrillig Jun 25, 2019
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
77 changes: 42 additions & 35 deletions VariableAnalysis/Lib/Helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,38 @@
use PHP_CodeSniffer\Files\File;

class Helpers {
/**
* @param int|bool $value
*
* @return ?int
*/
public static function getIntOrNull($value) {
return is_int($value) ? $value: null;
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findContainingOpeningSquareBracket(File $phpcsFile, $stackPtr) {
$previousStatementPtr = self::getPreviousStatementPtr($phpcsFile, $stackPtr);
return $phpcsFile->findPrevious(T_OPEN_SHORT_ARRAY, $stackPtr - 1, $previousStatementPtr);
return self::getIntOrNull($phpcsFile->findPrevious(T_OPEN_SHORT_ARRAY, $stackPtr - 1, $previousStatementPtr));
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findContainingClosingSquareBracket(File $phpcsFile, $stackPtr) {
$endOfStatementPtr = $phpcsFile->findNext([T_SEMICOLON], $stackPtr + 1);
if (is_bool($endOfStatementPtr)) {
return false;
$endOfStatementPtr = self::getIntOrNull($phpcsFile->findNext([T_SEMICOLON], $stackPtr + 1));
if (! $endOfStatementPtr) {
return null;
}
return $phpcsFile->findNext(T_CLOSE_SHORT_ARRAY, $stackPtr + 1, $endOfStatementPtr);
return self::getIntOrNull($phpcsFile->findNext(T_CLOSE_SHORT_ARRAY, $stackPtr + 1, $endOfStatementPtr));
}

/**
Expand All @@ -45,25 +54,25 @@ public static function getPreviousStatementPtr(File $phpcsFile, $stackPtr) {
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findContainingOpeningBracket(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
if (isset($tokens[$stackPtr]['nested_parenthesis'])) {
$openPtrs = array_keys($tokens[$stackPtr]['nested_parenthesis']);
return (int)end($openPtrs);
}
return false;
return null;
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findParenthesisOwner(File $phpcsFile, $stackPtr) {
return $phpcsFile->findPrevious(T_WHITESPACE, $stackPtr - 1, null, true);
return self::getIntOrNull($phpcsFile->findPrevious(T_WHITESPACE, $stackPtr - 1, null, true));
}

/**
Expand Down Expand Up @@ -123,22 +132,22 @@ public static function areConditionsWithinFunctionBeforeClass(array $conditions)
* @param File $phpcsFile
* @param int $openPtr
*
* @return int|bool
* @return ?int
*/
public static function findPreviousFunctionPtr(File $phpcsFile, $openPtr) {
// Function names are T_STRING, and return-by-reference is T_BITWISE_AND,
// so we look backwards from the opening bracket for the first thing that
// isn't a function name, reference sigil or whitespace and check if it's a
// function keyword.
$functionPtrTypes = [T_STRING, T_WHITESPACE, T_BITWISE_AND];
return $phpcsFile->findPrevious($functionPtrTypes, $openPtr - 1, null, true, null, true);
return self::getIntOrNull($phpcsFile->findPrevious($functionPtrTypes, $openPtr - 1, null, true, null, true));
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findFunctionCall(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
Expand All @@ -147,18 +156,18 @@ public static function findFunctionCall(File $phpcsFile, $stackPtr) {
if (is_int($openPtr)) {
// First non-whitespace thing and see if it's a T_STRING function name
$functionPtr = $phpcsFile->findPrevious(T_WHITESPACE, $openPtr - 1, null, true, null, true);
if ($tokens[$functionPtr]['code'] === T_STRING) {
if (is_int($functionPtr) && $tokens[$functionPtr]['code'] === T_STRING) {
return $functionPtr;
}
}
return false;
return null;
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return array[]|false
* @return array[]
*/
public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
Expand All @@ -167,19 +176,19 @@ public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
if (($tokens[$stackPtr]['code'] !== T_STRING) && ($tokens[$stackPtr]['code'] !== T_ARRAY)) {
// Assume $stackPtr is something within the brackets, find our function call
$stackPtr = Helpers::findFunctionCall($phpcsFile, $stackPtr);
if ($stackPtr === false) {
return false;
if ($stackPtr === null) {
return [];
}
}

// $stackPtr is the function name, find our brackets after it
$openPtr = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true, null, true);
if (($openPtr === false) || ($tokens[$openPtr]['code'] !== T_OPEN_PARENTHESIS)) {
return false;
return [];
}

if (!isset($tokens[$openPtr]['parenthesis_closer'])) {
return false;
return [];
}
$closePtr = $tokens[$openPtr]['parenthesis_closer'];

Expand Down Expand Up @@ -220,7 +229,7 @@ public static function findWhereAssignExecuted(File $phpcsFile, $stackPtr) {
$commaPtr = $phpcsFile->findNext(T_COMMA, $stackPtr + 1, null, false, null, true);
$closePtr = false;
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if ($openPtr !== false) {
if ($openPtr !== null) {
if (isset($tokens[$openPtr]['parenthesis_closer'])) {
$closePtr = $tokens[$openPtr]['parenthesis_closer'];
}
Expand All @@ -240,19 +249,17 @@ public static function findWhereAssignExecuted(File $phpcsFile, $stackPtr) {
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function isNextThingAnAssign(File $phpcsFile, $stackPtr) {
public static function getNextAssignPointer(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();

// Is the next non-whitespace an assignment?
$nextPtr = $phpcsFile->findNext(T_WHITESPACE, $stackPtr + 1, null, true, null, true);
if ($nextPtr !== false) {
if ($tokens[$nextPtr]['code'] === T_EQUAL) {
return $nextPtr;
}
if (is_int($nextPtr) && $tokens[$nextPtr]['code'] === T_EQUAL) {
return $nextPtr;
}
return false;
return null;
}

/**
Expand All @@ -269,27 +276,27 @@ public static function normalizeVarName($varName) {
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|bool
* @return ?int
*/
public static function findFunctionPrototype(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();

$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if (! is_int($openPtr)) {
return false;
return null;
}
$functionPtr = Helpers::findPreviousFunctionPtr($phpcsFile, $openPtr);
if (($functionPtr !== false) && ($tokens[$functionPtr]['code'] === T_FUNCTION)) {
if (($functionPtr !== null) && ($tokens[$functionPtr]['code'] === T_FUNCTION)) {
return $functionPtr;
}
return false;
return null;
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* @return int|false
* @return ?int
*/
public static function findVariableScope(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
Expand All @@ -314,7 +321,7 @@ public static function findVariableScope(File $phpcsFile, $stackPtr) {

if ($in_class) {
// Member var of a class, we don't care.
return false;
return null;
}

// File scope, hmm, lets use first token of file?
Expand Down
44 changes: 17 additions & 27 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,11 @@ protected function isGetDefinedVars(File $phpcsFile, $stackPtr) {
}

/**
* @param int|bool $currScope
* @param int $currScope
*
* @return string
*/
protected function getScopeKey($currScope) {
if ($currScope === false) {
Copy link
Owner Author

@sirbrillig sirbrillig Jun 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to review the history of this and make sure that it's no longer necessary.

Copy link
Owner Author

@sirbrillig sirbrillig Jun 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this guard is extremely old (from this change) and I think is just no longer necessary.

$currScope = 'file';
}
return ($this->currentFile ? $this->currentFile->getFilename() : 'unknown file') . ':' . $currScope;
}

Expand Down Expand Up @@ -442,7 +439,7 @@ protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $sta
*/
protected function markAllVariablesRead(File $phpcsFile, $stackPtr) {
$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
if (! $currScope) {
if ($currScope === null) {
return;
}
$scopeInfo = $this->getOrCreateScopeInfo($currScope);
Expand All @@ -469,7 +466,7 @@ protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varNam
// T_FUNCTION, but AbstractVariableSniff and AbstractScopeSniff define everything
// we need to do that as private or final, so we have to do it this hackish way.
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if (is_bool($openPtr)) {
if (! is_int($openPtr)) {
return false;
}

Expand All @@ -487,7 +484,7 @@ protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varNam
$varInfo->passByReference = true;
}
// Are we optional with a default?
if (Helpers::isNextThingAnAssign($phpcsFile, $stackPtr) !== false) {
if (Helpers::getNextAssignPointer($phpcsFile, $stackPtr) !== null) {
$this->markVariableAssignment($varName, $stackPtr, $functionPtr);
}
return true;
Expand Down Expand Up @@ -574,7 +571,7 @@ protected function checkForCatchBlock(File $phpcsFile, $stackPtr, $varName, $cur

// Are we a catch block parameter?
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if ($openPtr === false) {
if ($openPtr === null) {
return false;
}

Expand Down Expand Up @@ -736,7 +733,7 @@ protected function checkForStaticOutsideClass(File $phpcsFile, $stackPtr, $varNa
*/
protected function checkForAssignment(File $phpcsFile, $stackPtr, $varName, $currScope) {
// Is the next non-whitespace an assignment?
$assignPtr = Helpers::isNextThingAnAssign($phpcsFile, $stackPtr);
$assignPtr = Helpers::getNextAssignPointer($phpcsFile, $stackPtr);
if (! is_int($assignPtr)) {
return false;
}
Expand Down Expand Up @@ -787,7 +784,7 @@ protected function checkForVariableVariable(File $phpcsFile, $stackPtr, $varName
protected function checkForListShorthandAssignment(File $phpcsFile, $stackPtr, $varName, $currScope) {
// OK, are we within a [ ... ] construct?
$openPtr = Helpers::findContainingOpeningSquareBracket($phpcsFile, $stackPtr);
if ($openPtr === false) {
if (! is_int($openPtr)) {
return false;
}

Expand All @@ -796,7 +793,7 @@ protected function checkForListShorthandAssignment(File $phpcsFile, $stackPtr, $
if (! is_int($closePtr)) {
return false;
}
$assignPtr = Helpers::isNextThingAnAssign($phpcsFile, $closePtr);
$assignPtr = Helpers::getNextAssignPointer($phpcsFile, $closePtr);
if (! is_int($assignPtr)) {
return false;
}
Expand All @@ -820,7 +817,7 @@ protected function checkForListAssignment(File $phpcsFile, $stackPtr, $varName,

// OK, are we within a list (...) construct?
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if ($openPtr === false) {
if ($openPtr === null) {
return false;
}

Expand All @@ -831,7 +828,7 @@ protected function checkForListAssignment(File $phpcsFile, $stackPtr, $varName,

// OK, we're a list (...) construct... are we being assigned to?
$closePtr = $tokens[$openPtr]['parenthesis_closer'];
$assignPtr = Helpers::isNextThingAnAssign($phpcsFile, $closePtr);
$assignPtr = Helpers::getNextAssignPointer($phpcsFile, $closePtr);
if (! is_int($assignPtr)) {
return false;
}
Expand Down Expand Up @@ -927,7 +924,7 @@ protected function checkForStaticDeclaration(File $phpcsFile, $stackPtr, $varNam

// It's a static declaration.
$this->markVariableDeclaration($varName, 'static', null, $stackPtr, $currScope);
if (Helpers::isNextThingAnAssign($phpcsFile, $stackPtr) !== false) {
if (Helpers::getNextAssignPointer($phpcsFile, $stackPtr) !== null) {
$this->markVariableAssignment($varName, $stackPtr, $currScope);
}
return true;
Expand Down Expand Up @@ -1000,7 +997,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 || ! isset($tokens[$functionPtr])) {
if ($functionPtr === null || ! isset($tokens[$functionPtr])) {
return false;
}

Expand All @@ -1012,9 +1009,6 @@ protected function checkForPassByReferenceFunctionCall(File $phpcsFile, $stackPt
}

$argPtrs = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr);
if ($argPtrs === false) {
return false;
}

// We're within a function call arguments list, find which arg we are.
$argPos = false;
Expand Down Expand Up @@ -1094,7 +1088,7 @@ protected function processVariable(File $phpcsFile, $stackPtr) {
$varName = Helpers::normalizeVarName($token['content']);
Helpers::debug('examining token ' . $varName);
$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
if ($currScope === false) {
if ($currScope === null) {
Helpers::debug('no scope found');
return;
}
Expand Down Expand Up @@ -1239,7 +1233,7 @@ protected function processVariableInString(File $phpcsFile, $stackPtr) {
}

$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
if (! $currScope) {
if ($currScope === null) {
return;
}
foreach ($matches[1] as $varName) {
Expand Down Expand Up @@ -1287,9 +1281,7 @@ protected function processCompactArguments(File $phpcsFile, $stackPtr, $argument
if ($argument_first_token['code'] === T_ARRAY) {
// It's an array argument, recurse.
$array_arguments = Helpers::findFunctionCallArguments($phpcsFile, $argumentPtrs[0]);
if ($array_arguments !== false) {
$this->processCompactArguments($phpcsFile, $stackPtr, $array_arguments, $currScope);
}
$this->processCompactArguments($phpcsFile, $stackPtr, $array_arguments, $currScope);
continue;
}
if (count($argumentPtrs) > 1) {
Expand Down Expand Up @@ -1327,14 +1319,12 @@ protected function processCompactArguments(File $phpcsFile, $stackPtr, $argument
*/
protected function processCompact(File $phpcsFile, $stackPtr) {
$currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
if (! $currScope) {
if ($currScope === null) {
return;
}

$arguments = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr);
if ($arguments !== false) {
$this->processCompactArguments($phpcsFile, $stackPtr, $arguments, $currScope);
}
$this->processCompactArguments($phpcsFile, $stackPtr, $arguments, $currScope);
}

/**
Expand Down