From 7713002f53958901c929c6052108e82db920bab8 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 31 Mar 2025 20:39:39 +0200 Subject: [PATCH 1/2] Ensure that all `@dataProvider` and `#[DataProvider]` (multiple) are considered Ref: https://github.com/psalm/psalm-plugin-phpunit/pull/149#issuecomment-2766891341 Ref: https://github.com/theodorejb/saml-utils/blob/2392b576799dd76957799e82dcb6df9e089e76bc/tests/DataProviderTest.php /cc @theodorejb --- src/Hooks/TestCaseHandler.php | 15 ++++++--- tests/acceptance/TestCase.feature | 56 +++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index c051d19..acb860b 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -566,14 +566,19 @@ private static function attributeValue(ClassMethod $method, Aliases $aliases, st $aliases, ), ); + $matchingAttributes = array_merge(...array_map( + $attributesInGroupMatchingRequestedAttributeName, + $method->getAttrGroups(), + )); - foreach ($method->getAttrGroups() as $group) { - foreach ($attributesInGroupMatchingRequestedAttributeName($group) as $attribute) { - return $onlyStringLiteralExpressions($attribute); - } + if ($matchingAttributes === []) { + return null; } - return null; + return array_merge(...array_map( + $onlyStringLiteralExpressions, + $matchingAttributes, + )); } /** @return array> */ diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index fc673c4..f9d5882 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -231,6 +231,62 @@ Feature: TestCase When I run Psalm Then I see no errors + Scenario: Multiple valid iterable @dataProvider are allowed, and all are used + Given I have the following code + """ + use PHPUnit\Framework\Attributes; + + final class MyTestCase extends TestCase + { + /** @return iterable> */ + public function provider1() { + yield [1]; + } + + /** @return iterable> */ + public function provider2() { + yield [1]; + } + + /** + * @dataProvider provider1 + * @dataProvider provider2 + */ + public function testSomething(int $int): void { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: Multiple valid iterable #[DataProvider]s are allowed, and all are used + Given I have the following code + """ + use PHPUnit\Framework\Attributes; + + final class MyTestCase extends TestCase + { + /** @return iterable> */ + public function provider1() { + yield [1]; + } + + /** @return iterable> */ + public function provider2() { + yield [1]; + } + + #[Attributes\DataProvider('provider1')] + #[Attributes\DataProvider('provider2')] + public function testSomething(int $int): void { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + Scenario: Invalid generator data provider is reported Given I have the following code """ From cc6b85b391a8fc99de56f9df2725e6132608ee1e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 31 Mar 2025 20:43:01 +0200 Subject: [PATCH 2/2] Cluttering the code with `array_values()`, thanks named arguments, I guess? --- src/Hooks/TestCaseHandler.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index acb860b..12be2c1 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -566,19 +566,19 @@ private static function attributeValue(ClassMethod $method, Aliases $aliases, st $aliases, ), ); - $matchingAttributes = array_merge(...array_map( + $matchingAttributes = array_merge(...array_values(array_map( $attributesInGroupMatchingRequestedAttributeName, $method->getAttrGroups(), - )); + ))); if ($matchingAttributes === []) { return null; } - return array_merge(...array_map( + return array_merge(...array_values(array_map( $onlyStringLiteralExpressions, $matchingAttributes, - )); + ))); } /** @return array> */