Skip to content

Commit

Permalink
fix(exception): do nothing if node name is instance of Variable (clos…
Browse files Browse the repository at this point in the history
…ure case)
  • Loading branch information
mremi committed Apr 11, 2019
1 parent 72357b4 commit 233055a
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ app-install: ## to install app
composer install --prefer-dist

app-security-check: ## to check if any security issues in the PHP dependencies
vendor/bin/security-checker security:check --end-point=http://security.sensiolabs.org/check_lock
vendor/bin/security-checker security:check

app-static-analysis: ## to run static analysis
php -dmemory_limit=-1 vendor/bin/phpstan analyze src tests -l 7
Expand Down
5 changes: 5 additions & 0 deletions src/Rules/BannedNodesRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
Expand Down Expand Up @@ -55,6 +56,10 @@ public function processNode(Node $node, Scope $scope): array
}

if ($node instanceof FuncCall) {
if ($node->name instanceof Variable) {
return [];
}

if (!$node->name instanceof Name) {
throw new \RuntimeException(sprintf('Expected instance of %s for $node->name, %s given', Name::class, \get_class($node->name)));
}
Expand Down
21 changes: 13 additions & 8 deletions tests/Rules/BannedNodesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use PhpParser\Node\Expr\Exit_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Include_;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -39,9 +40,9 @@ class BannedNodesRuleTest extends TestCase
private $scope;

/**
* Initializes the tests.
* {@inheritdoc}
*/
protected function setUp()
protected function setUp(): void
{
$this->rule = new BannedNodesRule([
['type' => 'Stmt_Echo'],
Expand All @@ -55,7 +56,7 @@ protected function setUp()
/**
* Tests getNodeType.
*/
public function testGetNodeType()
public function testGetNodeType(): void
{
$this->assertSame(Node::class, $this->rule->getNodeType());
}
Expand All @@ -67,15 +68,15 @@ public function testGetNodeType()
*
* @dataProvider getUnhandledNodes
*/
public function testProcessNodeWithUnhandledType(Expr $node)
public function testProcessNodeWithUnhandledType(Expr $node): void
{
$this->assertCount(0, $this->rule->processNode($node, $this->scope));
}

/**
* Tests processNode with banned/allowed functions.
*/
public function testProcessNodeWithFunctions()
public function testProcessNodeWithFunctions(): void
{
foreach (['debug_backtrace', 'dump'] as $bannedFunction) {
$node = new FuncCall(new Name($bannedFunction));
Expand All @@ -88,6 +89,10 @@ public function testProcessNodeWithFunctions()

$this->assertCount(0, $this->rule->processNode($node, $this->scope));
}

$node = new FuncCall(new Variable('myClosure'));

$this->assertCount(0, $this->rule->processNode($node, $this->scope));
}

/**
Expand All @@ -97,23 +102,23 @@ public function testProcessNodeWithFunctions()
*
* @dataProvider getHandledNodes
*/
public function testProcessNodeWithHandledTypes(Expr $node)
public function testProcessNodeWithHandledTypes(Expr $node): void
{
$this->assertCount(1, $this->rule->processNode($node, $this->scope));
}

/**
* @return \Generator
*/
public function getUnhandledNodes()
public function getUnhandledNodes(): \Generator
{
yield [new Include_($this->createMock(Expr::class), Include_::TYPE_INCLUDE)];
}

/**
* @return \Generator
*/
public function getHandledNodes()
public function getHandledNodes(): \Generator
{
yield [new Eval_($this->createMock(Expr::class))];
yield [new Exit_()];
Expand Down
14 changes: 7 additions & 7 deletions tests/Rules/BannedUseTestRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ class BannedUseTestRuleTest extends TestCase
private $scope;

/**
* Initializes the tests.
* {@inheritdoc}
*/
protected function setUp()
protected function setUp(): void
{
$this->rule = new BannedUseTestRule();
$this->scope = $this->createMock(Scope::class);
Expand All @@ -47,15 +47,15 @@ protected function setUp()
/**
* Tests getNodeType.
*/
public function testGetNodeType()
public function testGetNodeType(): void
{
$this->assertSame(Use_::class, $this->rule->getNodeType());
}

/**
* Tests processNode if disabled.
*/
public function testProcessNodeIfDisabled()
public function testProcessNodeIfDisabled(): void
{
$this->scope->expects($this->never())->method('getNamespace');

Expand All @@ -65,7 +65,7 @@ public function testProcessNodeIfDisabled()
/**
* Tests processNode with test scope.
*/
public function testProcessNodeWithTestScope()
public function testProcessNodeWithTestScope(): void
{
$this->scope->expects($this->once())->method('getNamespace')->willReturn('Tests\\Foo\\Bar');

Expand All @@ -77,7 +77,7 @@ public function testProcessNodeWithTestScope()
*
* @expectedException \InvalidArgumentException
*/
public function testProcessNodeThrowsException()
public function testProcessNodeThrowsException(): void
{
$this->scope->expects($this->once())->method('getNamespace')->willReturn('Foo\\Bar');

Expand All @@ -87,7 +87,7 @@ public function testProcessNodeThrowsException()
/**
* Tests processNode with errors.
*/
public function testProcessNodeWithErrors()
public function testProcessNodeWithErrors(): void
{
$this->scope->expects($this->once())->method('getNamespace')->willReturn('Foo\\Bar');

Expand Down

0 comments on commit 233055a

Please sign in to comment.