diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index fcc843b472..517c55ad6d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -160,6 +160,11 @@ parameters: count: 1 path: inc/Engine/Common/JobManager/APIHandler/APIClient.php + - + message: "#^One or more @param tags has an invalid name or invalid syntax\\.$#" + count: 1 + path: inc/Engine/Common/PerformanceHints/Frontend/Subscriber.php + - message: "#^Usage of apply_filters\\(\\) is discouraged\\. Use wpm_apply_filters_typed\\(\\) instead\\.$#" count: 2 diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 7d2c7a5a48..fde76a2773 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -55,8 +55,8 @@ parameters: - %currentWorkingDirectory%/inc/classes/subscriber/third-party/ - %currentWorkingDirectory%/inc/3rd-party/ rules: + - WP_Rocket\Tests\phpstan\Rules\ApplyFiltersTypedDynamicFunctionReturnTypeExtension - WP_Rocket\Tests\phpstan\Rules\DiscourageUpdateOptionUsage - WP_Rocket\Tests\phpstan\Rules\DiscourageApplyFilters - WP_Rocket\Tests\phpstan\Rules\EnsureCallbackMethodsExistsInSubscribedEvents - WP_Rocket\Tests\phpstan\Rules\NoHooksInORM - diff --git a/tests/phpstan/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtension.php b/tests/phpstan/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtension.php new file mode 100644 index 0000000000..4ff3f23435 --- /dev/null +++ b/tests/phpstan/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtension.php @@ -0,0 +1,217 @@ + */ + private $errors; + + public function __construct( + FileTypeMapper $fileTypeMapper, + RuleLevelHelper $ruleLevelHelper + ) { + $this->hookDocBlock = new HookDocBlock($fileTypeMapper); + $this->ruleLevelHelper = $ruleLevelHelper; + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + // Ensure the node is a FuncCall instance. + if (! ($node instanceof FuncCall)) { + return []; + } + + $this->currentNode = $node; + $this->currentScope = $scope; + $this->errors = []; + + if (! ($node->name instanceof Name) + || ! in_array($node->name->toString(), self::SUPPORTED_FUNCTIONS, true) + ) { + return []; + } + + $resolvedPhpDoc = $this->hookDocBlock->getNullableHookDocBlock($node, $scope); + + if ($resolvedPhpDoc === null) { + $this->errors[] = RuleErrorBuilder::message( + 'Missing docblock for wpm_apply_filters_typed call.' + )->identifier('docblock.missing')->build(); + + return $this->errors; + } + + $this->validateDocBlock($resolvedPhpDoc); + + return $this->errors; + } + + /** + * Validates the `@param` tags documented in the given docblock. + */ + public function validateDocBlock(ResolvedPhpDocBlock $resolvedPhpDoc): void + { + // Count all documented `@param` tag strings in the docblock. + $numberOfParamTagStrings = substr_count($resolvedPhpDoc->getPhpDocString(), '* @param '); + + // A docblock with no param tags is allowed and gets skipped. + if ($numberOfParamTagStrings === 0) { + return; + } + + $this->validateParamCount($numberOfParamTagStrings); + + // If the number of param tags doesn't match the number of + // parameters, bail out early. There's no point trying to + // reconcile param tags in this situation. + if ($this->errors !== []) { + return; + } + + // Fetch the parsed `@param` tags from the docblock. + $paramTags = $resolvedPhpDoc->getParamTags(); + + $this->validateParamDocumentation(count($paramTags), $resolvedPhpDoc); + if ($this->errors !== []) { + return; + } + + $nodeArgs = $this->currentNode->getArgs(); + $paramIndex = 2; + + foreach ($paramTags as $paramName => $paramTag) { + $this->validateSingleParamTag($paramName, $paramTag, $nodeArgs[$paramIndex]); + $paramIndex += 1; + } + } + + /** + * Validates the number of documented `@param` tags in the docblock. + */ + public function validateParamCount(int $numberOfParamTagStrings): void + { + // The first parameter is the type, the second parameter is the hook name, so we subtract 2. + $numberOfParams = count($this->currentNode->getArgs()) - 2; + + // Correct number of `@param` tags. + if ($numberOfParams === $numberOfParamTagStrings) { + return; + } + + $this->errors[] = RuleErrorBuilder::message( + sprintf( + 'Expected %1$d @param tags, found %2$d.', + $numberOfParams, + $numberOfParamTagStrings + ) + )->identifier('paramTag.count')->build(); + } + + /** + * Validates the number of parsed and valid `@param` tags in the docblock. + */ + public function validateParamDocumentation( + int $numberOfParamTags, + ResolvedPhpDocBlock $resolvedPhpDoc + ): void { + $nodeArgs = $this->currentNode->getArgs(); + $numberOfParams = count($nodeArgs) - 2; + + // No invalid `@param` tags. + if ($numberOfParams === $numberOfParamTags) { + return; + } + + // We might have an invalid `@param` tag because it's named `$this`. + // PHPStan does not detect param tags named `$this`, it skips the tag. + // We can indirectly detect this by checking the actual parameter name, + // and if one of them is `$this` assume that's the problem. + $namedThis = false; + if (strpos($resolvedPhpDoc->getPhpDocString(), ' $this') !== false) { + foreach ($nodeArgs as $param) { + if (($param->value instanceof Variable) && $param->value->name === 'this') { + $namedThis = true; + break; + } + } + } + + $this->errors[] = RuleErrorBuilder::message( + $namedThis === true + ? '@param tag must not be named $this. Choose a descriptive alias, for example $instance.' + : 'One or more @param tags has an invalid name or invalid syntax.' + )->identifier('phpDoc.parseError')->build(); + } + + /** + * Validates a `@param` tag against its actual parameter. + * + * @param string $paramName The param tag name. + * @param ParamTag $paramTag The param tag instance. + * @param Arg $arg The actual parameter instance. + */ + protected function validateSingleParamTag(string $paramName, ParamTag $paramTag, Arg $arg): void + { + $paramTagType = $paramTag->getType(); + $paramType = $this->currentScope->getType($arg->value); + $accepted = $this->ruleLevelHelper->accepts( + $paramTagType, + $paramType, + $this->currentScope->isDeclareStrictTypes() + ); + + if ($accepted) { + return; + } + + $paramTagVerbosityLevel = VerbosityLevel::getRecommendedLevelByType($paramTagType); + $paramVerbosityLevel = VerbosityLevel::getRecommendedLevelByType($paramType); + + $this->errors[] = RuleErrorBuilder::message( + sprintf( + '@param %1$s $%2$s does not accept actual type of parameter: %3$s.', + $paramTagType->describe($paramTagVerbosityLevel), + $paramName, + $paramType->describe($paramVerbosityLevel) + ) + )->identifier('parameter.phpDocType')->build(); + } +} diff --git a/tests/phpstan/tests/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest.php b/tests/phpstan/tests/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest.php new file mode 100644 index 0000000000..82719fa26c --- /dev/null +++ b/tests/phpstan/tests/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest.php @@ -0,0 +1,43 @@ +getContainer()->getByType(FileTypeMapper::class), $this->getContainer()->getByType(RuleLevelHelper::class)); + } + + public function testShouldRaiseError() { + $this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/not-valid.php'], [ + [ + 'Missing docblock for wpm_apply_filters_typed call.', + 3, + ], + [ + 'Expected 2 @param tags, found 0.', + 8, + ], + [ + 'Expected 2 @param tags, found 1.', + 13, + ], + ]); + } + + public function testShouldNotRaiseError() { + $this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/valid.php'], []); + } + + public static function getAdditionalConfigFiles(): array + { + // path to your project's phpstan.neon, or extension.neon in case of custom extension packages + // this is only necessary if your custom rule relies on some extra configuration and other extensions + return [__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/extension.neon']; + } +} diff --git a/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/extension.neon b/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/extension.neon new file mode 100644 index 0000000000..383a8a393d --- /dev/null +++ b/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/extension.neon @@ -0,0 +1,2 @@ +includes: + - ../../../../../vendor/szepeviktor/phpstan-wordpress/extension.neon diff --git a/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/not-valid.php b/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/not-valid.php new file mode 100644 index 0000000000..63e59508bf --- /dev/null +++ b/tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtension/not-valid.php @@ -0,0 +1,13 @@ +