Normalize leading backslashes in banned function names (ekino#30)

* Normalize leading slashed in function names * Fix type hint * Fix codestyle * Fix types * changelog
Spomky · Oct 11, 2023 · 65a5752 · 65a5752
1 parent ac7406a
commit 65a5752
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ master
 * Added rule to ban print statements
 * Allow Composer plugin ergebnis/composer-normalize
 * Add Composer keyword for asking user to add this package to require-dev instead of require
+* Normalize leading backslashes in banned function names
 
 v1.0.0
 ------

diff --git a/src/Rules/BannedNodesRule.php b/src/Rules/BannedNodesRule.php
@@ -26,16 +26,22 @@
 class BannedNodesRule implements Rule
 {
     /**
-     * @var array<mixed>
+     * @var array<string, mixed>
      */
     private $bannedNodes;
 
     /**
-     * @param array<mixed> $bannedNodes
+     * @var array<string>
+     */
+    private $bannedFunctions;
+
+    /**
+     * @param array<array{type: string, functions?: array<string>}> $bannedNodes
      */
     public function __construct(array $bannedNodes)
     {
-        $this->bannedNodes = array_column($bannedNodes, null, 'type');
+        $this->bannedNodes     = array_column($bannedNodes, null, 'type');
+        $this->bannedFunctions = $this->normalizeFunctionNames($this->bannedNodes);
     }
 
     /**
@@ -64,7 +70,7 @@ public function processNode(Node $node, Scope $scope): array
 
             $function = $node->name->toString();
 
-            if (\in_array($function, $this->bannedNodes[$type]['functions'])) {
+            if (\in_array($function, $this->bannedFunctions)) {
                 return [sprintf('Should not use function "%s", please change the code.', $function)];
             }
 
@@ -73,4 +79,22 @@ public function processNode(Node $node, Scope $scope): array
 
         return [sprintf('Should not use node with type "%s", please change the code.', $type)];
     }
+
+    /**
+     * Strip leading slashes from function names.
+     *
+     * php-parser makes the same normalization.
+     *
+     * @param  array<string, mixed>  $bannedNodes
+     * @return array<string>
+     */
+    protected function normalizeFunctionNames(array $bannedNodes): array
+    {
+        return array_map(
+            static function (string $function): string {
+                return ltrim($function, '\\');
+            },
+            $bannedNodes['Expr_FuncCall']['functions'] ?? []
+        );
+    }
 }
diff --git a/tests/Rules/BannedNodesRuleTest.php b/tests/Rules/BannedNodesRuleTest.php
@@ -24,6 +24,7 @@
 use PhpParser\Node\Expr\ShellExec;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name;
+use PhpParser\Node\Name\FullyQualified;
 use PhpParser\Node\Scalar\LNumber;
 use PHPStan\Analyser\Scope;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -53,7 +54,7 @@ protected function setUp(): void
             ['type' => 'Stmt_Echo'],
             ['type' => 'Expr_Eval'],
             ['type' => 'Expr_Exit'],
-            ['type' => 'Expr_FuncCall', 'functions' => ['debug_backtrace', 'dump']],
+            ['type' => 'Expr_FuncCall', 'functions' => ['debug_backtrace', 'dump', 'Safe\namespaced']],
             ['type' => 'Expr_Print'],
             ['type' => 'Expr_ShellExec'],
         ]);
@@ -80,35 +81,73 @@ public function testProcessNodeWithUnhandledType(Expr $node): void
         $this->assertCount(0, $this->rule->processNode($node, $this->scope));
     }
 
-    /**
-     * Tests processNode with banned/allowed functions.
-     */
-    public function testProcessNodeWithFunctions(): void
+    public function testProcessNodeWithBannedFunctions(): void
+    {
+        $ruleWithoutLeadingSlashes = new BannedNodesRule([
+            [
+                'type'      => 'Expr_FuncCall',
+                'functions' => [
+                    'root',
+                    'Safe\namespaced',
+                ]
+            ],
+        ]);
+
+        $ruleWithLeadingSlashes = new BannedNodesRule([
+            [
+                'type'      => 'Expr_FuncCall',
+                'functions' => [
+                    '\root',
+                    '\Safe\namespaced',
+                ]
+            ],
+        ]);
+
+        $rootFunction = new FuncCall(new Name('root'));
+        $this->assertNodeTriggersError($ruleWithoutLeadingSlashes, $rootFunction);
+        $this->assertNodeTriggersError($ruleWithLeadingSlashes, $rootFunction);
+
+        $namespacedFunction = new FuncCall(new FullyQualified('Safe\namespaced'));
+        $this->assertNodeTriggersError($ruleWithoutLeadingSlashes, $namespacedFunction);
+        $this->assertNodeTriggersError($ruleWithLeadingSlashes, $namespacedFunction);
+    }
+
+    protected function assertNodeTriggersError(BannedNodesRule $rule, Node $node): void
     {
-        foreach (['debug_backtrace', 'dump'] as $bannedFunction) {
-            $node = new FuncCall(new Name($bannedFunction));
+        $this->assertCount(1, $rule->processNode($node, $this->scope));
+    }
 
-            $this->assertCount(1, $this->rule->processNode($node, $this->scope));
-        }
+    protected function assertNodePasses(BannedNodesRule $rule, Node $node): void
+    {
+        $this->assertCount(0, $rule->processNode($node, $this->scope));
+    }
 
-        foreach (['array_search', 'sprintf'] as $allowedFunction) {
-            $node = new FuncCall(new Name($allowedFunction));
+    public function testProcessNodeWithAllowedFunctions(): void
+    {
+        $rootFunction = new FuncCall(new Name('allowed'));
+        $this->assertNodePasses($this->rule, $rootFunction);
 
-            $this->assertCount(0, $this->rule->processNode($node, $this->scope));
-        }
+        $namespacedFunction = new FuncCall(new FullyQualified('Safe\allowed'));
+        $this->assertNodePasses($this->rule, $namespacedFunction);
+    }
 
+    public function testProcessNodeWithFunctionInClosure(): void
+    {
         $node = new FuncCall(new Variable('myClosure'));
 
-        $this->assertCount(0, $this->rule->processNode($node, $this->scope));
+        $this->assertNodePasses($this->rule, $node);
+    }
 
+    public function testProcessNodeWithArrayDimFetch(): void
+    {
         $node = new FuncCall(
             new Expr\ArrayDimFetch(
                 new Variable('myArray'),
                 LNumber::fromString('0', ['kind' => LNumber::KIND_DEC])
             )
         );
 
-        $this->assertCount(0, $this->rule->processNode($node, $this->scope));
+        $this->assertNodePasses($this->rule, $node);
     }
 
     /**
@@ -120,7 +159,7 @@ public function testProcessNodeWithFunctions(): void
      */
     public function testProcessNodeWithHandledTypes(Expr $node): void
     {
-        $this->assertCount(1, $this->rule->processNode($node, $this->scope));
+        $this->assertNodeTriggersError($this->rule, $node);
     }
 
     /**