feature: Introduce `PhpUnitDataProviderReturnTypeFixer` (#7156)

Co-authored-by: Kuba Werłos <9282069+kubawerlos@users.noreply.github.com>

doc/list.rst

@@ -2465,6 +2465,18 @@ List of Available Rules
    Part of rule set `@PhpCsFixer:risky <./ruleSets/PhpCsFixerRisky.rst>`_
 
    `Source PhpCsFixer\\Fixer\\PhpUnit\\PhpUnitDataProviderNameFixer <./../src/Fixer/PhpUnit/PhpUnitDataProviderNameFixer.php>`_
+-  `php_unit_data_provider_return_type <./rules/php_unit/php_unit_data_provider_return_type.rst>`_
+
+   The return type of PHPUnit data provider must be ``iterable``.
+
+   Data provider must return ``iterable``, either an array of arrays or an
+   object that implements the ``Traversable`` interface.
+
+   *warning risky* Risky when relying on signature of the data provider.
+
+   Part of rule set `@PhpCsFixer:risky <./ruleSets/PhpCsFixerRisky.rst>`_
+
+   `Source PhpCsFixer\\Fixer\\PhpUnit\\PhpUnitDataProviderReturnTypeFixer <./../src/Fixer/PhpUnit/PhpUnitDataProviderReturnTypeFixer.php>`_
 -  `php_unit_data_provider_static <./rules/php_unit/php_unit_data_provider_static.rst>`_
 
    Data providers must be static.

doc/ruleSets/PhpCsFixerRisky.rst

@@ -20,6 +20,7 @@ Rules
 - `no_unreachable_default_argument_value <./../rules/function_notation/no_unreachable_default_argument_value.rst>`_
 - `no_unset_on_property <./../rules/language_construct/no_unset_on_property.rst>`_
 - `php_unit_data_provider_name <./../rules/php_unit/php_unit_data_provider_name.rst>`_
+- `php_unit_data_provider_return_type <./../rules/php_unit/php_unit_data_provider_return_type.rst>`_
 - `php_unit_strict <./../rules/php_unit/php_unit_strict.rst>`_
 - `php_unit_test_case_static_method_calls <./../rules/php_unit/php_unit_test_case_static_method_calls.rst>`_
   config:

doc/rules/index.rst

@@ -609,6 +609,9 @@ PHPUnit
 - `php_unit_data_provider_name <./php_unit/php_unit_data_provider_name.rst>`_ *(risky)*
 
   Data provider names must match the name of the test.
+- `php_unit_data_provider_return_type <./php_unit/php_unit_data_provider_return_type.rst>`_ *(risky)*
+
+  The return type of PHPUnit data provider must be ``iterable``.
 - `php_unit_data_provider_static <./php_unit/php_unit_data_provider_static.rst>`_ *(risky)*
 
   Data providers must be static.

doc/rules/php_unit/php_unit_data_provider_return_type.rst

@@ -0,0 +1,64 @@
+===========================================
+Rule ``php_unit_data_provider_return_type``
+===========================================
+
+The return type of PHPUnit data provider must be ``iterable``.
+
+Description
+-----------
+
+Data provider must return ``iterable``, either an array of arrays or an object
+that implements the ``Traversable`` interface.
+
+Warning
+-------
+
+Using this rule is risky
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Risky when relying on signature of the data provider.
+
+Examples
+--------
+
+Example #1
+~~~~~~~~~~
+
+.. code-block:: diff
+
+   --- Original
+   +++ New
+    <?php
+    class FooTest extends TestCase {
+        /**
+         * @dataProvider provideSomethingCases
+         */
+        public function testSomething($expected, $actual) {}
+   -    public function provideSomethingCases(): array {}
+   +    public function provideSomethingCases(): iterable {}
+    }
+
+Example #2
+~~~~~~~~~~
+
+.. code-block:: diff
+
+   --- Original
+   +++ New
+    <?php
+    class FooTest extends TestCase {
+        /**
+         * @dataProvider provideSomethingCases
+         */
+        public function testSomething($expected, $actual) {}
+   -    public function provideSomethingCases() {}
+   +    public function provideSomethingCases(): iterable {}
+    }
+
+Rule sets
+---------
+
+The rule is part of the following rule set:
+
+@PhpCsFixer:risky
+  Using the `@PhpCsFixer:risky <./../../ruleSets/PhpCsFixerRisky.rst>`_ rule set will enable the ``php_unit_data_provider_return_type`` rule.

src/Fixer/FunctionNotation/ReturnTypeDeclarationFixer.php

@@ -55,7 +55,7 @@ final class ReturnTypeDeclarationFixer extends AbstractFixer implements Configur
     /**
      * {@inheritdoc}
      *
-     * Must run after PhpdocToReturnTypeFixer, VoidReturnFixer.
+     * Must run after PhpUnitDataProviderReturnTypeFixer, PhpdocToReturnTypeFixer, VoidReturnFixer.
      */
     public function getPriority(): int
     {

src/Fixer/NamespaceNotation/CleanNamespaceFixer.php

@@ -45,6 +45,16 @@ final class CleanNamespaceFixer extends AbstractFixer
         return \PHP_VERSION_ID < 8_00_00 && $tokens->isTokenKindFound(T_NS_SEPARATOR);
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * Must run before PhpUnitDataProviderReturnTypeFixer.
+     */
+    public function getPriority(): int
+    {
+        return 1;
+    }
+
     protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
     {
         $count = $tokens->count();

src/Fixer/PhpUnit/PhpUnitDataProviderReturnTypeFixer.php

@@ -0,0 +1,120 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *     Dariusz Rumiński <dariusz.ruminski@gmail.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Fixer\PhpUnit;
+
+use PhpCsFixer\Fixer\AbstractPhpUnitFixer;
+use PhpCsFixer\FixerDefinition\CodeSample;
+use PhpCsFixer\FixerDefinition\FixerDefinition;
+use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
+use PhpCsFixer\Tokenizer\Analyzer\DataProviderAnalyzer;
+use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer;
+use PhpCsFixer\Tokenizer\CT;
+use PhpCsFixer\Tokenizer\Token;
+use PhpCsFixer\Tokenizer\Tokens;
+
+/**
+ * @author Kuba Werłos <werlos@gmail.com>
+ */
+final class PhpUnitDataProviderReturnTypeFixer extends AbstractPhpUnitFixer
+{
+    public function getDefinition(): FixerDefinitionInterface
+    {
+        return new FixerDefinition(
+            'The return type of PHPUnit data provider must be `iterable`.',
+            [
+                new CodeSample(
+                    '<?php
+class FooTest extends TestCase {
+    /**
+     * @dataProvider provideSomethingCases
+     */
+    public function testSomething($expected, $actual) {}
+    public function provideSomethingCases(): array {}
+}
+',
+                ),
+                new CodeSample(
+                    '<?php
+class FooTest extends TestCase {
+    /**
+     * @dataProvider provideSomethingCases
+     */
+    public function testSomething($expected, $actual) {}
+    public function provideSomethingCases() {}
+}
+',
+                ),
+            ],
+            'Data provider must return `iterable`, either an array of arrays or an object that implements the `Traversable` interface.',
+            'Risky when relying on signature of the data provider.',
+        );
+    }
+
+    /**
+     * {@inheritdoc}
+     *
+     * Must run before ReturnTypeDeclarationFixer.
+     * Must run after CleanNamespaceFixer.
+     */
+    public function getPriority(): int
+    {
+        return 0;
+    }
+
+    public function isRisky(): bool
+    {
+        return true;
+    }
+
+    protected function applyPhpUnitClassFix(Tokens $tokens, int $startIndex, int $endIndex): void
+    {
+        $dataProviderAnalyzer = new DataProviderAnalyzer();
+        $functionsAnalyzer = new FunctionsAnalyzer();
+
+        foreach (array_reverse($dataProviderAnalyzer->getDataProviders($tokens, $startIndex, $endIndex)) as $dataProviderAnalysis) {
+            $typeAnalysis = $functionsAnalyzer->getFunctionReturnType($tokens, $dataProviderAnalysis->getNameIndex());
+
+            if (null === $typeAnalysis) {
+                $argumentsStart = $tokens->getNextTokenOfKind($dataProviderAnalysis->getNameIndex(), ['(']);
+                $argumentsEnd = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $argumentsStart);
+
+                $tokens->insertAt(
+                    $argumentsEnd + 1,
+                    [
+                        new Token([CT::T_TYPE_COLON, ':']),
+                        new Token([T_WHITESPACE, ' ']),
+                        new Token([T_STRING, 'iterable']),
+                    ],
+                );
+
+                continue;
+            }
+
+            if ('iterable' === $typeAnalysis->getName()) {
+                continue;
+            }
+
+            $typeStartIndex = $tokens->getNextMeaningfulToken($typeAnalysis->getStartIndex() - 1);
+            $typeEndIndex = $typeAnalysis->getEndIndex();
+
+            // @TODO: drop condition and it's body when PHP 8+ is required
+            if ($tokens->generatePartialCode($typeStartIndex, $typeEndIndex) !== $typeAnalysis->getName()) {
+                continue;
+            }
+
+            $tokens->overrideRange($typeStartIndex, $typeEndIndex, [new Token([T_STRING, 'iterable'])]);
+        }
+    }
+}

src/RuleSet/Sets/PhpCsFixerRiskySet.php

@@ -50,6 +50,7 @@ final class PhpCsFixerRiskySet extends AbstractRuleSetDescription
             'no_unreachable_default_argument_value' => true,
             'no_unset_on_property' => true,
             'php_unit_data_provider_name' => true,
+            'php_unit_data_provider_return_type' => true,
             'php_unit_strict' => true,
             'php_unit_test_case_static_method_calls' => ['call_type' => 'self'],
             'strict_comparison' => true,

tests/AutoReview/FixerFactoryTest.php

@@ -372,6 +372,9 @@ final class FixerFactoryTest extends TestCase
             'class_keyword_remove' => [
                 'no_unused_imports',
             ],
+            'clean_namespace' => [
+                'php_unit_data_provider_return_type',
+            ],
             'combine_consecutive_issets' => [
                 'multiline_whitespace_before_semicolons',
                 'no_singleline_whitespace_before_semicolons',
@@ -675,6 +678,9 @@ final class FixerFactoryTest extends TestCase
             'php_unit_construct' => [
                 'php_unit_dedicate_assert',
             ],
+            'php_unit_data_provider_return_type' => [
+                'return_type_declaration',
+            ],
             'php_unit_dedicate_assert' => [
                 'no_unused_imports',
                 'php_unit_dedicate_assert_internal_type',

tests/AutoReview/ProjectCodeTest.php

@@ -279,29 +279,6 @@ final class ProjectCodeTest extends TestCase
         );
     }
 
-    /**
-     * @dataProvider provideDataProviderMethodCases
-     */
-    public function testThatTestDataProvidersReturnIterableOrArray(string $testClassName, \ReflectionMethod $dataProvider): void
-    {
-        $dataProviderName = $dataProvider->getName();
-
-        $returnType = $dataProvider->getReturnType();
-
-        self::assertInstanceOf(
-            \ReflectionNamedType::class,
-            $returnType,
-            sprintf('Data provider in "%s" with name "%s" has no return type.', $dataProvider->getDeclaringClass()->getName(), $dataProviderName)
-        );
-
-        $returnTypeName = $returnType->getName();
-
-        self::assertTrue(
-            'array' === $returnTypeName || 'iterable' === $returnTypeName,
-            sprintf('Data provider in "%s" with name "%s" has return type "%s", expected "array" or "iterable".', $dataProvider->getDeclaringClass()->getName(), $dataProviderName, $returnTypeName)
-        );
-    }
-
     /**
      * @dataProvider provideDataProviderMethodCases
      */