From e8a4b37686ab4255f5a8d7fdfc306cb63d7d20c6 Mon Sep 17 00:00:00 2001
From: Fabien Villepinte <fabien.villepinte@gmail.com>
Date: Wed, 7 Dec 2022 12:42:53 +0100
Subject: [PATCH] Add rule to check @dataProvider

---
 extension.neon                                |   2 +
 .../PHPUnit/DataProviderDeclarationRule.php   |  81 +++++++++++++
 src/Rules/PHPUnit/DataProviderHelper.php      | 111 ++++++++++++++++++
 .../DataProviderDeclarationRuleTest.php       |  54 +++++++++
 .../data/data-provider-declaration.php        |  70 +++++++++++
 5 files changed, 318 insertions(+)
 create mode 100644 src/Rules/PHPUnit/DataProviderDeclarationRule.php
 create mode 100644 src/Rules/PHPUnit/DataProviderHelper.php
 create mode 100644 tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php
 create mode 100644 tests/Rules/PHPUnit/data/data-provider-declaration.php

diff --git a/extension.neon b/extension.neon
index f6f372e..5c6a90d 100644
--- a/extension.neon
+++ b/extension.neon
@@ -55,6 +55,8 @@ services:
 		class: PHPStan\Rules\PHPUnit\CoversHelper
 	-
 		class: PHPStan\Rules\PHPUnit\AnnotationHelper
+	-
+		class: PHPStan\Rules\PHPUnit\DataProviderHelper
 
 conditionalTags:
 	PHPStan\PhpDoc\PHPUnit\MockObjectTypeNodeResolverExtension:
diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php
new file mode 100644
index 0000000..865d137
--- /dev/null
+++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php
@@ -0,0 +1,81 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Type\FileTypeMapper;
+use PHPUnit\Framework\TestCase;
+use function array_merge;
+
+/**
+ * @implements Rule<Node\Stmt\ClassMethod>
+ */
+class DataProviderDeclarationRule implements Rule
+{
+
+	/**
+	 * Data provider helper.
+	 *
+	 * @var DataProviderHelper
+	 */
+	private $dataProviderHelper;
+
+	/**
+	 * The file type mapper.
+	 *
+	 * @var FileTypeMapper
+	 */
+	private $fileTypeMapper;
+
+	public function __construct(
+		DataProviderHelper $dataProviderHelper,
+		FileTypeMapper $fileTypeMapper
+	)
+	{
+		$this->dataProviderHelper = $dataProviderHelper;
+		$this->fileTypeMapper = $fileTypeMapper;
+	}
+
+	public function getNodeType(): string
+	{
+		return Node\Stmt\ClassMethod::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		$classReflection = $scope->getClassReflection();
+
+		if ($classReflection === null || !$classReflection->isSubclassOf(TestCase::class)) {
+			return [];
+		}
+
+		$docComment = $node->getDocComment();
+		if ($docComment === null) {
+			return [];
+		}
+
+		$methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc(
+			$scope->getFile(),
+			$classReflection->getName(),
+			$scope->isInTrait() ? $scope->getTraitReflection()->getName() : null,
+			$node->name->toString(),
+			$docComment->getText()
+		);
+
+		$annotations = $this->dataProviderHelper->getDataProviderAnnotations($methodPhpDoc);
+
+		$errors = [];
+
+		foreach ($annotations as $annotation) {
+			$errors = array_merge(
+				$errors,
+				$this->dataProviderHelper->processDataProvider($scope, $annotation)
+			);
+		}
+
+		return $errors;
+	}
+
+}
diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php
new file mode 100644
index 0000000..415b1a0
--- /dev/null
+++ b/src/Rules/PHPUnit/DataProviderHelper.php
@@ -0,0 +1,111 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\Analyser\Scope;
+use PHPStan\PhpDoc\ResolvedPhpDocBlock;
+use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
+use PHPStan\Reflection\MissingMethodFromReflectionException;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use function array_merge;
+use function preg_match;
+use function sprintf;
+use function trim;
+
+class DataProviderHelper
+{
+
+	/**
+	 * @return array<PhpDocTagNode>
+	 */
+	public function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array
+	{
+		if ($phpDoc === null) {
+			return [];
+		}
+
+		$phpDocNodes = $phpDoc->getPhpDocNodes();
+
+		$annotations = [];
+
+		foreach ($phpDocNodes as $docNode) {
+			$annotations = array_merge(
+				$annotations,
+				$docNode->getTagsByName('@dataProvider')
+			);
+		}
+
+		return $annotations;
+	}
+
+	/**
+	 * @return RuleError[] errors
+	 */
+	public function processDataProvider(
+		Scope $scope,
+		PhpDocTagNode $phpDocTag
+	): array
+	{
+		$dataProviderName = $this->getDataProviderName($phpDocTag);
+		if ($dataProviderName === null) {
+			// Missing name is already handled in NoMissingSpaceInMethodAnnotationRule
+			return [];
+		}
+
+		$classReflection = $scope->getClassReflection();
+		if ($classReflection === null) {
+			// Should not happen
+			return [];
+		}
+
+		try {
+			$dataProviderMethodReflection = $classReflection->getMethod($dataProviderName, $scope);
+		} catch (MissingMethodFromReflectionException $missingMethodFromReflectionException) {
+			$error = RuleErrorBuilder::message(sprintf(
+				'@dataProvider %s related method not found.',
+				$dataProviderName
+			))->build();
+
+			return [$error];
+		}
+
+		$errors = [];
+
+		if ($dataProviderName !== $dataProviderMethodReflection->getName()) {
+			$errors[] = RuleErrorBuilder::message(sprintf(
+				'@dataProvider %s related method is used with incorrect case: %s.',
+				$dataProviderName,
+				$dataProviderMethodReflection->getName()
+			))->build();
+		}
+
+		if (!$dataProviderMethodReflection->isPublic()) {
+			$errors[] = RuleErrorBuilder::message(sprintf(
+				'@dataProvider %s related method must be public.',
+				$dataProviderName
+			))->build();
+		}
+
+		if (!$dataProviderMethodReflection->isStatic()) {
+			$errors[] = RuleErrorBuilder::message(sprintf(
+				'@dataProvider %s related method must be static.',
+				$dataProviderName
+			))->build();
+		}
+
+		return $errors;
+	}
+
+	private function getDataProviderName(PhpDocTagNode $phpDocTag): ?string
+	{
+		$value = trim((string) $phpDocTag->value);
+
+		if (preg_match('/^[\S]+/', $value, $matches) !== 1) {
+			return null;
+		}
+
+		return $matches[0];
+	}
+
+}
diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php
new file mode 100644
index 0000000..27c3e7d
--- /dev/null
+++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php
@@ -0,0 +1,54 @@
+<?php declare(strict_types=1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPStan\Type\FileTypeMapper;
+
+/**
+ * @extends RuleTestCase<DataProviderDeclarationRule>
+ */
+class DataProviderDeclarationRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new DataProviderDeclarationRule(
+			new DataProviderHelper(),
+			self::getContainer()->getByType(FileTypeMapper::class)
+		);
+	}
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/data-provider-declaration.php'], [
+			[
+				'@dataProvider providebaz related method is used with incorrect case: provideBaz.',
+				13,
+			],
+			[
+				'@dataProvider provideQux related method must be static.',
+				13,
+			],
+			[
+				'@dataProvider provideQuux related method must be public.',
+				13,
+			],
+			[
+				'@dataProvider provideNonExisting related method not found.',
+				66,
+			],
+		]);
+	}
+
+	/**
+	 * @return string[]
+	 */
+	public static function getAdditionalConfigFiles(): array
+	{
+		return [
+			__DIR__ . '/../../../extension.neon',
+		];
+	}
+}
diff --git a/tests/Rules/PHPUnit/data/data-provider-declaration.php b/tests/Rules/PHPUnit/data/data-provider-declaration.php
new file mode 100644
index 0000000..7f63a43
--- /dev/null
+++ b/tests/Rules/PHPUnit/data/data-provider-declaration.php
@@ -0,0 +1,70 @@
+<?php declare(strict_types=1);
+
+namespace ExampleTestCase;
+
+class FooTestCase extends \PHPUnit\Framework\TestCase
+{
+	/**
+	 * @dataProvider provideBar
+	 * @dataProvider providebaz
+	 * @dataProvider provideQux
+	 * @dataProvider provideQuux
+	 */
+	public function testIsNotFoo(string $subject): void
+	{
+		self::assertNotSame('foo', $subject);
+	}
+
+	public static function provideBar(): iterable
+	{
+		return [
+			['bar'],
+		];
+	}
+
+	public static function provideBaz(): iterable
+	{
+		return [
+			['baz'],
+		];
+	}
+
+	public function provideQux(): iterable
+	{
+		return [
+			['qux'],
+		];
+	}
+
+	protected static function provideQuux(): iterable
+	{
+
+		return [
+			['quux'],
+		];
+	}
+}
+
+trait BarProvider
+{
+	public static function provideCorge(): iterable
+	{
+		return [
+			['corge'],
+		];
+	}
+}
+
+class BarTestCase extends \PHPUnit\Framework\TestCase
+{
+	use BarProvider;
+
+	/**
+	 * @dataProvider provideNonExisting
+	 * @dataProvider provideCorge
+	 */
+	public function testIsNotBar(string $subject): void
+	{
+		self::assertNotSame('bar', $subject);
+	}
+}