Skip to content

Commit 275cb81

Browse files
sirbrilligsirbrillig
authored
Add special cases for variables used in else blocks (#189)
* Add tests for if condition blocks * Process else conditions/blocks with more scrutiny * Add test for if block inside the else * Add test for definition within previous elseif * Check all previous if/elseif scopes rather than just the if * Make sure all assignments are in attached blocks
1 parent c6d3b02 commit 275cb81

File tree

4 files changed

+318
-0
lines changed

4 files changed

+318
-0
lines changed

‎Tests/VariableAnalysisSniff/IfConditionTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
3+
4+
use VariableAnalysis\Tests\BaseTestCase;
5+
6+
class IfConditionTest extends BaseTestCase {
7+
public function testIfConditionWarnings() {
8+
$fixtureFile = $this->getFixture('FunctionWithIfConditionFixture.php');
9+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
10+
$phpcsFile->ruleset->setSniffProperty(
11+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
12+
'allowUnusedParametersBeforeUsed',
13+
'true'
14+
);
15+
$phpcsFile->process();
16+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
17+
$expectedWarnings = [
18+
15,
19+
27,
20+
36,
21+
38,
22+
47,
23+
58,
24+
62,
25+
70,
26+
74,
27+
82,
28+
87,
29+
98,
30+
101,
31+
];
32+
$this->assertEquals($expectedWarnings, $lines);
33+
}
34+
}

‎Tests/VariableAnalysisSniff/fixtures/FunctionWithIfConditionFixture.php

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
<?php
2+
3+
function normalIfCondition($first, $second) {
4+
$name = 'human';
5+
if ($first) {
6+
$words = "hello {$name}";
7+
} elseif ($second) {
8+
$words = "bye {$name}";
9+
}
10+
echo $words;
11+
}
12+
13+
function undefinedInsideIfCondition($second) {
14+
$name = 'human';
15+
if ($first) { // undefined variable $first
16+
$words = "hello {$name}";
17+
} elseif ($second) {
18+
$words = "bye {$name}";
19+
}
20+
echo $words;
21+
}
22+
23+
function undefinedInsideElseCondition($first) {
24+
$name = 'human';
25+
if ($first) {
26+
$words = "hello {$name}";
27+
} elseif ($second) { // undefined variable $second
28+
$words = "bye {$name}";
29+
}
30+
echo $words;
31+
}
32+
33+
function definedInsideFirstBlockUndefinedInsideElseCondition($first) {
34+
$name = 'human';
35+
if ($first) {
36+
$second = true; // unused variable $second
37+
$words = "hello {$name}";
38+
} elseif ($second) { // undefined variable $second
39+
$words = "bye {$name}";
40+
}
41+
echo $words;
42+
}
43+
44+
function unusedInsideFirstBlock($first, $second) {
45+
$name = 'human';
46+
if ($first) {
47+
$unused = true; // unused variable $unused
48+
$words = "hello {$name}";
49+
} elseif ($second) {
50+
$words = "bye {$name}";
51+
}
52+
echo $words;
53+
}
54+
55+
function definedInsideFirstBlockUndefinedInsideElseIfBlock($first) {
56+
$name = 'human';
57+
if ($first) {
58+
$second = true; // unused variable $second
59+
$words = "hello {$name}";
60+
} elseif ($name) {
61+
$words = "bye {$name}";
62+
echo $second; // undefined variable $second
63+
}
64+
echo $words;
65+
}
66+
67+
function definedInsideFirstBlockUndefinedInsideElseBlock($first) {
68+
$name = 'human';
69+
if ($first) {
70+
$second = true; // unused variable $second
71+
$words = "hello {$name}";
72+
} else {
73+
$words = "bye {$name}";
74+
echo $second; // undefined variable $second
75+
}
76+
echo $words;
77+
}
78+
79+
function definedInsideFirstBlockUndefinedInsideElseBlockInsideAnotherIf($first) {
80+
$name = 'human';
81+
if ($first) {
82+
$second = true; // unused variable $second
83+
$words = "hello {$name}";
84+
} else {
85+
if ($name) {
86+
$words = "bye {$name}";
87+
echo $second; // undefined variable $second
88+
}
89+
}
90+
echo $words;
91+
}
92+
93+
function definedInsideElseIfBlockUndefinedInsideElseBlock($first) {
94+
$name = 'human';
95+
if ($first) {
96+
$words = "hello {$name}";
97+
} elseif ($name) {
98+
$second = true; // unused variable $second
99+
} else {
100+
$words = "bye {$name}";
101+
echo $second; // undefined variable $second
102+
}
103+
echo $words;
104+
}
105+
106+
function definedInsideFirstBlockUndefinedInsideUnconnectedElseCondition($first) {
107+
$name = 'human';
108+
if ($first) {
109+
$second = true;
110+
$words = "hello {$name}";
111+
} elseif ($name) {
112+
$words = "bye {$name}";
113+
}
114+
if ($first) {
115+
$second = true;
116+
$words = "hello {$name}";
117+
} elseif ($second) {
118+
$words = "bye {$name}";
119+
}
120+
echo $words;
121+
}
122+
123+
function definedInsideFirstBlockUndefinedInsideSecondCondition($first) {
124+
$name = 'human';
125+
if ($first) {
126+
$second = true;
127+
$words = "hello {$name}";
128+
}
129+
if ($second) {
130+
$words = "bye {$name}";
131+
}
132+
echo $words;
133+
}
134+
135+
function ifConditionWithPossibleDefinition($first) {
136+
if ($first) {
137+
$name = 'person';
138+
}
139+
echo $name;
140+
}
141+
142+
function ifConditionWithPossibleUse($first) {
143+
$name = 'person';
144+
if ($first) {
145+
echo $name;
146+
}
147+
}

‎VariableAnalysis/Lib/Helpers.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,4 +560,93 @@ public static function splitStringToArray($pattern, $value) {
560560
public static function isVariableANumericVariable($varName) {
561561
return is_numeric(substr($varName, 0, 1));
562562
}
563+
564+
/**
565+
* @param File $phpcsFile
566+
* @param int $stackPtr
567+
*
568+
* @return bool
569+
*/
570+
public static function isVariableInsideElseCondition(File $phpcsFile, $stackPtr) {
571+
$tokens = $phpcsFile->getTokens();
572+
$nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
573+
$nonFunctionTokenTypes[] = T_OPEN_PARENTHESIS;
574+
$nonFunctionTokenTypes[] = T_VARIABLE;
575+
$nonFunctionTokenTypes[] = T_ELLIPSIS;
576+
$nonFunctionTokenTypes[] = T_COMMA;
577+
$nonFunctionTokenTypes[] = T_STRING;
578+
$nonFunctionTokenTypes[] = T_BITWISE_AND;
579+
$elsePtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $stackPtr - 1, null, true, null, true));
580+
$elseTokenTypes = [
581+
T_ELSE,
582+
T_ELSEIF,
583+
];
584+
if (is_int($elsePtr) && in_array($tokens[$elsePtr]['code'], $elseTokenTypes, true)) {
585+
return true;
586+
}
587+
return false;
588+
}
589+
590+
/**
591+
* @param File $phpcsFile
592+
* @param int $stackPtr
593+
*
594+
* @return bool
595+
*/
596+
public static function isVariableInsideElseBody(File $phpcsFile, $stackPtr) {
597+
$tokens = $phpcsFile->getTokens();
598+
$token = $tokens[$stackPtr];
599+
$conditions = isset($token['conditions']) ? $token['conditions'] : [];
600+
$elseTokenTypes = [
601+
T_ELSE,
602+
T_ELSEIF,
603+
];
604+
foreach (array_reverse($conditions, true) as $scopeCode) {
605+
if (in_array($scopeCode, $elseTokenTypes, true)) {
606+
return true;
607+
}
608+
}
609+
return false;
610+
}
611+
612+
/**
613+
* @param File $phpcsFile
614+
* @param int $stackPtr
615+
*
616+
* @return int[]
617+
*/
618+
public static function getAttachedBlockIndicesForElse(File $phpcsFile, $stackPtr) {
619+
$currentElsePtr = $phpcsFile->findPrevious([T_ELSE, T_ELSEIF], $stackPtr - 1);
620+
if (! is_int($currentElsePtr)) {
621+
throw new \Exception("Cannot find expected else at {$stackPtr}");
622+
}
623+
624+
$ifPtr = $phpcsFile->findPrevious([T_IF], $currentElsePtr - 1);
625+
if (! is_int($ifPtr)) {
626+
throw new \Exception("Cannot find if for else at {$stackPtr}");
627+
}
628+
$blockIndices = [$ifPtr];
629+
630+
$previousElseIfPtr = $currentElsePtr;
631+
do {
632+
$elseIfPtr = $phpcsFile->findPrevious([T_ELSEIF], $previousElseIfPtr - 1, $ifPtr);
633+
if (is_int($elseIfPtr)) {
634+
$blockIndices[] = $elseIfPtr;
635+
$previousElseIfPtr = $elseIfPtr;
636+
}
637+
} while (is_int($elseIfPtr));
638+
639+
return $blockIndices;
640+
}
641+
642+
/**
643+
* @param int $needle
644+
* @param int $scopeStart
645+
* @param int $scopeEnd
646+
*
647+
* @return bool
648+
*/
649+
public static function isIndexInsideScope($needle, $scopeStart, $scopeEnd) {
650+
return ($needle > $scopeStart && $needle < $scopeEnd);
651+
}
563652
}

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,11 +1390,59 @@ protected function processVariable(File $phpcsFile, $stackPtr) {
13901390
return;
13911391
}
13921392

1393+
if (Helpers::isVariableInsideElseCondition($phpcsFile, $stackPtr) || Helpers::isVariableInsideElseBody($phpcsFile, $stackPtr)) {
1394+
Helpers::debug('found variable inside else condition or body');
1395+
$this->processVaribleInsideElse($phpcsFile, $stackPtr, $varName, $currScope);
1396+
return;
1397+
}
1398+
13931399
// OK, we don't appear to be a write to the var, assume we're a read.
13941400
Helpers::debug('looks like a variable read');
13951401
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
13961402
}
13971403

1404+
/**
1405+
* @param File $phpcsFile
1406+
* @param int $stackPtr
1407+
* @param string $varName
1408+
* @param int $currScope
1409+
*
1410+
* @return void
1411+
*/
1412+
protected function processVaribleInsideElse(File $phpcsFile, $stackPtr, $varName, $currScope) {
1413+
// Find all assignments to this variable inside the current scope.
1414+
$varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
1415+
$allAssignmentIndices = array_unique($varInfo->allAssignments);
1416+
// Find the attached 'if' and 'elseif' block start and end indices.
1417+
$blockIndices = Helpers::getAttachedBlockIndicesForElse($phpcsFile, $stackPtr);
1418+
1419+
// If all of the assignments are within the previous attached blocks, then warn about undefined.
1420+
$tokens = $phpcsFile->getTokens();
1421+
$assignmentsInsideAttachedBlocks = [];
1422+
foreach ($allAssignmentIndices as $index) {
1423+
foreach ($blockIndices as $blockIndex) {
1424+
Helpers::debug('looking at scope', $index, 'between', $tokens[$blockIndex]['scope_opener'], 'and', $tokens[$blockIndex]['scope_closer']);
1425+
if (Helpers::isIndexInsideScope($index, $tokens[$blockIndex]['scope_opener'], $tokens[$blockIndex]['scope_closer'])) {
1426+
$assignmentsInsideAttachedBlocks[] = $index;
1427+
}
1428+
}
1429+
}
1430+
1431+
if (count($assignmentsInsideAttachedBlocks) === count($allAssignmentIndices)) {
1432+
Helpers::debug("variable $varName inside else looks undefined");
1433+
$phpcsFile->addWarning(
1434+
"Variable %s is undefined.",
1435+
$stackPtr,
1436+
'UndefinedVariable',
1437+
["\${$varName}"]
1438+
);
1439+
return;
1440+
}
1441+
1442+
Helpers::debug('looks like a variable read inside else');
1443+
$this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
1444+
}
1445+
13981446
/**
13991447
* Called to process variables found in double quoted strings.
14001448
*

0 commit comments

Comments
 (0)