Closes #7150: Add tests for Ensure Docblock presence for wpm_apply_filters_typed() rule

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

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
-

tests/phpstan/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtension.php

@@ -0,0 +1,217 @@
+<?php
+
+namespace WP_Rocket\Tests\phpstan\Rules;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Name;
+use PHPStan\Analyser\Scope;
+use PHPStan\PhpDoc\ResolvedPhpDocBlock;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Type\FileTypeMapper;
+use PHPStan\Type\VerbosityLevel;
+use SzepeViktor\PHPStan\WordPress\HookDocBlock;
+use PHPStan\PhpDoc\Tag\ParamTag;
+use PhpParser\Node\Arg;
+
+class ApplyFiltersTypedDynamicFunctionReturnTypeExtension implements Rule
+{
+	private const SUPPORTED_FUNCTIONS = [
+		'wpm_apply_filters_typed',
+	];
+
+	/** @var HookDocBlock */
+	protected $hookDocBlock;
+
+	/** @var RuleLevelHelper */
+	protected $ruleLevelHelper;
+
+	/** @var FuncCall */
+	protected $currentNode;
+
+	/** @var Scope */
+	protected $currentScope;
+
+	/** @var list<IdentifierRuleError> */
+	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->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();
+	}
+}

tests/phpstan/tests/Rules/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace WP_Rocket\Tests\phpstan\tests\Rules;
+
+use Mockery;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Type\FileTypeMapper;
+use WP_Rocket\Tests\phpstan\Rules\ApplyFiltersTypedDynamicFunctionReturnTypeExtension;
+
+class ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest extends RuleTestCase {
+	protected function getRule(): Rule {
+		return new ApplyFiltersTypedDynamicFunctionReturnTypeExtension(
+			$this->getContainer()->getByType(FileTypeMapper::class),
+			$this->getContainer()->getByType(RuleLevelHelper::class)
+		);
+	}
+
+	public function testValidShouldNotHaveErrors() {
+		$this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/valid.php'], [
+		]);
+	}
+
+	public function testTypeNotValidShouldReturnError() {
+		$this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/missing-type.php'], [
+			[
+				"One or more @param tags has an invalid name or invalid syntax.",
+				18
+			],
+		]);
+	}
+
+	public function testWrongTypeShouldReturnError() {
+		$this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/wrong-type.php'], [
+			[
+				"One or more @param tags has an invalid name or invalid syntax.",
+				18
+			],
+		]);
+	}
+
+	public function testNoDocblockShouldReturnError() {
+		$this->analyse([__DIR__ . '/../data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/missing-docblock.php'], [
+			[
+				"No docblock.",
+				19
+			],
+		]);
+	}
+}

tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/missing-docblock.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace WP_Rocket\Engine\Admin;
+
+use WP_Rocket\ThirdParty\ReturnTypesTrait;
+
+class FilterTest {
+
+	use ReturnTypesTrait;
+	public function get_filter_value(string $buffer): string {
+
+		return wpm_apply_filters_typed( 'string', 'rocket_performance_hints_buffer', $buffer );
+
+	}
+}

tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/missing-type.php

@@ -0,0 +1,13 @@
+<?php
+
+$buffer = 'test';
+
+/**
+ * Filters the buffer content for performance hints.
+ *
+ * @since 3.17
+ *
+ * @param $buffer Page HTML content.
+ */
+wpm_apply_filters_typed( 'string', 'rocket_performance_hints_buffer', $buffer );
+

tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/valid.php

@@ -0,0 +1,10 @@
+<?php
+$buffer = 'Hello world';
+/**
+ * Filters the buffer content for performance hints.
+ *
+ * @since 3.17
+ *
+ * @param string $buffer Page HTML content.
+ */
+wpm_apply_filters_typed( 'string', 'rocket_performance_hints_buffer', $buffer );

tests/phpstan/tests/data/ApplyFiltersTypedDynamicFunctionReturnTypeExtensionTest/wrong-type.php

@@ -0,0 +1,13 @@
+<?php
+
+$buffer = true;
+
+/**
+ * Filters the buffer content for performance hints.
+ *
+ * @since 3.17
+ *
+ * @param boolean $buffer Page HTML content.
+ */
+wpm_apply_filters_typed( 'string', 'rocket_performance_hints_buffer', $buffer );
+