test: AutoReview - unify data provider returns (#7544)

PHP-CS-Fixer · Dec 10, 2023 · 095443e · 095443e
1 parent d7f6ea9
commit 095443e
Show file tree

Hide file tree

Showing 22 changed files with 117 additions and 58 deletions.
diff --git a/phpstan.dist.neon b/phpstan.dist.neon
@@ -17,11 +17,11 @@ parameters:
         - '/^Class [a-zA-Z\\]+ extends @final class PhpCsFixer\\(ConfigurationException\\InvalidConfigurationException|ConfigurationException\\InvalidFixerConfigurationException|Tokenizer\\Tokens|Console\\Command\\FixCommand)\.$/'
         - '/^\$this\(PhpCsFixer\\Tokenizer\\Tokens\) does not accept PhpCsFixer\\Tokenizer\\Token\|null\.$/'
 
-        # ignore PHPUnit data providers return type as they are not checked against the test methods anyway
+        # PHPUnit data providers return type were not maintained originally, this exception should shrink over time (eg with help of custom, re-usable type)
         -
-            message: '#^Method PhpCsFixer\\Tests\\.+::provide.+Cases\(\) return type has no value type specified in iterable type (array|iterable)\.$#'
+            message: '#^Method PhpCsFixer\\Tests\\.+::provide.+Cases\(\) return type has no value type specified in iterable type iterable\.$#'
             path: tests
-            count: 1126
+            count: 1120
 
         -
             message: '#Call to static method .+ with .+ will always evaluate to true.$#'

diff --git a/tests/AbstractDoctrineAnnotationFixerTestCase.php b/tests/AbstractDoctrineAnnotationFixerTestCase.php
@@ -23,7 +23,7 @@
 abstract class AbstractDoctrineAnnotationFixerTestCase extends AbstractFixerTestCase
 {
     /**
-     * @param array<mixed> $configuration
+     * @param array<string, mixed> $configuration
      *
      * @dataProvider provideConfigureWithInvalidConfigurationCases
      */
@@ -34,6 +34,9 @@ public function testConfigureWithInvalidConfiguration(array $configuration): voi
         $this->fixer->configure($configuration);
     }
 
+    /**
+     * @return iterable<array{array<string, mixed>}>
+     */
     public static function provideConfigureWithInvalidConfigurationCases(): iterable
     {
         yield [['foo' => 'bar']];

diff --git a/tests/AutoReview/ProjectCodeTest.php b/tests/AutoReview/ProjectCodeTest.php
@@ -452,6 +452,56 @@ public function testExpectedInputOrder(string $testClassName): void
         }
     }
 
+    /**
+     * @dataProvider provideTestClassCases
+     */
+    public function testDataProvidersDeclaredReturnType(string $testClassName): void
+    {
+        $reflectionClass = new \ReflectionClass($testClassName);
+
+        $publicMethods = array_filter(
+            $reflectionClass->getMethods(),
+            static fn (\ReflectionMethod $reflectionMethod): bool => $reflectionMethod->getDeclaringClass()->getName() === $reflectionClass->getName()
+        );
+
+        $dataProviderMethods = array_filter(
+            $publicMethods,
+            static fn (\ReflectionMethod $reflectionMethod): bool => str_starts_with($reflectionMethod->getName(), 'provide')
+        );
+
+        if ([] === $dataProviderMethods) {
+            $this->expectNotToPerformAssertions(); // no methods to test, all good!
+
+            return;
+        }
+
+        /** @var \ReflectionMethod $method */
+        foreach ($dataProviderMethods as $method) {
+            $methodId = $method->getDeclaringClass()->getName().'::'.$method->getName();
+
+            self::assertSame('iterable', $method->hasReturnType() ? $method->getReturnType()->__toString() : null, sprintf('DataProvider `%s` must provide `iterable` as return in method prototype.', $methodId));
+
+            $doc = new DocBlock(false !== $method->getDocComment() ? $method->getDocComment() : '/** */');
+
+            $returnDocs = $doc->getAnnotationsOfType('return');
+            if (\count($returnDocs) > 1) {
+                throw new \UnexpectedValueException(sprintf('Multiple `%s@return` annotations.', $methodId));
+            }
+            if (1 !== \count($returnDocs)) {
+                $this->addToAssertionCount(1); // no @return annotation, all good!
+
+                continue;
+            }
+
+            $returnDoc = $returnDocs[0];
+            $types = $returnDoc->getTypes();
+
+            self::assertCount(1, $types, sprintf('DataProvider `%s@return` must provide single type.', $methodId));
+            self::assertMatchesRegularExpression('/^iterable\</', $types[0], sprintf('DataProvider `%s@return` must return iterable.', $methodId));
+            self::assertMatchesRegularExpression('/^iterable\\<(?:(?:int\\|)?string, )?array\\{/', $types[0], sprintf('DataProvider `%s@return` must return iterable of tuples (eg `iterable<string, array{string, string}>`).', $methodId));
+        }
+    }
+
     /**
      * @dataProvider provideSrcClassCases
      * @dataProvider provideTestClassCases

diff --git a/tests/AutoReview/TransformerTest.php b/tests/AutoReview/TransformerTest.php
@@ -114,7 +114,7 @@ public static function provideTransformerPriorityCases(): iterable
     }
 
     /**
-     * @return TransformerInterface[]
+     * @return iterable<array{TransformerInterface}>
      */
     public static function provideTransformerPriorityIsListedCases(): iterable
     {

diff --git a/tests/Fixer/Alias/RandomApiMigrationFixerTest.php b/tests/Fixer/Alias/RandomApiMigrationFixerTest.php
@@ -70,7 +70,7 @@ public function testFix(string $expected, ?string $input = null, array $config =
     }
 
     /**
-     * @return array[]
+     * @return iterable<array{0: string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixCases(): iterable
     {

diff --git a/tests/Fixer/ClassNotation/ClassDefinitionFixerTest.php b/tests/Fixer/ClassNotation/ClassDefinitionFixerTest.php
@@ -889,7 +889,7 @@ private function doTestClassyInheritanceInfo(string $source, string $label, arra
         self::assertSame($expected, $result);
     }
 
-    private static function provideClassyCases(string $classy): array
+    private static function provideClassyCases(string $classy): iterable
     {
         return [
             [
@@ -957,7 +957,7 @@ private static function provideClassyCases(string $classy): array
         ];
     }
 
-    private static function provideClassyExtendingCases(string $classy): array
+    private static function provideClassyExtendingCases(string $classy): iterable
     {
         return [
             [
@@ -987,7 +987,7 @@ private static function provideClassyExtendingCases(string $classy): array
         ];
     }
 
-    private static function provideClassyImplementsCases(): array
+    private static function provideClassyImplementsCases(): iterable
     {
         return [
             [

diff --git a/tests/Fixer/ClassNotation/OrderedTraitsFixerTest.php b/tests/Fixer/ClassNotation/OrderedTraitsFixerTest.php
@@ -32,7 +32,7 @@ public function testFix(string $expected, ?string $input = null): void
     }
 
     /**
-     * @return iterable<array>
+     * @return iterable<string, array{string, 1?: ?string}>
      */
     public static function provideFixCases(): iterable
     {
@@ -320,25 +320,22 @@ class User
     }
 
     /**
-     * @param array<mixed> $configuration
+     * @param array<string, mixed> $configuration
      *
      * @dataProvider provideFixWithConfigurationCases
      */
-    public function testFixWithConfiguration(array $configuration, string $expected, ?string $input = null): void
+    public function testFixWithConfiguration(string $expected, string $input = null, array $configuration): void
     {
         $this->fixer->configure($configuration);
         $this->doTest($expected, $input);
     }
 
     /**
-     * @return iterable<mixed>
+     * @return iterable<string, array{string, 1: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixWithConfigurationCases(): iterable
     {
         yield 'with case sensitive order' => [
-            [
-                'case_sensitive' => true,
-            ],
             '<?php
 class Foo {
     use AA;
@@ -349,6 +346,9 @@ class Foo {
     use Aaa;
     use AA;
 }',
+            [
+                'case_sensitive' => true,
+            ],
         ];
     }
 }
diff --git a/tests/Fixer/ClassNotation/OrderedTypesFixerTest.php b/tests/Fixer/ClassNotation/OrderedTypesFixerTest.php
@@ -40,7 +40,7 @@ public function testFix(string $expected, ?string $input = null, ?array $config
     }
 
     /**
-     * @return iterable<string, (null|array<string, string>|string)[]|string[]>
+     * @return iterable<string, array{string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixCases(): iterable
     {
@@ -162,7 +162,7 @@ public function testFixPhp80(string $expected, ?string $input = null, ?array $co
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]>
+     * @return iterable<string, array{string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixPhp80Cases(): iterable
     {
@@ -214,7 +214,7 @@ public function testFixDefault(string $expected, ?string $input = null): void
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1?: ?string}>
      */
     public static function provideFixDefaultCases(): iterable
     {
@@ -330,7 +330,7 @@ public function testFixWithAlphaAlgorithmAndNullAlwaysLast(string $expected, ?st
     }
 
     /**
-     * @return iterable<(null|string|string[])[]|string[]>
+     * @return iterable<array{string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixWithAlphaAlgorithmAndNullAlwaysLastCases(): iterable
     {
@@ -446,7 +446,7 @@ public function testFixWithAlphaAlgorithmOnly(string $expected, ?string $input =
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1?: ?string}>
      */
     public static function provideFixWithAlphaAlgorithmOnlyCases(): iterable
     {
@@ -561,7 +561,7 @@ public function testFixWithSandwichedWhitespaceOrCommentInType(string $expected,
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1: ?string}>
      */
     public static function provideFixWithSandwichedWhitespaceOrCommentInTypeCases(): iterable
     {
@@ -603,7 +603,7 @@ public function testFixPhp81(string $expected, ?string $input = null, ?array $co
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixPhp81Cases(): iterable
     {
@@ -660,7 +660,7 @@ public function testFixPhp82(string $expected, ?string $input = null, ?array $co
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixPhp82Cases(): iterable
     {
@@ -721,7 +721,7 @@ public function testFixWithCaseSensitive(string $expected, ?string $input = null
     }
 
     /**
-     * @return iterable<(null|array<string, string>|string)[]|string[]>
+     * @return iterable<array{string, 1: ?string}>
      */
     public static function provideFixWithCaseSensitiveCases(): iterable
     {

diff --git a/tests/Fixer/FunctionNotation/VoidReturnFixerTest.php b/tests/Fixer/FunctionNotation/VoidReturnFixerTest.php
@@ -34,6 +34,9 @@ public function testFix(string $expected, ?string $input = null): void
         $this->doTest($expected, $input);
     }
 
+    /**
+     * @return iterable<array{string, 1?: ?string}>
+     */
     public static function provideFixCases(): iterable
     {
         yield ['<?php class Test { public function __construct() {} }'];
@@ -270,7 +273,7 @@ public function testFix80(string $expected, ?string $input = null): void
     }
 
     /**
-     * @return iterable<array<string>>
+     * @return iterable<array{string, 1?: ?string}>
      */
     public static function provideFix80Cases(): iterable
     {

diff --git a/tests/Fixer/LanguageConstruct/NullableTypeDeclarationFixerTest.php b/tests/Fixer/LanguageConstruct/NullableTypeDeclarationFixerTest.php
@@ -36,7 +36,7 @@ public function testDefaultFix(string $expected, ?string $input = null): void
     }
 
     /**
-     * @return iterable<string, array<int, null|string>>
+     * @return iterable<string, array{string, 1?: ?string}>
      */
     public static function provideDefaultFixCases(): iterable
     {
@@ -113,7 +113,7 @@ public function testFixWithUnionSyntax(string $expected, ?string $input = null):
     }
 
     /**
-     * @return iterable<string, array<int, null|string>>
+     * @return iterable<string, array{string, 1?: ?string}>
      */
     public static function provideFixWithUnionSyntaxCases(): iterable
     {
@@ -209,7 +209,7 @@ public function testFixPhp81(string $expected, ?string $input = null, ?array $co
     }
 
     /**
-     * @return iterable<string, array<int, null|array<string, string>|string>>
+     * @return iterable<string, array{string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixPhp81Cases(): iterable
     {
@@ -262,7 +262,7 @@ public function testFixPhp82(string $expected, ?string $input = null, ?array $co
     }
 
     /**
-     * @return iterable<string, array<int, null|array<string, string>|string>>
+     * @return iterable<string, array{string, 1?: ?string, 2?: array<string, mixed>}>
      */
     public static function provideFixPhp82Cases(): iterable
     {

diff --git a/tests/Fixer/PhpUnit/PhpUnitInternalClassFixerTest.php b/tests/Fixer/PhpUnit/PhpUnitInternalClassFixerTest.php
@@ -393,7 +393,7 @@ public function testFix80(string $expected, string $input): void
     }
 
     /**
-     * @return iterable<string[]>
+     * @return iterable<string, array{string, string}>
      */
     public static function provideFix80Cases(): iterable
     {
@@ -503,6 +503,9 @@ public function testFix82(string $expected, ?string $input = null, array $config
         $this->doTest($expected, $input);
     }
 
+    /**
+     * @return iterable<int|string, array{string, 1: ?string, 2?: array<string, mixed>}>
+     */
     public static function provideFix82Cases(): iterable
     {
         yield 'If final is not added as an option, final classes will not be marked internal' => [

diff --git a/tests/Fixer/PhpUnit/PhpUnitMethodCasingFixerTest.php b/tests/Fixer/PhpUnit/PhpUnitMethodCasingFixerTest.php
@@ -183,7 +183,7 @@ public function testFix80ToCamelCase(string $expected, string $input): void
     }
 
     /**
-     * @return iterable<string[]>
+     * @return iterable<string, array{string, string}>
      */
     public static function provideFix80ToCamelCaseCases(): iterable
     {

diff --git a/tests/Fixer/PhpUnit/PhpUnitSizeClassFixerTest.php b/tests/Fixer/PhpUnit/PhpUnitSizeClassFixerTest.php
@@ -334,7 +334,7 @@ public function testFix80(string $expected, string $input): void
     }
 
     /**
-     * @return iterable<string[]>
+     * @return iterable<string, array{string, string}>
      */
     public static function provideFix80Cases(): iterable
     {

diff --git a/tests/Fixer/PhpUnit/PhpUnitTestAnnotationFixerTest.php b/tests/Fixer/PhpUnit/PhpUnitTestAnnotationFixerTest.php
@@ -1082,7 +1082,7 @@ public function testFix80(string $expected, string $input, array $config): void
     }
 
     /**
-     * @return iterable<(array<string, string>|string)[]>
+     * @return iterable<array{string, 1?: ?string}>
      */
     public static function provideFix80Cases(): iterable
     {

diff --git a/tests/Fixer/Phpdoc/PhpdocAddMissingParamAnnotationFixerTest.php b/tests/Fixer/Phpdoc/PhpdocAddMissingParamAnnotationFixerTest.php
@@ -58,7 +58,7 @@ public function testConfigureRejectsInvalidConfigurationValue($value, string $ex
     }
 
     /**
-     * @return iterable<string, array>
+     * @return iterable<string, array{mixed, string}>
      */
     public static function provideConfigureRejectsInvalidConfigurationValueCases(): iterable
     {