Feature/disallow debug functions

README.md

@@ -133,6 +133,21 @@ Table::hasMany, Table::belongsToMany, Table::hasOne and AssociationCollection::l
 ### AddBehaviorExistsClassRule
 This rule check if the target behavior has a valid class when calling to Table::addBehavior and BehaviorRegistry::load.
 
+### DisallowDebugFuncCallRule
+This rule disallow use of debug functions (`dd, debug, debug_print_backtrace, debug_zval_dump, pr, print_r, stacktrace, var_dump and var_export`).
+
+The use of these functions in shipped code is discouraged because they can leak sensitive information or clutter output.
+
+### DisallowDebugStaticCallRule
+This rule disallow use of debug methods. The use of these methods in shipped code is discouraged because they can leak sensitive information or clutter output.
+
+Methods covered:
+
+    - Cake\Error\Debugger::dump
+    - Cake\Error\Debugger::printVar
+    - DebugKit\DebugSql::sql
+    - DebugKit\DebugSql::sqld
+
 ### DisallowEntityArrayAccessRule
 This rule disallow array access to entity in favor of object notation, is easier to detect a wrong property and to refactor code.
 

rules.neon

@@ -9,6 +9,8 @@ parameters:
 	 	getMailerExistsClassRule: true
 	 	loadComponentExistsClassRule: true
 	 	controllerMethodMustBeUsedRule: true
+	 	disallowDebugFuncCallRule: true
+	 	disallowDebugStaticCallRule: true
 parametersSchema:
 	cakeDC: structure([
 		addAssociationExistsTableClassRule: anyOf(bool(), arrayOf(bool()))
@@ -20,6 +22,8 @@ parametersSchema:
 		getMailerExistsClassRule: anyOf(bool(), arrayOf(bool()))
 		loadComponentExistsClassRule: anyOf(bool(), arrayOf(bool()))
 		controllerMethodMustBeUsedRule: anyOf(bool(), arrayOf(bool()))
+		disallowDebugFuncCallRule: anyOf(bool(), arrayOf(bool()))
+		disallowDebugStaticCallRule: anyOf(bool(), arrayOf(bool()))
 	])
 
 conditionalTags:
@@ -43,6 +47,10 @@ conditionalTags:
 		phpstan.rules.rule: %cakeDC.tableGetMatchOptionsTypesRule%
 	CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule:
 		phpstan.rules.rule: %cakeDC.ormSelectQueryFindMatchOptionsTypesRule%
+	CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule:
+		phpstan.rules.rule: %cakeDC.disallowDebugFuncCallRule%
+	CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule:
+		phpstan.rules.rule: %cakeDC.disallowDebugStaticCallRule%
 
 services:
 	-
@@ -65,3 +73,7 @@ services:
 		class: CakeDC\PHPStan\Rule\Model\TableGetMatchOptionsTypesRule
 	-
 		class: CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule
+	-
+		class: CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule
+	-
+		class: CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule

src/Rule/Debug/DisallowDebugFuncCallRule.php

@@ -0,0 +1,72 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Rule\Debug;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Expr\FuncCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+class DisallowDebugFuncCallRule implements Rule
+{
+    protected const BASIC = 'basic';
+    protected const RETURNABLE = 'returnable';
+
+    /**
+     * @var array<string, string>
+     */
+    private array $disallowedFunctions = [
+        'dd' => self::BASIC,
+        'debug' => self::BASIC,
+        'debug_print_backtrace' => self::BASIC,
+        'debug_zval_dump' => self::BASIC,
+        'pr' => self::BASIC,
+        'print_r' => self::RETURNABLE,
+        'stacktrace' => self::BASIC,
+        'var_dump' => self::BASIC,
+        'var_export' => self::RETURNABLE,
+    ];
+
+    /**
+     * @inheritDoc
+     */
+    public function getNodeType(): string
+    {
+        return FuncCall::class;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        assert($node instanceof FuncCall);
+        if (!$node->name instanceof Node\Name) {
+            return [];
+        }
+        $usedName = $node->name->name;
+        $name = strtolower($usedName);
+        if (!isset($this->disallowedFunctions[$name])) {
+            return [];
+        }
+        if ($this->disallowedFunctions[$name] === self::RETURNABLE) {
+            $arg = $node->getArgs()[1]->value ?? null;
+            if ($arg instanceof ConstFetch && $arg->name->name === 'true') {
+                return [];
+            }
+        }
+
+        return [
+            RuleErrorBuilder::message(sprintf(
+                'Use of debug function "%s" is not allowed. %s',
+                $usedName,
+                'The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+            ))
+            ->identifier('cake.debug.debugFuncCallUse')
+            ->build(),
+        ];
+    }
+}

src/Rule/Debug/DisallowDebugStaticCallRule.php

@@ -0,0 +1,65 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Rule\Debug;
+
+use Cake\Error\Debugger;
+use PhpParser\Node;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+class DisallowDebugStaticCallRule implements Rule
+{
+    /**
+     * @var array<string, array<int, string>>
+     */
+    private array $disallowed = [
+        //Methods must be lowercased
+        Debugger::class => ['dump', 'printvar'],
+        'DebugKit\DebugSql' => ['sql', 'sqld'],
+    ];
+
+    /**
+     * @inheritDoc
+     */
+    public function getNodeType(): string
+    {
+        return StaticCall::class;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        assert($node instanceof StaticCall);
+        if (!$node->class instanceof Name || !$node->name instanceof Identifier) {
+            return [];
+        }
+
+        $className = (string)$node->class;
+        if (!isset($this->disallowed[$className])) {
+            return [];
+        }
+        $methodUsed = (string)$node->name;
+        $method = strtolower($methodUsed);
+        if (!in_array($method, $this->disallowed[$className], true)) {
+            return [];
+        }
+
+        return [
+            RuleErrorBuilder::message(sprintf(
+                'Use of debug method "%s::%s" is not allowed. %s',
+                $className,
+                $methodUsed,
+                'The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+            ))
+            ->identifier('cake.debug.debugStaticCallUse')
+            ->build(),
+        ];
+    }
+}

tests/TestCase/Rule/Debug/DisallowDebugFuncCallRuleTest.php

@@ -0,0 +1,84 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug;
+
+use CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+class DisallowDebugFuncCallRuleTest extends RuleTestCase
+{
+    /**
+     * @return \PHPStan\Rules\Rule
+     */
+    protected function getRule(): Rule
+    {
+        // getRule() method needs to return an instance of the tested rule
+        return new DisallowDebugFuncCallRule();
+    }
+
+    /**
+     * @return void
+     */
+    public function testRule(): void
+    {
+        // first argument: path to the example file that contains some errors that should be reported by MyRule
+        // second argument: an array of expected errors,
+        // each error consists of the asserted error message, and the asserted error file line
+        $this->analyse([__DIR__ . '/Fake/FailingDebugUseLogic.php'], [
+            [
+                'Use of debug function "debug" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                14, // asserted error line
+            ],
+            [
+                'Use of debug function "debug_print_backtrace" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                15, // asserted error line
+            ],
+            [
+                'Use of debug function "debug_zval_dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                16, // asserted error line
+            ],
+            [
+                'Use of debug function "print_r" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                19, // asserted error line
+            ],
+            [
+                'Use of debug function "print_r" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                20, // asserted error line
+            ],
+            [
+                'Use of debug function "var_dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                21, // asserted error line
+            ],
+            [
+                'Use of debug function "var_export" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                23, // asserted error line
+            ],
+            [
+                'Use of debug function "var_export" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                24, // asserted error line
+            ],
+            [
+                'Use of debug function "stackTrace" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                25, // asserted error line
+            ],
+            [
+                'Use of debug function "pr" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                26, // asserted error line
+            ],
+            [
+                'Use of debug function "dd" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                28, // asserted error line
+            ],
+        ]);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [__DIR__ . '/../../../../extension.neon'];
+    }
+}

tests/TestCase/Rule/Debug/DisallowDebugStaticCallRuleTest.php

@@ -0,0 +1,55 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug;
+
+use CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+class DisallowDebugStaticCallRuleTest extends RuleTestCase
+{
+    /**
+     * @return \PHPStan\Rules\Rule
+     */
+    protected function getRule(): Rule
+    {
+        return new DisallowDebugStaticCallRule();
+    }
+
+    /**
+     * @return void
+     */
+    public function testRule(): void
+    {
+        // first argument: path to the example file that contains some errors that should be reported by MyRule
+        // second argument: an array of expected errors,
+        // each error consists of the asserted error message, and the asserted error file line
+        $this->analyse([__DIR__ . '/Fake/FailingDebugStaticUseLogic.php'], [
+            [
+                'Use of debug method "Cake\Error\Debugger::dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                16, // asserted error line
+            ],
+            [
+                'Use of debug method "Cake\Error\Debugger::printVar" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                17, // asserted error line
+            ],
+            [
+                'Use of debug method "DebugKit\DebugSql::sql" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                18, // asserted error line
+            ],
+            [
+                'Use of debug method "DebugKit\DebugSql::sqld" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
+                19, // asserted error line
+            ],
+        ]);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [__DIR__ . '/../../../../extension.neon'];
+    }
+}

tests/TestCase/Rule/Debug/Fake/FailingDebugStaticUseLogic.php

@@ -0,0 +1,21 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug\Fake;
+
+use Cake\Error\Debugger;
+use DebugKit\DebugSql;
+
+class FailingDebugStaticUseLogic
+{
+    /**
+     * @return void
+     */
+    public function execute(): void
+    {
+        Debugger::dump(['a' => 1, 'b' => 2, 'c' => 3]);
+        Debugger::printVar(['y' => 10, 'x' => 20, 'z' => 30]);
+        DebugSql::sql();
+        DebugSql::sqld();
+    }
+}

tests/TestCase/Rule/Debug/Fake/FailingDebugUseLogic.php

@@ -0,0 +1,30 @@
+<?php
+declare(strict_types=1);
+
+namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug\Fake;
+
+class FailingDebugUseLogic
+{
+    /**
+     * @return void
+     */
+    public function execute(): void
+    {
+        $list = [1, 2, 3, 4];
+        debug($list);//Error
+        debug_print_backtrace();//Error
+        debug_zval_dump('Hello World');//Error
+        ksort($list);//Not a debug should not fail
+        print_r(['Hello World!'], true);//No error, text is returned
+        print_r(['Hello World!']);//Error
+        print_r(['Hello World!'], false);//Error
+        var_dump(['a' => 1, 'b' => 2, 'c' => 3]);//Error
+        var_export($list, true);//No error, text is returned
+        var_export($list);//Error
+        var_export($list, false);//Error
+        stackTrace();
+        pr(['b' => 2, 'c' => 3]);
+        //Last
+        dd(['a' => 1, 'b' => 2, 'c' => 3]);
+    }
+}