diff --git a/composer.json b/composer.json index bd2e0f32..20e18af5 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,11 @@ "phpunit/phpunit": "^8 || ^9", "szepeviktor/phpcs-psr-12-neutron-hybrid-ruleset": "^0.6" }, + "autoload-dev": { + "classmap": [ + "tests/" + ] + }, "autoload": { "psr-4": { "SzepeViktor\\PHPStan\\WordPress\\": "src/" diff --git a/extension.neon b/extension.neon index c217aea1..bab58b2c 100644 --- a/extension.neon +++ b/extension.neon @@ -92,6 +92,7 @@ services: tags: - phpstan.parser.richParserNodeVisitor rules: + - SzepeViktor\PHPStan\WordPress\HookCallbackRule - SzepeViktor\PHPStan\WordPress\HookDocsRule - SzepeViktor\PHPStan\WordPress\IsWpErrorRule parameters: diff --git a/src/HookCallbackException.php b/src/HookCallbackException.php new file mode 100644 index 00000000..77c4131a --- /dev/null +++ b/src/HookCallbackException.php @@ -0,0 +1,14 @@ + + */ +class HookCallbackRule implements \PHPStan\Rules\Rule +{ + private const SUPPORTED_FUNCTIONS = [ + 'add_filter', + 'add_action', + ]; + + /** @var \PHPStan\Rules\RuleLevelHelper */ + protected $ruleLevelHelper; + + /** @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 + */ + public function processNode(Node $node, Scope $scope): array + { + $name = $node->name; + $this->currentScope = $scope; + + 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, let PHPStan handle the error: + if (count($args) < 2) { + return []; + } + + $callbackType = $scope->getType($args[1]->value); + + // If the callback is not valid, let PHPStan handle the error: + if (! $callbackType->isCallable()->yes()) { + return []; + } + + $callbackAcceptor = ParametersAcceptorSelector::selectSingle($callbackType->getCallableParametersAcceptors($scope)); + + try { + if ($name->toString() === 'add_action') { + $this->validateActionReturnType($callbackAcceptor); + } else { + $this->validateFilterReturnType($callbackAcceptor); + } + + $this->validateParamCount($callbackAcceptor, $args[3] ?? null); + } catch (\SzepeViktor\PHPStan\WordPress\HookCallbackException $e) { + return [RuleErrorBuilder::message($e->getMessage())->build()]; + } + + return []; + } + + protected function getAcceptedArgs(?Arg $acceptedArgsParam): ?int + { + $acceptedArgs = 1; + + if (isset($acceptedArgsParam)) { + $acceptedArgs = null; + $argumentType = $this->currentScope->getType($acceptedArgsParam->value); + + if ($argumentType instanceof ConstantIntegerType) { + $acceptedArgs = $argumentType->getValue(); + } + } + + return $acceptedArgs; + } + + protected function validateParamCount(ParametersAcceptor $callbackAcceptor, ?Arg $acceptedArgsParam): void + { + $acceptedArgs = $this->getAcceptedArgs($acceptedArgsParam); + + if ($acceptedArgs === null) { + return; + } + + $allParameters = $callbackAcceptor->getParameters(); + $requiredParameters = array_filter( + $allParameters, + static function (\PHPStan\Reflection\ParameterReflection $parameter): bool { + return ! $parameter->isOptional(); + } + ); + $maxArgs = count($allParameters); + $minArgs = count($requiredParameters); + + if (($acceptedArgs >= $minArgs) && ($acceptedArgs <= $maxArgs)) { + return; + } + + if (($acceptedArgs >= $minArgs) && $callbackAcceptor->isVariadic()) { + return; + } + + if ($minArgs === 0 && $acceptedArgs === 1) { + return; + } + + throw new \SzepeViktor\PHPStan\WordPress\HookCallbackException( + self::buildParameterCountMessage( + $minArgs, + $maxArgs, + $acceptedArgs, + $callbackAcceptor + ) + ); + } + + protected static function buildParameterCountMessage(int $minArgs, int $maxArgs, int $acceptedArgs, ParametersAcceptor $callbackAcceptor): string + { + $expectedParametersMessage = $minArgs; + + if ($maxArgs !== $minArgs) { + $expectedParametersMessage = sprintf( + '%1$d-%2$d', + $minArgs, + $maxArgs + ); + } + + $message = ($expectedParametersMessage === 1) + ? 'Callback expects %1$d parameter, $accepted_args is set to %2$d.' + : 'Callback expects %1$s parameters, $accepted_args is set to %2$d.'; + + if ($callbackAcceptor->isVariadic()) { + $message = ($minArgs === 1) + ? 'Callback expects at least %1$d parameter, $accepted_args is set to %2$d.' + : 'Callback expects at least %1$s parameters, $accepted_args is set to %2$d.'; + } + + return sprintf( + $message, + $expectedParametersMessage, + $acceptedArgs + ); + } + + protected function validateActionReturnType(ParametersAcceptor $callbackAcceptor): void + { + $acceptedType = $callbackAcceptor->getReturnType(); + $accepted = $this->ruleLevelHelper->accepts( + new VoidType(), + $acceptedType, + true + ); + + if ($accepted) { + return; + } + + throw new \SzepeViktor\PHPStan\WordPress\HookCallbackException( + sprintf( + 'Action callback returns %s but should not return anything.', + $acceptedType->describe(VerbosityLevel::getRecommendedLevelByType($acceptedType)) + ) + ); + } + + protected function validateFilterReturnType(ParametersAcceptor $callbackAcceptor): void + { + $returnType = $callbackAcceptor->getReturnType(); + $isVoidSuperType = $returnType->isSuperTypeOf(new VoidType()); + + if (! $isVoidSuperType->yes()) { + return; + } + + throw new \SzepeViktor\PHPStan\WordPress\HookCallbackException( + 'Filter callback return statement is missing.' + ); + } +} diff --git a/tests/HookCallbackRuleTest.php b/tests/HookCallbackRuleTest.php new file mode 100644 index 00000000..cc161fee --- /dev/null +++ b/tests/HookCallbackRuleTest.php @@ -0,0 +1,127 @@ + + */ +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 return statement is missing.', + 17, + ], + [ + 'Filter callback return statement is missing.', + 20, + ], + [ + 'Filter callback return statement is missing.', + 23, + ], + [ + 'Callback expects 1 parameter, $accepted_args is set to 0.', + 26, + ], + [ + 'Callback expects 1 parameter, $accepted_args is set to 2.', + 31, + ], + [ + 'Callback expects 0-1 parameters, $accepted_args is set to 2.', + 36, + ], + [ + 'Callback expects 2 parameters, $accepted_args is set to 1.', + 41, + ], + [ + 'Callback expects 2-4 parameters, $accepted_args is set to 1.', + 46, + ], + [ + 'Callback expects 2-3 parameters, $accepted_args is set to 4.', + 51, + ], + [ + 'Action callback returns true but should not return anything.', + 56, + ], + [ + 'Action callback returns true but should not return anything.', + 61, + ], + [ + 'Filter callback return statement is missing.', + 68, + ], + [ + 'Action callback returns false but should not return anything.', + 71, + ], + [ + 'Action callback returns int but should not return anything.', + 74, + ], + [ + 'Callback expects at least 1 parameter, $accepted_args is set to 0.', + 77, + ], + [ + 'Callback expects at least 1 parameter, $accepted_args is set to 0.', + 80, + ], + [ + 'Callback expects 0-3 parameters, $accepted_args is set to 4.', + 85, + ], + [ + 'Action callback returns int but should not return anything.', + 90, + ], + [ + 'Callback expects 0 parameters, $accepted_args is set to 2.', + 93, + ], + [ + 'Action callback returns false but should not return anything.', + 96, + ], + [ + 'Filter callback return statement is missing.', + 99, + ], + ] + ); + } + + public static function getAdditionalConfigFiles(): array + { + return [dirname(__DIR__) . '/vendor/szepeviktor/phpstan-wordpress/extension.neon']; + } +} diff --git a/tests/data/hook-callback.php b/tests/data/hook-callback.php new file mode 100644 index 00000000..bf57d22f --- /dev/null +++ b/tests/data/hook-callback.php @@ -0,0 +1,261 @@ +