Skip to content

Commit 65a5752

Browse files
authored
Normalize leading backslashes in banned function names (#30)
* Normalize leading slashed in function names * Fix type hint * Fix codestyle * Fix types * changelog
1 parent ac7406a commit 65a5752

File tree

3 files changed

+84
-20
lines changed

3 files changed

+84
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ master
88
* Added rule to ban print statements
99
* Allow Composer plugin ergebnis/composer-normalize
1010
* Add Composer keyword for asking user to add this package to require-dev instead of require
11+
* Normalize leading backslashes in banned function names
1112

1213
v1.0.0
1314
------

src/Rules/BannedNodesRule.php

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,22 @@
2626
class BannedNodesRule implements Rule
2727
{
2828
/**
29-
* @var array<mixed>
29+
* @var array<string, mixed>
3030
*/
3131
private $bannedNodes;
3232

3333
/**
34-
* @param array<mixed> $bannedNodes
34+
* @var array<string>
35+
*/
36+
private $bannedFunctions;
37+
38+
/**
39+
* @param array<array{type: string, functions?: array<string>}> $bannedNodes
3540
*/
3641
public function __construct(array $bannedNodes)
3742
{
38-
$this->bannedNodes = array_column($bannedNodes, null, 'type');
43+
$this->bannedNodes = array_column($bannedNodes, null, 'type');
44+
$this->bannedFunctions = $this->normalizeFunctionNames($this->bannedNodes);
3945
}
4046

4147
/**
@@ -64,7 +70,7 @@ public function processNode(Node $node, Scope $scope): array
6470

6571
$function = $node->name->toString();
6672

67-
if (\in_array($function, $this->bannedNodes[$type]['functions'])) {
73+
if (\in_array($function, $this->bannedFunctions)) {
6874
return [sprintf('Should not use function "%s", please change the code.', $function)];
6975
}
7076

@@ -73,4 +79,22 @@ public function processNode(Node $node, Scope $scope): array
7379

7480
return [sprintf('Should not use node with type "%s", please change the code.', $type)];
7581
}
82+
83+
/**
84+
* Strip leading slashes from function names.
85+
*
86+
* php-parser makes the same normalization.
87+
*
88+
* @param array<string, mixed> $bannedNodes
89+
* @return array<string>
90+
*/
91+
protected function normalizeFunctionNames(array $bannedNodes): array
92+
{
93+
return array_map(
94+
static function (string $function): string {
95+
return ltrim($function, '\\');
96+
},
97+
$bannedNodes['Expr_FuncCall']['functions'] ?? []
98+
);
99+
}
76100
}

tests/Rules/BannedNodesRuleTest.php

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use PhpParser\Node\Expr\ShellExec;
2525
use PhpParser\Node\Expr\Variable;
2626
use PhpParser\Node\Name;
27+
use PhpParser\Node\Name\FullyQualified;
2728
use PhpParser\Node\Scalar\LNumber;
2829
use PHPStan\Analyser\Scope;
2930
use PHPUnit\Framework\MockObject\MockObject;
@@ -53,7 +54,7 @@ protected function setUp(): void
5354
['type' => 'Stmt_Echo'],
5455
['type' => 'Expr_Eval'],
5556
['type' => 'Expr_Exit'],
56-
['type' => 'Expr_FuncCall', 'functions' => ['debug_backtrace', 'dump']],
57+
['type' => 'Expr_FuncCall', 'functions' => ['debug_backtrace', 'dump', 'Safe\namespaced']],
5758
['type' => 'Expr_Print'],
5859
['type' => 'Expr_ShellExec'],
5960
]);
@@ -80,35 +81,73 @@ public function testProcessNodeWithUnhandledType(Expr $node): void
8081
$this->assertCount(0, $this->rule->processNode($node, $this->scope));
8182
}
8283

83-
/**
84-
* Tests processNode with banned/allowed functions.
85-
*/
86-
public function testProcessNodeWithFunctions(): void
84+
public function testProcessNodeWithBannedFunctions(): void
85+
{
86+
$ruleWithoutLeadingSlashes = new BannedNodesRule([
87+
[
88+
'type' => 'Expr_FuncCall',
89+
'functions' => [
90+
'root',
91+
'Safe\namespaced',
92+
]
93+
],
94+
]);
95+
96+
$ruleWithLeadingSlashes = new BannedNodesRule([
97+
[
98+
'type' => 'Expr_FuncCall',
99+
'functions' => [
100+
'\root',
101+
'\Safe\namespaced',
102+
]
103+
],
104+
]);
105+
106+
$rootFunction = new FuncCall(new Name('root'));
107+
$this->assertNodeTriggersError($ruleWithoutLeadingSlashes, $rootFunction);
108+
$this->assertNodeTriggersError($ruleWithLeadingSlashes, $rootFunction);
109+
110+
$namespacedFunction = new FuncCall(new FullyQualified('Safe\namespaced'));
111+
$this->assertNodeTriggersError($ruleWithoutLeadingSlashes, $namespacedFunction);
112+
$this->assertNodeTriggersError($ruleWithLeadingSlashes, $namespacedFunction);
113+
}
114+
115+
protected function assertNodeTriggersError(BannedNodesRule $rule, Node $node): void
87116
{
88-
foreach (['debug_backtrace', 'dump'] as $bannedFunction) {
89-
$node = new FuncCall(new Name($bannedFunction));
117+
$this->assertCount(1, $rule->processNode($node, $this->scope));
118+
}
90119

91-
$this->assertCount(1, $this->rule->processNode($node, $this->scope));
92-
}
120+
protected function assertNodePasses(BannedNodesRule $rule, Node $node): void
121+
{
122+
$this->assertCount(0, $rule->processNode($node, $this->scope));
123+
}
93124

94-
foreach (['array_search', 'sprintf'] as $allowedFunction) {
95-
$node = new FuncCall(new Name($allowedFunction));
125+
public function testProcessNodeWithAllowedFunctions(): void
126+
{
127+
$rootFunction = new FuncCall(new Name('allowed'));
128+
$this->assertNodePasses($this->rule, $rootFunction);
96129

97-
$this->assertCount(0, $this->rule->processNode($node, $this->scope));
98-
}
130+
$namespacedFunction = new FuncCall(new FullyQualified('Safe\allowed'));
131+
$this->assertNodePasses($this->rule, $namespacedFunction);
132+
}
99133

134+
public function testProcessNodeWithFunctionInClosure(): void
135+
{
100136
$node = new FuncCall(new Variable('myClosure'));
101137

102-
$this->assertCount(0, $this->rule->processNode($node, $this->scope));
138+
$this->assertNodePasses($this->rule, $node);
139+
}
103140

141+
public function testProcessNodeWithArrayDimFetch(): void
142+
{
104143
$node = new FuncCall(
105144
new Expr\ArrayDimFetch(
106145
new Variable('myArray'),
107146
LNumber::fromString('0', ['kind' => LNumber::KIND_DEC])
108147
)
109148
);
110149

111-
$this->assertCount(0, $this->rule->processNode($node, $this->scope));
150+
$this->assertNodePasses($this->rule, $node);
112151
}
113152

114153
/**
@@ -120,7 +159,7 @@ public function testProcessNodeWithFunctions(): void
120159
*/
121160
public function testProcessNodeWithHandledTypes(Expr $node): void
122161
{
123-
$this->assertCount(1, $this->rule->processNode($node, $this->scope));
162+
$this->assertNodeTriggersError($this->rule, $node);
124163
}
125164

126165
/**

0 commit comments

Comments
 (0)