From c4597bf2a6106684919bd5d1e9b04132a85cd6b6 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 15:38:30 +0200 Subject: [PATCH 01/63] Implement DataProviderDataRule --- composer.json | 2 +- rules.neon | 1 + src/Rules/PHPUnit/DataProviderDataRule.php | 81 ++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 src/Rules/PHPUnit/DataProviderDataRule.php diff --git a/composer.json b/composer.json index 3c07436..39d7a03 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.1.18" + "phpstan/phpstan": "^2.1.32" }, "conflict": { "phpunit/phpunit": "<7.0" diff --git a/rules.neon b/rules.neon index 84b7149..9910730 100644 --- a/rules.neon +++ b/rules.neon @@ -8,6 +8,7 @@ rules: - PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule + - PHPStan\Rules\PHPUnit\DataProviderDataRule conditionalTags: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php new file mode 100644 index 0000000..467988f --- /dev/null +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -0,0 +1,81 @@ + + */ +class DataProviderDataRule implements Rule +{ + public function getNodeType(): string + { + return Node\Stmt\Return_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->expr instanceof Node\Expr\Array_) { + return []; + } + + if ($scope->getFunction() === null) { + return []; + } + + if ($scope->isInAnonymousFunction()) { + return []; + } + + $method = $scope->getFunction(); + if (!$method instanceof PhpMethodFromParserNodeReflection) { + return []; + } + + $classReflection = $scope->getClassReflection(); + + if ($classReflection === null || !$classReflection->is(TestCase::class)) { + return []; + } + + // XXX check whether the method is used as a data provider + + if (method_exists($scope, 'invokeNodeCallback')) { + foreach($node->expr->items as $item) { + if (!$item->value instanceof Node\Expr\Array_) { + return []; + } + + $args = $this->arrayItemsToArgs($item->value); + $var = new Node\Expr\New_(new Node\Name('test')); + $scope->invokeNodeCallback(new Node\Expr\MethodCall($var, 'testTrim', $args)); + } + } + + return []; + } + + /** + * @return array + */ + private function arrayItemsToArgs(Node\Expr\Array_ $array): ?array { + $args = []; + + foreach($array->items as $item) { + // XXX named args + $value = $item->value; + + $arg = new Node\Arg($value); + $args[] = $arg; + } + + return $args; + } + +} From ccff9927a7471f2b2b273193265d19e91ff49589 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 15:41:34 +0200 Subject: [PATCH 02/63] report correct line --- src/Rules/PHPUnit/DataProviderDataRule.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 467988f..c6f8367 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -54,7 +54,12 @@ public function processNode(Node $node, Scope $scope): array $args = $this->arrayItemsToArgs($item->value); $var = new Node\Expr\New_(new Node\Name('test')); - $scope->invokeNodeCallback(new Node\Expr\MethodCall($var, 'testTrim', $args)); + $scope->invokeNodeCallback(new Node\Expr\MethodCall( + $var, + 'testTrim', + $args, + ['startLine' => $item->getStartLine()] + )); } } From 0975030f2ad215604ef23d281db5725b550fb2e3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 18:07:05 +0200 Subject: [PATCH 03/63] Added DataProviderDataRuleTest --- src/Rules/PHPUnit/DataProviderDataRule.php | 42 +++++++++++------ .../PHPUnit/DataProviderDataRuleTest.php | 47 +++++++++++++++++++ tests/Rules/PHPUnit/data-provider-data.neon | 5 ++ .../Rules/PHPUnit/data/data-provider-data.php | 34 ++++++++++++++ 4 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 tests/Rules/PHPUnit/DataProviderDataRuleTest.php create mode 100644 tests/Rules/PHPUnit/data-provider-data.neon create mode 100644 tests/Rules/PHPUnit/data/data-provider-data.php diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index c6f8367..da99851 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -6,6 +6,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPUnit\Framework\TestCase; @@ -14,6 +15,15 @@ */ class DataProviderDataRule implements Rule { + private ReflectionProvider $reflectionProvider; + + public function __construct( + ReflectionProvider $reflectionProvider, + ) + { + $this->reflectionProvider = $reflectionProvider; + } + public function getNodeType(): string { return Node\Stmt\Return_::class; @@ -40,27 +50,29 @@ public function processNode(Node $node, Scope $scope): array $classReflection = $scope->getClassReflection(); - if ($classReflection === null || !$classReflection->is(TestCase::class)) { + if ( + $classReflection === null + || !$classReflection->is(TestCase::class) + || $classReflection->isAbstract() + ) { return []; } // XXX check whether the method is used as a data provider - if (method_exists($scope, 'invokeNodeCallback')) { - foreach($node->expr->items as $item) { - if (!$item->value instanceof Node\Expr\Array_) { - return []; - } - - $args = $this->arrayItemsToArgs($item->value); - $var = new Node\Expr\New_(new Node\Name('test')); - $scope->invokeNodeCallback(new Node\Expr\MethodCall( - $var, - 'testTrim', - $args, - ['startLine' => $item->getStartLine()] - )); + foreach($node->expr->items as $item) { + if (!$item->value instanceof Node\Expr\Array_) { + return []; } + + $args = $this->arrayItemsToArgs($item->value); + $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); + $scope->invokeNodeCallback(new Node\Expr\MethodCall( + $var, + 'testTrim', + $args, + ['startLine' => $item->getStartLine()] + )); } return []; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php new file mode 100644 index 0000000..fbd228d --- /dev/null +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -0,0 +1,47 @@ + + */ +class DataProviderDataRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $reflection = $this->createReflectionProvider(); + + return new DataProviderDataRule( + $reflection + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ + [ + 'Parameter #2 $input of method test::testTrim() expects string, int given.', + 25, + ], + [ + 'Parameter #2 $input of method test::testTrim() expects string, false given.', + 29, + ], + ]); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/data-provider-data.neon', + ]; + } +} diff --git a/tests/Rules/PHPUnit/data-provider-data.neon b/tests/Rules/PHPUnit/data-provider-data.neon new file mode 100644 index 0000000..e0df8db --- /dev/null +++ b/tests/Rules/PHPUnit/data-provider-data.neon @@ -0,0 +1,5 @@ +includes: + - ../../../extension.neon + +rules: + - PHPStan\Rules\Methods\CallMethodsRule diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php new file mode 100644 index 0000000..a447022 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -0,0 +1,34 @@ + Date: Mon, 13 Oct 2025 18:54:41 +0200 Subject: [PATCH 04/63] implement CompositeRule --- tests/PHPStan/Rules/CompositeRule.php | 34 +++++++++++++++++++ .../PHPUnit/DataProviderDataRuleTest.php | 34 ++++++++++++++----- .../Rules/PHPUnit/data/data-provider-data.php | 2 +- 3 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/CompositeRule.php diff --git a/tests/PHPStan/Rules/CompositeRule.php b/tests/PHPStan/Rules/CompositeRule.php new file mode 100644 index 0000000..db80d30 --- /dev/null +++ b/tests/PHPStan/Rules/CompositeRule.php @@ -0,0 +1,34 @@ +registry = $ruleRegisty; + } + + + public function getNodeType(): string + { + return Node::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $errors = []; + + $nodeType = get_class($node); + foreach ($this->registry->getRules($nodeType) as $rule) { + foreach ($rule->processNode($node, $scope) as $error) { + $errors[] = $error; + } + } + + return $errors; + } +} diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index fbd228d..e001a59 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -2,8 +2,17 @@ namespace Rules\PHPUnit; +use PHPStan\Rules\CompositeRule; +use PHPStan\Rules\DirectRegistry; +use PHPStan\Rules\FunctionCallParametersCheck; +use PHPStan\Rules\Methods\CallMethodsRule; +use PHPStan\Rules\Methods\MethodCallCheck; +use PHPStan\Rules\NullsafeCheck; +use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; use PHPStan\Rules\PHPUnit\DataProviderDataRule; +use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; /** @@ -14,23 +23,32 @@ class DataProviderDataRuleTest extends RuleTestCase protected function getRule(): Rule { - $reflection = $this->createReflectionProvider(); + $reflectionProvider = $this->createReflectionProvider(); - return new DataProviderDataRule( - $reflection - ); + $ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true); + + + return new CompositeRule(new DirectRegistry([ + new DataProviderDataRule( + $reflectionProvider + ), + new CallMethodsRule( + new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true), + new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true), + ) + ])); } public function testRule(): void { $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ - 'Parameter #2 $input of method test::testTrim() expects string, int given.', - 25, + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testTrim() expects string, int given.', + 23, ], [ - 'Parameter #2 $input of method test::testTrim() expects string, false given.', - 29, + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testTrim() expects string, false given.', + 27, ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index a447022..25c52f2 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -13,7 +13,7 @@ public function testTrim(string $expectedResult, string $input): void { } - public function aProvider(): array /** @phpstan-ignore missingType.iterableValue */ + public function aProvider(): array { return [ [ From ff6843eb9ec98f061bdfd80bed568bb8bb0e944a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 18:56:27 +0200 Subject: [PATCH 05/63] cleanup --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 2 +- tests/Rules/PHPUnit/data-provider-data.neon | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) delete mode 100644 tests/Rules/PHPUnit/data-provider-data.neon diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index e001a59..c4826c9 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -59,7 +59,7 @@ public function testRule(): void public static function getAdditionalConfigFiles(): array { return [ - __DIR__ . '/data-provider-data.neon', + __DIR__ . '/../../../extension.neon', ]; } } diff --git a/tests/Rules/PHPUnit/data-provider-data.neon b/tests/Rules/PHPUnit/data-provider-data.neon deleted file mode 100644 index e0df8db..0000000 --- a/tests/Rules/PHPUnit/data-provider-data.neon +++ /dev/null @@ -1,5 +0,0 @@ -includes: - - ../../../extension.neon - -rules: - - PHPStan\Rules\Methods\CallMethodsRule From 1dd180c9c983a9e162f524999c20589bfcc8a508 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 19:50:27 +0200 Subject: [PATCH 06/63] Implement reflection based dataprovider detection --- extension.neon | 6 + src/Rules/PHPUnit/DataProviderDataRule.php | 23 +++- src/Rules/PHPUnit/TestMethodsHelper.php | 113 ++++++++++++++++++ .../PHPUnit/DataProviderDataRuleTest.php | 5 +- 4 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 src/Rules/PHPUnit/TestMethodsHelper.php diff --git a/extension.neon b/extension.neon index 7083122..3a56a5d 100644 --- a/extension.neon +++ b/extension.neon @@ -49,6 +49,12 @@ services: class: PHPStan\Rules\PHPUnit\CoversHelper - class: PHPStan\Rules\PHPUnit\AnnotationHelper + + - + class: PHPStan\Rules\PHPUnit\TestMethodsHelper + arguments: + parser: @defaultAnalysisParser + - class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index da99851..e5f6fa1 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -17,11 +17,15 @@ class DataProviderDataRule implements Rule { private ReflectionProvider $reflectionProvider; + private TestMethodsHelper $testMethodsHelper; + public function __construct( ReflectionProvider $reflectionProvider, + TestMethodsHelper $testMethodsHelper, ) { $this->reflectionProvider = $reflectionProvider; + $this->testMethodsHelper = $testMethodsHelper; } public function getNodeType(): string @@ -58,7 +62,22 @@ public function processNode(Node $node, Scope $scope): array return []; } - // XXX check whether the method is used as a data provider + $testsWithProvider = []; + $testMethods = $this->testMethodsHelper->getTestMethods($classReflection); + foreach($testMethods as $testMethod) + { + foreach($this->testMethodsHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [$providerMethod]) + { + if ($providerMethod === $method->getName()) { + $testsWithProvider[] = $testMethod; + continue 2; + } + } + } + + if (count($testsWithProvider) === 0) { + return []; + } foreach($node->expr->items as $item) { if (!$item->value instanceof Node\Expr\Array_) { @@ -69,7 +88,7 @@ public function processNode(Node $node, Scope $scope): array $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); $scope->invokeNodeCallback(new Node\Expr\MethodCall( $var, - 'testTrim', + $testsWithProvider[0]->getName(), $args, ['startLine' => $item->getStartLine()] )); diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php new file mode 100644 index 0000000..480286a --- /dev/null +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -0,0 +1,113 @@ +reflectionProvider = $reflectionProvider; + $this->fileTypeMapper = $fileTypeMapper; + $this->parser = $parser; + } + + /** + * @return array + */ + public function getTestMethods(ClassReflection $class): array + { + $testMethods = []; + foreach($class->getNativeReflection()->getMethods() as $reflectionMethod) { + if (str_starts_with(strtolower($reflectionMethod->getName()), 'test')) { + $testMethods[] = $reflectionMethod; + continue; + } + + // todo: detect tests with @test annotation + + $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attribute\Test'); + if ($testAttributes !== []) { + $testMethods[] = $reflectionMethod; + } + } + + return $testMethods; + } + + /** + * @return iterable + */ + public function getDataProviderMethods( + Scope $scope, + ReflectionMethod $node, + ClassReflection $classReflection + ): iterable + { + /* + $docComment = $node->getDocComment(); + if ($docComment !== null) { + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $node->name->toString(), + $docComment->getText(), + ); + foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { + $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); + if ($dataProviderValue === null) { + // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule + continue; + } + + $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); + $dataProviderMethod[] = $node->getStartLine(); + + yield $dataProviderValue => $dataProviderMethod; + } + } + + if (!$this->phpunit10OrNewer) { + return; + } + */ + + foreach ($node->getAttributes('PHPUnit\Framework\Attributes\DataProvider') as $attr) { + $args = $attr->getArguments(); + if (count($args) !== 1) { + continue; + } + + yield [$args[0]]; + } + } + +} diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index c4826c9..a127cc5 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -10,10 +10,12 @@ use PHPStan\Rules\NullsafeCheck; use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; use PHPStan\Rules\PHPUnit\DataProviderDataRule; +use PHPStan\Rules\PHPUnit\TestMethodsHelper; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; +use PHPStan\Type\FileTypeMapper; /** * @extends RuleTestCase @@ -30,7 +32,8 @@ protected function getRule(): Rule return new CompositeRule(new DirectRegistry([ new DataProviderDataRule( - $reflectionProvider + $reflectionProvider, + new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')) ), new CallMethodsRule( new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true), From cff1f79398e102b78545ada13181a54ae39b1c20 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 19:50:59 +0200 Subject: [PATCH 07/63] cs --- src/Rules/PHPUnit/DataProviderDataRule.php | 20 ++++++++-------- src/Rules/PHPUnit/TestMethodsHelper.php | 27 +++++++++------------- tests/PHPStan/Rules/CompositeRule.php | 11 ++++++--- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index e5f6fa1..ea1b433 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -2,26 +2,27 @@ namespace PHPStan\Rules\PHPUnit; -use LogicException; use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPUnit\Framework\TestCase; +use function count; /** * @implements Rule */ class DataProviderDataRule implements Rule { + private ReflectionProvider $reflectionProvider; private TestMethodsHelper $testMethodsHelper; public function __construct( ReflectionProvider $reflectionProvider, - TestMethodsHelper $testMethodsHelper, + TestMethodsHelper $testMethodsHelper ) { $this->reflectionProvider = $reflectionProvider; @@ -64,10 +65,8 @@ public function processNode(Node $node, Scope $scope): array $testsWithProvider = []; $testMethods = $this->testMethodsHelper->getTestMethods($classReflection); - foreach($testMethods as $testMethod) - { - foreach($this->testMethodsHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [$providerMethod]) - { + foreach ($testMethods as $testMethod) { + foreach ($this->testMethodsHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [$providerMethod]) { if ($providerMethod === $method->getName()) { $testsWithProvider[] = $testMethod; continue 2; @@ -79,7 +78,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - foreach($node->expr->items as $item) { + foreach ($node->expr->items as $item) { if (!$item->value instanceof Node\Expr\Array_) { return []; } @@ -90,7 +89,7 @@ public function processNode(Node $node, Scope $scope): array $var, $testsWithProvider[0]->getName(), $args, - ['startLine' => $item->getStartLine()] + ['startLine' => $item->getStartLine()], )); } @@ -100,10 +99,11 @@ public function processNode(Node $node, Scope $scope): array /** * @return array */ - private function arrayItemsToArgs(Node\Expr\Array_ $array): ?array { + private function arrayItemsToArgs(Node\Expr\Array_ $array): ?array + { $args = []; - foreach($array->items as $item) { + foreach ($array->items as $item) { // XXX named args $value = $item->value; diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index 480286a..cfb4e23 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -2,26 +2,19 @@ namespace PHPStan\Rules\PHPUnit; -use PhpParser\Node; -use PhpParser\Node\Name; use PHPStan\Analyser\Scope; -use PHPStan\Node\ClassMethod; use PHPStan\Parser\Parser; -use PHPStan\PhpDoc\ResolvedPhpDocBlock; -use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Rules\IdentifierRuleError; -use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\FileTypeMapper; use ReflectionMethod; -use function array_merge; -use function explode; -use function sprintf; -use function strpos; +use function count; +use function str_starts_with; +use function strtolower; final class TestMethodsHelper { + private ReflectionProvider $reflectionProvider; private FileTypeMapper $fileTypeMapper; @@ -45,7 +38,7 @@ public function __construct( public function getTestMethods(ClassReflection $class): array { $testMethods = []; - foreach($class->getNativeReflection()->getMethods() as $reflectionMethod) { + foreach ($class->getNativeReflection()->getMethods() as $reflectionMethod) { if (str_starts_with(strtolower($reflectionMethod->getName()), 'test')) { $testMethods[] = $reflectionMethod; continue; @@ -54,9 +47,11 @@ public function getTestMethods(ClassReflection $class): array // todo: detect tests with @test annotation $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attribute\Test'); - if ($testAttributes !== []) { - $testMethods[] = $reflectionMethod; + if ($testAttributes === []) { + continue; } + + $testMethods[] = $reflectionMethod; } return $testMethods; @@ -66,9 +61,9 @@ public function getTestMethods(ClassReflection $class): array * @return iterable */ public function getDataProviderMethods( - Scope $scope, + Scope $scope, ReflectionMethod $node, - ClassReflection $classReflection + ClassReflection $classReflection ): iterable { /* diff --git a/tests/PHPStan/Rules/CompositeRule.php b/tests/PHPStan/Rules/CompositeRule.php index db80d30..993ef93 100644 --- a/tests/PHPStan/Rules/CompositeRule.php +++ b/tests/PHPStan/Rules/CompositeRule.php @@ -1,14 +1,18 @@ -registry = $ruleRegisty; } @@ -31,4 +35,5 @@ public function processNode(Node $node, Scope $scope): array return $errors; } + } From 0ab211d54c16b8d5268c76e3549e4f8fecc837d1 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 19:52:00 +0200 Subject: [PATCH 08/63] simplify --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index a127cc5..203e942 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -35,10 +35,7 @@ protected function getRule(): Rule $reflectionProvider, new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')) ), - new CallMethodsRule( - new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true), - new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new UnresolvableTypeHelper(), new PropertyReflectionFinder(), true, true, true, true), - ) + self::getContainer()->getByType(CallMethodsRule::class) ])); } From 7f7b38b1762406486d5eec79d7804b6fb0f4644d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 13 Oct 2025 19:56:05 +0200 Subject: [PATCH 09/63] fix --- src/Rules/PHPUnit/DataProviderDataRule.php | 10 +++++----- tests/PHPStan/Rules/CompositeRule.php | 5 ++++- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 6 +----- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index ea1b433..5537a1e 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -16,16 +16,12 @@ class DataProviderDataRule implements Rule { - private ReflectionProvider $reflectionProvider; - private TestMethodsHelper $testMethodsHelper; public function __construct( - ReflectionProvider $reflectionProvider, TestMethodsHelper $testMethodsHelper ) { - $this->reflectionProvider = $reflectionProvider; $this->testMethodsHelper = $testMethodsHelper; } @@ -80,10 +76,14 @@ public function processNode(Node $node, Scope $scope): array foreach ($node->expr->items as $item) { if (!$item->value instanceof Node\Expr\Array_) { - return []; + continue; } $args = $this->arrayItemsToArgs($item->value); + if ($args === null) { + continue; + } + $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); $scope->invokeNodeCallback(new Node\Expr\MethodCall( $var, diff --git a/tests/PHPStan/Rules/CompositeRule.php b/tests/PHPStan/Rules/CompositeRule.php index 993ef93..31b8bd3 100644 --- a/tests/PHPStan/Rules/CompositeRule.php +++ b/tests/PHPStan/Rules/CompositeRule.php @@ -6,7 +6,10 @@ use PHPStan\Analyser\Scope; use function get_class; -class CompositeRule implements Rule +/** + * @implements Rule + */ +final class CompositeRule implements Rule { private DirectRegistry $registry; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 203e942..ca6c1a9 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -18,7 +18,7 @@ use PHPStan\Type\FileTypeMapper; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class DataProviderDataRuleTest extends RuleTestCase { @@ -27,12 +27,8 @@ protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); - $ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, false, true, true, false, false, true); - - return new CompositeRule(new DirectRegistry([ new DataProviderDataRule( - $reflectionProvider, new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')) ), self::getContainer()->getByType(CallMethodsRule::class) From d74aeec7e3af526808ea305a014fca1c0b6d8d80 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 06:58:18 +0200 Subject: [PATCH 10/63] Refactor --- src/Rules/PHPUnit/DataProviderDataRule.php | 11 ++-- src/Rules/PHPUnit/TestMethodsHelper.php | 50 ------------------- .../PHPUnit/DataProviderDataRuleTest.php | 4 +- 3 files changed, 10 insertions(+), 55 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 5537a1e..fbeb477 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -5,7 +5,6 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; -use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPUnit\Framework\TestCase; use function count; @@ -18,11 +17,15 @@ class DataProviderDataRule implements Rule private TestMethodsHelper $testMethodsHelper; + private DataProviderHelper $dataProviderHelper; + public function __construct( - TestMethodsHelper $testMethodsHelper + TestMethodsHelper $testMethodsHelper, + DataProviderHelper $dataProviderHelper ) { $this->testMethodsHelper = $testMethodsHelper; + $this->dataProviderHelper = $dataProviderHelper; } public function getNodeType(): string @@ -62,8 +65,8 @@ public function processNode(Node $node, Scope $scope): array $testsWithProvider = []; $testMethods = $this->testMethodsHelper->getTestMethods($classReflection); foreach ($testMethods as $testMethod) { - foreach ($this->testMethodsHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [$providerMethod]) { - if ($providerMethod === $method->getName()) { + foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [, $providerMethodName]) { + if ($providerMethodName === $method->getName()) { $testsWithProvider[] = $testMethod; continue 2; } diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index cfb4e23..dccc570 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -2,13 +2,11 @@ namespace PHPStan\Rules\PHPUnit; -use PHPStan\Analyser\Scope; use PHPStan\Parser\Parser; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; use ReflectionMethod; -use function count; use function str_starts_with; use function strtolower; @@ -57,52 +55,4 @@ public function getTestMethods(ClassReflection $class): array return $testMethods; } - /** - * @return iterable - */ - public function getDataProviderMethods( - Scope $scope, - ReflectionMethod $node, - ClassReflection $classReflection - ): iterable - { - /* - $docComment = $node->getDocComment(); - if ($docComment !== null) { - $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( - $scope->getFile(), - $classReflection->getName(), - $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, - $node->name->toString(), - $docComment->getText(), - ); - foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { - $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); - if ($dataProviderValue === null) { - // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule - continue; - } - - $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); - $dataProviderMethod[] = $node->getStartLine(); - - yield $dataProviderValue => $dataProviderMethod; - } - } - - if (!$this->phpunit10OrNewer) { - return; - } - */ - - foreach ($node->getAttributes('PHPUnit\Framework\Attributes\DataProvider') as $attr) { - $args = $attr->getArguments(); - if (count($args) !== 1) { - continue; - } - - yield [$args[0]]; - } - } - } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index ca6c1a9..9f1c25d 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -10,6 +10,7 @@ use PHPStan\Rules\NullsafeCheck; use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; use PHPStan\Rules\PHPUnit\DataProviderDataRule; +use PHPStan\Rules\PHPUnit\DataProviderHelper; use PHPStan\Rules\PHPUnit\TestMethodsHelper; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; @@ -29,7 +30,8 @@ protected function getRule(): Rule return new CompositeRule(new DirectRegistry([ new DataProviderDataRule( - new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')) + new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')), + new DataProviderHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), true), ), self::getContainer()->getByType(CallMethodsRule::class) ])); From 09b226b61b7ca75b891a9bf2ad5ba419396c8dd2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:00:10 +0200 Subject: [PATCH 11/63] test annotations --- .../PHPUnit/DataProviderDataRuleTest.php | 12 ++++++-- .../Rules/PHPUnit/data/data-provider-data.php | 29 ++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 9f1c25d..28fbe92 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -41,13 +41,21 @@ public function testRule(): void { $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ - 'Parameter #2 $input of method DataProviderDataTest\FooTest::testTrim() expects string, int given.', + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', 23, ], [ - 'Parameter #2 $input of method DataProviderDataTest\FooTest::testTrim() expects string, false given.', + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, false given.', 27, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, int given.', + 50, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', + 54, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 25c52f2..9e5d59c 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -9,7 +9,34 @@ class FooTest extends TestCase { #[DataProvider('aProvider')] - public function testTrim(string $expectedResult, string $input): void + public function testWithAttribute(string $expectedResult, string $input): void + { + } + + static public function aProvider(): array + { + return [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 123, + ], + [ + 'Hello World', + false, + ], + ]; + } +} + +class BarTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testWithAnnotation(string $expectedResult, string $input): void { } From f9c8af7b5d248d9d4a87fc7e78a461d4b6c83371 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:22:18 +0200 Subject: [PATCH 12/63] support yield --- src/Rules/PHPUnit/DataProviderDataRule.php | 32 +++++++++++++++---- .../PHPUnit/DataProviderDataRuleTest.php | 8 +++++ .../Rules/PHPUnit/data/data-provider-data.php | 30 +++++++++++++++++ 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index fbeb477..5a2edbb 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -6,6 +6,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; +use PHPStan\ShouldNotHappenException; use PHPUnit\Framework\TestCase; use function count; @@ -30,12 +31,29 @@ public function __construct( public function getNodeType(): string { - return Node\Stmt\Return_::class; + return Node::class; } public function processNode(Node $node, Scope $scope): array { - if (!$node->expr instanceof Node\Expr\Array_) { + if ($node instanceof Node\Stmt\Return_) { + if (!$node->expr instanceof Node\Expr\Array_) { + return []; + } + + $arrayExprs = []; + foreach ($node->expr->items as $item) { + if (!$item->value instanceof Node\Expr\Array_) { + return []; + } + $arrayExprs[] = $item->value; + } + } elseif ($node instanceof Node\Expr\Yield_) { + if (!$node->value instanceof Node\Expr\Array_) { + return []; + } + $arrayExprs = [$node->value]; + } else { return []; } @@ -77,12 +95,12 @@ public function processNode(Node $node, Scope $scope): array return []; } - foreach ($node->expr->items as $item) { - if (!$item->value instanceof Node\Expr\Array_) { - continue; + foreach ($arrayExprs as $arrayExpr) { + if (!$arrayExpr instanceof Node\Expr\Array_) { + throw new ShouldNotHappenException(); } - $args = $this->arrayItemsToArgs($item->value); + $args = $this->arrayItemsToArgs($arrayExpr); if ($args === null) { continue; } @@ -92,7 +110,7 @@ public function processNode(Node $node, Scope $scope): array $var, $testsWithProvider[0]->getName(), $args, - ['startLine' => $item->getStartLine()], + ['startLine' => $arrayExpr->getStartLine()], )); } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 28fbe92..8c0181d 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -56,6 +56,14 @@ public function testRule(): void 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', 54, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldingTest::testYield() expects string, int given.', + 79, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldingTest::testYield() expects string, false given.', + 85, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 9e5d59c..4d22d20 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -59,3 +59,33 @@ public function aProvider(): array } } +class YieldingTest extends TestCase +{ + + /** @dataProvider yieldProvider */ + public function testYield(string $expectedResult, string $input): void + { + } + + public function yieldProvider(): iterable + { + yield + [ + 'Hello World', + " Hello World \n", + ]; + + yield + [ + 'Hello World', + 123, + ]; + + yield + [ + 'Hello World', + false, + ]; + } +} + From d3a2b9b9e9d1639c66d6f76c4ca1ded69ba5282f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:25:42 +0200 Subject: [PATCH 13/63] support yield from --- src/Rules/PHPUnit/DataProviderDataRule.php | 2 +- .../PHPUnit/DataProviderDataRuleTest.php | 12 ++++++-- .../Rules/PHPUnit/data/data-provider-data.php | 29 ++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 5a2edbb..db9a9b1 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -36,7 +36,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if ($node instanceof Node\Stmt\Return_) { + if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { if (!$node->expr instanceof Node\Expr\Array_) { return []; } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 8c0181d..7298cc2 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -57,13 +57,21 @@ public function testRule(): void 54, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldingTest::testYield() expects string, int given.', + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::testYield() expects string, int given.', 79, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldingTest::testYield() expects string, false given.', + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::testYield() expects string, false given.', 85, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::testYieldFrom() expects string, int given.', + 107, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::testYieldFrom() expects string, false given.', + 111, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 4d22d20..0c5e483 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -59,7 +59,7 @@ public function aProvider(): array } } -class YieldingTest extends TestCase +class YieldTest extends TestCase { /** @dataProvider yieldProvider */ @@ -89,3 +89,30 @@ public function yieldProvider(): iterable } } +class YieldFromTest extends TestCase +{ + + /** @dataProvider yieldProvider */ + public function testYieldFrom(string $expectedResult, string $input): void + { + } + + public function yieldProvider(): iterable + { + yield from [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 123, + ], + [ + 'Hello World', + false, + ] + ]; + } +} + From f42bd8a1a018585baf59aebe36098e8a7babfde9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:26:51 +0200 Subject: [PATCH 14/63] fix --- src/Rules/PHPUnit/DataProviderDataRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index db9a9b1..ed18614 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -11,7 +11,7 @@ use function count; /** - * @implements Rule + * @implements Rule */ class DataProviderDataRule implements Rule { From 3318b73e165b98f60c3e866811d4ecc428096b42 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:40:37 +0200 Subject: [PATCH 15/63] Support `test` annotation and attribute --- src/Rules/PHPUnit/DataProviderDataRule.php | 2 +- src/Rules/PHPUnit/TestMethodsHelper.php | 51 +++++++++++++++++-- .../PHPUnit/DataProviderDataRuleTest.php | 24 ++++----- .../Rules/PHPUnit/data/data-provider-data.php | 13 +++-- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index ed18614..85d4a5e 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -81,7 +81,7 @@ public function processNode(Node $node, Scope $scope): array } $testsWithProvider = []; - $testMethods = $this->testMethodsHelper->getTestMethods($classReflection); + $testMethods = $this->testMethodsHelper->getTestMethods($classReflection, $scope); foreach ($testMethods as $testMethod) { foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [, $providerMethodName]) { if ($providerMethodName === $method->getName()) { diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index dccc570..08ba4ca 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -2,7 +2,9 @@ namespace PHPStan\Rules\PHPUnit; +use PHPStan\Analyser\Scope; use PHPStan\Parser\Parser; +use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; @@ -33,18 +35,43 @@ public function __construct( /** * @return array */ - public function getTestMethods(ClassReflection $class): array + public function getTestMethods(ClassReflection $classReflection, Scope $scope): array { $testMethods = []; - foreach ($class->getNativeReflection()->getMethods() as $reflectionMethod) { + foreach ($classReflection->getNativeReflection()->getMethods() as $reflectionMethod) { + if (!$reflectionMethod->isPublic()) { + continue; + } + if (str_starts_with(strtolower($reflectionMethod->getName()), 'test')) { $testMethods[] = $reflectionMethod; continue; } + $docComment = $reflectionMethod->getDocComment(); + if ($docComment !== false) { + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $reflectionMethod->getName(), + $docComment, + ); + + if ($this->hasTestAnnotation($methodPhpDoc)) { + $testMethods[] = $reflectionMethod; + continue; + } + } + // todo: detect tests with @test annotation - $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attribute\Test'); + // XXX + //if (!$this->phpunit10OrNewer) { + // return; + //} + + $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attributes\Test'); if ($testAttributes === []) { continue; } @@ -55,4 +82,22 @@ public function getTestMethods(ClassReflection $class): array return $testMethods; } + private function hasTestAnnotation(?ResolvedPhpDocBlock $phpDoc): bool + { + if ($phpDoc === null) { + return false; + } + + $phpDocNodes = $phpDoc->getPhpDocNodes(); + + foreach ($phpDocNodes as $docNode) { + $tags = $docNode->getTagsByName('@test'); + if ($tags !== []) { + return true; + } + } + + return false; + } + } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 7298cc2..8a9ff36 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -42,35 +42,35 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', - 23, + 24, ], [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, false given.', - 27, + 28, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, int given.', - 50, + 51, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', - 54, + 55, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldTest::testYield() expects string, int given.', - 79, + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, int given.', + 81, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldTest::testYield() expects string, false given.', - 85, + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, false given.', + 87, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::testYieldFrom() expects string, int given.', - 107, + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, int given.', + 112, ], [ - 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::testYieldFrom() expects string, false given.', - 111, + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, false given.', + 116, ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 0c5e483..8aa536e 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -3,6 +3,7 @@ namespace DataProviderDataTest; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; class FooTest extends TestCase @@ -62,8 +63,9 @@ public function aProvider(): array class YieldTest extends TestCase { - /** @dataProvider yieldProvider */ - public function testYield(string $expectedResult, string $input): void + #[DataProvider('yieldProvider')] + #[Test] + public function myTestMethod(string $expectedResult, string $input): void { } @@ -92,8 +94,11 @@ public function yieldProvider(): iterable class YieldFromTest extends TestCase { - /** @dataProvider yieldProvider */ - public function testYieldFrom(string $expectedResult, string $input): void + /** + * @dataProvider yieldProvider + * @test + */ + public function myTestMethod(string $expectedResult, string $input): void { } From f715fbf0506b860944a2ad3cd36754b0f0eaae96 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 07:57:01 +0200 Subject: [PATCH 16/63] extract PHPUnitVersionDetector --- extension.neon | 6 +++ .../PHPUnit/DataProviderHelperFactory.php | 8 +++- src/Rules/PHPUnit/TestMethodsHelper.php | 15 ++++--- .../PHPUnit/TestMethodsHelperFactory.php | 44 +++++++++++++++++++ .../PHPUnit/DataProviderDataRuleTest.php | 16 ++++++- .../DataProviderDeclarationRuleTest.php | 7 ++- 6 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 src/Rules/PHPUnit/TestMethodsHelperFactory.php diff --git a/extension.neon b/extension.neon index 3a56a5d..dcca805 100644 --- a/extension.neon +++ b/extension.neon @@ -50,8 +50,14 @@ services: - class: PHPStan\Rules\PHPUnit\AnnotationHelper + - + class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector + - class: PHPStan\Rules\PHPUnit\TestMethodsHelper + factory: @PHPStan\Rules\PHPUnit\TestMethodsHelperFactory::create() + - + class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory arguments: parser: @defaultAnalysisParser diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 23a5f34..d241b08 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -5,6 +5,12 @@ use PHPStan\Parser\Parser; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; +use PHPUnit\Framework\TestCase; +use function dirname; +use function explode; +use function file_get_contents; +use function is_file; +use function json_decode; class DataProviderHelperFactory { @@ -21,7 +27,7 @@ public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - PHPUnitVersionDetector $PHPUnitVersionDetector + PHPUnitVersionDetector $PHPUnitVersionDetector, ) { $this->reflectionProvider = $reflectionProvider; diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index 08ba4ca..2ae3b99 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -21,15 +21,19 @@ final class TestMethodsHelper private Parser $parser; + private bool $phpunit10OrNewer; + public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, - Parser $parser + Parser $parser, + bool $phpunit10OrNewer, ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; $this->parser = $parser; + $this->phpunit10OrNewer = $phpunit10OrNewer; } /** @@ -64,12 +68,9 @@ public function getTestMethods(ClassReflection $classReflection, Scope $scope): } } - // todo: detect tests with @test annotation - - // XXX - //if (!$this->phpunit10OrNewer) { - // return; - //} + if (!$this->phpunit10OrNewer) { + continue; + } $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attributes\Test'); if ($testAttributes === []) { diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php new file mode 100644 index 0000000..47f0c65 --- /dev/null +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -0,0 +1,44 @@ +reflectionProvider = $reflectionProvider; + $this->fileTypeMapper = $fileTypeMapper; + $this->parser = $parser; + $this->PHPUnitVersionDetector = $PHPUnitVersionDetector; + } + + public function create(): TestMethodsHelper + { + return new TestMethodsHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); + } + +} diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 8a9ff36..51e7be8 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -11,6 +11,7 @@ use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; use PHPStan\Rules\PHPUnit\DataProviderDataRule; use PHPStan\Rules\PHPUnit\DataProviderHelper; +use PHPStan\Rules\PHPUnit\PHPUnitVersionDetector; use PHPStan\Rules\PHPUnit\TestMethodsHelper; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; @@ -30,8 +31,19 @@ protected function getRule(): Rule return new CompositeRule(new DirectRegistry([ new DataProviderDataRule( - new TestMethodsHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser')), - new DataProviderHelper($reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), true), + new TestMethodsHelper( + $reflectionProvider, + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + true + ), + new DataProviderHelper( + $reflectionProvider, + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + true + ), + ), self::getContainer()->getByType(CallMethodsRule::class) ])); diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index cbe40bf..67f55ed 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -17,7 +17,12 @@ protected function getRule(): Rule $reflection = $this->createReflectionProvider(); return new DataProviderDeclarationRule( - new DataProviderHelper($reflection, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), true), + new DataProviderHelper( + $reflection, + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + true + ), true, true ); From 67a40d632c6c2b51e82622f31e55500d922ac209 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 08:00:31 +0200 Subject: [PATCH 17/63] fix php 7.4 --- src/Rules/PHPUnit/DataProviderHelperFactory.php | 2 +- src/Rules/PHPUnit/TestMethodsHelper.php | 2 +- src/Rules/PHPUnit/TestMethodsHelperFactory.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index d241b08..03855d1 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -27,7 +27,7 @@ public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - PHPUnitVersionDetector $PHPUnitVersionDetector, + PHPUnitVersionDetector $PHPUnitVersionDetector ) { $this->reflectionProvider = $reflectionProvider; diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index 2ae3b99..63c5976 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -27,7 +27,7 @@ public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - bool $phpunit10OrNewer, + bool $phpunit10OrNewer ) { $this->reflectionProvider = $reflectionProvider; diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php index 47f0c65..9164f30 100644 --- a/src/Rules/PHPUnit/TestMethodsHelperFactory.php +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -27,7 +27,7 @@ public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - PHPUnitVersionDetector $PHPUnitVersionDetector, + PHPUnitVersionDetector $PHPUnitVersionDetector ) { $this->reflectionProvider = $reflectionProvider; From 6f4dcc9890334d1e398065ec7e9c2f0b661b4c1d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 08:00:48 +0200 Subject: [PATCH 18/63] cs --- src/Rules/PHPUnit/DataProviderHelperFactory.php | 6 ------ src/Rules/PHPUnit/TestMethodsHelperFactory.php | 6 ------ 2 files changed, 12 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 03855d1..23a5f34 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -5,12 +5,6 @@ use PHPStan\Parser\Parser; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; -use PHPUnit\Framework\TestCase; -use function dirname; -use function explode; -use function file_get_contents; -use function is_file; -use function json_decode; class DataProviderHelperFactory { diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php index 9164f30..1bc5539 100644 --- a/src/Rules/PHPUnit/TestMethodsHelperFactory.php +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -5,12 +5,6 @@ use PHPStan\Parser\Parser; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; -use PHPUnit\Framework\TestCase; -use function dirname; -use function explode; -use function file_get_contents; -use function is_file; -use function json_decode; class TestMethodsHelperFactory { From 20d14088ef686695b72b78b253d168a93c79fe0a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 11:56:42 +0200 Subject: [PATCH 19/63] bleeding edge only --- rules.neon | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rules.neon b/rules.neon index 9910730..85b3064 100644 --- a/rules.neon +++ b/rules.neon @@ -8,12 +8,14 @@ rules: - PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule - - PHPStan\Rules\PHPUnit\DataProviderDataRule conditionalTags: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] + PHPStan\Rules\PHPUnit\DataProviderDataRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% + services: - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule From 5cb8c63509d89c8aa2ef9bdbcd778eab4d50176d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 12:02:38 +0200 Subject: [PATCH 20/63] test too many/few arguments --- .../PHPUnit/DataProviderDataRuleTest.php | 8 +++++ .../Rules/PHPUnit/data/data-provider-data.php | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 51e7be8..3bbdd1c 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -84,6 +84,14 @@ public function testRule(): void 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, false given.', 116, ], + [ + 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 3 parameters, 2 required.', + 141, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 1 parameter, 2 required.', + 146, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 8aa536e..f4bdf33 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -121,3 +121,32 @@ public function yieldProvider(): iterable } } +class DifferentArgumentCount extends TestCase +{ + + /** + * @dataProvider yieldProvider + */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function yieldProvider(): iterable + { + yield from [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 'abc', + 123, + ], + [ + 'Hello World', + ] + ]; + } +} + From 72d5655baf9c50f50edd2f284b63af1da6177a01 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 12:16:59 +0200 Subject: [PATCH 21/63] report too many/too few with when provider re-used --- src/Rules/PHPUnit/DataProviderDataRule.php | 45 ++++++++++++------- .../PHPUnit/DataProviderDataRuleTest.php | 8 ++++ .../Rules/PHPUnit/data/data-provider-data.php | 36 +++++++++++++++ 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 85d4a5e..a586971 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -8,7 +8,10 @@ use PHPStan\Rules\Rule; use PHPStan\ShouldNotHappenException; use PHPUnit\Framework\TestCase; +use function array_slice; use function count; +use function max; +use function min; /** * @implements Rule @@ -95,23 +98,35 @@ public function processNode(Node $node, Scope $scope): array return []; } - foreach ($arrayExprs as $arrayExpr) { - if (!$arrayExpr instanceof Node\Expr\Array_) { - throw new ShouldNotHappenException(); - } + $maxNumberOfParameters = 0; + $trimArgs = count($testsWithProvider) > 1; + foreach ($testsWithProvider as $testMethod) { + $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters()); + } - $args = $this->arrayItemsToArgs($arrayExpr); - if ($args === null) { - continue; - } + foreach ($testsWithProvider as $testMethod) { + foreach ($arrayExprs as $arrayExpr) { + if (!$arrayExpr instanceof Node\Expr\Array_) { + throw new ShouldNotHappenException(); + } - $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); - $scope->invokeNodeCallback(new Node\Expr\MethodCall( - $var, - $testsWithProvider[0]->getName(), - $args, - ['startLine' => $arrayExpr->getStartLine()], - )); + $args = $this->arrayItemsToArgs($arrayExpr); + if ($args === null) { + continue; + } + + if ($trimArgs && $maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { + $args = array_slice($args, 0, min($testMethod->getNumberOfParameters(), $maxNumberOfParameters)); + } + + $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); + $scope->invokeNodeCallback(new Node\Expr\MethodCall( + $var, + $testMethod->getName(), + $args, + ['startLine' => $arrayExpr->getStartLine()], + )); + } } return []; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 3bbdd1c..e14629f 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -92,6 +92,14 @@ public function testRule(): void 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 1 parameter, 2 required.', 146, ], + [ + 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 3 parameters, 2 required.', + 177, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 1 parameter, 2 required.', + 182, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index f4bdf33..c609583 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -150,3 +150,39 @@ public function yieldProvider(): iterable } } +class DifferentArgumentCountWithReusedDataprovider extends TestCase +{ + + /** + * @dataProvider yieldProvider + */ + public function testFoo(string $expectedResult, string $input): void + { + } + + /** + * @dataProvider yieldProvider + */ + public function testBar(string $expectedResult): void + { + } + + public function yieldProvider(): iterable + { + yield from [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 'abc', + 123, + ], + [ + 'Hello World', + ] + ]; + } +} + From 1453c2022c52316258ad005c526800be941e9422 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 12:25:47 +0200 Subject: [PATCH 22/63] cleanup --- extension.neon | 2 -- rules.neon | 3 +++ src/Rules/PHPUnit/DataProviderDataRule.php | 4 ---- src/Rules/PHPUnit/TestMethodsHelper.php | 10 +--------- src/Rules/PHPUnit/TestMethodsHelperFactory.php | 9 +-------- tests/PHPStan/Rules/CompositeRule.php | 2 ++ tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 2 -- 7 files changed, 7 insertions(+), 25 deletions(-) diff --git a/extension.neon b/extension.neon index dcca805..662a6ce 100644 --- a/extension.neon +++ b/extension.neon @@ -58,8 +58,6 @@ services: factory: @PHPStan\Rules\PHPUnit\TestMethodsHelperFactory::create() - class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory - arguments: - parser: @defaultAnalysisParser - class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector diff --git a/rules.neon b/rules.neon index 85b3064..63e10b4 100644 --- a/rules.neon +++ b/rules.neon @@ -27,3 +27,6 @@ services: - class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule + + - + class: PHPStan\Rules\PHPUnit\DataProviderDataRule diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index a586971..bdbe252 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -106,10 +106,6 @@ public function processNode(Node $node, Scope $scope): array foreach ($testsWithProvider as $testMethod) { foreach ($arrayExprs as $arrayExpr) { - if (!$arrayExpr instanceof Node\Expr\Array_) { - throw new ShouldNotHappenException(); - } - $args = $this->arrayItemsToArgs($arrayExpr); if ($args === null) { continue; diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index 63c5976..c85d9d3 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -15,24 +15,16 @@ final class TestMethodsHelper { - private ReflectionProvider $reflectionProvider; - private FileTypeMapper $fileTypeMapper; - private Parser $parser; - private bool $phpunit10OrNewer; public function __construct( - ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, - Parser $parser, bool $phpunit10OrNewer ) { - $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; - $this->parser = $parser; $this->phpunit10OrNewer = $phpunit10OrNewer; } @@ -72,7 +64,7 @@ public function getTestMethods(ClassReflection $classReflection, Scope $scope): continue; } - $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attributes\Test'); + $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attributes\Test'); // @phpstan-ignore argument.type if ($testAttributes === []) { continue; } diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php index 1bc5539..be754e9 100644 --- a/src/Rules/PHPUnit/TestMethodsHelperFactory.php +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -9,30 +9,23 @@ class TestMethodsHelperFactory { - private ReflectionProvider $reflectionProvider; private FileTypeMapper $fileTypeMapper; - private Parser $parser; - private PHPUnitVersionDetector $PHPUnitVersionDetector; public function __construct( - ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, - Parser $parser, PHPUnitVersionDetector $PHPUnitVersionDetector ) { - $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; - $this->parser = $parser; $this->PHPUnitVersionDetector = $PHPUnitVersionDetector; } public function create(): TestMethodsHelper { - return new TestMethodsHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); + return new TestMethodsHelper($this->fileTypeMapper, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); } } diff --git a/tests/PHPStan/Rules/CompositeRule.php b/tests/PHPStan/Rules/CompositeRule.php index 31b8bd3..5e8f291 100644 --- a/tests/PHPStan/Rules/CompositeRule.php +++ b/tests/PHPStan/Rules/CompositeRule.php @@ -8,6 +8,8 @@ /** * @implements Rule + * + * @api */ final class CompositeRule implements Rule { diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index e14629f..0f1cd8f 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -32,9 +32,7 @@ protected function getRule(): Rule return new CompositeRule(new DirectRegistry([ new DataProviderDataRule( new TestMethodsHelper( - $reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), - self::getContainer()->getService('defaultAnalysisParser'), true ), new DataProviderHelper( From 1506a06890703481c87768ca5ab235e09d41ee94 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 13:46:32 +0200 Subject: [PATCH 23/63] Update extension.neon --- extension.neon | 3 --- 1 file changed, 3 deletions(-) diff --git a/extension.neon b/extension.neon index 662a6ce..c6ce9bf 100644 --- a/extension.neon +++ b/extension.neon @@ -59,9 +59,6 @@ services: - class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory - - - class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector - - class: PHPStan\Rules\PHPUnit\DataProviderHelper factory: @PHPStan\Rules\PHPUnit\DataProviderHelperFactory::create() From 9b69121e4483ac37cdb7ca7624cec7b45474772b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:10:00 +0200 Subject: [PATCH 24/63] use types --- src/Rules/PHPUnit/DataProviderDataRule.php | 39 +++++++++++-------- src/Rules/PHPUnit/TestMethodsHelper.php | 2 - .../PHPUnit/TestMethodsHelperFactory.php | 3 -- .../PHPUnit/DataProviderDataRuleTest.php | 28 +++++++------ .../Rules/PHPUnit/data/data-provider-data.php | 30 ++++++++++++++ 5 files changed, 68 insertions(+), 34 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index bdbe252..301b274 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -4,9 +4,10 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\Node\Expr\TypeExpr; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; -use PHPStan\ShouldNotHappenException; +use PHPStan\Type\Constant\ConstantArrayType; use PHPUnit\Framework\TestCase; use function array_slice; use function count; @@ -40,22 +41,28 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { - if (!$node->expr instanceof Node\Expr\Array_) { + $exprType = $scope->getType($node->expr); + if (!$exprType->isConstantArray()->yes()) { return []; } - $arrayExprs = []; - foreach ($node->expr->items as $item) { - if (!$item->value instanceof Node\Expr\Array_) { - return []; + $constArrays = $exprType->getConstantArrays(); + $constantArrays = []; + foreach ($constArrays as $constArray) { + foreach ($constArray->getValueTypes() as $valueType) { + if (!$valueType->isConstantArray()->yes()) { + return []; + } + $constantArrays[] = $valueType; } - $arrayExprs[] = $item->value; } } elseif ($node instanceof Node\Expr\Yield_) { - if (!$node->value instanceof Node\Expr\Array_) { + $exprType = $scope->getType($node->value); + if (!$exprType->isConstantArray()->yes()) { return []; } - $arrayExprs = [$node->value]; + + $constantArrays = $exprType->getConstantArrays(); } else { return []; } @@ -105,8 +112,8 @@ public function processNode(Node $node, Scope $scope): array } foreach ($testsWithProvider as $testMethod) { - foreach ($arrayExprs as $arrayExpr) { - $args = $this->arrayItemsToArgs($arrayExpr); + foreach ($constantArrays as $constantArray) { + $args = $this->arrayItemsToArgs($constantArray); if ($args === null) { continue; } @@ -120,7 +127,7 @@ public function processNode(Node $node, Scope $scope): array $var, $testMethod->getName(), $args, - ['startLine' => $arrayExpr->getStartLine()], + ['startLine' => $node->getStartLine()], )); } } @@ -131,15 +138,13 @@ public function processNode(Node $node, Scope $scope): array /** * @return array */ - private function arrayItemsToArgs(Node\Expr\Array_ $array): ?array + private function arrayItemsToArgs(ConstantArrayType $array): ?array { $args = []; - foreach ($array->items as $item) { + foreach ($array->getValueTypes() as $valueType) { // XXX named args - $value = $item->value; - - $arg = new Node\Arg($value); + $arg = new Node\Arg(new TypeExpr($valueType)); $args[] = $arg; } diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index c85d9d3..d098444 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -3,10 +3,8 @@ namespace PHPStan\Rules\PHPUnit; use PHPStan\Analyser\Scope; -use PHPStan\Parser\Parser; use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\Reflection\ClassReflection; -use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; use ReflectionMethod; use function str_starts_with; diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php index be754e9..202f88c 100644 --- a/src/Rules/PHPUnit/TestMethodsHelperFactory.php +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -2,14 +2,11 @@ namespace PHPStan\Rules\PHPUnit; -use PHPStan\Parser\Parser; -use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; class TestMethodsHelperFactory { - private FileTypeMapper $fileTypeMapper; private PHPUnitVersionDetector $PHPUnitVersionDetector; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 0f1cd8f..b9868f1 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -52,51 +52,55 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', - 24, + 19, ], [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, false given.', - 28, + 19, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, int given.', - 51, + 46, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', - 55, + 46, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, int given.', - 81, + 80, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, false given.', - 87, + 86, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, int given.', - 112, + 107, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, false given.', - 116, + 107, ], [ 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 3 parameters, 2 required.', - 141, + 136, ], [ 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 1 parameter, 2 required.', - 146, + 136, ], [ 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 3 parameters, 2 required.', - 177, + 172, ], [ 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 1 parameter, 2 required.', - 182, + 172, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testWithAnnotation() expects string, int given.', + 216, ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index c609583..7ef3b8c 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -186,3 +186,33 @@ public function yieldProvider(): iterable } } + +class UnionTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testWithAnnotation(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + $arr = [ + [ + 'Hello World', + " Hello World \n" + ] + ]; + + if (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 123 + ] + ]; + } + + return $arr; + } +} From 459eb1d5b3a309cb2f0e8af05f4a96b85bdca1db Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:24:06 +0200 Subject: [PATCH 25/63] support named args --- src/Rules/PHPUnit/DataProviderDataRule.php | 24 +++++++++-- .../PHPUnit/DataProviderDataRuleTest.php | 10 ++++- .../Rules/PHPUnit/data/data-provider-data.php | 41 ++++++++++++++++++- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 301b274..f28b41c 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -142,9 +142,27 @@ private function arrayItemsToArgs(ConstantArrayType $array): ?array { $args = []; - foreach ($array->getValueTypes() as $valueType) { - // XXX named args - $arg = new Node\Arg(new TypeExpr($valueType)); + $keyTypes = $array->getKeyTypes(); + foreach ($array->getValueTypes() as $i => $valueType) { + $key = $keyTypes[$i]->getConstantStrings(); + if (count($key) > 1) { + return null; + } + + if (count($key) === 0) { + $arg = new Node\Arg(new TypeExpr($valueType)); + $args[] = $arg; + continue; + + } + + $arg = new Node\Arg( + new TypeExpr($valueType), + false, + false, + [], + new Node\Identifier($key[0]->getValue()), + ); $args[] = $arg; } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index b9868f1..895cb56 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -99,9 +99,17 @@ public function testRule(): void 172, ], [ - 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testWithAnnotation() expects string, int given.', + 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testFoo() expects string, int given.', 216, ], + [ + 'Parameter $input of method DataProviderDataTest\NamedArgsInProvider::testFoo() expects string, int given.', + 255, + ], + [ + 'Parameter $input of method DataProviderDataTest\NamedArgsInProvider::testFoo() expects string, false given.', + 255, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 7ef3b8c..62da499 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -191,7 +191,7 @@ class UnionTypeReturnTest extends TestCase { /** @dataProvider aProvider */ - public function testWithAnnotation(string $expectedResult, string $input): void + public function testFoo(string $expectedResult, string $input): void { } @@ -216,3 +216,42 @@ public function aProvider(): array return $arr; } } + + +class NamedArgsInProvider extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + $arr = [ + [ + "input" => 'Hello World', + "expectedResult" => " Hello World \n" + ] + ]; + + if (rand(0,1)) { + $arr = [ + [ + "input" => 123, + "expectedResult" => " Hello World \n" + ] + ]; + } + if (rand(0,1)) { + $arr = [ + [ + "input" => false, + "expectedResult" => " Hello World \n" + ] + ]; + } + + return $arr; + } +} From 5fcf01d855000c1e0b726c93fd834957e57865e0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:26:07 +0200 Subject: [PATCH 26/63] fix invalid call on null --- src/Rules/PHPUnit/DataProviderDataRule.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index f28b41c..8544a68 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -41,6 +41,10 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { + if ($node->expr === null) { + return []; + } + $exprType = $scope->getType($node->expr); if (!$exprType->isConstantArray()->yes()) { return []; From a978aa8ca399a5d47d401ee93466a0704c7216fa Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:27:01 +0200 Subject: [PATCH 27/63] fix invalid call on null in yield --- src/Rules/PHPUnit/DataProviderDataRule.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 8544a68..9f25b9b 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -61,6 +61,10 @@ public function processNode(Node $node, Scope $scope): array } } } elseif ($node instanceof Node\Expr\Yield_) { + if ($node->value === null) { + return []; + } + $exprType = $scope->getType($node->value); if (!$exprType->isConstantArray()->yes()) { return []; From a1dad842b39bb082fa6945fb9f5c72513acd0b58 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:32:51 +0200 Subject: [PATCH 28/63] fix type error --- src/Rules/PHPUnit/DataProviderDataRule.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 9f25b9b..6a17493 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -54,10 +54,9 @@ public function processNode(Node $node, Scope $scope): array $constantArrays = []; foreach ($constArrays as $constArray) { foreach ($constArray->getValueTypes() as $valueType) { - if (!$valueType->isConstantArray()->yes()) { - return []; + foreach($valueType->getConstantArrays() as $constValueArray) { + $constantArrays[] = $constValueArray; } - $constantArrays[] = $valueType; } } } elseif ($node instanceof Node\Expr\Yield_) { From 1d3db5449dec8ece5c548852f2e6db16e855fee4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 14:36:10 +0200 Subject: [PATCH 29/63] new TypeExpr(new ObjectType()) --- src/Rules/PHPUnit/DataProviderDataRule.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 6a17493..1d09e1a 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -8,6 +8,7 @@ use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\ObjectType; use PHPUnit\Framework\TestCase; use function array_slice; use function count; @@ -54,7 +55,7 @@ public function processNode(Node $node, Scope $scope): array $constantArrays = []; foreach ($constArrays as $constArray) { foreach ($constArray->getValueTypes() as $valueType) { - foreach($valueType->getConstantArrays() as $constValueArray) { + foreach ($valueType->getConstantArrays() as $constValueArray) { $constantArrays[] = $constValueArray; } } @@ -129,9 +130,8 @@ public function processNode(Node $node, Scope $scope): array $args = array_slice($args, 0, min($testMethod->getNumberOfParameters(), $maxNumberOfParameters)); } - $var = new Node\Expr\New_(new Node\Name($classReflection->getName())); $scope->invokeNodeCallback(new Node\Expr\MethodCall( - $var, + new TypeExpr(new ObjectType($classReflection->getName())), $testMethod->getName(), $args, ['startLine' => $node->getStartLine()], From 7417fbc2889dcd1206b031cc5caf2cef50548830 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 15:10:26 +0200 Subject: [PATCH 30/63] test constant array return from delegated method --- .../PHPUnit/DataProviderDataRuleTest.php | 8 ++++ .../Rules/PHPUnit/data/data-provider-data.php | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 895cb56..6a73b24 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -110,6 +110,14 @@ public function testRule(): void 'Parameter $input of method DataProviderDataTest\NamedArgsInProvider::testFoo() expects string, false given.', 255, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, int given.', + 275, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, true given.', + 277, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 62da499..a6a6a50 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -255,3 +255,41 @@ public function aProvider(): array return $arr; } } + + +class YieldFromExpr extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): iterable + { + yield [ + 'Hello World', + " Hello World \n", + ]; + + yield from $this->moreData(); + + yield [ + 'Hello World', + true, + ]; + } + + /** + * @return array{array{'Hello World', 123}} + */ + private function moreData(): array + { + return [ + [ + 'Hello World', + 123, + ] + ]; + } +} From 74b67cf3ce252a08a80a7ea2c70a21a28b96871a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 16:04:33 +0200 Subject: [PATCH 31/63] use CompositeRule from phpstan-src --- tests/PHPStan/Rules/CompositeRule.php | 44 ------------------- .../PHPUnit/DataProviderDataRuleTest.php | 14 ++---- 2 files changed, 3 insertions(+), 55 deletions(-) delete mode 100644 tests/PHPStan/Rules/CompositeRule.php diff --git a/tests/PHPStan/Rules/CompositeRule.php b/tests/PHPStan/Rules/CompositeRule.php deleted file mode 100644 index 5e8f291..0000000 --- a/tests/PHPStan/Rules/CompositeRule.php +++ /dev/null @@ -1,44 +0,0 @@ - - * - * @api - */ -final class CompositeRule implements Rule -{ - - private DirectRegistry $registry; - - public function __construct(DirectRegistry $ruleRegisty) - { - $this->registry = $ruleRegisty; - } - - - public function getNodeType(): string - { - return Node::class; - } - - public function processNode(Node $node, Scope $scope): array - { - $errors = []; - - $nodeType = get_class($node); - foreach ($this->registry->getRules($nodeType) as $rule) { - foreach ($rule->processNode($node, $scope) as $error) { - $errors[] = $error; - } - } - - return $errors; - } - -} diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 6a73b24..8ce5243 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -2,20 +2,12 @@ namespace Rules\PHPUnit; -use PHPStan\Rules\CompositeRule; -use PHPStan\Rules\DirectRegistry; -use PHPStan\Rules\FunctionCallParametersCheck; +use PHPStan\Testing\CompositeRule; use PHPStan\Rules\Methods\CallMethodsRule; -use PHPStan\Rules\Methods\MethodCallCheck; -use PHPStan\Rules\NullsafeCheck; -use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; use PHPStan\Rules\PHPUnit\DataProviderDataRule; use PHPStan\Rules\PHPUnit\DataProviderHelper; -use PHPStan\Rules\PHPUnit\PHPUnitVersionDetector; use PHPStan\Rules\PHPUnit\TestMethodsHelper; -use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; -use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\FileTypeMapper; @@ -29,7 +21,7 @@ protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); - return new CompositeRule(new DirectRegistry([ + return new CompositeRule([ new DataProviderDataRule( new TestMethodsHelper( self::getContainer()->getByType(FileTypeMapper::class), @@ -44,7 +36,7 @@ protected function getRule(): Rule ), self::getContainer()->getByType(CallMethodsRule::class) - ])); + ]); } public function testRule(): void From 5cb7312501ac336cccec25bedf40e3448d2d3106 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 16:41:33 +0200 Subject: [PATCH 32/63] fix PHP 7.4 prevents "Named arguments are supported only on PHP 8.0 and later." --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 8ce5243..01c2815 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -41,6 +41,10 @@ protected function getRule(): Rule public function testRule(): void { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped(); + } + $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', From 9ee26b107721de3657a706e7c6733e8ed303c9ae Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 17:02:55 +0200 Subject: [PATCH 33/63] Update data-provider-data.php --- .../Rules/PHPUnit/data/data-provider-data.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index a6a6a50..0e54037 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -293,3 +293,25 @@ private function moreData(): array ]; } } + +class TestValidVariadic extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(string $s): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, string ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", "foo", "bar"], + ["hi", "ho"], + ["nope"] + ]; + } +} From 4cb94049e4e86eaf9d9940a2a51a199225a57913 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 17:05:27 +0200 Subject: [PATCH 34/63] test more variadics --- .../PHPUnit/DataProviderDataRuleTest.php | 8 +++++++ .../Rules/PHPUnit/data/data-provider-data.php | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 01c2815..1cc838d 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -114,6 +114,14 @@ public function testRule(): void 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, true given.', 277, ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestInValidVariadic::testBar() expects int, string given.', + 333, + ], + [ + 'Parameter #1 $s of method DataProviderDataTest\TestInValidVariadic::testFoo() expects string, int given.', + 333, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 0e54037..0254342 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -315,3 +315,24 @@ public function aProvider(): iterable ]; } } + +class TestInValidVariadic extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, string ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", "foo", "bar"], + [123] + ]; + } +} From 34a0a0e624e944954d821864152f391f79bd1b5a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 17:07:52 +0200 Subject: [PATCH 35/63] more variadic tests --- .../PHPUnit/DataProviderDataRuleTest.php | 20 ++++++++++++++-- .../Rules/PHPUnit/data/data-provider-data.php | 24 ++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 1cc838d..1ca0acc 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -115,13 +115,29 @@ public function testRule(): void 277, ], [ - 'Parameter #1 $si of method DataProviderDataTest\TestInValidVariadic::testBar() expects int, string given.', + 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic::testBar() expects int, string given.', 333, ], [ - 'Parameter #1 $s of method DataProviderDataTest\TestInValidVariadic::testFoo() expects string, int given.', + 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic::testFoo() expects string, int given.', 333, ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic2::testBar() expects int, string given.', + 355, + ], + [ + 'Parameter #2 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', + 355, + ], + [ + 'Parameter #4 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', + 355, + ], + [ + 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', + 355, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 0254342..d4f723f 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -316,7 +316,7 @@ public function aProvider(): iterable } } -class TestInValidVariadic extends TestCase +class TestInvalidVariadic extends TestCase { /** @dataProvider aProvider */ public function testBar(int $si): void @@ -336,3 +336,25 @@ public function aProvider(): iterable ]; } } + + +class TestInvalidVariadic2 extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, int ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", 5, "bar"], + [123] + ]; + } +} From bb855f0ea899451fec2128e61e5b0de3d58c89b6 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 18:32:34 +0200 Subject: [PATCH 36/63] fix --- src/Rules/PHPUnit/DataProviderDataRule.php | 4 +--- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 1d09e1a..e4ed7da 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -5,7 +5,6 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Node\Expr\TypeExpr; -use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; use PHPStan\Rules\Rule; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\ObjectType; @@ -84,12 +83,11 @@ public function processNode(Node $node, Scope $scope): array } $method = $scope->getFunction(); - if (!$method instanceof PhpMethodFromParserNodeReflection) { + if ($method === null) { return []; } $classReflection = $scope->getClassReflection(); - if ( $classReflection === null || !$classReflection->is(TestCase::class) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 1ca0acc..145f2dc 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -1,6 +1,6 @@ Date: Tue, 14 Oct 2025 18:34:18 +0200 Subject: [PATCH 37/63] Update DataProviderDataRuleTest.php --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 145f2dc..5a59016 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -35,7 +35,7 @@ protected function getRule(): Rule ), ), - self::getContainer()->getByType(CallMethodsRule::class) + self::getContainer()->getByType(CallMethodsRule::class) /** @phpstan-ignore phpstanApi.classConstant */ ]); } From e31a234cf943543f8082f9b5eeaefa848aeb5135 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 18:35:42 +0200 Subject: [PATCH 38/63] Update DataProviderDataRule.php --- src/Rules/PHPUnit/DataProviderDataRule.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index e4ed7da..9c4fcb9 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -83,10 +83,6 @@ public function processNode(Node $node, Scope $scope): array } $method = $scope->getFunction(); - if ($method === null) { - return []; - } - $classReflection = $scope->getClassReflection(); if ( $classReflection === null From 1532c26effc6c5b98b408a6f9ca46c2e470f7e27 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 19:04:45 +0200 Subject: [PATCH 39/63] support iterables --- src/Rules/PHPUnit/DataProviderDataRule.php | 33 ++++++++++--------- .../PHPUnit/DataProviderDataRuleTest.php | 8 +++++ .../Rules/PHPUnit/data/data-provider-data.php | 20 +++++++++++ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 9c4fcb9..e39c928 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -40,24 +40,31 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { + if ($scope->getFunction() === null) { + return []; + } + if ($scope->isInAnonymousFunction()) { + return []; + } + if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { if ($node->expr === null) { return []; } - $exprType = $scope->getType($node->expr); - if (!$exprType->isConstantArray()->yes()) { - return []; - } - - $constArrays = $exprType->getConstantArrays(); $constantArrays = []; - foreach ($constArrays as $constArray) { - foreach ($constArray->getValueTypes() as $valueType) { - foreach ($valueType->getConstantArrays() as $constValueArray) { - $constantArrays[] = $constValueArray; + $exprType = $scope->getType($node->expr); + $exprConstArrays = $exprType->getConstantArrays(); + if ($exprConstArrays !== []) { + foreach ($exprConstArrays as $constArray) { + foreach ($constArray->getValueTypes() as $valueType) { + foreach ($valueType->getConstantArrays() as $constValueArray) { + $constantArrays[] = $constValueArray; + } } } + } else { + $constantArrays = $exprType->getIterableValueType()->getConstantArrays(); } } elseif ($node instanceof Node\Expr\Yield_) { if ($node->value === null) { @@ -74,11 +81,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($scope->getFunction() === null) { - return []; - } - - if ($scope->isInAnonymousFunction()) { + if ($constantArrays === []) { return []; } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 5a59016..8570184 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -138,6 +138,14 @@ public function testRule(): void 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', 355, ], + [ + 'Unknown parameter $foo in call to method DataProviderDataTest\TestIterable::testBar().', + 371, + ], + [ + 'Missing parameter $si (int) in call to method DataProviderDataTest\TestIterable::testBar().', + 371, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index d4f723f..c1cd6f5 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -358,3 +358,23 @@ public function aProvider(): iterable ]; } } + +class TestIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} From 68e65e983d06070d128737911fde4669f64eb058 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 19:09:55 +0200 Subject: [PATCH 40/63] test ArrayIterator --- .../PHPUnit/DataProviderDataRuleTest.php | 12 ++++++++ .../Rules/PHPUnit/data/data-provider-data.php | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 8570184..0822173 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -146,6 +146,18 @@ public function testRule(): void 'Missing parameter $si (int) in call to method DataProviderDataTest\TestIterable::testBar().', 371, ], + [ + 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testBar() expects int, int|string given.', + 401, + ], + [ + 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testFoo() expects int, int|string given.', + 401, + ], + [ + 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', + 401, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index c1cd6f5..30c6ca9 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -378,3 +378,31 @@ public function data(): iterable { } } + +class TestArrayIterator extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $i): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(int $i, string $si): void + { + } + + /** @dataProvider aProvider */ + public function testFooBar(string $s1, string $s2): void + { + } + + public function aProvider(): iterable + { + return new \ArrayIterator([ + [1], + [2, "hello"], + ["no"], + ["no", "yes"], + ]); + } +} From 30e27db0624ad78d35dd5548dc2eb96ce5b1bc9d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 15 Oct 2025 07:09:39 +0200 Subject: [PATCH 41/63] cleanup --- src/Rules/PHPUnit/DataProviderDataRule.php | 14 ++++++------ .../PHPUnit/DataProviderDataRuleTest.php | 10 ++++----- .../Rules/PHPUnit/data/data-provider-data.php | 22 ++++++++++++++++++- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index e39c928..dd4b3f0 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -55,15 +55,15 @@ public function processNode(Node $node, Scope $scope): array $constantArrays = []; $exprType = $scope->getType($node->expr); $exprConstArrays = $exprType->getConstantArrays(); - if ($exprConstArrays !== []) { - foreach ($exprConstArrays as $constArray) { - foreach ($constArray->getValueTypes() as $valueType) { - foreach ($valueType->getConstantArrays() as $constValueArray) { - $constantArrays[] = $constValueArray; - } + foreach ($exprConstArrays as $constArray) { + foreach ($constArray->getValueTypes() as $valueType) { + foreach ($valueType->getConstantArrays() as $constValueArray) { + $constantArrays[] = $constValueArray; } } - } else { + } + + if ($constantArrays === []) { $constantArrays = $exprType->getIterableValueType()->getConstantArrays(); } } elseif ($node instanceof Node\Expr\Yield_) { diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 0822173..50211d1 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -139,24 +139,24 @@ public function testRule(): void 355, ], [ - 'Unknown parameter $foo in call to method DataProviderDataTest\TestIterable::testBar().', + 'Unknown parameter $foo in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', 371, ], [ - 'Missing parameter $si (int) in call to method DataProviderDataTest\TestIterable::testBar().', + 'Missing parameter $si (int) in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', 371, ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testBar() expects int, int|string given.', - 401, + 421, ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testFoo() expects int, int|string given.', - 401, + 421, ], [ 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', - 401, + 421, ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 30c6ca9..1c827e2 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -359,7 +359,7 @@ public function aProvider(): iterable } } -class TestIterable extends TestCase +class TestArrayShapeIterable extends TestCase { /** @dataProvider aProvider */ public function testBar(int $si): void @@ -379,6 +379,26 @@ public function data(): iterable } } +class TestTypedIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable> + */ + public function data(): iterable + { + } +} + class TestArrayIterator extends TestCase { /** @dataProvider aProvider */ From aafaf8f71f5a410a396b6b068eda3d69df70cb73 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 15 Oct 2025 07:14:08 +0200 Subject: [PATCH 42/63] simplify --- src/Rules/PHPUnit/DataProviderDataRule.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index dd4b3f0..6b7fe88 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -47,12 +47,12 @@ public function processNode(Node $node, Scope $scope): array return []; } + $constantArrays = []; if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { if ($node->expr === null) { return []; } - $constantArrays = []; $exprType = $scope->getType($node->expr); $exprConstArrays = $exprType->getConstantArrays(); foreach ($exprConstArrays as $constArray) { @@ -72,13 +72,7 @@ public function processNode(Node $node, Scope $scope): array } $exprType = $scope->getType($node->value); - if (!$exprType->isConstantArray()->yes()) { - return []; - } - $constantArrays = $exprType->getConstantArrays(); - } else { - return []; } if ($constantArrays === []) { From b6f11fea79192a6cc0f0932f4f7f8f30b8929c46 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 15 Oct 2025 07:24:10 +0200 Subject: [PATCH 43/63] Report wrongly typed generic arrays --- src/Rules/PHPUnit/DataProviderDataRule.php | 48 +++++++++++++------ .../PHPUnit/DataProviderDataRuleTest.php | 4 ++ .../Rules/PHPUnit/data/data-provider-data.php | 20 ++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 6b7fe88..609c7bf 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -6,8 +6,8 @@ use PHPStan\Analyser\Scope; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Rules\Rule; -use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; use PHPUnit\Framework\TestCase; use function array_slice; use function count; @@ -47,7 +47,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - $constantArrays = []; + $arraysTypes = []; if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { if ($node->expr === null) { return []; @@ -58,13 +58,17 @@ public function processNode(Node $node, Scope $scope): array foreach ($exprConstArrays as $constArray) { foreach ($constArray->getValueTypes() as $valueType) { foreach ($valueType->getConstantArrays() as $constValueArray) { - $constantArrays[] = $constValueArray; + $arraysTypes[] = $constValueArray; } } } - if ($constantArrays === []) { - $constantArrays = $exprType->getIterableValueType()->getConstantArrays(); + if ($arraysTypes === []) { + $arraysTypes = $exprType->getIterableValueType()->getConstantArrays(); + } + + if ($arraysTypes === []) { + $arraysTypes = $exprType->getIterableValueType()->getArrays(); } } elseif ($node instanceof Node\Expr\Yield_) { if ($node->value === null) { @@ -72,10 +76,10 @@ public function processNode(Node $node, Scope $scope): array } $exprType = $scope->getType($node->value); - $constantArrays = $exprType->getConstantArrays(); + $arraysTypes = $exprType->getConstantArrays(); } - if ($constantArrays === []) { + if ($arraysTypes === []) { return []; } @@ -111,14 +115,16 @@ public function processNode(Node $node, Scope $scope): array } foreach ($testsWithProvider as $testMethod) { - foreach ($constantArrays as $constantArray) { - $args = $this->arrayItemsToArgs($constantArray); + $numberOfParameters = $testMethod->getNumberOfParameters(); + + foreach ($arraysTypes as $arraysType) { + $args = $this->arrayItemsToArgs($arraysType, $numberOfParameters); if ($args === null) { continue; } - if ($trimArgs && $maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { - $args = array_slice($args, 0, min($testMethod->getNumberOfParameters(), $maxNumberOfParameters)); + if ($trimArgs && $maxNumberOfParameters !== $numberOfParameters) { + $args = array_slice($args, 0, min($numberOfParameters, $maxNumberOfParameters)); } $scope->invokeNodeCallback(new Node\Expr\MethodCall( @@ -136,12 +142,26 @@ public function processNode(Node $node, Scope $scope): array /** * @return array */ - private function arrayItemsToArgs(ConstantArrayType $array): ?array + private function arrayItemsToArgs(Type $array, int $numberOfParameters): ?array { $args = []; - $keyTypes = $array->getKeyTypes(); - foreach ($array->getValueTypes() as $i => $valueType) { + $constArrays = $array->getConstantArrays(); + if ($constArrays !== [] && count($constArrays) === 1) { + $keyTypes = $constArrays[0]->getKeyTypes(); + $valueTypes = $constArrays[0]->getValueTypes(); + } elseif ($array->isArray()->yes()) { + $keyTypes = []; + $valueTypes = []; + for ($i = 0; $i < $numberOfParameters; ++$i) { + $keyTypes[$i] = $array->getIterableKeyType(); + $valueTypes[$i] = $array->getIterableValueType(); + } + } else { + return null; + } + + foreach ($valueTypes as $i => $valueType) { $key = $keyTypes[$i]->getConstantStrings(); if (count($key) > 1) { return null; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 50211d1..71697cd 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -158,6 +158,10 @@ public function testRule(): void 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', 421, ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestWrongTypedIterable::testBar() expects int, string given.', + 439, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 1c827e2..a0cd73e 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -426,3 +426,23 @@ public function aProvider(): iterable ]); } } + +class TestWrongTypedIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable> + */ + public function data(): iterable + { + } +} From 72dc9e04ae3649c73d1cf86dff1ed6c6a69b32ce Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 15 Oct 2025 07:25:17 +0200 Subject: [PATCH 44/63] simplify --- src/Rules/PHPUnit/DataProviderDataRule.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 609c7bf..d730fb9 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -63,10 +63,6 @@ public function processNode(Node $node, Scope $scope): array } } - if ($arraysTypes === []) { - $arraysTypes = $exprType->getIterableValueType()->getConstantArrays(); - } - if ($arraysTypes === []) { $arraysTypes = $exprType->getIterableValueType()->getArrays(); } From ccb7b04ed80935bdf21c67f55c7f1370154cdf20 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 16 Oct 2025 11:42:49 +0200 Subject: [PATCH 45/63] Fix SA error --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 71697cd..e6bd1aa 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\PHPUnit; +use PhpParser\Node; use PHPStan\Testing\CompositeRule; use PHPStan\Rules\Methods\CallMethodsRule; use PHPStan\Rules\PHPUnit\DataProviderDataRule; @@ -21,7 +22,8 @@ protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); - return new CompositeRule([ + /** @var list> $rules */ + $rules = [ new DataProviderDataRule( new TestMethodsHelper( self::getContainer()->getByType(FileTypeMapper::class), @@ -36,7 +38,9 @@ protected function getRule(): Rule ), self::getContainer()->getByType(CallMethodsRule::class) /** @phpstan-ignore phpstanApi.classConstant */ - ]); + ]; + + return new CompositeRule($rules); } public function testRule(): void From 8491dd7675482d62f994660d161e2dc28cb078a2 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 16 Oct 2025 11:43:05 +0200 Subject: [PATCH 46/63] Fix CS --- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index e6bd1aa..120b0a5 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -5,9 +5,6 @@ use PhpParser\Node; use PHPStan\Testing\CompositeRule; use PHPStan\Rules\Methods\CallMethodsRule; -use PHPStan\Rules\PHPUnit\DataProviderDataRule; -use PHPStan\Rules\PHPUnit\DataProviderHelper; -use PHPStan\Rules\PHPUnit\TestMethodsHelper; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\FileTypeMapper; From aea7e4217222a490fb8a796b9ec85ecea3489616 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 07:14:21 +0200 Subject: [PATCH 47/63] abstract test-class --- src/Rules/PHPUnit/DataProviderDataRule.php | 1 - .../PHPUnit/DataProviderDataRuleTest.php | 8 ++++++ .../Rules/PHPUnit/data/data-provider-data.php | 28 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index d730fb9..1c87adc 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -84,7 +84,6 @@ public function processNode(Node $node, Scope $scope): array if ( $classReflection === null || !$classReflection->is(TestCase::class) - || $classReflection->isAbstract() ) { return []; } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 120b0a5..2132b8a 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -163,6 +163,14 @@ public function testRule(): void 'Parameter #1 $si of method DataProviderDataTest\TestWrongTypedIterable::testBar() expects int, string given.', 439, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, int given.', + 461, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', + 461, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index a0cd73e..3435a25 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -446,3 +446,31 @@ public function data(): iterable { } } + + +abstract class AbstractBaseTest extends TestCase +{ + + #[DataProvider('aProvider')] + public function testWithAttribute(string $expectedResult, string $input): void + { + } + + static public function aProvider(): array + { + return [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 123, + ], + [ + 'Hello World', + false, + ], + ]; + } +} From a1e5a19a0de8e76f90feabb3a261765134afed4c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 07:20:36 +0200 Subject: [PATCH 48/63] factor out buildArrayTypesFromNode() --- src/Rules/PHPUnit/DataProviderDataRule.php | 70 ++++++++++++++-------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 1c87adc..d29ad0d 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -47,34 +47,15 @@ public function processNode(Node $node, Scope $scope): array return []; } - $arraysTypes = []; - if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { - if ($node->expr === null) { - return []; - } - - $exprType = $scope->getType($node->expr); - $exprConstArrays = $exprType->getConstantArrays(); - foreach ($exprConstArrays as $constArray) { - foreach ($constArray->getValueTypes() as $valueType) { - foreach ($valueType->getConstantArrays() as $constValueArray) { - $arraysTypes[] = $constValueArray; - } - } - } - - if ($arraysTypes === []) { - $arraysTypes = $exprType->getIterableValueType()->getArrays(); - } - } elseif ($node instanceof Node\Expr\Yield_) { - if ($node->value === null) { - return []; - } - - $exprType = $scope->getType($node->value); - $arraysTypes = $exprType->getConstantArrays(); + if ( + !$node instanceof Node\Stmt\Return_ + && !$node instanceof Node\Expr\Yield_ + && !$node instanceof Node\Expr\YieldFrom + ) { + return []; } + $arraysTypes = $this->buildArrayTypesFromNode($node, $scope); if ($arraysTypes === []) { return []; } @@ -182,4 +163,41 @@ private function arrayItemsToArgs(Type $array, int $numberOfParameters): ?array return $args; } + /** + * @param Node\Stmt\Return_|Node\Expr\Yield_|Node\Expr\YieldFrom $node + * @return array + */ + private function buildArrayTypesFromNode(Node $node, Scope $scope): array + { + $arraysTypes = []; + if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { + if ($node->expr === null) { + return []; + } + + $exprType = $scope->getType($node->expr); + $exprConstArrays = $exprType->getConstantArrays(); + foreach ($exprConstArrays as $constArray) { + foreach ($constArray->getValueTypes() as $valueType) { + foreach ($valueType->getConstantArrays() as $constValueArray) { + $arraysTypes[] = $constValueArray; + } + } + } + + if ($arraysTypes === []) { + $arraysTypes = $exprType->getIterableValueType()->getArrays(); + } + } elseif ($node instanceof Node\Expr\Yield_) { + if ($node->value === null) { + return []; + } + + $exprType = $scope->getType($node->value); + $arraysTypes = $exprType->getConstantArrays(); + } + + return $arraysTypes; + } + } From 7f9dbf2987e5ee417f5952b1b1f19aa318f3d528 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 07:52:02 +0200 Subject: [PATCH 49/63] test more const array variants --- .../PHPUnit/DataProviderDataRuleTest.php | 16 +++ .../Rules/PHPUnit/data/data-provider-data.php | 102 ++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 2132b8a..0dfde48 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -171,6 +171,22 @@ public function testRule(): void 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', 461, ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionTypeReturnTest::testFoo() expects string, int given.', + 505, + ], + [ + 'Method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() invoked with 3 parameters, 2 required.', + 543, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() expects string, int given.', + 543, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionWithDifferentValueTypeReturnTest::testFoo() expects string, int|string given.', + 576, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 3435a25..4574287 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -474,3 +474,105 @@ static public function aProvider(): array ]; } } + + +class ConstantArrayUnionTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 123 + ] + ]; + } else { + $arr = [ + [ + 'Hello World', + " Hello World \n" + ] + ]; + } + + return $arr; + } +} + +class ConstantArrayDifferentLengthUnionTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 123 + ] + ]; + } elseif (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 'Hello World', + ] + ]; + } else { + $arr = [ + [ + 'Hello World', + " Hello World \n", + " Too much \n", + ] + ]; + } + + return $arr; + } +} + +class ConstantArrayUnionWithDifferentValueTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hellooo', + ' World', + ] + ]; + } else { + $a = rand(0,1) ? 'Hello' : 'World'; + $b = rand(0,1) ? " Hello World \n" : 123; + + $arr = [ + [ + $a, + $b + ] + ]; + } + + return $arr; + } +} From c689efec4e091b1d82689195afa82e83355f9529 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 08:10:33 +0200 Subject: [PATCH 50/63] special case for providers only containing static data, so we get more precise error lines --- src/Rules/PHPUnit/DataProviderDataRule.php | 43 ++++++++++++++++--- .../PHPUnit/DataProviderDataRuleTest.php | 24 +++++------ 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index d29ad0d..8c6e6f3 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -93,7 +93,7 @@ public function processNode(Node $node, Scope $scope): array foreach ($testsWithProvider as $testMethod) { $numberOfParameters = $testMethod->getNumberOfParameters(); - foreach ($arraysTypes as $arraysType) { + foreach ($arraysTypes as [$startLine, $arraysType]) { $args = $this->arrayItemsToArgs($arraysType, $numberOfParameters); if ($args === null) { continue; @@ -107,7 +107,7 @@ public function processNode(Node $node, Scope $scope): array new TypeExpr(new ObjectType($classReflection->getName())), $testMethod->getName(), $args, - ['startLine' => $node->getStartLine()], + ['startLine' => $startLine], )); } } @@ -165,11 +165,38 @@ private function arrayItemsToArgs(Type $array, int $numberOfParameters): ?array /** * @param Node\Stmt\Return_|Node\Expr\Yield_|Node\Expr\YieldFrom $node - * @return array + * + * @return list */ private function buildArrayTypesFromNode(Node $node, Scope $scope): array { $arraysTypes = []; + + // special case for providers only containing static data, so we get more precise error lines + if ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) { + foreach ($node->expr->items as $item) { + if (!$item->value instanceof Node\Expr\Array_) { + $arraysTypes = []; + break; + } + + $constArrays = $scope->getType($item->value)->getConstantArrays(); + if ($constArrays === []) { + $arraysTypes = []; + break; + } + + foreach ($constArrays as $constArray) { + $arraysTypes[] = [$item->value->getStartLine(), $constArray]; + } + } + + if ($arraysTypes !== []) { + return $arraysTypes; + } + } + + // general case with less precise error message lines if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { if ($node->expr === null) { return []; @@ -180,13 +207,15 @@ private function buildArrayTypesFromNode(Node $node, Scope $scope): array foreach ($exprConstArrays as $constArray) { foreach ($constArray->getValueTypes() as $valueType) { foreach ($valueType->getConstantArrays() as $constValueArray) { - $arraysTypes[] = $constValueArray; + $arraysTypes[] = [$node->getStartLine(), $constValueArray]; } } } if ($arraysTypes === []) { - $arraysTypes = $exprType->getIterableValueType()->getArrays(); + foreach ($exprType->getIterableValueType()->getArrays() as $arrayType) { + $arraysTypes[] = [$node->getStartLine(), $arrayType]; + } } } elseif ($node instanceof Node\Expr\Yield_) { if ($node->value === null) { @@ -194,7 +223,9 @@ private function buildArrayTypesFromNode(Node $node, Scope $scope): array } $exprType = $scope->getType($node->value); - $arraysTypes = $exprType->getConstantArrays(); + foreach ($exprType->getConstantArrays() as $constValueArray) { + $arraysTypes[] = [$node->getStartLine(), $constValueArray]; + } } return $arraysTypes; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 0dfde48..8169305 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -49,19 +49,19 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', - 19, + 24, ], [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, false given.', - 19, + 28, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, int given.', - 46, + 51, ], [ 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', - 46, + 55, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, int given.', @@ -117,27 +117,27 @@ public function testRule(): void ], [ 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic::testBar() expects int, string given.', - 333, + 334, ], [ 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic::testFoo() expects string, int given.', - 333, + 335, ], [ 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic2::testBar() expects int, string given.', - 355, + 356, ], [ 'Parameter #2 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', - 355, + 356, ], [ 'Parameter #4 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', - 355, + 356, ], [ 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', - 355, + 357, ], [ 'Unknown parameter $foo in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', @@ -165,11 +165,11 @@ public function testRule(): void ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, int given.', - 461, + 466, ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', - 461, + 470, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionTypeReturnTest::testFoo() expects string, int given.', From d6c11be3f57350d0238d302e0f9acbf3b685952f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 08:14:34 +0200 Subject: [PATCH 51/63] apply static data only special case also for "yield from" --- src/Rules/PHPUnit/DataProviderDataRule.php | 5 ++++- tests/Rules/PHPUnit/DataProviderDataRuleTest.php | 12 ++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 8c6e6f3..236229e 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -173,7 +173,10 @@ private function buildArrayTypesFromNode(Node $node, Scope $scope): array $arraysTypes = []; // special case for providers only containing static data, so we get more precise error lines - if ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) { + if ( + ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) + || ($node instanceof Node\Expr\YieldFrom && $node->expr instanceof Node\Expr\Array_) + ) { foreach ($node->expr->items as $item) { if (!$item->value instanceof Node\Expr\Array_) { $arraysTypes = []; diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 8169305..80bf0d5 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -73,27 +73,27 @@ public function testRule(): void ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, int given.', - 107, + 112, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, false given.', - 107, + 116, ], [ 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 3 parameters, 2 required.', - 136, + 141, ], [ 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 1 parameter, 2 required.', - 136, + 146, ], [ 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 3 parameters, 2 required.', - 172, + 177, ], [ 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 1 parameter, 2 required.', - 172, + 182, ], [ 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testFoo() expects string, int given.', From 21740cb141910643da35c30812a7a0abb329c21e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 08:27:27 +0200 Subject: [PATCH 52/63] moved cheapest check to the front --- src/Rules/PHPUnit/DataProviderDataRule.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 236229e..92608fa 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -40,13 +40,6 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if ($scope->getFunction() === null) { - return []; - } - if ($scope->isInAnonymousFunction()) { - return []; - } - if ( !$node instanceof Node\Stmt\Return_ && !$node instanceof Node\Expr\Yield_ @@ -55,6 +48,13 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($scope->getFunction() === null) { + return []; + } + if ($scope->isInAnonymousFunction()) { + return []; + } + $arraysTypes = $this->buildArrayTypesFromNode($node, $scope); if ($arraysTypes === []) { return []; From 41acc04610be3a48373dde74c18c29d985e66528 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 17 Oct 2025 10:42:03 +0200 Subject: [PATCH 53/63] Failing test - args trimming logic needs some work --- .../PHPUnit/DataProviderDataRuleTest.php | 38 ++++++++++++ .../data/data-provider-variadic-method.php | 61 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 tests/Rules/PHPUnit/data/data-provider-variadic-method.php diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 80bf0d5..34d65e4 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -190,6 +190,44 @@ public function testRule(): void ]); } + public function testVariadicMethod(): void + { + $this->analyse([__DIR__ . '/data/data-provider-variadic-method.php'], [ + [ + 'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.', + 12, + ], + [ + 'Parameter #1 $a of method DataProviderVariadicMethod\FooTest::testProvide() expects int, string given.', + 13, + ], + [ + 'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.', + 13, + ], + [ + 'Parameter #1 $a of method DataProviderVariadicMethod\FooTest::testProvide2() expects int, string given.', + 13, + ], + [ + 'Parameter #2 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide() expects string, int given.', + 15, + ], + [ + 'Parameter #3 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide() expects string, int given.', + 15, + ], + [ + 'Parameter #2 $two of method DataProviderVariadicMethod\FooTest::testProvide2() expects string, int given.', + 15, + ], + [ + 'Parameter #3 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide2() expects string, int given.', + 15, + ], + ]); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/data-provider-variadic-method.php b/tests/Rules/PHPUnit/data/data-provider-variadic-method.php new file mode 100644 index 0000000..978d197 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-variadic-method.php @@ -0,0 +1,61 @@ + Date: Fri, 17 Oct 2025 10:53:35 +0200 Subject: [PATCH 54/63] Not a failing test, just checking :) --- .../PHPUnit/DataProviderDataRuleTest.php | 22 +++++++++++++ .../data/data-provider-trimming-args.php | 32 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 tests/Rules/PHPUnit/data/data-provider-trimming-args.php diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 34d65e4..da2b4dd 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -228,6 +228,28 @@ public function testVariadicMethod(): void ]); } + public function testTrimmingArgs(): void + { + $this->analyse([__DIR__ . '/data/data-provider-trimming-args.php'], [ + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.', + 12, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide2() invoked with 2 parameters, 1 required.', + 12, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.', + 13, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide2() invoked with 2 parameters, 1 required.', + 13, + ], + ]); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php new file mode 100644 index 0000000..60fc7cf --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php @@ -0,0 +1,32 @@ + Date: Fri, 17 Oct 2025 16:03:07 +0200 Subject: [PATCH 55/63] separated php8 only tests --- .../PHPUnit/DataProviderDataRuleTest.php | 71 ++++++++++--------- .../PHPUnit/data/data-provider-data-named.php | 46 ++++++++++++ .../Rules/PHPUnit/data/data-provider-data.php | 39 ---------- 3 files changed, 85 insertions(+), 71 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/data-provider-data-named.php diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index da2b4dd..c9a87e4 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -42,10 +42,6 @@ protected function getRule(): Rule public function testRule(): void { - if (PHP_VERSION_ID < 80000) { - self::markTestSkipped(); - } - $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', @@ -99,97 +95,108 @@ public function testRule(): void 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testFoo() expects string, int given.', 216, ], - [ - 'Parameter $input of method DataProviderDataTest\NamedArgsInProvider::testFoo() expects string, int given.', - 255, - ], - [ - 'Parameter $input of method DataProviderDataTest\NamedArgsInProvider::testFoo() expects string, false given.', - 255, - ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, int given.', - 275, + 236, ], [ 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, true given.', - 277, + 238, ], [ 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic::testBar() expects int, string given.', - 334, + 295, ], [ 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic::testFoo() expects string, int given.', - 335, + 296, ], [ 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic2::testBar() expects int, string given.', - 356, + 317, ], [ 'Parameter #2 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', - 356, + 317, ], [ 'Parameter #4 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', - 356, + 317, ], [ 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', - 357, + 318, ], [ 'Unknown parameter $foo in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', - 371, + 332, ], [ 'Missing parameter $si (int) in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', - 371, + 332, ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testBar() expects int, int|string given.', - 421, + 382, ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testFoo() expects int, int|string given.', - 421, + 382, ], [ 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', - 421, + 382, ], [ 'Parameter #1 $si of method DataProviderDataTest\TestWrongTypedIterable::testBar() expects int, string given.', - 439, + 400, ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, int given.', - 466, + 427, ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', - 470, + 431, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionTypeReturnTest::testFoo() expects string, int given.', - 505, + 466, ], [ 'Method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() invoked with 3 parameters, 2 required.', - 543, + 504, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() expects string, int given.', - 543, + 504, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionWithDifferentValueTypeReturnTest::testFoo() expects string, int|string given.', - 576, + 537, ], ]); } + public function testRulePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped(); + } + + $this->analyse([__DIR__ . '/data/data-provider-data-named.php'], [ + [ + 'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.', + 44 + ], + [ + 'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, false given.', + 44 + ], + ]); + } + + public function testVariadicMethod(): void { $this->analyse([__DIR__ . '/data/data-provider-variadic-method.php'], [ diff --git a/tests/Rules/PHPUnit/data/data-provider-data-named.php b/tests/Rules/PHPUnit/data/data-provider-data-named.php new file mode 100644 index 0000000..3789f0f --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-data-named.php @@ -0,0 +1,46 @@ + 'Hello World', + "expectedResult" => " Hello World \n" + ] + ]; + + if (rand(0,1)) { + $arr = [ + [ + "input" => 123, + "expectedResult" => " Hello World \n" + ] + ]; + } + if (rand(0,1)) { + $arr = [ + [ + "input" => false, + "expectedResult" => " Hello World \n" + ] + ]; + } + + return $arr; + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 4574287..092b4d9 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -218,45 +218,6 @@ public function aProvider(): array } -class NamedArgsInProvider extends TestCase -{ - - /** @dataProvider aProvider */ - public function testFoo(string $expectedResult, string $input): void - { - } - - public function aProvider(): array - { - $arr = [ - [ - "input" => 'Hello World', - "expectedResult" => " Hello World \n" - ] - ]; - - if (rand(0,1)) { - $arr = [ - [ - "input" => 123, - "expectedResult" => " Hello World \n" - ] - ]; - } - if (rand(0,1)) { - $arr = [ - [ - "input" => false, - "expectedResult" => " Hello World \n" - ] - ]; - } - - return $arr; - } -} - - class YieldFromExpr extends TestCase { From 9a0d0399c0477d491250291d47281333547a3e74 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 16:11:12 +0200 Subject: [PATCH 56/63] separate named args expectations --- .../PHPUnit/DataProviderDataRuleTest.php | 14 ++++++++----- .../PHPUnit/data/data-provider-data-named.php | 21 +++++++++++++++++++ .../Rules/PHPUnit/data/data-provider-data.php | 2 +- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index c9a87e4..f76b19d 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -128,11 +128,7 @@ public function testRule(): void 318, ], [ - 'Unknown parameter $foo in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', - 332, - ], - [ - 'Missing parameter $si (int) in call to method DataProviderDataTest\TestArrayShapeIterable::testBar().', + 'Parameter $si of method DataProviderDataTest\TestArrayShapeIterable::testBar() expects int, string given.', 332, ], [ @@ -193,6 +189,14 @@ public function testRulePhp8(): void 'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, false given.', 44 ], + [ + 'Unknown parameter $wrong in call to method DataProviderDataTestPhp8\TestArrayShapeIterable::testBar().', + 58 + ], + [ + 'Missing parameter $si (int) in call to method DataProviderDataTestPhp8\TestArrayShapeIterable::testBar().', + 58 + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data-named.php b/tests/Rules/PHPUnit/data/data-provider-data-named.php index 3789f0f..bae58bf 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data-named.php +++ b/tests/Rules/PHPUnit/data/data-provider-data-named.php @@ -44,3 +44,24 @@ public function aProvider(): array return $arr; } } + + +class TestArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 092b4d9..6397507 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -333,7 +333,7 @@ public function aProvider(): iterable } /** - * @return iterable + * @return iterable */ public function data(): iterable { From daf978eed3ebf6da52cc96c8382740e8f66471a5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 17 Oct 2025 16:20:31 +0200 Subject: [PATCH 57/63] separated more named args tests --- .../PHPUnit/DataProviderDataRuleTest.php | 32 +++++++------- .../PHPUnit/data/data-provider-data-named.php | 44 ++++++++++++++++++- .../Rules/PHPUnit/data/data-provider-data.php | 20 --------- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index f76b19d..fa3aa9f 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -127,49 +127,45 @@ public function testRule(): void 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', 318, ], - [ - 'Parameter $si of method DataProviderDataTest\TestArrayShapeIterable::testBar() expects int, string given.', - 332, - ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testBar() expects int, int|string given.', - 382, + 362, ], [ 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testFoo() expects int, int|string given.', - 382, + 362, ], [ 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', - 382, + 362, ], [ 'Parameter #1 $si of method DataProviderDataTest\TestWrongTypedIterable::testBar() expects int, string given.', - 400, + 380, ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, int given.', - 427, + 407, ], [ 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', - 431, + 411, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionTypeReturnTest::testFoo() expects string, int given.', - 466, + 446, ], [ 'Method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() invoked with 3 parameters, 2 required.', - 504, + 484, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() expects string, int given.', - 504, + 484, ], [ 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionWithDifferentValueTypeReturnTest::testFoo() expects string, int|string given.', - 537, + 517, ], ]); } @@ -190,13 +186,17 @@ public function testRulePhp8(): void 44 ], [ - 'Unknown parameter $wrong in call to method DataProviderDataTestPhp8\TestArrayShapeIterable::testBar().', + 'Unknown parameter $wrong in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().', 58 ], [ - 'Missing parameter $si (int) in call to method DataProviderDataTestPhp8\TestArrayShapeIterable::testBar().', + 'Missing parameter $si (int) in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().', 58 ], + [ + 'Parameter $si of method DataProviderDataTestPhp8\TestWrongTypeInArrayShapeIterable::testBar() expects int, string given.', + 79 + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-data-named.php b/tests/Rules/PHPUnit/data/data-provider-data-named.php index bae58bf..bea3e50 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data-named.php +++ b/tests/Rules/PHPUnit/data/data-provider-data-named.php @@ -46,7 +46,7 @@ public function aProvider(): array } -class TestArrayShapeIterable extends TestCase +class TestWrongOffsetNameArrayShapeIterable extends TestCase { /** @dataProvider aProvider */ public function testBar(int $si): void @@ -65,3 +65,45 @@ public function data(): iterable { } } + + +class TestWrongTypeInArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} + + +class TestValidArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php index 6397507..d684df7 100644 --- a/tests/Rules/PHPUnit/data/data-provider-data.php +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -320,26 +320,6 @@ public function aProvider(): iterable } } -class TestArrayShapeIterable extends TestCase -{ - /** @dataProvider aProvider */ - public function testBar(int $si): void - { - } - - public function aProvider(): iterable - { - return $this->data(); - } - - /** - * @return iterable - */ - public function data(): iterable - { - } -} - class TestTypedIterable extends TestCase { /** @dataProvider aProvider */ From 534694d6561a99f33bfa939ba271c15f9e314007 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Oct 2025 09:06:51 +0200 Subject: [PATCH 58/63] fix arg trimming --- src/Rules/PHPUnit/DataProviderDataRule.php | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 92608fa..4e7a9aa 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -84,10 +84,20 @@ public function processNode(Node $node, Scope $scope): array return []; } - $maxNumberOfParameters = 0; - $trimArgs = count($testsWithProvider) > 1; - foreach ($testsWithProvider as $testMethod) { - $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters()); + $trimArgs = false; + if (count($testsWithProvider) > 1) { + $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); + foreach ($testsWithProvider as $testMethod) { + if ($testMethod->isVariadic()) { + $trimArgs = true; + break; + } + + if ($maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { + $trimArgs = true; + break; + } + } } foreach ($testsWithProvider as $testMethod) { @@ -99,8 +109,8 @@ public function processNode(Node $node, Scope $scope): array continue; } - if ($trimArgs && $maxNumberOfParameters !== $numberOfParameters) { - $args = array_slice($args, 0, min($numberOfParameters, $maxNumberOfParameters)); + if (!$testMethod->isVariadic() && $trimArgs) { + $args = array_slice($args, 0, $numberOfParameters); } $scope->invokeNodeCallback(new Node\Expr\MethodCall( From 58461d78df2ddd9c7a9f430769d7a733800e19b2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Oct 2025 09:32:24 +0200 Subject: [PATCH 59/63] more fixes --- src/Rules/PHPUnit/DataProviderDataRule.php | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 4e7a9aa..43e53d2 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -85,18 +85,19 @@ public function processNode(Node $node, Scope $scope): array } $trimArgs = false; + $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); if (count($testsWithProvider) > 1) { - $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); foreach ($testsWithProvider as $testMethod) { - if ($testMethod->isVariadic()) { + if ($maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { $trimArgs = true; - break; } - if ($maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { + if ($testMethod->isVariadic()) { $trimArgs = true; - break; + $maxNumberOfParameters = PHP_INT_MAX; } + + $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters()); } } @@ -104,13 +105,17 @@ public function processNode(Node $node, Scope $scope): array $numberOfParameters = $testMethod->getNumberOfParameters(); foreach ($arraysTypes as [$startLine, $arraysType]) { - $args = $this->arrayItemsToArgs($arraysType, $numberOfParameters); + $args = $this->arrayItemsToArgs($arraysType, $maxNumberOfParameters); if ($args === null) { continue; } - if (!$testMethod->isVariadic() && $trimArgs) { - $args = array_slice($args, 0, $numberOfParameters); + if ( + $trimArgs + && !$testMethod->isVariadic() + && $numberOfParameters !== $maxNumberOfParameters + ) { + $args = array_slice($args, 0, min($numberOfParameters, $maxNumberOfParameters)); } $scope->invokeNodeCallback(new Node\Expr\MethodCall( From b36a404f4e45c51627633a419b10f51f6952f1e5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Oct 2025 09:35:33 +0200 Subject: [PATCH 60/63] cs --- src/Rules/PHPUnit/DataProviderDataRule.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 43e53d2..9c96cd0 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -13,6 +13,7 @@ use function count; use function max; use function min; +use const PHP_INT_MAX; /** * @implements Rule From f083e6319a52692b7dfa5ddcc97c7b8f74c49cbf Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 18 Oct 2025 10:35:00 +0200 Subject: [PATCH 61/63] Manual mutation testing ep. 1 --- src/Rules/PHPUnit/DataProviderDataRule.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 9c96cd0..b13e502 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -12,7 +12,6 @@ use function array_slice; use function count; use function max; -use function min; use const PHP_INT_MAX; /** @@ -116,7 +115,7 @@ public function processNode(Node $node, Scope $scope): array && !$testMethod->isVariadic() && $numberOfParameters !== $maxNumberOfParameters ) { - $args = array_slice($args, 0, min($numberOfParameters, $maxNumberOfParameters)); + $args = array_slice($args, 0, $numberOfParameters); } $scope->invokeNodeCallback(new Node\Expr\MethodCall( From 42ad04ed09ae916e230a21bc0e26e0eae0b3d9c0 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 18 Oct 2025 10:35:45 +0200 Subject: [PATCH 62/63] Manual mutation testing ep. 2 --- src/Rules/PHPUnit/DataProviderDataRule.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index b13e502..045bd95 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -84,16 +84,10 @@ public function processNode(Node $node, Scope $scope): array return []; } - $trimArgs = false; $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); if (count($testsWithProvider) > 1) { foreach ($testsWithProvider as $testMethod) { - if ($maxNumberOfParameters !== $testMethod->getNumberOfParameters()) { - $trimArgs = true; - } - if ($testMethod->isVariadic()) { - $trimArgs = true; $maxNumberOfParameters = PHP_INT_MAX; } @@ -111,8 +105,7 @@ public function processNode(Node $node, Scope $scope): array } if ( - $trimArgs - && !$testMethod->isVariadic() + !$testMethod->isVariadic() && $numberOfParameters !== $maxNumberOfParameters ) { $args = array_slice($args, 0, $numberOfParameters); From 073102699b5b0ebc5fcc61fdb90669ccd654ea4d Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 18 Oct 2025 10:37:25 +0200 Subject: [PATCH 63/63] break --- src/Rules/PHPUnit/DataProviderDataRule.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 045bd95..e241053 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -89,6 +89,7 @@ public function processNode(Node $node, Scope $scope): array foreach ($testsWithProvider as $testMethod) { if ($testMethod->isVariadic()) { $maxNumberOfParameters = PHP_INT_MAX; + break; } $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters());