Skip to content

Commit 9ef0985

Browse files
sirbrilligsirbrillig
authored
Handle inline if/else (#199)
* Add tests for inline if/else statements * Handle inline if/else statements
1 parent 352d9d4 commit 9ef0985

File tree

4 files changed

+181
-2
lines changed

4 files changed

+181
-2
lines changed

‎Tests/VariableAnalysisSniff/IfConditionTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,32 @@ public function testIfConditionWarnings() {
3131
];
3232
$this->assertEquals($expectedWarnings, $lines);
3333
}
34+
35+
public function testInlineIfConditionWarnings() {
36+
$fixtureFile = $this->getFixture('FunctionWithInlineIfConditionFixture.php');
37+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
38+
$phpcsFile->ruleset->setSniffProperty(
39+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
40+
'allowUnusedParametersBeforeUsed',
41+
'true'
42+
);
43+
$phpcsFile->process();
44+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
45+
$expectedWarnings = [
46+
14,
47+
25,
48+
34,
49+
35,
50+
44,
51+
54,
52+
56,
53+
64,
54+
66,
55+
74,
56+
77,
57+
86,
58+
88,
59+
];
60+
$this->assertEquals($expectedWarnings, $lines);
61+
}
3462
}

‎Tests/VariableAnalysisSniff/fixtures/FunctionWithInlineIfConditionFixture.php‎

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

‎VariableAnalysis/Lib/Helpers.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,18 @@ public static function isVariableInsideElseBody(File $phpcsFile, $stackPtr) {
589589
return true;
590590
}
591591
}
592+
593+
// Some else body code will not have conditions because it is inline (no
594+
// curly braces) so we have to look in other ways.
595+
$previousSemicolonPtr = $phpcsFile->findPrevious([T_SEMICOLON], $stackPtr - 1);
596+
if (! is_int($previousSemicolonPtr)) {
597+
$previousSemicolonPtr = 0;
598+
}
599+
$elsePtr = $phpcsFile->findPrevious([T_ELSE, T_ELSEIF], $stackPtr - 1, $previousSemicolonPtr);
600+
if (is_int($elsePtr)) {
601+
return true;
602+
}
603+
592604
return false;
593605
}
594606

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,8 +1411,21 @@ protected function processVaribleInsideElse(File $phpcsFile, $stackPtr, $varName
14111411
$assignmentsInsideAttachedBlocks = [];
14121412
foreach ($allAssignmentIndices as $index) {
14131413
foreach ($blockIndices as $blockIndex) {
1414-
Helpers::debug('looking at scope', $index, 'between', $tokens[$blockIndex]['scope_opener'], 'and', $tokens[$blockIndex]['scope_closer']);
1415-
if (Helpers::isIndexInsideScope($index, $tokens[$blockIndex]['scope_opener'], $tokens[$blockIndex]['scope_closer'])) {
1414+
$blockToken = $tokens[$blockIndex];
1415+
Helpers::debug('looking at assignment', $index, 'at block index', $blockIndex, 'which is token', $blockToken);
1416+
if (isset($blockToken['scope_opener']) && isset($blockToken['scope_closer'])) {
1417+
$scopeOpener = $blockToken['scope_opener'];
1418+
$scopeCloser = $blockToken['scope_closer'];
1419+
} else {
1420+
// If the `if` statement has no scope, it is probably inline, which means its scope is up until the next semicolon
1421+
$scopeOpener = $blockIndex + 1;
1422+
$scopeCloser = $phpcsFile->findNext([T_SEMICOLON], $scopeOpener);
1423+
if (! $scopeCloser) {
1424+
throw new \Exception("Cannot find scope for if condition block at index {$stackPtr} while examining variable {$varName}");
1425+
}
1426+
}
1427+
Helpers::debug('looking at scope', $index, 'between', $scopeOpener, 'and', $scopeCloser);
1428+
if (Helpers::isIndexInsideScope($index, $scopeOpener, $scopeCloser)) {
14161429
$assignmentsInsideAttachedBlocks[] = $index;
14171430
}
14181431
}

0 commit comments

Comments
 (0)