Skip to content

Commit

Permalink
implemented a rule to detect overwriting parent scope vars in for loo…
Browse files Browse the repository at this point in the history
…p init
  • Loading branch information
dktapps committed Aug 30, 2020
1 parent 9b86e1e commit 334898a
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 0 deletions.
7 changes: 7 additions & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,10 @@ services:
universalObjectCratesClasses: %universalObjectCratesClasses%
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule

conditionalTags:
PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule:
phpstan.rules.rule: %featureToggles.bleedingEdge%
78 changes: 78 additions & 0 deletions src/Rules/ForLoop/OverwriteVariablesWithForLoopInitRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\ForLoop;

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Stmt\For_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;

/**
* @implements Rule<For_>
*/
class OverwriteVariablesWithForLoopInitRule implements Rule
{

public function getNodeType(): string
{
return For_::class;
}

/**
* @param For_ $node
* @param Scope $scope
* @return string[]
*/
public function processNode(Node $node, Scope $scope): array
{
$errors = [];
foreach ($node->init as $expr) {
if (!($expr instanceof Assign)) {
continue;
}

foreach ($this->checkValueVar($scope, $expr->var) as $error) {
$errors[] = $error;
}
}

return $errors;
}

/**
* @param Scope $scope
* @param Expr $expr
* @return string[]
*/
private function checkValueVar(Scope $scope, Expr $expr): array
{
$errors = [];
if (
$expr instanceof Node\Expr\Variable
&& is_string($expr->name)
&& $scope->hasVariableType($expr->name)->yes()
) {
$errors[] = sprintf('For loop initial assignment overwrites variable $%s.', $expr->name);
}

if (
$expr instanceof Node\Expr\List_
|| $expr instanceof Node\Expr\Array_
) {
foreach ($expr->items as $item) {
if ($item === null) {
continue;
}

foreach ($this->checkValueVar($scope, $item->value) as $error) {
$errors[] = $error;
}
}
}

return $errors;
}

}
65 changes: 65 additions & 0 deletions tests/Rules/ForLoop/OverwriteVariablesWithForLoopInitRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\ForLoop;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @template-extends RuleTestCase<OverwriteVariablesWithForLoopInitRule>
*/
class OverwriteVariablesWithForLoopInitRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new OverwriteVariablesWithForLoopInitRule();
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/data.php'], [
[
'For loop initial assignment overwrites variable $i.',
9,
],
[
'For loop initial assignment overwrites variable $i.',
20,
],
[
'For loop initial assignment overwrites variable $j.',
20,
],
[
'For loop initial assignment overwrites variable $i.',
24,
],
[
'For loop initial assignment overwrites variable $i.',
35,
],
[
'For loop initial assignment overwrites variable $j.',
35,
],
[
'For loop initial assignment overwrites variable $i.',
39,
],
[
'For loop initial assignment overwrites variable $j.',
39,
],
[
'For loop initial assignment overwrites variable $i.',
50,
],
[
'For loop initial assignment overwrites variable $i.',
54,
],
]);
}

}
62 changes: 62 additions & 0 deletions tests/Rules/ForLoop/data/data.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php

namespace OverwriteVariablesWithForLoopInit;

class Foo{

public function simple(int $i): void
{
for($i = 0; $i < 10; ++$i){

}

for($j = 0; $j < 10; ++$j){

}
}

public function multi(int $i, int $j): void
{
for($i = 0, $j = 0; $i < 10; ++$i){

}

for($i = 0, $k = 0; $i < 10; ++$i){

}

for($k = 0, $l = 0; $k < 10; ++$k){

}
}

public function list(int $i, int $j, array $b): void
{
for(list($i, $j) = $b; $i < 10; ++$i){

}

for(list($i, list($j, $k)) = $b; $i < 10; ++$i){

}

for(list($k, list($l, $m)) = $b; $k < 10; ++$k){

}
}

public function array(int $i, array $b): void
{
for([$i, $j] = $b; $i < 10; ++$i){

}

for([$i, [$j, $k]] = $b; $i < 10; ++$i){

}

for([$k, [$l, $m]] = $b; $k < 10; ++$k){

}
}
}

0 comments on commit 334898a

Please sign in to comment.