Skip to content

Commit 1087f11

Browse files
sirbrilligsirbrillig
authored
Allow for loop increment expression to use variable defined inside loop (#262)
* Add tests for for loops * Reprocess for loop increment vars after loop ends * Prevent treating for loops (and other blocks) as function calls * Add test for for more specialized for loops * Add tests for unusual loops to test third expressions * Handle for loops without scope * Support 'endfor' as scope closer * Handle inline for loops * Fix indentation
1 parent 2747116 commit 1087f11

File tree

5 files changed

+549
-10
lines changed

5 files changed

+549
-10
lines changed

‎Tests/VariableAnalysisSniff/ForLoopTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
4+
5+
use VariableAnalysis\Tests\BaseTestCase;
6+
7+
class ForLoopTest extends BaseTestCase
8+
{
9+
public function testFunctionWithForLoopWarningsReportsCorrectLines()
10+
{
11+
$fixtureFile = $this->getFixture('FunctionWithForLoopFixture.php');
12+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
13+
$phpcsFile->process();
14+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
15+
$expectedWarnings = [
16+
22,
17+
65,
18+
80,
19+
94,
20+
110,
21+
112,
22+
113,
23+
118,
24+
123,
25+
129,
26+
];
27+
$this->assertSame($expectedWarnings, $lines);
28+
}
29+
30+
public function testFunctionWithForLoopWarningsHasCorrectSniffCodes()
31+
{
32+
$fixtureFile = $this->getFixture('FunctionWithForLoopFixture.php');
33+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
34+
$phpcsFile->process();
35+
$warnings = $phpcsFile->getWarnings();
36+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[22][17][0]['source']);
37+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[65][16][0]['source']);
38+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[80][16][0]['source']);
39+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[94][5][0]['source']);
40+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[110][7][0]['source']);
41+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[112][7][0]['source']);
42+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[113][16][0]['source']);
43+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[118][5][0]['source']);
44+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[123][5][0]['source']);
45+
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[129][14][0]['source']);
46+
}
47+
}

‎Tests/VariableAnalysisSniff/fixtures/FunctionWithForLoopFixture.php‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
<?php
2+
3+
function defineIncrementBeforeLoop($fp, $string) {
4+
$count = 10;
5+
for (
6+
$written = 0;
7+
$written < strlen($string);
8+
$written += $count
9+
) {
10+
$fwrite = fwrite($fp, substr($string, $written));
11+
if ($fwrite === false) {
12+
return $written;
13+
}
14+
}
15+
return $written;
16+
}
17+
18+
function defineIncrementAfterLoop($fp, $string) {
19+
for (
20+
$written = 0;
21+
$written < strlen($string);
22+
$written += $count // Undefined variable $count
23+
) {
24+
$fwrite = fwrite($fp, substr($string, $written));
25+
if ($fwrite === false) {
26+
return $written;
27+
}
28+
}
29+
$count = 10; // This is an unused variable, but it's hard to tell because it has been read above in the same scope.
30+
return $written;
31+
}
32+
33+
function defineIncrementInsideLoop($fp, $string) {
34+
for (
35+
$written = 0;
36+
$written < strlen($string);
37+
$written += $fwrite
38+
) {
39+
$fwrite = fwrite($fp, substr($string, $written));
40+
if ($fwrite === false) {
41+
return $written;
42+
}
43+
}
44+
return $written;
45+
}
46+
47+
function defineConditionBeforeLoop($fp, $string) {
48+
$count = 10;
49+
for (
50+
$written = 0;
51+
$written < $count;
52+
$written += 1
53+
) {
54+
$fwrite = fwrite($fp, substr($string, $written));
55+
if ($fwrite === false) {
56+
return $written;
57+
}
58+
}
59+
return $written;
60+
}
61+
62+
function defineConditionAfterLoop($fp, $string) {
63+
for (
64+
$written = 0;
65+
$written < $count; // Undefined variable $count
66+
$written += 1
67+
) {
68+
$fwrite = fwrite($fp, substr($string, $written));
69+
if ($fwrite === false) {
70+
return $written;
71+
}
72+
}
73+
$count = 10;
74+
return $written;
75+
}
76+
77+
function defineConditionInsideLoop($fp, $string) {
78+
for (
79+
$written = 0;
80+
$written < $fwrite; // Undefined variable $fwrite
81+
$written += 1
82+
) {
83+
$fwrite = fwrite($fp, substr($string, $written));
84+
if ($fwrite === false) {
85+
return $written;
86+
}
87+
}
88+
return $written;
89+
}
90+
91+
function unusedInitializer($fp, $string) {
92+
$written = 0;
93+
for (
94+
$foo = 0; // Unused variable $foo
95+
;
96+
$written += 1
97+
) {
98+
fwrite($fp, substr($string, $written));
99+
if ($written > 10) {
100+
break;
101+
}
102+
}
103+
}
104+
105+
function closureInsideLoopInit() {
106+
for (
107+
$closure = function(
108+
$a,
109+
$b,
110+
$c // Unused variable $c
111+
) {
112+
$m = 1; // Unused variable $m
113+
var_dump($i); // Undefined variable $i
114+
return $a * $b;
115+
},
116+
$a = 0,
117+
$b = 0,
118+
$c = 0, // Unused variable $c
119+
$i = 10;
120+
$closure($a, $b, 4) < $i;
121+
$i++,
122+
$a++,
123+
$t++, // Undefined variable $t
124+
$b++
125+
)
126+
{
127+
var_dump($a);
128+
var_dump($b);
129+
var_dump($m); // Undefined variable $m
130+
}
131+
}
132+
133+
function veryBoringForLoop() {
134+
for (
135+
;
136+
;
137+
) {
138+
}
139+
}
140+
141+
function reallySmallForLoop() {
142+
for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++);
143+
}
144+
145+
function colonSyntaxForLoop() {
146+
for ($i = 0; $i < 10; $i++):
147+
print $i;
148+
endfor;
149+
}
150+
151+
function colonSyntaxForLoopAndLateDefinition() {
152+
for ($i = 0; $i < 10; var_dump($foo), $i++):
153+
$foo = 'hello';
154+
print $i;
155+
endfor;
156+
}
157+
158+
function forLoopWithOneStatement() {
159+
for ($i = 0; $i < 10; $i++) echo $i;
160+
}
161+
162+
function forLoopWithOneStatementAndLateDefinition() {
163+
for ($i = 0; $i < 10; var_dump($foo), $i++) $foo = 'hello';
164+
}

‎VariableAnalysis/Lib/ForLoopInfo.php‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
namespace VariableAnalysis\Lib;
4+
5+
/**
6+
* Holds details of a for loop.
7+
*/
8+
class ForLoopInfo
9+
{
10+
/**
11+
* The position of the `for` token.
12+
*
13+
* @var int
14+
*/
15+
public $forIndex;
16+
17+
/**
18+
* The position of the initialization expression opener for the loop.
19+
*
20+
* @var int
21+
*/
22+
public $initStart;
23+
24+
/**
25+
* The position of the initialization expression closer for the loop.
26+
*
27+
* @var int
28+
*/
29+
public $initEnd;
30+
31+
/**
32+
* The position of the condition expression opener for the loop.
33+
*
34+
* @var int
35+
*/
36+
public $conditionStart;
37+
38+
/**
39+
* The position of the condition expression closer for the loop.
40+
*
41+
* @var int
42+
*/
43+
public $conditionEnd;
44+
45+
/**
46+
* The position of the increment expression opener for the loop.
47+
*
48+
* @var int
49+
*/
50+
public $incrementStart;
51+
52+
/**
53+
* The position of the increment expression closer for the loop.
54+
*
55+
* @var int
56+
*/
57+
public $incrementEnd;
58+
59+
/**
60+
* The position of the block opener for the loop.
61+
*
62+
* @var int
63+
*/
64+
public $blockStart;
65+
66+
/**
67+
* The position of the block closer for the loop.
68+
*
69+
* @var int
70+
*/
71+
public $blockEnd;
72+
73+
/**
74+
* Any variables defined inside the third expression of the loop.
75+
*
76+
* The key is the variable index.
77+
*
78+
* @var array<int, \VariableAnalysis\Lib\VariableInfo>
79+
*/
80+
public $incrementVariables = [];
81+
82+
/**
83+
* @param int $forIndex
84+
* @param int $blockStart
85+
* @param int $blockEnd
86+
* @param int $initStart
87+
* @param int $initEnd
88+
* @param int $conditionStart
89+
* @param int $conditionEnd
90+
* @param int $incrementStart
91+
* @param int $incrementEnd
92+
*/
93+
public function __construct(
94+
$forIndex,
95+
$blockStart,
96+
$blockEnd,
97+
$initStart,
98+
$initEnd,
99+
$conditionStart,
100+
$conditionEnd,
101+
$incrementStart,
102+
$incrementEnd
103+
) {
104+
$this->forIndex = $forIndex;
105+
$this->blockStart = $blockStart;
106+
$this->blockEnd = $blockEnd;
107+
$this->initStart = $initStart;
108+
$this->initEnd = $initEnd;
109+
$this->conditionStart = $conditionStart;
110+
$this->conditionEnd = $conditionEnd;
111+
$this->incrementStart = $incrementStart;
112+
$this->incrementEnd = $incrementEnd;
113+
}
114+
}

0 commit comments

Comments
 (0)