diff --git a/Tests/VariableAnalysisSniff/IfConditionTest.php b/Tests/VariableAnalysisSniff/IfConditionTest.php new file mode 100644 index 00000000..729bc2a9 --- /dev/null +++ b/Tests/VariableAnalysisSniff/IfConditionTest.php @@ -0,0 +1,34 @@ +getFixture('FunctionWithIfConditionFixture.php'); + $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile); + $phpcsFile->ruleset->setSniffProperty( + 'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff', + 'allowUnusedParametersBeforeUsed', + 'true' + ); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + $expectedWarnings = [ + 15, + 27, + 36, + 38, + 47, + 58, + 62, + 70, + 74, + 82, + 87, + 98, + 101, + ]; + $this->assertEquals($expectedWarnings, $lines); + } +} diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithIfConditionFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithIfConditionFixture.php new file mode 100644 index 00000000..0e90f494 --- /dev/null +++ b/Tests/VariableAnalysisSniff/fixtures/FunctionWithIfConditionFixture.php @@ -0,0 +1,147 @@ +getTokens(); + $nonFunctionTokenTypes = array_values(Tokens::$emptyTokens); + $nonFunctionTokenTypes[] = T_OPEN_PARENTHESIS; + $nonFunctionTokenTypes[] = T_VARIABLE; + $nonFunctionTokenTypes[] = T_ELLIPSIS; + $nonFunctionTokenTypes[] = T_COMMA; + $nonFunctionTokenTypes[] = T_STRING; + $nonFunctionTokenTypes[] = T_BITWISE_AND; + $elsePtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $stackPtr - 1, null, true, null, true)); + $elseTokenTypes = [ + T_ELSE, + T_ELSEIF, + ]; + if (is_int($elsePtr) && in_array($tokens[$elsePtr]['code'], $elseTokenTypes, true)) { + return true; + } + return false; + } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return bool + */ + public static function isVariableInsideElseBody(File $phpcsFile, $stackPtr) { + $tokens = $phpcsFile->getTokens(); + $token = $tokens[$stackPtr]; + $conditions = isset($token['conditions']) ? $token['conditions'] : []; + $elseTokenTypes = [ + T_ELSE, + T_ELSEIF, + ]; + foreach (array_reverse($conditions, true) as $scopeCode) { + if (in_array($scopeCode, $elseTokenTypes, true)) { + return true; + } + } + return false; + } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return int[] + */ + public static function getAttachedBlockIndicesForElse(File $phpcsFile, $stackPtr) { + $currentElsePtr = $phpcsFile->findPrevious([T_ELSE, T_ELSEIF], $stackPtr - 1); + if (! is_int($currentElsePtr)) { + throw new \Exception("Cannot find expected else at {$stackPtr}"); + } + + $ifPtr = $phpcsFile->findPrevious([T_IF], $currentElsePtr - 1); + if (! is_int($ifPtr)) { + throw new \Exception("Cannot find if for else at {$stackPtr}"); + } + $blockIndices = [$ifPtr]; + + $previousElseIfPtr = $currentElsePtr; + do { + $elseIfPtr = $phpcsFile->findPrevious([T_ELSEIF], $previousElseIfPtr - 1, $ifPtr); + if (is_int($elseIfPtr)) { + $blockIndices[] = $elseIfPtr; + $previousElseIfPtr = $elseIfPtr; + } + } while (is_int($elseIfPtr)); + + return $blockIndices; + } + + /** + * @param int $needle + * @param int $scopeStart + * @param int $scopeEnd + * + * @return bool + */ + public static function isIndexInsideScope($needle, $scopeStart, $scopeEnd) { + return ($needle > $scopeStart && $needle < $scopeEnd); + } } diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 4c2081c3..2d75e921 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -1390,11 +1390,59 @@ protected function processVariable(File $phpcsFile, $stackPtr) { return; } + if (Helpers::isVariableInsideElseCondition($phpcsFile, $stackPtr) || Helpers::isVariableInsideElseBody($phpcsFile, $stackPtr)) { + Helpers::debug('found variable inside else condition or body'); + $this->processVaribleInsideElse($phpcsFile, $stackPtr, $varName, $currScope); + return; + } + // OK, we don't appear to be a write to the var, assume we're a read. Helpers::debug('looks like a variable read'); $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope); } + /** + * @param File $phpcsFile + * @param int $stackPtr + * @param string $varName + * @param int $currScope + * + * @return void + */ + protected function processVaribleInsideElse(File $phpcsFile, $stackPtr, $varName, $currScope) { + // Find all assignments to this variable inside the current scope. + $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); + $allAssignmentIndices = array_unique($varInfo->allAssignments); + // Find the attached 'if' and 'elseif' block start and end indices. + $blockIndices = Helpers::getAttachedBlockIndicesForElse($phpcsFile, $stackPtr); + + // If all of the assignments are within the previous attached blocks, then warn about undefined. + $tokens = $phpcsFile->getTokens(); + $assignmentsInsideAttachedBlocks = []; + foreach ($allAssignmentIndices as $index) { + foreach ($blockIndices as $blockIndex) { + Helpers::debug('looking at scope', $index, 'between', $tokens[$blockIndex]['scope_opener'], 'and', $tokens[$blockIndex]['scope_closer']); + if (Helpers::isIndexInsideScope($index, $tokens[$blockIndex]['scope_opener'], $tokens[$blockIndex]['scope_closer'])) { + $assignmentsInsideAttachedBlocks[] = $index; + } + } + } + + if (count($assignmentsInsideAttachedBlocks) === count($allAssignmentIndices)) { + Helpers::debug("variable $varName inside else looks undefined"); + $phpcsFile->addWarning( + "Variable %s is undefined.", + $stackPtr, + 'UndefinedVariable', + ["\${$varName}"] + ); + return; + } + + Helpers::debug('looks like a variable read inside else'); + $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope); + } + /** * Called to process variables found in double quoted strings. *