Add type checking for hook callbacks in add_action and add_filter

composer.json

@@ -12,6 +12,7 @@
     ],
     "require": {
         "php": "^7.2 || ^8.0",
+        "johnbillion/wp-hooks": "^0.8.0",
         "php-stubs/wordpress-stubs": "^4.7 || ^5.0 || ^6.0",
         "phpstan/phpstan": "^1.6",
         "symfony/polyfill-php73": "^1.12.0"

extension.neon

@@ -80,6 +80,7 @@ services:
         tags:
             - phpstan.parser.richParserNodeVisitor
 rules:
+    - SzepeViktor\PHPStan\WordPress\HookCallbackRule
     - SzepeViktor\PHPStan\WordPress\HookDocsRule
     - SzepeViktor\PHPStan\WordPress\IsWpErrorRule
 parameters:

src/HookCallbackRule.php

@@ -0,0 +1,129 @@
+<?php
+
+/**
+ * Custom rule to validate the callback function for WordPress core actions and filters.
+ */
+
+declare(strict_types=1);
+
+namespace SzepeViktor\PHPStan\WordPress;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Type\Constant\ConstantIntegerType;
+use PHPStan\Type\Constant\ConstantStringType;
+
+/**
+ * @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\FuncCall>
+ */
+class HookCallbackRule implements \PHPStan\Rules\Rule
+{
+    private const SUPPORTED_FUNCTIONS = [
+        'add_filter',
+        'add_action',
+    ];
+
+    /** @var \PHPStan\Rules\RuleLevelHelper */
+    protected $ruleLevelHelper;
+
+    /** @var \PhpParser\Node\Expr\FuncCall */
+    protected $currentNode;
+
+    /** @var \PHPStan\Analyser\Scope */
+    protected $currentScope;
+
+    public function __construct(
+        RuleLevelHelper $ruleLevelHelper
+    ) {
+        $this->ruleLevelHelper = $ruleLevelHelper;
+    }
+
+    public function getNodeType(): string
+    {
+        return FuncCall::class;
+    }
+
+    /**
+     * @param \PhpParser\Node\Expr\FuncCall $node
+     * @param \PHPStan\Analyser\Scope       $scope
+     * @return array<int, \PHPStan\Rules\RuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $name = $node->name;
+
+        if (!($name instanceof \PhpParser\Node\Name)) {
+            return [];
+        }
+
+        if (!in_array($name->toString(), self::SUPPORTED_FUNCTIONS, true)) {
+            return [];
+        }
+
+        $args = $node->getArgs();
+
+        // If we don't have enough arguments, bail out and let PHPStan handle the error:
+        if (count($args) < 2) {
+            return [];
+        }
+
+        list(
+            $hookNameArg,
+            $callbackArg
+        ) = $args;
+
+        $hookNameType = $scope->getType($hookNameArg->value);
+        $hookNameValue = null;
+
+        if ($hookNameType instanceof ConstantStringType) {
+            $hookNameValue = $hookNameType->getValue();
+        }
+
+        $callbackType = $scope->getType($callbackArg->value);
+
+        // If the callback is not valid, bail out and let PHPStan handle the error:
+        if ($callbackType->isCallable()->no()) {
+            return [];
+        }
+
+        $acceptedArgs = 1;
+
+        if (isset($args[3])) {
+            $acceptedArgs = null;
+            $argumentType = $scope->getType($args[3]->value);
+
+            if ($argumentType instanceof ConstantIntegerType) {
+                $acceptedArgs = $argumentType->getValue();
+            }
+        }
+
+        if ($acceptedArgs !== null) {
+            $callbackAcceptor = $callbackType->getCallableParametersAcceptors($scope)[0];
+
+            $callbackParameters = $callbackAcceptor->getParameters();
+            $expectedArgs = count($callbackParameters);
+
+            if ($expectedArgs !== $acceptedArgs && ($expectedArgs !== 0 && $acceptedArgs !== 1)) {
+                $message = (1 === $expectedArgs)
+                    ? 'Callback expects %1$d argument, $accepted_args is set to %2$d.'
+                    : 'Callback expects %1$d arguments, $accepted_args is set to %2$d.'
+                ;
+
+                return [
+                    RuleErrorBuilder::message(
+                        sprintf(
+                            $message,
+                            $expectedArgs,
+                            $acceptedArgs
+                        )
+                    )->build()
+                ];
+            }
+        }
+
+        return [];
+    }
+}

tests/HookCallbackRuleTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SzepeViktor\PHPStan\WordPress\Tests;
+
+use PHPStan\Rules\RuleLevelHelper;
+use SzepeViktor\PHPStan\WordPress\HookCallbackRule;
+
+/**
+ * @extends \PHPStan\Testing\RuleTestCase<\SzepeViktor\PHPStan\WordPress\HookCallbackRule>
+ */
+class HookCallbackRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+    protected function getRule(): \PHPStan\Rules\Rule
+    {
+        /** @var \PHPStan\Rules\RuleLevelHelper */
+        $ruleLevelHelper = self::getContainer()->getByType(RuleLevelHelper::class);
+
+        // getRule() method needs to return an instance of the tested rule
+        return new HookCallbackRule($ruleLevelHelper);
+    }
+
+    // phpcs:ignore NeutronStandard.Functions.LongFunction.LongFunction
+    public function testRule(): void
+    {
+        // first argument: path to the example file that contains some errors that should be reported by HookCallbackRule
+        // second argument: an array of expected errors,
+        // each error consists of the asserted error message, and the asserted error file line
+        $this->analyse(
+            [
+                __DIR__ . '/data/hook-callback.php',
+            ],
+            [
+                [
+                    'Filter callback has no return value.',
+                    17,
+                ],
+                [
+                    'Filter callback has no return value.',
+                    20,
+                ],
+                [
+                    'Filter callback has no return value.',
+                    21,
+                ],
+                [
+                    'Callback expects 1 argument, $accepted_args is set to 0.',
+                    24,
+                ],
+                [
+                    'Callback expects 1 argument, $accepted_args is set to 2.',
+                    27,
+                ],
+                [
+                    'Callback expects 2 arguments, $accepted_args is set to 1.',
+                    30,
+                ],
+            ]
+        );
+    }
+
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [dirname(__DIR__) . '/vendor/szepeviktor/phpstan-wordpress/extension.neon'];
+    }
+}

tests/data/hook-callback.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SzepeViktor\PHPStan\WordPress\Tests;
+
+use function add_filter;
+use function add_action;
+
+// phpcs:disable Squiz.NamingConventions.ValidFunctionName.NotCamelCaps,Squiz.NamingConventions.ValidVariableName.NotCamelCaps,Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps
+
+/**
+ * Incorrect usage:
+ */
+
+// Not a core filter, but callback is missing a return value
+add_filter('not_a_core_filter', function() {});
+
+// Core filter, callback is missing a return value
+add_filter('post_class', function() {});
+add_filter('post_class', function(array $classes) {});
+
+// Not a core filter, accepted args are incorrect
+add_filter('not_a_core_filter', function($value) {
+    return 123;
+}, 10, 0);
+add_filter('not_a_core_filter', function($value) {
+    return 123;
+}, 10, 2);
+add_filter('not_a_core_filter', function($value1, $value2) {
+    return 123;
+});
+
+// Core filter, callback is missing a return value
+add_filter('post_class', function() {});
+add_filter('post_class', function(array $classes) {});
+
+/**
+ * Incorrect usage that's handled by PHPStan:
+ *
+ * These are here to ensure the rule doesn't trigger unwanted errors.
+ */
+
+// Too few parameters:
+add_filter('post_class');
+add_filter();
+
+// Invalid callback:
+add_filter('post_class','i_do_not_exist');
+
+// Invalid parameters:
+add_filter('post_class', function() {
+    return 123;
+}, false);
+add_filter('post_class', function() {
+    return 123;
+}, 10, false);
+
+/**
+ * Correct usage:
+ */
+
+// Not a core filter, but callback is ok
+add_filter('not_a_core_filter', function() {
+    return 123;
+}, 10, 0);
+add_filter('not_a_core_filter', function() {
+    // We're allowing 0 parameters when `$accepted_args` is default value.
+    // This might change in the future to get more strict.
+    return 123;
+});
+add_filter('not_a_core_filter', function($value) {
+    return 123;
+});
+add_filter('not_a_core_filter', function($value) {
+    return 123;
+}, 10, 1);
+add_filter('not_a_core_filter', function($value1, $value2) {
+    return 123;
+}, 10, 2);
+
+// Various callback types
+add_filter('not_a_core_filter', '__return_false');
+add_filter('not_a_core_filter', __NAMESPACE__ . '\\filter_callback');
+add_filter('not_a_core_filter', new TestInvokable(), 10, 2);
+
+function filter_callback() {
+    return 123;
+}
+
+class TestInvokable {
+	public function __invoke($one, $two) {
+        return 123;
+	}
+}