diff --git a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php index 8e77b1a..2020e86 100644 --- a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php +++ b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php @@ -1040,6 +1040,7 @@ public function testAssignmentByReference() { 34, 35, 43, + 70, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -1058,6 +1059,7 @@ public function testAssignmentByReferenceWithIgnoreUnusedMatch() { 26, 34, 35, + 70, ]; $this->assertEquals($expectedWarnings, $lines); } diff --git a/Tests/VariableAnalysisSniff/fixtures/AssignmentByReferenceFixture.php b/Tests/VariableAnalysisSniff/fixtures/AssignmentByReferenceFixture.php index 0f138c4..2561c94 100644 --- a/Tests/VariableAnalysisSniff/fixtures/AssignmentByReferenceFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/AssignmentByReferenceFixture.php @@ -54,3 +54,21 @@ function doubleUsedThenUsedAssignmentByReference() { $var = &$bee; return $var; } + +function somefunc($choice, &$arr1, &$arr_default) { + $var = &$arr_default; + + if ($choice) { + $var = &$arr1; + } + + echo $var; +} + +function somefunc($choice, &$arr1, &$arr_default) { + if ($choice) { + $var = &$arr_default; // unused variable $var + $var = &$arr1; + echo $var; + } +} diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index d9dab91..b84ac49 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -102,6 +102,25 @@ public static function areConditionsWithinFunctionBeforeClass(array $conditions) return false; } + /** + * @param (int|string)[] $conditions + * + * @return int|string|null + */ + public static function getClosestIfPositionIfBeforeOtherConditions(array $conditions) { + // Return true if the token conditions are within an if block before + // they are within a class or function. + $conditionsInsideOut = array_reverse($conditions, true); + if (empty($conditions)) { + return null; + } + $scopeCode = reset($conditionsInsideOut); + if ($scopeCode === T_IF) { + return key($conditionsInsideOut); + } + return null; + } + /** * @param File $phpcsFile * @param int $stackPtr diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index fecf0b5..bfe41f2 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -937,9 +937,18 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN Helpers::debug('processVariableAsAssignment: found reference variable'); $varInfo = $this->getOrCreateVariableInfo($varName, $currScope); // If the variable was already declared, but was not yet read, it is - // unused because we're about to change the binding. + // unused because we're about to change the binding; that is, unless we + // are inside a conditional block because in that case the condition may + // never activate. $scopeInfo = $this->getOrCreateScopeInfo($currScope); - $this->processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo); + $ifPtr = Helpers::getClosestIfPositionIfBeforeOtherConditions($tokens[$referencePtr]['conditions']); + $lastAssignmentPtr = $varInfo->firstDeclared; + if (! $ifPtr && $lastAssignmentPtr) { + $this->processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo); + } + if ($ifPtr && $lastAssignmentPtr && $ifPtr <= $lastAssignmentPtr) { + $this->processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo); + } // The referenced variable may have a different name, but we don't // actually need to mark it as used in this case because the act of this // assignment will mark it used on the next token.