From 37a7dc6f50a8d0fe539f80e5cca6a2e5ea58fc5d Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:02:20 +0100 Subject: [PATCH 01/24] fix(dead-code): mark PHPUnit hook methods as used #251 --- src/Hooks/TestCaseHandler.php | 53 ++++++++- tests/acceptance/TestCase.feature | 179 ++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 4 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..b3eb236 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -24,7 +24,10 @@ use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use PhpParser\NodeVisitorAbstract; +use PHPUnit\Framework\Attributes\After; +use PHPUnit\Framework\Attributes\AfterClass; use PHPUnit\Framework\Attributes\Before; +use PHPUnit\Framework\Attributes\BeforeClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\Attributes\Depends; @@ -33,6 +36,8 @@ use PHPUnit\Framework\Attributes\DependsExternalUsingShallowClone; use PHPUnit\Framework\Attributes\DependsUsingDeepClone; use PHPUnit\Framework\Attributes\DependsUsingShallowClone; +use PHPUnit\Framework\Attributes\PostCondition; +use PHPUnit\Framework\Attributes\PreCondition; use PHPUnit\Framework\Attributes\Test; use Psalm\Aliases; use Psalm\Codebase; @@ -95,6 +100,21 @@ final class TestCaseHandler implements DependsExternalUsingShallowClone::class => true, ]; + /** + * The hook attributes, which are also the names of the PHPUnit 11 docblock tags => whether PHPUnit only runs + * them on static methods. + * + * @var array + */ + private const HOOKS = [ + 'before' => false, + 'after' => false, + 'preCondition' => false, + 'postCondition' => false, + 'beforeClass' => true, + 'afterClass' => true, + ]; + private const DATA_PROVIDER_CLOSURE = 'PHPUnit\\Framework\\Attributes\\DataProviderClosure'; private const EXPECT_EXCEPTION_METHODS = [ @@ -348,8 +368,8 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ); $is_test = 0 === strpos($method_name_lc, 'test') || isset($specials['test']); - if (!$is_test) { - continue; // skip non-test methods + if (!$is_test && !self::isHook($stmt_method, $method_aliases, $method_storage)) { + continue; // skip methods PHPUnit never calls } $codebase->methodExists( @@ -357,6 +377,10 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event calling_method_id: 'PHPUnit\Framework\TestSuite::run', ); + if (!$is_test) { + continue; + } + $providers = self::getDataProviders($stmt_method, $method_aliases, $aliases, $class_storage); $closure_providers = self::getClosureProviders($stmt_method, $method_aliases); if ($providers === [] && $closure_providers === []) { @@ -1498,12 +1522,33 @@ public static function classFromExpression( return Type::getFQCLNFromString($expression->class->toCodeString(), $aliases); } + /** PHPUnit ignores a docblock tag on a method that has any PHPUnit attribute. */ + private static function isHook(ClassMethod $method, Aliases $aliases, MethodStorage $storage): bool + { + $specials = self::hasPhpUnitAttribute($method, $aliases) + ? self::getAttributeSpecials($method, $aliases) + : self::docblockSpecials($method); + + foreach (self::HOOKS as $hook => $static_only) { + if (isset($specials[$hook]) && (!$static_only || $storage->is_static)) { + return true; + } + } + + return false; + } + /** @return array> */ private static function getAttributeSpecials(ClassMethod $method, Aliases $aliases): array { return array_filter([ - 'before' => self::attributeValue($method, $aliases, Before::class), - 'test' => self::attributeValue($method, $aliases, Test::class), + 'before' => self::attributeValue($method, $aliases, Before::class), + 'after' => self::attributeValue($method, $aliases, After::class), + 'preCondition' => self::attributeValue($method, $aliases, PreCondition::class), + 'postCondition' => self::attributeValue($method, $aliases, PostCondition::class), + 'beforeClass' => self::attributeValue($method, $aliases, BeforeClass::class), + 'afterClass' => self::attributeValue($method, $aliases, AfterClass::class), + 'test' => self::attributeValue($method, $aliases, Test::class), ], static fn(array|null $special): bool => $special !== null); } diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..d7ebec9 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,182 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: Methods with a hook attribute are used by PHPUnit + Given I have the following code + """ + use PHPUnit\Framework\Attributes\After; + use PHPUnit\Framework\Attributes\AfterClass; + use PHPUnit\Framework\Attributes\Before; + use PHPUnit\Framework\Attributes\BeforeClass; + use PHPUnit\Framework\Attributes\PostCondition; + use PHPUnit\Framework\Attributes\PreCondition; + + final class MyTestCase extends TestCase { + #[BeforeClass] + public static function prepareClass(): void { + self::assertTrue(true); + } + + #[AfterClass] + public static function cleanUpClass(): void { + self::assertTrue(true); + } + + #[Before] + public function prepare(): void { + $this->assertTrue(true); + } + + #[After] + protected function cleanUp(): void { + $this->assertTrue(true); + } + + #[PreCondition] + public function checkBefore(): void { + $this->assertTrue(true); + } + + #[PostCondition] + public function checkAfter(): void { + $this->assertTrue(true); + } + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: Methods with a hook docblock tag are used by PHPUnit 11 + Given PHPUnit identifies as version "11.5.0" + And I have the following code + """ + final class MyTestCase extends TestCase { + /** + * @beforeClass + */ + public static function prepareClass(): void { + self::assertTrue(true); + } + + /** + * @afterClass + */ + public static function cleanUpClass(): void { + self::assertTrue(true); + } + + /** + * @before + */ + public function prepare(): void { + $this->assertTrue(true); + } + + /** + * @after + */ + protected function cleanUp(): void { + $this->assertTrue(true); + } + + /** + * @preCondition + */ + public function checkBefore(): void { + $this->assertTrue(true); + } + + /** + * @postCondition + */ + public function checkAfter(): void { + $this->assertTrue(true); + } + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: A method with a hook docblock tag is unused on PHPUnit 12 + Given PHPUnit identifies as version "12.5.0" + And I have the following code + """ + final class MyTestCase extends TestCase { + /** + * @before + */ + public function prepare(): void { + $this->assertTrue(true); + } + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | IgnoredDocblockMetadata | PHPUnit 12 ignores the @before docblock tag, use #[Before] instead | + | PossiblyUnusedMethod | Cannot find any calls to method NS\MyTestCase::prepare | + And I see no other errors + + Scenario: A hook attribute of a method of a trait used by the test case is used by PHPUnit + Given I have the following code + """ + use PHPUnit\Framework\Attributes\Before; + use PHPUnit\Framework\Attributes\BeforeClass; + + trait Prepares { + #[BeforeClass] + public static function prepareClass(): void { + self::assertTrue(true); + } + + #[Before] + public function prepare(): void { + $this->assertTrue(true); + } + } + + final class MyTestCase extends TestCase { + use Prepares; + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: A non-static #[BeforeClass] method is not run by PHPUnit + Given I have the following code + """ + use PHPUnit\Framework\Attributes\BeforeClass; + + final class MyTestCase extends TestCase { + #[BeforeClass] + public function prepareClass(): void { + $this->assertTrue(true); + } + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | PossiblyUnusedMethod | Cannot find any calls to method NS\MyTestCase::prepareClass | + And I see no other errors From 9de65a68ae76ffaff3b8dccabf887b8b4d7b5677 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:03:13 +0100 Subject: [PATCH 02/24] fix(provider): report argument type coercion like Psalm core #248 --- src/Hooks/TestCaseHandler.php | 22 +++++- tests/acceptance/TestCase.feature | 113 ++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..173f477 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -39,6 +39,8 @@ use Psalm\CodeLocation; use Psalm\Internal\Analyzer\ClassLikeAnalyzer; use Psalm\Internal\MethodIdentifier; +use Psalm\Internal\Type\Comparator\TypeComparisonResult; +use Psalm\Internal\Type\Comparator\UnionTypeComparator; use Psalm\IssueBuffer; use Psalm\Issue; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; @@ -634,8 +636,26 @@ static function ( if ($is_optional) { $param_type = $param_type->setPossiblyUndefined(true); } - if ($codebase->isTypeContainedByType($potential_argument_type, $param_type)) { + $comparison = new TypeComparisonResult(); + if (UnionTypeComparator::isContainedBy( + $codebase, + $potential_argument_type, + $param_type, + false, + false, + $comparison, + )) { // ok + } elseif ($comparison->type_coerced && !$potential_argument_type->hasMixed()) { + $message = $argument . ' of ' . $method_name + . ' expects ' . $param_type->getId() . ', but parent type ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')'; + IssueBuffer::accepts( + $comparison->type_coerced_from_mixed + ? new Issue\MixedArgumentTypeCoercion($message, $provider_docblock_location) + : new Issue\ArgumentTypeCoercion($message, $provider_docblock_location) + ); } elseif ($codebase->canTypeBeContainedByType($potential_argument_type, $param_type)) { IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( $argument . ' of ' . $method_name diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..7ea46f8 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,116 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: A provider value that is a parent type of the parameter type is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a']; + } + + /** + * @param non-empty-string $s + */ + #[DataProvider('provide')] + public function testSomething(string $s): void { + $this->assertNotSame('', $s); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects non-empty-string, but parent type string provided by NS\MyTestCase::provide():(iterable[pure]) | + And I see no other errors + + Scenario: A nullable provider value that is a parent type of the nullable parameter type is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [null]; + } + + /** + * @param ?non-empty-string $s + */ + #[DataProvider('provide')] + public function testSomething(?string $s): void { + $this->assertNull($s); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects non-empty-string\|null, but parent type null\|string provided by NS\MyTestCase::provide():(iterable[pure]) | + And I see no other errors + + Scenario: A named dataset value that is a parent type of the parameter type is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['s' => 'a']; + } + + /** + * @param non-empty-string $s + */ + #[DataProvider('provide')] + public function testSomething(string $s): void { + $this->assertNotSame('', $s); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 ($s) of NS\MyTestCase::testSomething expects non-empty-string, but parent type string provided by NS\MyTestCase::provide():(iterable[pure]) | + And I see no other errors + + Scenario: A provider value that does not fit the parameter type at all stays an invalid argument + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [1]; + } + + #[DataProvider('provide')] + public function testSomething(string $s): void { + $this->assertSame('', $s); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]) | + And I see no other errors From fed591d654210d84e110535cb81df648021c7f30 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:03:25 +0100 Subject: [PATCH 03/24] fix(missing-constructor): honour setUp/#[Before] from traits #252 --- src/Hooks/TestCaseHandler.php | 26 +++++- tests/acceptance/TestCase.feature | 142 ++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 2 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..3178d74 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -161,8 +161,10 @@ public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event } foreach ($codebase->classlike_storage_provider->getAll() as $name => $storage) { - $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); - if ($codebase->classExtends($name, 'PHPUnit\Framework\TestCase') && ($meta['hasInitializers'] ?? false)) { + if ( + $codebase->classExtends($name, 'PHPUnit\Framework\TestCase') + && self::hasInitializersFromTraits($codebase, $storage) + ) { $storage->suppressed_issues[] = 'MissingConstructor'; foreach (self::getDescendants($codebase, $name) as $dependent_name) { @@ -173,6 +175,26 @@ public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event } } + /** Trait methods are not merged into the class yet when the class is visited, so its own flag misses them. */ + private static function hasInitializersFromTraits(Codebase $codebase, ClassLikeStorage $storage): bool + { + $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); + if (($meta['hasInitializers'] ?? false) || isset($storage->trait_alias_map['setup'])) { + return true; + } + + foreach ($storage->used_traits as $trait_name) { + if ( + $codebase->classlike_storage_provider->has($trait_name) + && self::hasInitializersFromTraits($codebase, $codebase->classlike_storage_provider->get($trait_name)) + ) { + return true; + } + } + + return false; + } + /** * @return string[] * diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..6d5d656 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,145 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: A trait providing setUp produces no MissingConstructor + Given I have the following code + """ + trait Init { + private int $x; + + protected function setUp(): void { + $this->x = 1; + } + } + + final class MyTestCase extends TestCase { + use Init; + + public function testSomething(): void { + $this->assertSame(1, $this->x); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: A trait providing a #[Before] method produces no MissingConstructor + Given I have the following code + """ + use PHPUnit\Framework\Attributes\Before; + + trait Init { + private int $x; + + #[Before] + protected function init(): void { + $this->x = 1; + } + } + + final class MyTestCase extends TestCase { + use Init; + + public function testSomething(): void { + $this->assertSame(1, $this->x); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: A trait used by a trait providing setUp produces no MissingConstructor + Given I have the following code + """ + trait Init { + private int $x; + + protected function setUp(): void { + $this->x = 1; + } + } + + trait Wrapper { + use Init; + } + + final class MyTestCase extends TestCase { + use Wrapper; + + public function testSomething(): void { + $this->assertSame(1, $this->x); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: A trait method aliased as setUp produces no MissingConstructor + Given I have the following code + """ + trait Init { + private int $x; + + protected function init(): void { + $this->x = 1; + } + } + + final class MyTestCase extends TestCase { + use Init { + init as protected setUp; + } + + public function testSomething(): void { + $this->assertSame(1, $this->x); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Descendant of a test case whose trait provides setUp produces no MissingConstructor + Given I have the following code + """ + trait Init { + protected int $x; + + protected function setUp(): void { + $this->x = 1; + } + } + + abstract class BaseTestCase extends TestCase { + use Init; + } + + final class MyTestCase extends BaseTestCase { + public function testSomething(): void { + $this->assertSame(1, $this->x); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: A trait without initializers still produces MissingConstructor + Given I have the following code + """ + trait State { + private int $x; + } + + final class MyTestCase extends TestCase { + use State; + + public function testSomething(): void { + $this->assertSame(1, 1); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | MissingConstructor | NS\MyTestCase has an uninitialized property NS\State::$x, but no constructor | + And I see no other errors From 1bbc943571a72ab54df989b7bd447872c6998b0a Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:03:26 +0100 Subject: [PATCH 04/24] fix(provider): check datasets of iterable providers #246 --- src/Hooks/TestCaseHandler.php | 7 +- src/Hooks/TestCaseHandler.php.orig | 1536 ++++++++++++++++++++++++++++ tests/acceptance/TestCase.feature | 110 ++ 3 files changed, 1652 insertions(+), 1 deletion(-) create mode 100644 src/Hooks/TestCaseHandler.php.orig diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..0979720 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -561,7 +561,12 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event new Type\Union($non_null_provider_return_types) ); - $provider_key_type_is_compatible = $codebase->isTypeContainedByType( + // A single-argument iterable such as iterable has a mixed key, and PHPUnit rejects keys + // that are not int|string at runtime. Only a mixed value reveals an untyped provider. + $provider_key_type_is_compatible = ( + $provider_return_type->type_params[0]->isMixed() + && !$provider_return_type->type_params[1]->hasMixed() + ) || $codebase->isTypeContainedByType( $provider_return_type->type_params[0], $expected_provider_return_type->type_params[0] ); diff --git a/src/Hooks/TestCaseHandler.php.orig b/src/Hooks/TestCaseHandler.php.orig new file mode 100644 index 0000000..e2819c5 --- /dev/null +++ b/src/Hooks/TestCaseHandler.php.orig @@ -0,0 +1,1536 @@ + attribute class => whether it names the target class explicitly */ + private const DEPENDS_ATTRIBUTES = [ + Depends::class => false, + DependsUsingDeepClone::class => false, + DependsUsingShallowClone::class => false, + DependsExternal::class => true, + DependsExternalUsingDeepClone::class => true, + DependsExternalUsingShallowClone::class => true, + ]; + + private const DATA_PROVIDER_CLOSURE = 'PHPUnit\\Framework\\Attributes\\DataProviderClosure'; + + private const EXPECT_EXCEPTION_METHODS = [ + 'phpunit\\framework\\testcase::expectexception' => true, + 'phpunit\\framework\\testcase::expectexceptioncode' => true, + 'phpunit\\framework\\testcase::expectexceptionmessage' => true, + 'phpunit\\framework\\testcase::expectexceptionmessagematches' => true, + 'phpunit\\framework\\testcase::expectexceptionobject' => true, + ]; + + /** + * PHPUnit's own exceptions are handled by the test runner, nobody documents or catches them in tests. + * Psalm matches a thrown class only against listed parent classes, so classes that merely implement one of + * the interfaces are listed on their own. + */ + private const IGNORED_EXCEPTIONS = [ + 'PHPUnit\\Exception', + 'PHPUnit\\Framework\\Exception', + 'PHPUnit\\Framework\\MockObject\\Exception', + 'PHPUnit\\Framework\\MockObject\\ReflectionException', + 'PHPUnit\\Framework\\MockObject\\RuntimeException', + 'PHPUnit\\Event\\NoPreviousThrowableException', + 'PHPUnit\\Util\\Exception', + 'PHPUnit\\Util\\InvalidJsonException', + 'PHPUnit\\Util\\Xml\\XmlException', + ]; + + /** + * Statements after expectException*() may exist only to throw, so their results are unused and PHPUnit catches + * the exception. + */ + #[\Override] + public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void + { + if (!isset(self::EXPECT_EXCEPTION_METHODS[strtolower($event->getDeclaringMethodId())])) { + return; + } + + // PHPUnit stores the expectation on the receiver, so only the current test's own expectations count. + $expr = $event->getExpr(); + $isOwnExpectation = $expr instanceof MethodCall + ? $expr->var instanceof Variable && $expr->var->name === 'this' + : $expr->class instanceof Name + && in_array(strtolower($expr->class->toString()), ['self', 'static', 'parent'], true); + if (!$isOwnExpectation) { + return; + } + + $event->getStatementsSource()->addSuppressedIssues([ + 'UnusedFunctionCall', + 'UnusedMethodCall', + 'MissingThrowsDocblock', + ]); + } + + #[\Override] + public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event) + { + $codebase = $event->getCodebase(); + + foreach (self::IGNORED_EXCEPTIONS as $exception) { + $codebase->config->ignored_exceptions_and_descendants[$exception] = true; + $codebase->config->ignored_exceptions_and_descendants_in_global_scope[$exception] = true; + } + + foreach ($codebase->classlike_storage_provider->getAll() as $name => $storage) { + $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); + if ($codebase->classExtends($name, 'PHPUnit\Framework\TestCase') && ($meta['hasInitializers'] ?? false)) { + $storage->suppressed_issues[] = 'MissingConstructor'; + + foreach (self::getDescendants($codebase, $name) as $dependent_name) { + $dependent_storage = $codebase->classlike_storage_provider->get($dependent_name); + $dependent_storage->suppressed_issues[] = 'MissingConstructor'; + } + } + } + } + + /** + * @return string[] + * + * @psalm-mutation-free + */ + private static function getDescendants(Codebase $codebase, string $name): array + { + if (!$codebase->classlike_storage_provider->has($name)) { + return []; + } + + $storage = $codebase->classlike_storage_provider->get($name); + $ret = []; + + foreach ($storage->dependent_classlikes as $dependent => $_) { + if ($codebase->classExtends($dependent, $name)) { + $ret[] = $dependent; + $ret = array_merge($ret, self::getDescendants($codebase, $dependent)); + } + } + return $ret; + } + + #[\Override] + public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) + { + $class_node = $event->getStmt(); + $class_storage = $event->getStorage(); + $statements_source = $event->getStatementsSource(); + $codebase = $event->getCodebase(); + $aliases = $statements_source->getAliases(); + + // Psalm 7 seals methods by default, so ObjectProphecy::__call() is ignored + if (strtolower($class_storage->name) === 'prophecy\\prophecy\\objectprophecy') { + $class_storage->sealed_methods = false; + } + + if (self::hasInitializers($class_storage, $class_node, $aliases)) { + $class_storage->custom_metadata[__NAMESPACE__] = ['hasInitializers' => true]; + } + + $file_path = $statements_source->getFilePath(); + $file_storage = $codebase->file_storage_provider->get($file_path); + + foreach ($class_node->getMethods() as $method) { + $providers = self::getDataProviders($method, $aliases, $aliases, $class_storage); + foreach ($providers as $provider) { + if ($provider['class'] === null) { + continue; + } + $codebase->queueClassLikeForScanning($provider['class']); + $file_storage->referenced_classlikes[strtolower($provider['class'])] = $provider['class']; + } + + $method_storage = $class_storage->methods[strtolower($method->name->name)] ?? null; + // PHPUnit ignores docblock annotations on a method that has any PHPUnit attribute + $has_attribute = self::hasPhpUnitAttribute($method, $aliases); + if ( + $method_storage === null + || $providers === [] + || ($has_attribute && self::getAttributeDataProviders($method, $aliases, $class_storage) === null) + || count($providers) < self::countProviderAttributes($method, $aliases) + ) { + continue; + } + + $test_specials = $has_attribute + ? self::getAttributeSpecials($method, $aliases) + : self::docblockSpecials($method); + $method_storage->custom_metadata[DataProviderNarrowingHandler::METADATA_KEY] = [ + // the test's own class is resolved at analysis, where a trait method has the class that uses it + 'providers' => array_map( + static fn(array $provider): array => [$provider['class'] ?? '', $provider['method']], + $providers, + ), + 'test' => isset($test_specials['test']), + 'depends' => self::getDependencyTargets($method, $aliases, $class_storage) !== [], + ]; + } + } + + #[\Override] + public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event) + { + $class_node = $event->getStmt(); + $class_storage = $event->getClasslikeStorage(); + $codebase = $event->getCodebase(); + $statements_source = $event->getStatementsSource(); + $aliases = $statements_source->getAliases(); + + if (!$codebase->classExtends($class_storage->name, 'PHPUnit\Framework\TestCase')) { + return null; + } + + // This should always pass, we're calling it for the side-effect of adding self-reference + // in order to + // 1. Inform Psalm that class is used + // 2. Make Psalm analyze unused methods + // + // Marking class as used is required to get more detailed dead-code analysis (like unused + // methods). If we instead just suppress UnusedClass, unused methods are not analyzed. + if (!$codebase->classOrInterfaceExists($class_storage->name, $class_storage->location)) { + return null; + } + + self::reportMalformedTags( + $event, + $class_node, + new CodeLocation($statements_source, $class_node), + [], + ); + + // PHPUnit merges the class docblocks of the traits used directly by the test case, not of nested ones + foreach ($class_storage->used_traits as $trait_name) { + $trait_storage = $codebase->classlike_storage_provider->get($trait_name); + if (!$trait_storage->location) { + continue; + } + + self::reportMalformedTags( + $event, + $codebase->classlikes->getTraitNode($trait_name), + $trait_storage->stmt_location ?? $trait_storage->location, + $trait_storage->suppressed_issues, + ); + } + + foreach ($class_storage->declaring_method_ids as $method_name_lc => $declaring_method_id) { + $method_name = $codebase->getCasedMethodId($class_storage->name . '::' . $method_name_lc); + $method_storage = $codebase->methods->getStorage($declaring_method_id); + [$declaring_method_class, $declaring_method_name] = explode('::', (string)$declaring_method_id); + $declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_method_class); + + $declaring_class_node = $class_node; + if ($declaring_class_storage->is_trait) { + $declaring_class_node = $codebase->classlikes->getTraitNode($declaring_class_storage->name); + } + + if (!$method_storage->location) { + continue; + } + + // The class declaring an inherited test does not see the providers its descendants implement + if (!$declaring_class_storage->is_trait && $declaring_class_storage->name !== $class_storage->name) { + self::checkInheritedTestProviders( + $codebase, + $statements_source, + $class_storage, + $method_name_lc, + $method_storage, + ); + continue; + } + + $stmt_method = $declaring_class_node->getMethod($declaring_method_name); + + if (!$stmt_method) { + continue; + } + + // Use the declaring class's aliases so trait-declared attributes resolve via the trait's imports. + $method_aliases = $declaring_class_storage->aliases ?? $aliases; + $specials = self::getSpecials($stmt_method, $method_aliases); + // PHPUnit ignores docblock annotations on a method that has any PHPUnit attribute + $docblock_providers_ignored = self::hasPhpUnitAttribute($stmt_method, $method_aliases) + && self::getAttributeDataProviders($stmt_method, $method_aliases, $class_storage) === null; + + self::reportMalformedTags( + $event, + $stmt_method, + $method_storage->location, + $method_storage->suppressed_issues + $declaring_class_storage->suppressed_issues, + ); + + $is_test = 0 === strpos($method_name_lc, 'test') || isset($specials['test']); + if (!$is_test) { + continue; // skip non-test methods + } + + $codebase->methodExists( + (string) $declaring_method_id, + calling_method_id: 'PHPUnit\Framework\TestSuite::run', + ); + + $providers = self::getDataProviders($stmt_method, $method_aliases, $aliases, $class_storage); + $closure_providers = self::getClosureProviders($stmt_method, $method_aliases); + if ($providers === [] && $closure_providers === []) { + continue; + } + + $dependency_values = self::countDependencyValues( + $codebase, + $stmt_method, + $method_aliases, + $class_storage, + ); + $appends_dependency_value = !$docblock_providers_ignored && self::appendsDependencyValue( + $codebase, + $class_node, + $class_storage, + $stmt_method, + $method_aliases, + ); + + /** + * @var list $provided_types + */ + $provided_types = []; + foreach ($providers as $provider) { + $provider_docblock_location = $method_storage->location->setCommentLine($provider['line']); + + if ($provider['class'] !== null) { + $fq_class_name = $provider['class']; + + if (!$codebase->classOrInterfaceExists($fq_class_name, $provider_docblock_location)) { + // Psalm reports the missing class of an attribute argument itself + if (!$provider['external']) { + IssueBuffer::accepts(new Issue\UndefinedClass( + 'Class ' . $fq_class_name . ' does not exist', + $provider_docblock_location, + $fq_class_name + )); + } + continue; + } + $apparent_provider_method_name = $fq_class_name . '::' . $provider['method']; + } else { + $apparent_provider_method_name = $class_storage->name . '::' . $provider['method']; + } + + // getDeclaringMethodId() would truncate "Class::A::b" to "Class::A", so never look such a name up + $provider_method_id = str_contains($provider['method'], '::') + ? null + : $codebase->getDeclaringMethodId($apparent_provider_method_name); + + if (null === $provider_method_id) { + IssueBuffer::accepts(new Issue\UndefinedMethod( + 'Provider method ' . $apparent_provider_method_name . ' is not defined', + $provider_docblock_location, + $apparent_provider_method_name + )); + continue; + } + + // methodExists also can mark methods as used (weird, but handy) + $provider_method_exists = $codebase->methodExists( + $provider_method_id, + calling_method_id: (string) $declaring_method_id, + code_location: $provider_docblock_location, + ); + + if (!$provider_method_exists) { + IssueBuffer::accepts(new Issue\UndefinedMethod( + 'Provider method ' . $apparent_provider_method_name . ' is not defined', + $provider_docblock_location, + $apparent_provider_method_name + )); + continue; + } + + $provider_id = MethodIdentifier::wrap($provider_method_id); + $provider_method_storage = $codebase->methods->getStorage($provider_id); + + // Abstract providers are checked on the concrete classes implementing them + if (!$provider_method_storage->abstract && !$docblock_providers_ignored) { + $suppressed_issues = $method_storage->suppressed_issues + + $declaring_class_storage->suppressed_issues + + $statements_source->getSuppressedIssues(); + + self::reportInvalidProviderSignature( + $codebase, + $provider_method_storage, + $apparent_provider_method_name, + $provider_docblock_location, + $suppressed_issues, + ); + + // Providers declared elsewhere are not read + $provider_node = $provider_id->fq_class_name === $class_storage->name + ? $class_node->getMethod($provider_id->method_name) + : null; + if ($provider_node !== null) { + self::reportPositionalAfterNamed( + $statements_source, + $provider_node->stmts ?? [], + 'a dataset provided by ' . $provider_method_id . '()', + $suppressed_issues, + ); + } + } + + $provider_return_type = $codebase->getMethodReturnType($provider_method_id, $_); + + if (!$provider_return_type) { + continue; + } + + $provider_return_type_location = $codebase->getMethodReturnTypeLocation($provider_method_id); + assert(null !== $provider_return_type_location); + $provided_types[] = [ + 'name' => $provider_method_id . '()', + 'type' => $provider_return_type, + 'location' => $provider_docblock_location, + 'return_type_location' => $provider_return_type_location, + ]; + } + + foreach ($closure_providers as $closure) { + // A closure of a trait lives in another file than the one analysed + if (!$declaring_class_storage->is_trait) { + self::reportPositionalAfterNamed( + $statements_source, + $closure['statements'], + 'a dataset provided by #[DataProviderClosure]', + $method_storage->suppressed_issues + + $declaring_class_storage->suppressed_issues + + $statements_source->getSuppressedIssues(), + ); + } + + $closure_return_type = DataProviderClosureHandler::returnType($method_storage, $closure['position']); + if ($closure_return_type === null) { + continue; + } + + $closure_location = $method_storage->location->setCommentLine($closure['line']); + $provided_types[] = [ + 'name' => '#[DataProviderClosure]', + 'type' => $closure_return_type, + 'location' => $closure_location, + 'return_type_location' => $closure_location, + ]; + } + + foreach ($provided_types as $provided) { + $provider_name = $provided['name']; + $provider_return_type = $provided['type']; + $provider_docblock_location = $provided['location']; + $provider_return_type_location = $provided['return_type_location']; + $provider_return_type_string = $provider_return_type->getId(); + + $expected_provider_return_type = new Type\Atomic\TIterable([ + Type::getArrayKey(), + Type::getArray(), + ]); + + $non_null_provider_return_types = []; + foreach (self::getAtomics($provider_return_type) as $type) { + // PHPUnit allows returning null from providers and treats it as an empty set + // resulting in the test being skipped + if ($type instanceof TNull) { + continue; + } + + if (!$type->isIterable($codebase)) { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + continue 2; + } + $non_null_provider_return_types[] = $type; + } + + if ([] === $non_null_provider_return_types) { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + continue; + } + + // unionize iterable so that instead of array|Traversable + // we get iterable + // + // TODO: this may get implemented in a future Psalm version, remove it then + $provider_return_type = self::unionizeIterables( + $codebase, + new Type\Union($non_null_provider_return_types) + ); + + $provider_key_type_is_compatible = $codebase->isTypeContainedByType( + $provider_return_type->type_params[0], + $expected_provider_return_type->type_params[0] + ); + + if (!$provider_key_type_is_compatible) { + // XXX: isn't array key allowed by the $expected_provider_return_type ? + $provider_key_type_is_uncertain = + $provider_return_type->type_params[0]->hasMixed() + || $provider_return_type->type_params[0]->hasArrayKey(); + + if ($provider_key_type_is_uncertain) { + IssueBuffer::accepts(new Issue\MixedReturnStatement( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', possibly different ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } else { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } + continue; + } + + $provider_value_type_is_compatible = $codebase->isTypeContainedByType( + $provider_return_type->type_params[1], + $expected_provider_return_type->type_params[1] + ); + + if (!$provider_value_type_is_compatible) { + if ($provider_return_type->type_params[1]->hasMixed()) { + IssueBuffer::accepts(new Issue\MixedReturnStatement( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', possibly different ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } else { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } + continue; + } + + // An empty array has no datasets to check + if ($provider_return_type->type_params[1]->isNever()) { + continue; + } + + $checkParam = + static function ( + Type\Union $potential_argument_type, + Type\Union $param_type, + bool $is_optional, + int $param_offset, + ?string $param_name = null + ) use ( + $codebase, + $method_name, + $provider_name, + $provider_return_type_string, + $provider_docblock_location + ): void { + $argument = 'Argument ' . ($param_offset + 1) + . ($param_name === null ? '' : ' ($' . $param_name . ')'); + if ($is_optional) { + $param_type = $param_type->setPossiblyUndefined(true); + } + if ($codebase->isTypeContainedByType($potential_argument_type, $param_type)) { + // ok + } elseif ($codebase->canTypeBeContainedByType($potential_argument_type, $param_type)) { + IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( + $argument . ' of ' . $method_name + . ' expects ' . $param_type->getId() . ', ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } elseif ($potential_argument_type->possibly_undefined && !$is_optional) { + IssueBuffer::accepts(new Issue\InvalidArgument( + $argument . ' of ' . $method_name + . ' is not optional, but possibly undefined ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } else { + IssueBuffer::accepts(new Issue\InvalidArgument( + $argument . ' of ' . $method_name + . ' expects ' . $param_type->getId() . ', ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } + }; + + /** @var Type\Atomic\TArray|Type\Atomic\TKeyedArray $dataset_type */ + $dataset_type = self::getAtomics($provider_return_type->type_params[1])['array']; + + if ($dataset_type instanceof Type\Atomic\TArray) { + // The dataset length is unknown, so assume the trailing params receive the return values + // of depended-upon tests, unless a variadic param could consume them. + $last_param = array_slice($method_storage->params, -1)[0] ?? null; + $provider_params = array_slice( + $method_storage->params, + 0, + $last_param !== null && $last_param->is_variadic + ? null + : max(0, count($method_storage->params) - $dependency_values), + ); + // check that all of the required (?) params accept value type + $potential_argument_type = $dataset_type->type_params[1]; + foreach ($provider_params as $param_offset => $param) { + if (!$param->type) { + continue; + } + $checkParam($potential_argument_type, $param->type, $param->is_optional, $param_offset); + } + } elseif (self::hasStringKeys($dataset_type)) { + $provided_by = $provider_name . ':(' . $provider_return_type_string . ')'; + $binding = DatasetBinding::bind($dataset_type, $method_storage->params, $codebase); + + foreach ($binding->arguments as $argument) { + $checkParam( + $argument->type, + $argument->param_type, + $argument->is_optional, + $argument->position, + $argument->name, + ); + } + + foreach ($binding->unknown as $name) { + IssueBuffer::accepts(new Issue\InvalidNamedArgument( + 'Parameter $' . $name . ' does not exist on ' . $method_name + . ', provided by ' . $provided_by, + $provider_docblock_location, + $method_name + )); + } + + foreach ($binding->duplicate as $name) { + IssueBuffer::accepts(new Issue\InvalidNamedArgument( + 'Parameter $' . $name . ' has already been used in ' . $method_name + . ', provided by ' . $provided_by, + $provider_docblock_location, + $method_name + )); + } + + if ($appends_dependency_value && self::hasStringKeys($dataset_type, required_only: true)) { + IssueBuffer::accepts(new Issue\InvalidNamedArgument( + 'Cannot use positional argument after named argument in ' . $method_name + . ': PHPUnit appends the return value of a depended-upon test to dataset ' . $provided_by, + $provider_docblock_location, + $method_name + )); + + continue; + } + + // the return values of depended-upon tests fill the first parameters the dataset leaves open + $missing = array_slice($binding->missing, $dependency_values); + if ($missing !== []) { + IssueBuffer::accepts(new Issue\TooFewArguments( + 'Too few arguments for ' . $method_name . ' - parameter $' . $missing[0] + . ' is not provided by ' . $provided_by, + $provider_docblock_location, + $method_name + )); + } + } else { // TKeyedArray + // iterate over all params checking if corresponding value type is acceptable + // let's hope properties are sorted in array order + $potential_argument_types = array_values($dataset_type->properties); + + foreach ($method_storage->params as $param_offset => $param) { + if (!isset($potential_argument_types[$param_offset])) { + // variadics are never required + // and they always come last + if ($param->is_variadic) { + break; + } + // reached default params, so it's fine, but let's continue + // because MisplacedRequiredParam could be suppressed + if ($param->is_optional) { + continue; + } + // the return values of depended-upon tests follow the provider values + if ($param_offset < $dataset_type->getMinCount() + $dependency_values) { + continue; + } + + IssueBuffer::accepts(new Issue\TooFewArguments( + 'Too few arguments for ' . $method_name + . ' - expecting at least ' . ($param_offset + 1 - $dependency_values) + . ', but saw ' . $dataset_type->getMinCount() + . ' provided by ' . $provider_name + . ':(' . $provider_return_type_string . ')', + $provider_docblock_location, + $method_name + )); + break; + } + $potential_argument_type = $potential_argument_types[$param_offset]; + + $param_type = $param->type === null ? Type::getMixed() : $param->type; + if ($param->is_variadic) { + $param_types = self::getAtomics($param_type); + $variadic_param_type = new Type\Union(array_values($param_types)); + + // check remaining argument types + for (; $param_offset < count($potential_argument_types); $param_offset++) { + $potential_argument_type = $potential_argument_types[$param_offset]; + $checkParam( + $potential_argument_type, + $variadic_param_type, + true, + $param_offset + ); + } + break; + } + + $checkParam($potential_argument_type, $param_type, $param->is_optional, $param_offset); + } + } + } + } + } + + /** + * Checks the providers of a test inherited from an ancestor that the class implements or overrides itself. + * + * @param lowercase-string $test_name_lc + */ + private static function checkInheritedTestProviders( + Codebase $codebase, + StatementsSource $statements_source, + ClassLikeStorage $class_storage, + string $test_name_lc, + MethodStorage $test_storage + ): void { + $metadata = $test_storage->custom_metadata[DataProviderNarrowingHandler::METADATA_KEY] ?? null; + if ( + $class_storage->parent_class === null + || !is_array($metadata) + || !((bool) $metadata['test'] || str_starts_with($test_name_lc, 'test')) + ) { + return; + } + + /** @var list $providers */ + $providers = $metadata['providers']; + foreach ($providers as [$provider_class, $provider_name]) { + if ($provider_class !== '' || str_contains($provider_name, '::')) { + continue; + } + + // The parent already checked the provider it resolves + $provider_method_id = $codebase->getDeclaringMethodId($class_storage->name . '::' . $provider_name); + if ( + $provider_method_id === null + || $provider_method_id === $codebase->getDeclaringMethodId( + $class_storage->parent_class . '::' . $provider_name + ) + ) { + continue; + } + + $provider_id = MethodIdentifier::wrap($provider_method_id); + $provider_storage = $codebase->methods->getStorage($provider_id); + if ($provider_storage->abstract || $provider_storage->location === null) { + continue; + } + + // Keys are source offsets, which array_merge() would renumber + $suppressed_issues = $provider_storage->suppressed_issues + + $codebase->classlike_storage_provider->get($provider_id->fq_class_name)->suppressed_issues; + // The analysed class's offsets mean nothing in a trait's file + if ($provider_storage->location->file_path === $statements_source->getFilePath()) { + $suppressed_issues += $class_storage->suppressed_issues + $statements_source->getSuppressedIssues(); + } + + self::reportInvalidProviderSignature( + $codebase, + $provider_storage, + $class_storage->name . '::' . $provider_name, + $provider_storage->location, + $suppressed_issues, + ); + } + } + + /** @param array $suppressed_issues */ + private static function reportInvalidProviderSignature( + Codebase $codebase, + MethodStorage $provider_method_storage, + string $apparent_provider_method_name, + CodeLocation $location, + array $suppressed_issues + ): void { + $provider_visibility = $provider_method_storage->visibility; + $appearing_provider_id = $codebase->methods->getAppearingMethodId( + MethodIdentifier::wrap($apparent_provider_method_name) + ); + if (null !== $appearing_provider_id) { + $provider_visibility = $codebase->classlike_storage_provider + ->get($appearing_provider_id->fq_class_name) + ->trait_visibility_map[$appearing_provider_id->method_name] + ?? $provider_visibility; + } + + if (ClassLikeAnalyzer::VISIBILITY_PUBLIC !== $provider_visibility) { + IssueBuffer::accepts(new InvalidDataProvider( + 'Data provider method ' . $apparent_provider_method_name . ' must be public', + $location + ), $suppressed_issues); + } + + if (!$provider_method_storage->is_static) { + IssueBuffer::accepts(new InvalidDataProvider( + 'Data provider method ' . $apparent_provider_method_name . ' must be static', + $location + ), $suppressed_issues); + } + } + + /** + * PHPUnit passes string dataset keys as named arguments, and PHP rejects a positional argument after them. + * Only literal datasets are read: Psalm's array shapes do not keep the key order. + * + * @param array $statements the body of a provider method or closure + * @param array $suppressed_issues + */ + private static function reportPositionalAfterNamed( + StatementsSource $source, + array $statements, + string $provided_by, + array $suppressed_issues + ): void { + $visitor = new class extends NodeVisitorAbstract { + public bool $generator = false; + + /** @var list */ + public array $returned = []; + + /** @var list */ + public array $yielded = []; + + #[\Override] + public function enterNode(Node $node) + { + if ($node instanceof Node\FunctionLike || $node instanceof ClassLike) { + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + + $sets = $node instanceof Return_ || $node instanceof YieldFrom ? $node->expr : null; + foreach ($sets instanceof Array_ ? $sets->items : [] as $item) { + if (!$item->unpack && $item->value instanceof Array_) { + if ($node instanceof Return_) { + $this->returned[] = $item->value; + } else { + $this->yielded[] = $item->value; + } + } + } + + if ($node instanceof YieldFrom || $node instanceof Yield_) { + $this->generator = true; + } + + if ($node instanceof Yield_ && $node->value instanceof Array_) { + $this->yielded[] = $node->value; + } + + return null; + } + }; + (new NodeTraverser($visitor))->traverse($statements); + + // PHPUnit ignores the return value of a generator + foreach ($visitor->generator ? $visitor->yielded : $visitor->returned as $dataset) { + $named = false; + $violated = false; + $seen = []; + $next = 0; + foreach ($dataset->items as $item) { + $key = $item->key; + if ($item->unpack || ($key !== null && !$key instanceof String_ && !$key instanceof Int_)) { + continue 2; + } + + // PHP casts keys like '1' to int + if ($key instanceof String_ && (string) (int) $key->value !== $key->value) { + $named = true; + continue; + } + + $position = $key === null ? $next : (int) $key->value; + $next = max($next, $position + 1); + + // An array keeps the place of a key's first insertion, so overwriting a positional key is harmless + if ($named && !isset($seen[$position])) { + $violated = true; + } + + $seen[$position] = true; + } + + if ($violated) { + IssueBuffer::accepts(new Issue\InvalidNamedArgument( + 'Cannot use positional argument after named argument in ' . $provided_by, + new CodeLocation($source, $dataset) + ), $suppressed_issues); + } + } + } + + /** @psalm-pure */ + public static function hasStringKeys(Type\Atomic\TKeyedArray $dataset_type, bool $required_only = false): bool + { + foreach ($dataset_type->properties as $key => $type) { + if (is_string($key) && !($required_only && $type->possibly_undefined)) { + return true; + } + } + + return false; + } + + /** + * @return non-empty-array + * + * @psalm-pure + */ + private static function getAtomics(Type\Union $union): array + { + return $union->getAtomicTypes(); + } + + public static function unionizeIterables(Codebase $codebase, Type\Union $iterables): Type\Atomic\TIterable + { + /** @var Type\Union[] $key_types */ + $key_types = []; + + /** @var Type\Union[] $value_types */ + $value_types = []; + + foreach (self::getAtomics($iterables) as $type) { + if (!$type->isIterable($codebase)) { + throw new RuntimeException('should be iterable'); + } + + if ($type instanceof Type\Atomic\TArray) { + $key_types[] = $type->type_params[0] ?? Type::getMixed(); + $value_types[] = $type->type_params[1] ?? Type::getMixed(); + } elseif ($type instanceof Type\Atomic\TKeyedArray) { + $key_types[] = $type->getGenericKeyType(); + $value_types[] = $type->getGenericValueType(); + } elseif ($type instanceof Type\Atomic\TNamedObject || $type instanceof Type\Atomic\TIterable) { + [$key_types[], $value_types[]] = $codebase->getKeyValueParamsForTraversableObject($type); + } else { + throw new RuntimeException('unexpected type'); + } + } + + $combine = + /** + * @param null|Type\Union $a + * + * @psalm-external-mutation-free + */ + static function ($a, Type\Union $b) use ($codebase): Type\Union { + return $a ? Type::combineUnionTypes($a, $b, $codebase) : $b; + }; + + return new Type\Atomic\TIterable([ + array_reduce($key_types, $combine), + array_reduce($value_types, $combine), + ]); + } + + + private static function hasInitializers(ClassLikeStorage $storage, ClassLike $stmt, Aliases $aliases): bool + { + if (isset($storage->methods['setup'])) { + return true; + } + + foreach ($storage->methods as $method => $_) { + $stmt_method = $stmt->getMethod($method); + if (!$stmt_method) { + continue; + } + if (self::isBeforeInitializer($stmt_method, $aliases)) { + return true; + } + } + return false; + } + + private static function isBeforeInitializer(ClassMethod $method, Aliases $aliases): bool + { + return isset(self::getSpecials($method, $aliases)['before']); + } + + /** + * PHPUnit 12 ignores docblock metadata altogether. + * + * @return array> Each entry is [file line, value] + */ + private static function docblockSpecials(ClassMethod $method): array + { + return PhpUnitVersion::ignoresDocblockMetadata() ? [] : DocblockSpecials::of($method); + } + + /** @return array> Each entry is [file line, value] */ + private static function getSpecials(ClassMethod $method, Aliases $aliases): array + { + return array_merge( + self::docblockSpecials($method), + // Attributes take priority over docblocks + self::getAttributeSpecials($method, $aliases), + ); + } + + /** + * @template T of object + * @param class-string $attributeClass + * @return list|null + */ + private static function attributeValue(ClassMethod $method, Aliases $aliases, string $attributeClass): array|null + { + $onlyStringLiteralExpressions = static fn (Attribute $attribute): array => array_map( + static fn(String_ $string): array => [$string->getStartLine(), $string->value], + array_filter( + array_column($attribute->args, 'value'), + // For our purposes, we only care about string literals: everything else is currently out of scope. + // If you need more complex expressions supported, add a constant expression evaluator here. + static fn(Expr $expression): bool => $expression instanceof String_, + ), + ); + $attributesInGroupMatchingRequestedAttributeName = static fn(AttributeGroup $group): array => array_filter( + $group->attrs, + static fn(Attribute $attribute): bool => $attributeClass === Type::getFQCLNFromString( + // toCodeString() keeps the leading backslash toString() strips, so fully-qualified attributes resolve. + $attribute->name->toCodeString(), + $aliases, + ), + ); + $matchingAttributes = array_merge(...array_values(array_map( + $attributesInGroupMatchingRequestedAttributeName, + $method->getAttrGroups(), + ))); + + if ($matchingAttributes === []) { + return null; + } + + return array_merge(...array_values(array_map( + $onlyStringLiteralExpressions, + $matchingAttributes, + ))); + } + + /** + * Attributes take priority over docblocks, like in PHPUnit's ParserChain. + * + * $aliases resolve the attribute names and the arguments of #[DataProviderExternal]. The "Class::method" + * references of docblock tags are resolved with $reference_aliases (of the analysed file). + * + * @return list + * class is null for a provider of the test's own class + */ + private static function getDataProviders( + ClassMethod $method, + Aliases $aliases, + Aliases $reference_aliases, + ClassLikeStorage $class + ): array { + return self::getAttributeDataProviders($method, $aliases, $class) + ?? array_map( + static fn(array $entry): array => self::parseProviderReference( + $entry[0], + $entry[1], + $reference_aliases, + ), + self::docblockSpecials($method)['dataProvider'] ?? [], + ); + } + + /** + * #[DataProvider] and #[DataProviderExternal], or null when the method has neither. A method with only a + * #[DataProviderClosure] has none of these providers, but is not left to its docblock either + * (see getClosureProviders()). + * + * @return list|null + */ + private static function getAttributeDataProviders( + ClassMethod $method, + Aliases $aliases, + ClassLikeStorage $class + ): array|null { + $references = self::attributeValue($method, $aliases, DataProvider::class); + $has_attributes = $references !== null; + // PHPUnit takes a #[DataProvider] argument literally as a method name of the test class + $providers = array_map( + static fn(array $entry): array => [ + 'line' => $entry[0], + 'class' => null, + 'method' => $entry[1], + 'external' => false, + ], + $references ?? [], + ); + + foreach ($method->getAttrGroups() as $group) { + foreach ($group->attrs as $attribute) { + $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); + if ($attribute_class === self::DATA_PROVIDER_CLOSURE) { + $has_attributes = true; + } + + if ($attribute_class !== DataProviderExternal::class) { + continue; + } + + $has_attributes = true; + $args = self::attributeArguments($attribute); + + $class_name = self::classFromExpression($args['className'] ?? $args[0] ?? null, $aliases, $class); + $method_name = $args['methodName'] ?? $args[1] ?? null; + if ($class_name === null || !$method_name instanceof String_) { + continue; + } + + $providers[] = [ + 'line' => $attribute->getStartLine(), + 'class' => $class_name, + 'method' => $method_name->value, + 'external' => true, + ]; + } + } + + return $has_attributes ? $providers : null; + } + + /** + * The closure of each #[DataProviderClosure] whose argument is a literal closure, by the start offset that + * DataProviderClosureHandler keys its return type by. + * + * @return list}> + */ + private static function getClosureProviders(ClassMethod $method, Aliases $aliases): array + { + $closures = []; + foreach ($method->getAttrGroups() as $group) { + foreach ($group->attrs as $attribute) { + $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); + if ($attribute_class !== self::DATA_PROVIDER_CLOSURE) { + continue; + } + + $args = self::attributeArguments($attribute); + $closure = $args['closure'] ?? $args[0] ?? null; + if ($closure instanceof Expr\Closure || $closure instanceof Expr\ArrowFunction) { + $closures[] = [ + 'line' => $attribute->getStartLine(), + 'position' => $closure->getStartFilePos(), + 'statements' => $closure instanceof Expr\Closure + ? $closure->stmts + : [new Return_($closure->expr)], + ]; + } + } + } + + return $closures; + } + + /** + * "Class::method" or "method" (optionally with trailing parentheses), as written in a dataProvider + * docblock tag. + * + * @psalm-mutation-free + * + * @return array{line: int, class: string|null, method: string, external: bool} + */ + private static function parseProviderReference(int $line, string $reference, Aliases $aliases): array + { + $parts = explode('::', $reference, 2); + $method = preg_replace('/\(\s*\)$/', '', $parts[1] ?? $parts[0]); + assert($method !== null); + + return [ + 'line' => $line, + 'class' => isset($parts[1]) ? Type::getFQCLNFromString($parts[0], $aliases) : null, + 'method' => $method, + 'external' => false, + ]; + } + + /** + * PHPUnit appends the return value of every distinct depended-upon test that is not declared void or never + * to the data provider values. Depending on a whole class passes no value. + */ + private static function countDependencyValues( + Codebase $codebase, + ClassMethod $method, + Aliases $aliases, + ClassLikeStorage $class + ): int { + $count = 0; + $seen = []; + foreach (self::getDependencyTargets($method, $aliases, $class) as $target) { + if ($target !== null) { + if (isset($seen[strtolower($target)])) { + continue; + } + $seen[strtolower($target)] = true; + + if (self::dependencyReturnsValue($codebase, $target) === false) { + continue; + } + } + $count++; + } + + return $count; + } + + /** + * Up to PHPUnit 12.5.39 and 13.4.2 (and on 11.5) only the first dependency counts: collecting stops at the first + * void dependency and at the first one larger than this test. A test of the same class shares its class-level + * size, so only size groups in the two method docblocks (PHPUnit 11) can make it larger. Newer releases skip + * both and pass the value of any later dependency. + */ + private static function appendsDependencyValue( + Codebase $codebase, + ClassLike $class_node, + ClassLikeStorage $class, + ClassMethod $method, + Aliases $aliases + ): bool { + $targets = self::getDependencyTargets($method, $aliases, $class); + + if (!PhpUnitVersion::stopsCollectingDependencyValuesEarly()) { + foreach ($targets as $target) { + if ($target !== null && self::dependencyReturnsValue($codebase, $target) === true) { + return true; + } + } + + return false; + } + + $first = $targets[0] ?? null; + if ($first === null || self::dependencyReturnsValue($codebase, $first) !== true) { + return false; + } + + [$target_class, $target_method] = explode('::', $first); + $target_node = strtolower($target_class) === strtolower($class->name) + ? $class_node->getMethod($target_method) + : null; + if ($target_node === null) { + return false; + } + + $size_group = '/@(?:(?:group|ticket)\s+)?(?:small|medium|large)\b/'; + foreach ([$method->getDocComment(), $target_node->getDocComment()] as $docblock) { + if ($docblock !== null && preg_match($size_group, $docblock->getText()) === 1) { + return false; + } + } + + return true; + } + + /** + * @return bool|null null when the target method cannot be resolved + */ + private static function dependencyReturnsValue(Codebase $codebase, string $target): ?bool + { + $target_class = explode('::', $target)[0]; + $target_id = $codebase->classOrInterfaceExists($target_class) + ? $codebase->getDeclaringMethodId($target) + : null; + if ($target_id === null) { + return null; + } + + $return_type = $codebase->methods->getStorage(MethodIdentifier::wrap($target_id))->signature_return_type; + + return $return_type === null || !($return_type->isVoid() || $return_type->isNever()); + } + + /** + * getAttributeDataProviders() leaves out the attributes it cannot read, such as #[DataProvider(self::NAME)] and + * #[DataProviderClosure], which is named by string as not every supported PHPUnit version has it. + */ + private static function countProviderAttributes(ClassMethod $method, Aliases $aliases): int + { + $provider_attributes = [ + DataProvider::class, + DataProviderExternal::class, + self::DATA_PROVIDER_CLOSURE, + ]; + $count = 0; + foreach ($method->getAttrGroups() as $group) { + foreach ($group->attrs as $attribute) { + $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); + if (in_array($attribute_class, $provider_attributes, true)) { + $count++; + } + } + } + + return $count; + } + + /** + * PHPUnit's ParserChain ignores docblock annotations on a class or method that has any PHPUnit attribute. + * + * @internal + */ + public static function hasPhpUnitAttribute(ClassLike|ClassMethod $node, Aliases $aliases): bool + { + foreach ($node->attrGroups as $group) { + foreach ($group->attrs as $attribute) { + $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); + if (str_starts_with($attribute_class, 'PHPUnit\\Framework\\Attributes\\')) { + return true; + } + } + } + + return false; + } + + /** + * Mirrors PHPUnit's ParserChain: docblock annotations are only read when the method has no PHPUnit attribute. + * + * @return list "Class::method" per method dependency, null when the target is not a literal + */ + private static function getDependencyTargets(ClassMethod $method, Aliases $aliases, ClassLikeStorage $class): array + { + $has_phpunit_attribute = self::hasPhpUnitAttribute($method, $aliases); + $targets = []; + foreach ($method->getAttrGroups() as $group) { + foreach ($group->attrs as $attribute) { + $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); + if (!isset(self::DEPENDS_ATTRIBUTES[$attribute_class])) { + continue; + } + + $args = self::attributeArguments($attribute); + + $is_external = self::DEPENDS_ATTRIBUTES[$attribute_class]; + $target_name = $args['methodName'] ?? $args[$is_external ? 1 : 0] ?? null; + $target_class = $is_external + ? self::classFromExpression($args['className'] ?? $args[0] ?? null, $aliases, $class) + : $class->name; + if ($target_name instanceof String_ && $target_name->value === 'class') { + continue; // PHPUnit treats this as a class dependency, which passes no value + } + + $targets[] = $target_class !== null && $target_name instanceof String_ + ? $target_class . '::' . $target_name->value + : null; + } + } + + if ($has_phpunit_attribute) { + return $targets; + } + + foreach (self::docblockSpecials($method)['depends'] ?? [] as [, $value]) { + $is_target = preg_match('/^(?:!?(?:clone|shallowClone)\s+)?(\S+)/', $value, $matches) === 1; + if (!$is_target || str_ends_with(strtolower($matches[1]), '::class')) { + continue; + } + + $target = $matches[1]; + $targets[] = str_contains($target, '::') ? ltrim($target, '\\') : $class->name . '::' . $target; + } + + return $targets; + } + + /** + * @return array named arguments by name, the others by position + * @psalm-mutation-free + * @internal + */ + public static function attributeArguments(Attribute $attribute): array + { + $args = []; + foreach ($attribute->args as $position => $arg) { + $args[$arg->name?->toString() ?? $position] = $arg->value; + } + + return $args; + } + + /** @internal */ + public static function classFromExpression( + Expr|null $expression, + Aliases $aliases, + ClassLikeStorage $class + ): string|null { + if ($expression instanceof String_) { + return ltrim($expression->value, '\\'); + } + + $is_class_fetch = $expression instanceof ClassConstFetch + && $expression->class instanceof Name + && $expression->name instanceof Identifier + && $expression->name->toLowerString() === 'class'; + if (!$is_class_fetch) { + return null; + } + + if ($expression->class->isSpecialClassName()) { + return $expression->class->toLowerString() === 'parent' ? $class->parent_class : $class->name; + } + + return Type::getFQCLNFromString($expression->class->toCodeString(), $aliases); + } + /** @return array> */ + private static function getAttributeSpecials(ClassMethod $method, Aliases $aliases): array + { + return array_filter([ + 'before' => self::attributeValue($method, $aliases, Before::class), + 'test' => self::attributeValue($method, $aliases, Test::class), + ], static fn(array|null $special): bool => $special !== null); + } + + /** @param array $suppressed_issues */ + private static function reportMalformedTags( + AfterClassLikeAnalysisEvent $event, + ClassLike|ClassMethod $node, + CodeLocation $location, + array $suppressed_issues, + ): void { + // Nothing is read from a docblock on PHPUnit 12, malformed or not + if (PhpUnitVersion::ignoresDocblockMetadata()) { + return; + } + + // Suppression keys are source offsets: those of the analysed class mean nothing in another file + $statements_source = $event->getStatementsSource(); + if ($location->file_path === $statements_source->getFilePath()) { + $suppressed_issues += $event->getClasslikeStorage()->suppressed_issues + + $statements_source->getSuppressedIssues(); + } + + foreach (DocblockSpecials::malformed($node) as ['tag' => $tag, 'line' => $line, 'problem' => $problem]) { + IssueBuffer::accepts(new InvalidPhpUnitAnnotation( + 'Annotation "@' . $tag . '" ' . $problem, + $location->setCommentLine($line), + ), $suppressed_issues); + } + } +} diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..3f21035 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,113 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: An iterable provider without a key type has its datasets checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield [1]; + } + + #[DataProvider('provide')] + public function testSomething(int $i): void { + $this->assertSame(1, $i); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: An iterable provider without a key type reports datasets that do not match the test + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield [1]; + } + + #[DataProvider('provide')] + public function testSomething(string $s): void { + $this->assertSame('', $s); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | Line | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]) | 17 | + And I see no other errors + + Scenario: A generator provider without a key type has its datasets checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return \Generator + * @psalm-pure + */ + public static function provide(): \Generator { + yield [1]; + } + + #[DataProvider('provide')] + public function testSomething(int $i): void { + $this->assertSame(1, $i); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: An IteratorAggregate provider without a key type has its datasets checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** + * @implements \IteratorAggregate + * @psalm-immutable + */ + final class Datasets implements \IteratorAggregate { + #[\Override] + public function getIterator(): \Iterator { + return new \ArrayIterator([[1]]); + } + } + + final class MyTestCase extends TestCase + { + /** + * @return Datasets + * @psalm-pure + */ + public static function provide(): Datasets { + return new Datasets(); + } + + #[DataProvider('provide')] + public function testSomething(int $i): void { + $this->assertSame(1, $i); + } + } + """ + When I run Psalm + Then I see no errors From dfbb7d5bfc73961eb578318a98d72f0e84d460c0 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:04:07 +0100 Subject: [PATCH 05/24] fix(provider): expand type aliases of test params in dataset checks #249 --- src/Hooks/TestCaseHandler.php | 39 ++++++- tests/acceptance/TestCase.feature | 187 ++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 5 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..93904cd 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -39,6 +39,7 @@ use Psalm\CodeLocation; use Psalm\Internal\Analyzer\ClassLikeAnalyzer; use Psalm\Internal\MethodIdentifier; +use Psalm\Internal\Type\TypeExpander; use Psalm\IssueBuffer; use Psalm\Issue; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; @@ -56,6 +57,7 @@ use Psalm\Plugin\EventHandler\Event\AfterMethodCallAnalysisEvent; use Psalm\StatementsSource; use Psalm\Storage\ClassLikeStorage; +use Psalm\Storage\FunctionLikeParameter; use Psalm\Storage\MethodStorage; use Psalm\Type; use Psalm\Type\Atomic\TNull; @@ -512,6 +514,9 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ]; } + // Test parameters may use type aliases, which a provider's datasets are never typed with + $params = self::expandParams($codebase, $method_storage->params, $class_storage); + foreach ($provided_types as $provided) { $provider_name = $provided['name']; $provider_return_type = $provided['type']; @@ -669,13 +674,13 @@ static function ( if ($dataset_type instanceof Type\Atomic\TArray) { // The dataset length is unknown, so assume the trailing params receive the return values // of depended-upon tests, unless a variadic param could consume them. - $last_param = array_slice($method_storage->params, -1)[0] ?? null; + $last_param = array_slice($params, -1)[0] ?? null; $provider_params = array_slice( - $method_storage->params, + $params, 0, $last_param !== null && $last_param->is_variadic ? null - : max(0, count($method_storage->params) - $dependency_values), + : max(0, count($params) - $dependency_values), ); // check that all of the required (?) params accept value type $potential_argument_type = $dataset_type->type_params[1]; @@ -687,7 +692,7 @@ static function ( } } elseif (self::hasStringKeys($dataset_type)) { $provided_by = $provider_name . ':(' . $provider_return_type_string . ')'; - $binding = DatasetBinding::bind($dataset_type, $method_storage->params, $codebase); + $binding = DatasetBinding::bind($dataset_type, $params, $codebase); foreach ($binding->arguments as $argument) { $checkParam( @@ -743,7 +748,7 @@ static function ( // let's hope properties are sorted in array order $potential_argument_types = array_values($dataset_type->properties); - foreach ($method_storage->params as $param_offset => $param) { + foreach ($params as $param_offset => $param) { if (!isset($potential_argument_types[$param_offset])) { // variadics are never required // and they always come last @@ -798,6 +803,30 @@ static function ( } } + /** + * @param list $params + * @return list + */ + private static function expandParams(Codebase $codebase, array $params, ClassLikeStorage $class): array + { + $expanded = []; + foreach ($params as $param) { + $param = clone $param; + if ($param->type !== null) { + $param->type = TypeExpander::expandUnion( + $codebase, + $param->type, + $class->name, + $class->name, + $class->parent_class, + ); + } + $expanded[] = $param; + } + + return $expanded; + } + /** * Checks the providers of a test inherited from an ancestor that the class implements or overrides itself. * diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..71a5828 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,190 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: An imported type alias in a test parameter is expanded in the provider check + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** @psalm-type Options = array{host: string} */ + final class Connection {} + + /** @psalm-import-type Options from Connection */ + final class MyTestCase extends TestCase { + /** + * @return list}> + * @psalm-pure + */ + public static function provide(): array { + return [[['host' => 'x']]]; + } + + /** + * @param Options $options + */ + #[DataProvider('provide')] + public function testAlias(array $options): void { + $this->assertSame('x', $options['host']); + } + + /** + * @param array{host: string} $options + */ + #[DataProvider('provide')] + public function testShape(array $options): void { + $this->assertSame('x', $options['host']); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | Line | + | InvalidArgument | Argument 1 of NS\MyTestCase::testAlias expects array{host: string}, array provided by NS\MyTestCase::provide():(list}>) | 23 | + | InvalidArgument | Argument 1 of NS\MyTestCase::testShape expects array{host: string}, array provided by NS\MyTestCase::provide():(list}>) | 31 | + And I see no other errors + + Scenario: An imported type alias in a test parameter accepts a matching dataset + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** @psalm-type Options = array{host: string} */ + final class Connection {} + + /** @psalm-import-type Options from Connection */ + final class MyTestCase extends TestCase { + /** + * @return list + * @psalm-pure + */ + public static function provide(): array { + return [[['host' => 'x']]]; + } + + /** + * @param Options $options + */ + #[DataProvider('provide')] + public function testSomething(array $options): void { + $this->assertSame('x', $options['host']); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: An imported type alias in a test parameter is expanded for named and unknown length datasets + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** @psalm-type Options = array{host: string} */ + final class Connection {} + + /** @psalm-import-type Options from Connection */ + final class MyTestCase extends TestCase { + /** + * @return list}> + * @psalm-pure + */ + public static function named(): array { + return [['options' => ['host' => 'x']]]; + } + + /** + * @return list>> + * @psalm-pure + */ + public static function lengthless(): array { + return [[['host' => 'x']]]; + } + + /** + * @param Options $options + */ + #[DataProvider('named')] + public function testNamed(array $options): void { + $this->assertSame('x', $options['host']); + } + + /** + * @param Options $options + */ + #[DataProvider('lengthless')] + public function testLengthless(array $options): void { + $this->assertSame('x', $options['host']); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | Line | + | InvalidArgument | Argument 1 ($options) of NS\MyTestCase::testNamed expects array{host: string}, array provided by NS\MyTestCase::named():(list}>) | 31 | + | InvalidArgument | Argument 1 of NS\MyTestCase::testLengthless expects array{host: string}, array provided by NS\MyTestCase::lengthless():(list>>) | 39 | + And I see no other errors + + Scenario: A test parameter typed with an imported type alias is narrowed to the dataset type + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** @psalm-type Options = array{host: string} */ + final class Connection {} + + /** @psalm-import-type Options from Connection */ + final class MyTestCase extends TestCase { + /** + * @return list + * @psalm-pure + */ + public static function provide(): array { + return [[['host' => 'x']]]; + } + + /** + * @param Options $options + */ + #[DataProvider('provide')] + public function testSomething(array $options): void { + /** @psalm-check-type-exact $options = array{host: 'x'} */ + $this->expectNotToPerformAssertions(); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: A variadic parameter typed with an imported type alias is expanded in the provider check + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + /** @psalm-type Options = array{host: string} */ + final class Connection {} + + /** @psalm-import-type Options from Connection */ + final class MyTestCase extends TestCase { + /** + * @return list, array}> + * @psalm-pure + */ + public static function provide(): array { + return [[['host' => 'x'], ['host' => 'y']]]; + } + + /** + * @param Options ...$options + */ + #[DataProvider('provide')] + public function testSomething(array ...$options): void { + $this->assertNotEmpty($options); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | + | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | + And I see no other errors From 34b586494fc31758c94ffc3344e3a1da8d22e829 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:04:30 +0100 Subject: [PATCH 06/24] fix(tests): ignore non-public methods as tests #250 --- src/Hooks/DataProviderNarrowingHandler.php | 5 +- src/Hooks/TestCaseHandler.php | 20 +-- src/TestMethod.php | 29 ++++ tests/acceptance/TestCase.feature | 151 +++++++++++++++++++++ 4 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 src/TestMethod.php diff --git a/src/Hooks/DataProviderNarrowingHandler.php b/src/Hooks/DataProviderNarrowingHandler.php index 4a762c3..1bd9596 100644 --- a/src/Hooks/DataProviderNarrowingHandler.php +++ b/src/Hooks/DataProviderNarrowingHandler.php @@ -5,11 +5,11 @@ namespace Psalm\PhpUnitPlugin\Hooks; use Psalm\Codebase; -use Psalm\Internal\Analyzer\ClassLikeAnalyzer; use Psalm\Internal\Analyzer\MethodAnalyzer; use Psalm\Internal\MethodIdentifier; use Psalm\Internal\Type\TypeExpander; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; +use Psalm\PhpUnitPlugin\TestMethod; use Psalm\Plugin\EventHandler\BeforeStatementAnalysisInterface; use Psalm\Plugin\EventHandler\Event\BeforeStatementAnalysisEvent; use Psalm\Storage\ClassLikeStorage; @@ -159,9 +159,8 @@ private static function isTest( continue; } - $visibility = $class->trait_visibility_map[$name] ?? $method->visibility; if ( - $visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC + TestMethod::isPublic($class, $method, $name) && ($marked || str_starts_with(self::casedName($class, $method, $name), 'test')) ) { return true; diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..89af85e 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -46,6 +46,7 @@ use Psalm\PhpUnitPlugin\Issue\InvalidDataProvider; use Psalm\PhpUnitPlugin\Issue\InvalidPhpUnitAnnotation; use Psalm\PhpUnitPlugin\PhpUnitVersion; +use Psalm\PhpUnitPlugin\TestMethod; use Psalm\Plugin\EventHandler\AfterClassLikeAnalysisInterface; use Psalm\Plugin\EventHandler\AfterClassLikeVisitInterface; use Psalm\Plugin\EventHandler\AfterCodebasePopulatedInterface; @@ -317,13 +318,15 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event // The class declaring an inherited test does not see the providers its descendants implement if (!$declaring_class_storage->is_trait && $declaring_class_storage->name !== $class_storage->name) { - self::checkInheritedTestProviders( - $codebase, - $statements_source, - $class_storage, - $method_name_lc, - $method_storage, - ); + if (TestMethod::isPublic($declaring_class_storage, $method_storage, $method_name_lc)) { + self::checkInheritedTestProviders( + $codebase, + $statements_source, + $class_storage, + $method_name_lc, + $method_storage, + ); + } continue; } @@ -347,7 +350,8 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event $method_storage->suppressed_issues + $declaring_class_storage->suppressed_issues, ); - $is_test = 0 === strpos($method_name_lc, 'test') || isset($specials['test']); + $is_test = TestMethod::isPublic($class_storage, $method_storage, $method_name_lc) + && (0 === strpos($method_name_lc, 'test') || isset($specials['test'])); if (!$is_test) { continue; // skip non-test methods } diff --git a/src/TestMethod.php b/src/TestMethod.php new file mode 100644 index 0000000..05e76f0 --- /dev/null +++ b/src/TestMethod.php @@ -0,0 +1,29 @@ +trait_visibility_map[$name_lc] ?? $method->visibility; + + return $visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC; + } +} diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..4ae4722 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,154 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: The data provider of a private test-named method is not checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return list + * @psalm-pure + */ + public static function ints(): array { + return [[1]]; + } + + #[DataProvider('ints')] + private function testHelper(string $s): void { + $this->assertSame('a', $s); + } + + public function testUses(): void { + $this->testHelper('a'); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: The data provider of a protected method marked with #[Test] is not checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + use PHPUnit\Framework\Attributes\Test; + + final class MyTestCase extends TestCase + { + /** + * @return list + * @psalm-pure + */ + public static function ints(): array { + return [[1]]; + } + + #[Test] + #[DataProvider('ints')] + protected function helper(string $s): void { + $this->assertSame('a', $s); + } + + public function testUses(): void { + $this->helper('a'); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: The provider of an inherited protected test-named method is not checked against the child's provider + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + abstract class BaseTestCase extends TestCase + { + /** + * @return iterable> + * @psalm-pure + */ + abstract public function provide(); + + #[DataProvider('provide')] + protected function testHelper(int $int): void { + $this->assertSame(1, $int); + } + + public function testUses(): void { + $this->testHelper(1); + } + } + + final class MyTestCase extends BaseTestCase + { + /** + * @return iterable> + * @psalm-pure + */ + #[\Override] + public function provide() { + yield [1]; + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: The data provider of a trait test method imported as protected is not checked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + trait HelperTrait + { + /** + * @return list + * @psalm-pure + */ + public static function ints(): array { + return [[1]]; + } + + #[DataProvider('ints')] + public function testHelper(string $s): void { + $this->assertSame('a', $s); + } + } + + final class MyTestCase extends TestCase + { + use HelperTrait { + testHelper as protected; + } + + public function testUses(): void { + $this->testHelper('a'); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: An unused private test-named method is not marked as used by PHPUnit + Given I have the following code + """ + final class MyTestCase extends TestCase + { + private function testHelper(): void { + $this->assertSame(1, 1); + } + + public function testSomething(): void { + $this->assertSame(1, 1); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | UnusedMethod | Cannot find any calls to private method NS\MyTestCase::testHelper | + And I see no other errors From 46909265adc117d7386ac8c163d6e0639a07c727 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:05:29 +0100 Subject: [PATCH 07/24] fix(dataset): accept open list datasets for further parameters #247 --- src/Hooks/TestCaseHandler.php | 39 +++++- tests/acceptance/TestCase.feature | 198 ++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+), 4 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e2819c5..a9bb77d 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -743,8 +743,22 @@ static function ( // let's hope properties are sorted in array order $potential_argument_types = array_values($dataset_type->properties); + $params_count = count($method_storage->params); foreach ($method_storage->params as $param_offset => $param) { - if (!isset($potential_argument_types[$param_offset])) { + $potential_argument_type = $potential_argument_types[$param_offset] ?? null; + if ($potential_argument_type === null) { + // an open list can supply any further position + $fallback_type = self::getPositionalFallbackType($codebase, $dataset_type, $param_offset); + if ($fallback_type !== null) { + // the return values of depended-upon tests follow the provider values + if (!$param->is_variadic && $param_offset >= $params_count - $dependency_values) { + break; + } + $potential_argument_type = $fallback_type; + } + } + + if ($potential_argument_type === null) { // variadics are never required // and they always come last if ($param->is_variadic) { @@ -771,7 +785,6 @@ static function ( )); break; } - $potential_argument_type = $potential_argument_types[$param_offset]; $param_type = $param->type === null ? Type::getMixed() : $param->type; if ($param->is_variadic) { @@ -780,14 +793,17 @@ static function ( // check remaining argument types for (; $param_offset < count($potential_argument_types); $param_offset++) { - $potential_argument_type = $potential_argument_types[$param_offset]; $checkParam( - $potential_argument_type, + $potential_argument_types[$param_offset], $variadic_param_type, true, $param_offset ); } + $fallback_type = self::getPositionalFallbackType($codebase, $dataset_type, $param_offset); + if ($fallback_type !== null) { + $checkParam($fallback_type, $variadic_param_type, true, $param_offset); + } break; } @@ -986,6 +1002,21 @@ public function enterNode(Node $node) } } + /** The value type an unsealed dataset supplies at a position its known entries do not cover. */ + private static function getPositionalFallbackType( + Codebase $codebase, + Type\Atomic\TKeyedArray $dataset_type, + int $position + ): ?Type\Union { + if ($dataset_type->fallback_params === null) { + return null; + } + + [$key_type, $value_type] = $dataset_type->fallback_params; + + return $codebase->canTypeBeContainedByType(Type::getInt(false, $position), $key_type) ? $value_type : null; + } + /** @psalm-pure */ public static function hasStringKeys(Type\Atomic\TKeyedArray $dataset_type, bool $required_only = false): bool { diff --git a/tests/acceptance/TestCase.feature b/tests/acceptance/TestCase.feature index 6c9a5a0..2fda414 100644 --- a/tests/acceptance/TestCase.feature +++ b/tests/acceptance/TestCase.feature @@ -10028,3 +10028,201 @@ final class MyTestCase extends TestCase | Type | Message | | InvalidNamedArgument | Cannot use positional argument after named argument in a dataset provided by #[DataProviderClosure] | And I see no other errors + + Scenario: A non-empty-list dataset can supply any number of parameters + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a', 'b']; + } + + #[DataProvider('provide')] + public function testSomething(string $a, string $b): void { + $this->assertSame($a, $b); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: The values of an open list dataset are checked against the parameters they can fill + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [1, 2]; + } + + #[DataProvider('provide')] + public function testSomething(string $a, string $b): void { + $this->assertSame($a, $b); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]>) | + | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]>) | + And I see no other errors + + Scenario: A sealed list dataset still cannot supply more parameters than it has entries + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a']; + } + + #[DataProvider('provide')] + public function testSomething(string $a, string $b): void { + $this->assertSame($a, $b); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | TooFewArguments | Too few arguments for NS\MyTestCase::testSomething - expecting at least 2, but saw 1 provided by NS\MyTestCase::provide():(iterable[pure]) | + And I see no other errors + + Scenario: A possibly empty list dataset is reported for a required first parameter only + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a', 'b']; + } + + #[DataProvider('provide')] + public function testSomething(string $a, string $b): void { + $this->assertSame($a, $b); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething is not optional, but possibly undefined string provided by NS\MyTestCase::provide():(iterable[pure]>) | + And I see no other errors + + Scenario: The values of an open list dataset are checked against a variadic parameter + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable> + * @psalm-pure + */ + public static function ok(): iterable { + yield 'x' => ['a', 'b', 'c']; + } + + /** + * @return iterable> + * @psalm-pure + */ + public static function wrong(): iterable { + yield 'x' => [1, 2, 3]; + } + + #[DataProvider('ok')] + public function testOk(string $a, string ...$rest): void { + $this->assertSame($a, $rest[0] ?? $a); + } + + #[DataProvider('wrong')] + public function testWrong(string $a, string ...$rest): void { + $this->assertSame($a, $rest[0] ?? $a); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testWrong expects string, int provided by NS\MyTestCase::wrong():(iterable[pure]>) | + | InvalidArgument | Argument 2 of NS\MyTestCase::testWrong expects string, int provided by NS\MyTestCase::wrong():(iterable[pure]>) | + And I see no other errors + + Scenario: The values of an open list dataset are checked against optional parameters + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a', 'b']; + } + + #[DataProvider('provide')] + public function testSomething(string $a, int $b = 1): void { + $this->assertSame($a, (string) $b); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects int, string provided by NS\MyTestCase::provide():(iterable[pure]>) | + And I see no other errors + + Scenario: An open list dataset leaves the last parameters to the values of depended-upon tests + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + use PHPUnit\Framework\Attributes\Depends; + + final class MyTestCase extends TestCase { + public function testOne(): int { + $this->assertTrue(true); + return 1; + } + + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a', 'b']; + } + + #[DataProvider('provide')] + #[Depends('testOne')] + public function testSecond(string $a, string $b, int $one): void { + $this->assertSame($a . $b, (string) $one); + } + } + """ + When I run Psalm + Then I see no errors From 04c337eb937b77a1790f4fa6c78152e372ee8cdd Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:12:32 +0100 Subject: [PATCH 08/24] refactor(dead-code): simplify hook detection, cover attribute precedence #251 --- src/Hooks/TestCaseHandler.php | 29 +++++++---------------------- tests/acceptance/DeadCode.feature | 28 ++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index b3eb236..238d749 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -100,21 +100,6 @@ final class TestCaseHandler implements DependsExternalUsingShallowClone::class => true, ]; - /** - * The hook attributes, which are also the names of the PHPUnit 11 docblock tags => whether PHPUnit only runs - * them on static methods. - * - * @var array - */ - private const HOOKS = [ - 'before' => false, - 'after' => false, - 'preCondition' => false, - 'postCondition' => false, - 'beforeClass' => true, - 'afterClass' => true, - ]; - private const DATA_PROVIDER_CLOSURE = 'PHPUnit\\Framework\\Attributes\\DataProviderClosure'; private const EXPECT_EXCEPTION_METHODS = [ @@ -1522,6 +1507,7 @@ public static function classFromExpression( return Type::getFQCLNFromString($expression->class->toCodeString(), $aliases); } + /** PHPUnit ignores a docblock tag on a method that has any PHPUnit attribute. */ private static function isHook(ClassMethod $method, Aliases $aliases, MethodStorage $storage): bool { @@ -1529,13 +1515,12 @@ private static function isHook(ClassMethod $method, Aliases $aliases, MethodStor ? self::getAttributeSpecials($method, $aliases) : self::docblockSpecials($method); - foreach (self::HOOKS as $hook => $static_only) { - if (isset($specials[$hook]) && (!$static_only || $storage->is_static)) { - return true; - } - } - - return false; + // PHPUnit runs the class-level hooks only when they are static + return isset($specials['before']) + || isset($specials['after']) + || isset($specials['preCondition']) + || isset($specials['postCondition']) + || ($storage->is_static && (isset($specials['beforeClass']) || isset($specials['afterClass']))); } /** @return array> */ diff --git a/tests/acceptance/DeadCode.feature b/tests/acceptance/DeadCode.feature index 71b666b..9098f4b 100644 --- a/tests/acceptance/DeadCode.feature +++ b/tests/acceptance/DeadCode.feature @@ -402,8 +402,7 @@ Feature: Dead code Then I see no errors Scenario: Methods with a hook docblock tag are used by PHPUnit 11 - Given PHPUnit identifies as version "11.5.0" - And I have the following code + Given I have the following code """ final class MyTestCase extends TestCase { /** @@ -480,6 +479,31 @@ Feature: Dead code | PossiblyUnusedMethod | Cannot find any calls to method NS\MyTestCase::prepare | And I see no other errors + Scenario: A docblock hook tag is ignored on a method that has a PHPUnit attribute + Given I have the following code + """ + use PHPUnit\Framework\Attributes\Group; + + final class MyTestCase extends TestCase { + /** + * @before + */ + #[Group('hooks')] + public function prepare(): void { + $this->assertTrue(true); + } + + public function testSomething(): void { + $this->assertTrue(true); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | PossiblyUnusedMethod | Cannot find any calls to method NS\MyTestCase::prepare | + And I see no other errors + Scenario: A hook attribute of a method of a trait used by the test case is used by PHPUnit Given I have the following code """ From b0f403cfc9fa14ae29a5b251d8136eab8a5da101 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:13:12 +0100 Subject: [PATCH 09/24] fix(provider): simplify mixed key exemption, add guard scenarios #246 --- src/Hooks/TestCaseHandler.php | 14 +- src/Hooks/TestCaseHandler.php.orig | 1536 ------------------ tests/acceptance/ProviderReturnTypes.feature | 54 + 3 files changed, 60 insertions(+), 1544 deletions(-) delete mode 100644 src/Hooks/TestCaseHandler.php.orig diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 0979720..62f1413 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -562,14 +562,12 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ); // A single-argument iterable such as iterable has a mixed key, and PHPUnit rejects keys - // that are not int|string at runtime. Only a mixed value reveals an untyped provider. - $provider_key_type_is_compatible = ( - $provider_return_type->type_params[0]->isMixed() - && !$provider_return_type->type_params[1]->hasMixed() - ) || $codebase->isTypeContainedByType( - $provider_return_type->type_params[0], - $expected_provider_return_type->type_params[0] - ); + // that are not int|string at runtime. + $provider_key_type_is_compatible = $provider_return_type->type_params[0]->isMixed() + || $codebase->isTypeContainedByType( + $provider_return_type->type_params[0], + $expected_provider_return_type->type_params[0] + ); if (!$provider_key_type_is_compatible) { // XXX: isn't array key allowed by the $expected_provider_return_type ? diff --git a/src/Hooks/TestCaseHandler.php.orig b/src/Hooks/TestCaseHandler.php.orig deleted file mode 100644 index e2819c5..0000000 --- a/src/Hooks/TestCaseHandler.php.orig +++ /dev/null @@ -1,1536 +0,0 @@ - attribute class => whether it names the target class explicitly */ - private const DEPENDS_ATTRIBUTES = [ - Depends::class => false, - DependsUsingDeepClone::class => false, - DependsUsingShallowClone::class => false, - DependsExternal::class => true, - DependsExternalUsingDeepClone::class => true, - DependsExternalUsingShallowClone::class => true, - ]; - - private const DATA_PROVIDER_CLOSURE = 'PHPUnit\\Framework\\Attributes\\DataProviderClosure'; - - private const EXPECT_EXCEPTION_METHODS = [ - 'phpunit\\framework\\testcase::expectexception' => true, - 'phpunit\\framework\\testcase::expectexceptioncode' => true, - 'phpunit\\framework\\testcase::expectexceptionmessage' => true, - 'phpunit\\framework\\testcase::expectexceptionmessagematches' => true, - 'phpunit\\framework\\testcase::expectexceptionobject' => true, - ]; - - /** - * PHPUnit's own exceptions are handled by the test runner, nobody documents or catches them in tests. - * Psalm matches a thrown class only against listed parent classes, so classes that merely implement one of - * the interfaces are listed on their own. - */ - private const IGNORED_EXCEPTIONS = [ - 'PHPUnit\\Exception', - 'PHPUnit\\Framework\\Exception', - 'PHPUnit\\Framework\\MockObject\\Exception', - 'PHPUnit\\Framework\\MockObject\\ReflectionException', - 'PHPUnit\\Framework\\MockObject\\RuntimeException', - 'PHPUnit\\Event\\NoPreviousThrowableException', - 'PHPUnit\\Util\\Exception', - 'PHPUnit\\Util\\InvalidJsonException', - 'PHPUnit\\Util\\Xml\\XmlException', - ]; - - /** - * Statements after expectException*() may exist only to throw, so their results are unused and PHPUnit catches - * the exception. - */ - #[\Override] - public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void - { - if (!isset(self::EXPECT_EXCEPTION_METHODS[strtolower($event->getDeclaringMethodId())])) { - return; - } - - // PHPUnit stores the expectation on the receiver, so only the current test's own expectations count. - $expr = $event->getExpr(); - $isOwnExpectation = $expr instanceof MethodCall - ? $expr->var instanceof Variable && $expr->var->name === 'this' - : $expr->class instanceof Name - && in_array(strtolower($expr->class->toString()), ['self', 'static', 'parent'], true); - if (!$isOwnExpectation) { - return; - } - - $event->getStatementsSource()->addSuppressedIssues([ - 'UnusedFunctionCall', - 'UnusedMethodCall', - 'MissingThrowsDocblock', - ]); - } - - #[\Override] - public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event) - { - $codebase = $event->getCodebase(); - - foreach (self::IGNORED_EXCEPTIONS as $exception) { - $codebase->config->ignored_exceptions_and_descendants[$exception] = true; - $codebase->config->ignored_exceptions_and_descendants_in_global_scope[$exception] = true; - } - - foreach ($codebase->classlike_storage_provider->getAll() as $name => $storage) { - $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); - if ($codebase->classExtends($name, 'PHPUnit\Framework\TestCase') && ($meta['hasInitializers'] ?? false)) { - $storage->suppressed_issues[] = 'MissingConstructor'; - - foreach (self::getDescendants($codebase, $name) as $dependent_name) { - $dependent_storage = $codebase->classlike_storage_provider->get($dependent_name); - $dependent_storage->suppressed_issues[] = 'MissingConstructor'; - } - } - } - } - - /** - * @return string[] - * - * @psalm-mutation-free - */ - private static function getDescendants(Codebase $codebase, string $name): array - { - if (!$codebase->classlike_storage_provider->has($name)) { - return []; - } - - $storage = $codebase->classlike_storage_provider->get($name); - $ret = []; - - foreach ($storage->dependent_classlikes as $dependent => $_) { - if ($codebase->classExtends($dependent, $name)) { - $ret[] = $dependent; - $ret = array_merge($ret, self::getDescendants($codebase, $dependent)); - } - } - return $ret; - } - - #[\Override] - public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) - { - $class_node = $event->getStmt(); - $class_storage = $event->getStorage(); - $statements_source = $event->getStatementsSource(); - $codebase = $event->getCodebase(); - $aliases = $statements_source->getAliases(); - - // Psalm 7 seals methods by default, so ObjectProphecy::__call() is ignored - if (strtolower($class_storage->name) === 'prophecy\\prophecy\\objectprophecy') { - $class_storage->sealed_methods = false; - } - - if (self::hasInitializers($class_storage, $class_node, $aliases)) { - $class_storage->custom_metadata[__NAMESPACE__] = ['hasInitializers' => true]; - } - - $file_path = $statements_source->getFilePath(); - $file_storage = $codebase->file_storage_provider->get($file_path); - - foreach ($class_node->getMethods() as $method) { - $providers = self::getDataProviders($method, $aliases, $aliases, $class_storage); - foreach ($providers as $provider) { - if ($provider['class'] === null) { - continue; - } - $codebase->queueClassLikeForScanning($provider['class']); - $file_storage->referenced_classlikes[strtolower($provider['class'])] = $provider['class']; - } - - $method_storage = $class_storage->methods[strtolower($method->name->name)] ?? null; - // PHPUnit ignores docblock annotations on a method that has any PHPUnit attribute - $has_attribute = self::hasPhpUnitAttribute($method, $aliases); - if ( - $method_storage === null - || $providers === [] - || ($has_attribute && self::getAttributeDataProviders($method, $aliases, $class_storage) === null) - || count($providers) < self::countProviderAttributes($method, $aliases) - ) { - continue; - } - - $test_specials = $has_attribute - ? self::getAttributeSpecials($method, $aliases) - : self::docblockSpecials($method); - $method_storage->custom_metadata[DataProviderNarrowingHandler::METADATA_KEY] = [ - // the test's own class is resolved at analysis, where a trait method has the class that uses it - 'providers' => array_map( - static fn(array $provider): array => [$provider['class'] ?? '', $provider['method']], - $providers, - ), - 'test' => isset($test_specials['test']), - 'depends' => self::getDependencyTargets($method, $aliases, $class_storage) !== [], - ]; - } - } - - #[\Override] - public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event) - { - $class_node = $event->getStmt(); - $class_storage = $event->getClasslikeStorage(); - $codebase = $event->getCodebase(); - $statements_source = $event->getStatementsSource(); - $aliases = $statements_source->getAliases(); - - if (!$codebase->classExtends($class_storage->name, 'PHPUnit\Framework\TestCase')) { - return null; - } - - // This should always pass, we're calling it for the side-effect of adding self-reference - // in order to - // 1. Inform Psalm that class is used - // 2. Make Psalm analyze unused methods - // - // Marking class as used is required to get more detailed dead-code analysis (like unused - // methods). If we instead just suppress UnusedClass, unused methods are not analyzed. - if (!$codebase->classOrInterfaceExists($class_storage->name, $class_storage->location)) { - return null; - } - - self::reportMalformedTags( - $event, - $class_node, - new CodeLocation($statements_source, $class_node), - [], - ); - - // PHPUnit merges the class docblocks of the traits used directly by the test case, not of nested ones - foreach ($class_storage->used_traits as $trait_name) { - $trait_storage = $codebase->classlike_storage_provider->get($trait_name); - if (!$trait_storage->location) { - continue; - } - - self::reportMalformedTags( - $event, - $codebase->classlikes->getTraitNode($trait_name), - $trait_storage->stmt_location ?? $trait_storage->location, - $trait_storage->suppressed_issues, - ); - } - - foreach ($class_storage->declaring_method_ids as $method_name_lc => $declaring_method_id) { - $method_name = $codebase->getCasedMethodId($class_storage->name . '::' . $method_name_lc); - $method_storage = $codebase->methods->getStorage($declaring_method_id); - [$declaring_method_class, $declaring_method_name] = explode('::', (string)$declaring_method_id); - $declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_method_class); - - $declaring_class_node = $class_node; - if ($declaring_class_storage->is_trait) { - $declaring_class_node = $codebase->classlikes->getTraitNode($declaring_class_storage->name); - } - - if (!$method_storage->location) { - continue; - } - - // The class declaring an inherited test does not see the providers its descendants implement - if (!$declaring_class_storage->is_trait && $declaring_class_storage->name !== $class_storage->name) { - self::checkInheritedTestProviders( - $codebase, - $statements_source, - $class_storage, - $method_name_lc, - $method_storage, - ); - continue; - } - - $stmt_method = $declaring_class_node->getMethod($declaring_method_name); - - if (!$stmt_method) { - continue; - } - - // Use the declaring class's aliases so trait-declared attributes resolve via the trait's imports. - $method_aliases = $declaring_class_storage->aliases ?? $aliases; - $specials = self::getSpecials($stmt_method, $method_aliases); - // PHPUnit ignores docblock annotations on a method that has any PHPUnit attribute - $docblock_providers_ignored = self::hasPhpUnitAttribute($stmt_method, $method_aliases) - && self::getAttributeDataProviders($stmt_method, $method_aliases, $class_storage) === null; - - self::reportMalformedTags( - $event, - $stmt_method, - $method_storage->location, - $method_storage->suppressed_issues + $declaring_class_storage->suppressed_issues, - ); - - $is_test = 0 === strpos($method_name_lc, 'test') || isset($specials['test']); - if (!$is_test) { - continue; // skip non-test methods - } - - $codebase->methodExists( - (string) $declaring_method_id, - calling_method_id: 'PHPUnit\Framework\TestSuite::run', - ); - - $providers = self::getDataProviders($stmt_method, $method_aliases, $aliases, $class_storage); - $closure_providers = self::getClosureProviders($stmt_method, $method_aliases); - if ($providers === [] && $closure_providers === []) { - continue; - } - - $dependency_values = self::countDependencyValues( - $codebase, - $stmt_method, - $method_aliases, - $class_storage, - ); - $appends_dependency_value = !$docblock_providers_ignored && self::appendsDependencyValue( - $codebase, - $class_node, - $class_storage, - $stmt_method, - $method_aliases, - ); - - /** - * @var list $provided_types - */ - $provided_types = []; - foreach ($providers as $provider) { - $provider_docblock_location = $method_storage->location->setCommentLine($provider['line']); - - if ($provider['class'] !== null) { - $fq_class_name = $provider['class']; - - if (!$codebase->classOrInterfaceExists($fq_class_name, $provider_docblock_location)) { - // Psalm reports the missing class of an attribute argument itself - if (!$provider['external']) { - IssueBuffer::accepts(new Issue\UndefinedClass( - 'Class ' . $fq_class_name . ' does not exist', - $provider_docblock_location, - $fq_class_name - )); - } - continue; - } - $apparent_provider_method_name = $fq_class_name . '::' . $provider['method']; - } else { - $apparent_provider_method_name = $class_storage->name . '::' . $provider['method']; - } - - // getDeclaringMethodId() would truncate "Class::A::b" to "Class::A", so never look such a name up - $provider_method_id = str_contains($provider['method'], '::') - ? null - : $codebase->getDeclaringMethodId($apparent_provider_method_name); - - if (null === $provider_method_id) { - IssueBuffer::accepts(new Issue\UndefinedMethod( - 'Provider method ' . $apparent_provider_method_name . ' is not defined', - $provider_docblock_location, - $apparent_provider_method_name - )); - continue; - } - - // methodExists also can mark methods as used (weird, but handy) - $provider_method_exists = $codebase->methodExists( - $provider_method_id, - calling_method_id: (string) $declaring_method_id, - code_location: $provider_docblock_location, - ); - - if (!$provider_method_exists) { - IssueBuffer::accepts(new Issue\UndefinedMethod( - 'Provider method ' . $apparent_provider_method_name . ' is not defined', - $provider_docblock_location, - $apparent_provider_method_name - )); - continue; - } - - $provider_id = MethodIdentifier::wrap($provider_method_id); - $provider_method_storage = $codebase->methods->getStorage($provider_id); - - // Abstract providers are checked on the concrete classes implementing them - if (!$provider_method_storage->abstract && !$docblock_providers_ignored) { - $suppressed_issues = $method_storage->suppressed_issues - + $declaring_class_storage->suppressed_issues - + $statements_source->getSuppressedIssues(); - - self::reportInvalidProviderSignature( - $codebase, - $provider_method_storage, - $apparent_provider_method_name, - $provider_docblock_location, - $suppressed_issues, - ); - - // Providers declared elsewhere are not read - $provider_node = $provider_id->fq_class_name === $class_storage->name - ? $class_node->getMethod($provider_id->method_name) - : null; - if ($provider_node !== null) { - self::reportPositionalAfterNamed( - $statements_source, - $provider_node->stmts ?? [], - 'a dataset provided by ' . $provider_method_id . '()', - $suppressed_issues, - ); - } - } - - $provider_return_type = $codebase->getMethodReturnType($provider_method_id, $_); - - if (!$provider_return_type) { - continue; - } - - $provider_return_type_location = $codebase->getMethodReturnTypeLocation($provider_method_id); - assert(null !== $provider_return_type_location); - $provided_types[] = [ - 'name' => $provider_method_id . '()', - 'type' => $provider_return_type, - 'location' => $provider_docblock_location, - 'return_type_location' => $provider_return_type_location, - ]; - } - - foreach ($closure_providers as $closure) { - // A closure of a trait lives in another file than the one analysed - if (!$declaring_class_storage->is_trait) { - self::reportPositionalAfterNamed( - $statements_source, - $closure['statements'], - 'a dataset provided by #[DataProviderClosure]', - $method_storage->suppressed_issues - + $declaring_class_storage->suppressed_issues - + $statements_source->getSuppressedIssues(), - ); - } - - $closure_return_type = DataProviderClosureHandler::returnType($method_storage, $closure['position']); - if ($closure_return_type === null) { - continue; - } - - $closure_location = $method_storage->location->setCommentLine($closure['line']); - $provided_types[] = [ - 'name' => '#[DataProviderClosure]', - 'type' => $closure_return_type, - 'location' => $closure_location, - 'return_type_location' => $closure_location, - ]; - } - - foreach ($provided_types as $provided) { - $provider_name = $provided['name']; - $provider_return_type = $provided['type']; - $provider_docblock_location = $provided['location']; - $provider_return_type_location = $provided['return_type_location']; - $provider_return_type_string = $provider_return_type->getId(); - - $expected_provider_return_type = new Type\Atomic\TIterable([ - Type::getArrayKey(), - Type::getArray(), - ]); - - $non_null_provider_return_types = []; - foreach (self::getAtomics($provider_return_type) as $type) { - // PHPUnit allows returning null from providers and treats it as an empty set - // resulting in the test being skipped - if ($type instanceof TNull) { - continue; - } - - if (!$type->isIterable($codebase)) { - IssueBuffer::accepts(new Issue\InvalidReturnType( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - continue 2; - } - $non_null_provider_return_types[] = $type; - } - - if ([] === $non_null_provider_return_types) { - IssueBuffer::accepts(new Issue\InvalidReturnType( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - continue; - } - - // unionize iterable so that instead of array|Traversable - // we get iterable - // - // TODO: this may get implemented in a future Psalm version, remove it then - $provider_return_type = self::unionizeIterables( - $codebase, - new Type\Union($non_null_provider_return_types) - ); - - $provider_key_type_is_compatible = $codebase->isTypeContainedByType( - $provider_return_type->type_params[0], - $expected_provider_return_type->type_params[0] - ); - - if (!$provider_key_type_is_compatible) { - // XXX: isn't array key allowed by the $expected_provider_return_type ? - $provider_key_type_is_uncertain = - $provider_return_type->type_params[0]->hasMixed() - || $provider_return_type->type_params[0]->hasArrayKey(); - - if ($provider_key_type_is_uncertain) { - IssueBuffer::accepts(new Issue\MixedReturnStatement( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', possibly different ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - } else { - IssueBuffer::accepts(new Issue\InvalidReturnType( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - } - continue; - } - - $provider_value_type_is_compatible = $codebase->isTypeContainedByType( - $provider_return_type->type_params[1], - $expected_provider_return_type->type_params[1] - ); - - if (!$provider_value_type_is_compatible) { - if ($provider_return_type->type_params[1]->hasMixed()) { - IssueBuffer::accepts(new Issue\MixedReturnStatement( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', possibly different ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - } else { - IssueBuffer::accepts(new Issue\InvalidReturnType( - 'Providers must return ' . $expected_provider_return_type->getId() - . ', ' . $provider_return_type_string . ' provided', - $provider_return_type_location - )); - } - continue; - } - - // An empty array has no datasets to check - if ($provider_return_type->type_params[1]->isNever()) { - continue; - } - - $checkParam = - static function ( - Type\Union $potential_argument_type, - Type\Union $param_type, - bool $is_optional, - int $param_offset, - ?string $param_name = null - ) use ( - $codebase, - $method_name, - $provider_name, - $provider_return_type_string, - $provider_docblock_location - ): void { - $argument = 'Argument ' . ($param_offset + 1) - . ($param_name === null ? '' : ' ($' . $param_name . ')'); - if ($is_optional) { - $param_type = $param_type->setPossiblyUndefined(true); - } - if ($codebase->isTypeContainedByType($potential_argument_type, $param_type)) { - // ok - } elseif ($codebase->canTypeBeContainedByType($potential_argument_type, $param_type)) { - IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( - $argument . ' of ' . $method_name - . ' expects ' . $param_type->getId() . ', ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location - )); - } elseif ($potential_argument_type->possibly_undefined && !$is_optional) { - IssueBuffer::accepts(new Issue\InvalidArgument( - $argument . ' of ' . $method_name - . ' is not optional, but possibly undefined ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location - )); - } else { - IssueBuffer::accepts(new Issue\InvalidArgument( - $argument . ' of ' . $method_name - . ' expects ' . $param_type->getId() . ', ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location - )); - } - }; - - /** @var Type\Atomic\TArray|Type\Atomic\TKeyedArray $dataset_type */ - $dataset_type = self::getAtomics($provider_return_type->type_params[1])['array']; - - if ($dataset_type instanceof Type\Atomic\TArray) { - // The dataset length is unknown, so assume the trailing params receive the return values - // of depended-upon tests, unless a variadic param could consume them. - $last_param = array_slice($method_storage->params, -1)[0] ?? null; - $provider_params = array_slice( - $method_storage->params, - 0, - $last_param !== null && $last_param->is_variadic - ? null - : max(0, count($method_storage->params) - $dependency_values), - ); - // check that all of the required (?) params accept value type - $potential_argument_type = $dataset_type->type_params[1]; - foreach ($provider_params as $param_offset => $param) { - if (!$param->type) { - continue; - } - $checkParam($potential_argument_type, $param->type, $param->is_optional, $param_offset); - } - } elseif (self::hasStringKeys($dataset_type)) { - $provided_by = $provider_name . ':(' . $provider_return_type_string . ')'; - $binding = DatasetBinding::bind($dataset_type, $method_storage->params, $codebase); - - foreach ($binding->arguments as $argument) { - $checkParam( - $argument->type, - $argument->param_type, - $argument->is_optional, - $argument->position, - $argument->name, - ); - } - - foreach ($binding->unknown as $name) { - IssueBuffer::accepts(new Issue\InvalidNamedArgument( - 'Parameter $' . $name . ' does not exist on ' . $method_name - . ', provided by ' . $provided_by, - $provider_docblock_location, - $method_name - )); - } - - foreach ($binding->duplicate as $name) { - IssueBuffer::accepts(new Issue\InvalidNamedArgument( - 'Parameter $' . $name . ' has already been used in ' . $method_name - . ', provided by ' . $provided_by, - $provider_docblock_location, - $method_name - )); - } - - if ($appends_dependency_value && self::hasStringKeys($dataset_type, required_only: true)) { - IssueBuffer::accepts(new Issue\InvalidNamedArgument( - 'Cannot use positional argument after named argument in ' . $method_name - . ': PHPUnit appends the return value of a depended-upon test to dataset ' . $provided_by, - $provider_docblock_location, - $method_name - )); - - continue; - } - - // the return values of depended-upon tests fill the first parameters the dataset leaves open - $missing = array_slice($binding->missing, $dependency_values); - if ($missing !== []) { - IssueBuffer::accepts(new Issue\TooFewArguments( - 'Too few arguments for ' . $method_name . ' - parameter $' . $missing[0] - . ' is not provided by ' . $provided_by, - $provider_docblock_location, - $method_name - )); - } - } else { // TKeyedArray - // iterate over all params checking if corresponding value type is acceptable - // let's hope properties are sorted in array order - $potential_argument_types = array_values($dataset_type->properties); - - foreach ($method_storage->params as $param_offset => $param) { - if (!isset($potential_argument_types[$param_offset])) { - // variadics are never required - // and they always come last - if ($param->is_variadic) { - break; - } - // reached default params, so it's fine, but let's continue - // because MisplacedRequiredParam could be suppressed - if ($param->is_optional) { - continue; - } - // the return values of depended-upon tests follow the provider values - if ($param_offset < $dataset_type->getMinCount() + $dependency_values) { - continue; - } - - IssueBuffer::accepts(new Issue\TooFewArguments( - 'Too few arguments for ' . $method_name - . ' - expecting at least ' . ($param_offset + 1 - $dependency_values) - . ', but saw ' . $dataset_type->getMinCount() - . ' provided by ' . $provider_name - . ':(' . $provider_return_type_string . ')', - $provider_docblock_location, - $method_name - )); - break; - } - $potential_argument_type = $potential_argument_types[$param_offset]; - - $param_type = $param->type === null ? Type::getMixed() : $param->type; - if ($param->is_variadic) { - $param_types = self::getAtomics($param_type); - $variadic_param_type = new Type\Union(array_values($param_types)); - - // check remaining argument types - for (; $param_offset < count($potential_argument_types); $param_offset++) { - $potential_argument_type = $potential_argument_types[$param_offset]; - $checkParam( - $potential_argument_type, - $variadic_param_type, - true, - $param_offset - ); - } - break; - } - - $checkParam($potential_argument_type, $param_type, $param->is_optional, $param_offset); - } - } - } - } - } - - /** - * Checks the providers of a test inherited from an ancestor that the class implements or overrides itself. - * - * @param lowercase-string $test_name_lc - */ - private static function checkInheritedTestProviders( - Codebase $codebase, - StatementsSource $statements_source, - ClassLikeStorage $class_storage, - string $test_name_lc, - MethodStorage $test_storage - ): void { - $metadata = $test_storage->custom_metadata[DataProviderNarrowingHandler::METADATA_KEY] ?? null; - if ( - $class_storage->parent_class === null - || !is_array($metadata) - || !((bool) $metadata['test'] || str_starts_with($test_name_lc, 'test')) - ) { - return; - } - - /** @var list $providers */ - $providers = $metadata['providers']; - foreach ($providers as [$provider_class, $provider_name]) { - if ($provider_class !== '' || str_contains($provider_name, '::')) { - continue; - } - - // The parent already checked the provider it resolves - $provider_method_id = $codebase->getDeclaringMethodId($class_storage->name . '::' . $provider_name); - if ( - $provider_method_id === null - || $provider_method_id === $codebase->getDeclaringMethodId( - $class_storage->parent_class . '::' . $provider_name - ) - ) { - continue; - } - - $provider_id = MethodIdentifier::wrap($provider_method_id); - $provider_storage = $codebase->methods->getStorage($provider_id); - if ($provider_storage->abstract || $provider_storage->location === null) { - continue; - } - - // Keys are source offsets, which array_merge() would renumber - $suppressed_issues = $provider_storage->suppressed_issues - + $codebase->classlike_storage_provider->get($provider_id->fq_class_name)->suppressed_issues; - // The analysed class's offsets mean nothing in a trait's file - if ($provider_storage->location->file_path === $statements_source->getFilePath()) { - $suppressed_issues += $class_storage->suppressed_issues + $statements_source->getSuppressedIssues(); - } - - self::reportInvalidProviderSignature( - $codebase, - $provider_storage, - $class_storage->name . '::' . $provider_name, - $provider_storage->location, - $suppressed_issues, - ); - } - } - - /** @param array $suppressed_issues */ - private static function reportInvalidProviderSignature( - Codebase $codebase, - MethodStorage $provider_method_storage, - string $apparent_provider_method_name, - CodeLocation $location, - array $suppressed_issues - ): void { - $provider_visibility = $provider_method_storage->visibility; - $appearing_provider_id = $codebase->methods->getAppearingMethodId( - MethodIdentifier::wrap($apparent_provider_method_name) - ); - if (null !== $appearing_provider_id) { - $provider_visibility = $codebase->classlike_storage_provider - ->get($appearing_provider_id->fq_class_name) - ->trait_visibility_map[$appearing_provider_id->method_name] - ?? $provider_visibility; - } - - if (ClassLikeAnalyzer::VISIBILITY_PUBLIC !== $provider_visibility) { - IssueBuffer::accepts(new InvalidDataProvider( - 'Data provider method ' . $apparent_provider_method_name . ' must be public', - $location - ), $suppressed_issues); - } - - if (!$provider_method_storage->is_static) { - IssueBuffer::accepts(new InvalidDataProvider( - 'Data provider method ' . $apparent_provider_method_name . ' must be static', - $location - ), $suppressed_issues); - } - } - - /** - * PHPUnit passes string dataset keys as named arguments, and PHP rejects a positional argument after them. - * Only literal datasets are read: Psalm's array shapes do not keep the key order. - * - * @param array $statements the body of a provider method or closure - * @param array $suppressed_issues - */ - private static function reportPositionalAfterNamed( - StatementsSource $source, - array $statements, - string $provided_by, - array $suppressed_issues - ): void { - $visitor = new class extends NodeVisitorAbstract { - public bool $generator = false; - - /** @var list */ - public array $returned = []; - - /** @var list */ - public array $yielded = []; - - #[\Override] - public function enterNode(Node $node) - { - if ($node instanceof Node\FunctionLike || $node instanceof ClassLike) { - return NodeVisitor::DONT_TRAVERSE_CHILDREN; - } - - $sets = $node instanceof Return_ || $node instanceof YieldFrom ? $node->expr : null; - foreach ($sets instanceof Array_ ? $sets->items : [] as $item) { - if (!$item->unpack && $item->value instanceof Array_) { - if ($node instanceof Return_) { - $this->returned[] = $item->value; - } else { - $this->yielded[] = $item->value; - } - } - } - - if ($node instanceof YieldFrom || $node instanceof Yield_) { - $this->generator = true; - } - - if ($node instanceof Yield_ && $node->value instanceof Array_) { - $this->yielded[] = $node->value; - } - - return null; - } - }; - (new NodeTraverser($visitor))->traverse($statements); - - // PHPUnit ignores the return value of a generator - foreach ($visitor->generator ? $visitor->yielded : $visitor->returned as $dataset) { - $named = false; - $violated = false; - $seen = []; - $next = 0; - foreach ($dataset->items as $item) { - $key = $item->key; - if ($item->unpack || ($key !== null && !$key instanceof String_ && !$key instanceof Int_)) { - continue 2; - } - - // PHP casts keys like '1' to int - if ($key instanceof String_ && (string) (int) $key->value !== $key->value) { - $named = true; - continue; - } - - $position = $key === null ? $next : (int) $key->value; - $next = max($next, $position + 1); - - // An array keeps the place of a key's first insertion, so overwriting a positional key is harmless - if ($named && !isset($seen[$position])) { - $violated = true; - } - - $seen[$position] = true; - } - - if ($violated) { - IssueBuffer::accepts(new Issue\InvalidNamedArgument( - 'Cannot use positional argument after named argument in ' . $provided_by, - new CodeLocation($source, $dataset) - ), $suppressed_issues); - } - } - } - - /** @psalm-pure */ - public static function hasStringKeys(Type\Atomic\TKeyedArray $dataset_type, bool $required_only = false): bool - { - foreach ($dataset_type->properties as $key => $type) { - if (is_string($key) && !($required_only && $type->possibly_undefined)) { - return true; - } - } - - return false; - } - - /** - * @return non-empty-array - * - * @psalm-pure - */ - private static function getAtomics(Type\Union $union): array - { - return $union->getAtomicTypes(); - } - - public static function unionizeIterables(Codebase $codebase, Type\Union $iterables): Type\Atomic\TIterable - { - /** @var Type\Union[] $key_types */ - $key_types = []; - - /** @var Type\Union[] $value_types */ - $value_types = []; - - foreach (self::getAtomics($iterables) as $type) { - if (!$type->isIterable($codebase)) { - throw new RuntimeException('should be iterable'); - } - - if ($type instanceof Type\Atomic\TArray) { - $key_types[] = $type->type_params[0] ?? Type::getMixed(); - $value_types[] = $type->type_params[1] ?? Type::getMixed(); - } elseif ($type instanceof Type\Atomic\TKeyedArray) { - $key_types[] = $type->getGenericKeyType(); - $value_types[] = $type->getGenericValueType(); - } elseif ($type instanceof Type\Atomic\TNamedObject || $type instanceof Type\Atomic\TIterable) { - [$key_types[], $value_types[]] = $codebase->getKeyValueParamsForTraversableObject($type); - } else { - throw new RuntimeException('unexpected type'); - } - } - - $combine = - /** - * @param null|Type\Union $a - * - * @psalm-external-mutation-free - */ - static function ($a, Type\Union $b) use ($codebase): Type\Union { - return $a ? Type::combineUnionTypes($a, $b, $codebase) : $b; - }; - - return new Type\Atomic\TIterable([ - array_reduce($key_types, $combine), - array_reduce($value_types, $combine), - ]); - } - - - private static function hasInitializers(ClassLikeStorage $storage, ClassLike $stmt, Aliases $aliases): bool - { - if (isset($storage->methods['setup'])) { - return true; - } - - foreach ($storage->methods as $method => $_) { - $stmt_method = $stmt->getMethod($method); - if (!$stmt_method) { - continue; - } - if (self::isBeforeInitializer($stmt_method, $aliases)) { - return true; - } - } - return false; - } - - private static function isBeforeInitializer(ClassMethod $method, Aliases $aliases): bool - { - return isset(self::getSpecials($method, $aliases)['before']); - } - - /** - * PHPUnit 12 ignores docblock metadata altogether. - * - * @return array> Each entry is [file line, value] - */ - private static function docblockSpecials(ClassMethod $method): array - { - return PhpUnitVersion::ignoresDocblockMetadata() ? [] : DocblockSpecials::of($method); - } - - /** @return array> Each entry is [file line, value] */ - private static function getSpecials(ClassMethod $method, Aliases $aliases): array - { - return array_merge( - self::docblockSpecials($method), - // Attributes take priority over docblocks - self::getAttributeSpecials($method, $aliases), - ); - } - - /** - * @template T of object - * @param class-string $attributeClass - * @return list|null - */ - private static function attributeValue(ClassMethod $method, Aliases $aliases, string $attributeClass): array|null - { - $onlyStringLiteralExpressions = static fn (Attribute $attribute): array => array_map( - static fn(String_ $string): array => [$string->getStartLine(), $string->value], - array_filter( - array_column($attribute->args, 'value'), - // For our purposes, we only care about string literals: everything else is currently out of scope. - // If you need more complex expressions supported, add a constant expression evaluator here. - static fn(Expr $expression): bool => $expression instanceof String_, - ), - ); - $attributesInGroupMatchingRequestedAttributeName = static fn(AttributeGroup $group): array => array_filter( - $group->attrs, - static fn(Attribute $attribute): bool => $attributeClass === Type::getFQCLNFromString( - // toCodeString() keeps the leading backslash toString() strips, so fully-qualified attributes resolve. - $attribute->name->toCodeString(), - $aliases, - ), - ); - $matchingAttributes = array_merge(...array_values(array_map( - $attributesInGroupMatchingRequestedAttributeName, - $method->getAttrGroups(), - ))); - - if ($matchingAttributes === []) { - return null; - } - - return array_merge(...array_values(array_map( - $onlyStringLiteralExpressions, - $matchingAttributes, - ))); - } - - /** - * Attributes take priority over docblocks, like in PHPUnit's ParserChain. - * - * $aliases resolve the attribute names and the arguments of #[DataProviderExternal]. The "Class::method" - * references of docblock tags are resolved with $reference_aliases (of the analysed file). - * - * @return list - * class is null for a provider of the test's own class - */ - private static function getDataProviders( - ClassMethod $method, - Aliases $aliases, - Aliases $reference_aliases, - ClassLikeStorage $class - ): array { - return self::getAttributeDataProviders($method, $aliases, $class) - ?? array_map( - static fn(array $entry): array => self::parseProviderReference( - $entry[0], - $entry[1], - $reference_aliases, - ), - self::docblockSpecials($method)['dataProvider'] ?? [], - ); - } - - /** - * #[DataProvider] and #[DataProviderExternal], or null when the method has neither. A method with only a - * #[DataProviderClosure] has none of these providers, but is not left to its docblock either - * (see getClosureProviders()). - * - * @return list|null - */ - private static function getAttributeDataProviders( - ClassMethod $method, - Aliases $aliases, - ClassLikeStorage $class - ): array|null { - $references = self::attributeValue($method, $aliases, DataProvider::class); - $has_attributes = $references !== null; - // PHPUnit takes a #[DataProvider] argument literally as a method name of the test class - $providers = array_map( - static fn(array $entry): array => [ - 'line' => $entry[0], - 'class' => null, - 'method' => $entry[1], - 'external' => false, - ], - $references ?? [], - ); - - foreach ($method->getAttrGroups() as $group) { - foreach ($group->attrs as $attribute) { - $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); - if ($attribute_class === self::DATA_PROVIDER_CLOSURE) { - $has_attributes = true; - } - - if ($attribute_class !== DataProviderExternal::class) { - continue; - } - - $has_attributes = true; - $args = self::attributeArguments($attribute); - - $class_name = self::classFromExpression($args['className'] ?? $args[0] ?? null, $aliases, $class); - $method_name = $args['methodName'] ?? $args[1] ?? null; - if ($class_name === null || !$method_name instanceof String_) { - continue; - } - - $providers[] = [ - 'line' => $attribute->getStartLine(), - 'class' => $class_name, - 'method' => $method_name->value, - 'external' => true, - ]; - } - } - - return $has_attributes ? $providers : null; - } - - /** - * The closure of each #[DataProviderClosure] whose argument is a literal closure, by the start offset that - * DataProviderClosureHandler keys its return type by. - * - * @return list}> - */ - private static function getClosureProviders(ClassMethod $method, Aliases $aliases): array - { - $closures = []; - foreach ($method->getAttrGroups() as $group) { - foreach ($group->attrs as $attribute) { - $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); - if ($attribute_class !== self::DATA_PROVIDER_CLOSURE) { - continue; - } - - $args = self::attributeArguments($attribute); - $closure = $args['closure'] ?? $args[0] ?? null; - if ($closure instanceof Expr\Closure || $closure instanceof Expr\ArrowFunction) { - $closures[] = [ - 'line' => $attribute->getStartLine(), - 'position' => $closure->getStartFilePos(), - 'statements' => $closure instanceof Expr\Closure - ? $closure->stmts - : [new Return_($closure->expr)], - ]; - } - } - } - - return $closures; - } - - /** - * "Class::method" or "method" (optionally with trailing parentheses), as written in a dataProvider - * docblock tag. - * - * @psalm-mutation-free - * - * @return array{line: int, class: string|null, method: string, external: bool} - */ - private static function parseProviderReference(int $line, string $reference, Aliases $aliases): array - { - $parts = explode('::', $reference, 2); - $method = preg_replace('/\(\s*\)$/', '', $parts[1] ?? $parts[0]); - assert($method !== null); - - return [ - 'line' => $line, - 'class' => isset($parts[1]) ? Type::getFQCLNFromString($parts[0], $aliases) : null, - 'method' => $method, - 'external' => false, - ]; - } - - /** - * PHPUnit appends the return value of every distinct depended-upon test that is not declared void or never - * to the data provider values. Depending on a whole class passes no value. - */ - private static function countDependencyValues( - Codebase $codebase, - ClassMethod $method, - Aliases $aliases, - ClassLikeStorage $class - ): int { - $count = 0; - $seen = []; - foreach (self::getDependencyTargets($method, $aliases, $class) as $target) { - if ($target !== null) { - if (isset($seen[strtolower($target)])) { - continue; - } - $seen[strtolower($target)] = true; - - if (self::dependencyReturnsValue($codebase, $target) === false) { - continue; - } - } - $count++; - } - - return $count; - } - - /** - * Up to PHPUnit 12.5.39 and 13.4.2 (and on 11.5) only the first dependency counts: collecting stops at the first - * void dependency and at the first one larger than this test. A test of the same class shares its class-level - * size, so only size groups in the two method docblocks (PHPUnit 11) can make it larger. Newer releases skip - * both and pass the value of any later dependency. - */ - private static function appendsDependencyValue( - Codebase $codebase, - ClassLike $class_node, - ClassLikeStorage $class, - ClassMethod $method, - Aliases $aliases - ): bool { - $targets = self::getDependencyTargets($method, $aliases, $class); - - if (!PhpUnitVersion::stopsCollectingDependencyValuesEarly()) { - foreach ($targets as $target) { - if ($target !== null && self::dependencyReturnsValue($codebase, $target) === true) { - return true; - } - } - - return false; - } - - $first = $targets[0] ?? null; - if ($first === null || self::dependencyReturnsValue($codebase, $first) !== true) { - return false; - } - - [$target_class, $target_method] = explode('::', $first); - $target_node = strtolower($target_class) === strtolower($class->name) - ? $class_node->getMethod($target_method) - : null; - if ($target_node === null) { - return false; - } - - $size_group = '/@(?:(?:group|ticket)\s+)?(?:small|medium|large)\b/'; - foreach ([$method->getDocComment(), $target_node->getDocComment()] as $docblock) { - if ($docblock !== null && preg_match($size_group, $docblock->getText()) === 1) { - return false; - } - } - - return true; - } - - /** - * @return bool|null null when the target method cannot be resolved - */ - private static function dependencyReturnsValue(Codebase $codebase, string $target): ?bool - { - $target_class = explode('::', $target)[0]; - $target_id = $codebase->classOrInterfaceExists($target_class) - ? $codebase->getDeclaringMethodId($target) - : null; - if ($target_id === null) { - return null; - } - - $return_type = $codebase->methods->getStorage(MethodIdentifier::wrap($target_id))->signature_return_type; - - return $return_type === null || !($return_type->isVoid() || $return_type->isNever()); - } - - /** - * getAttributeDataProviders() leaves out the attributes it cannot read, such as #[DataProvider(self::NAME)] and - * #[DataProviderClosure], which is named by string as not every supported PHPUnit version has it. - */ - private static function countProviderAttributes(ClassMethod $method, Aliases $aliases): int - { - $provider_attributes = [ - DataProvider::class, - DataProviderExternal::class, - self::DATA_PROVIDER_CLOSURE, - ]; - $count = 0; - foreach ($method->getAttrGroups() as $group) { - foreach ($group->attrs as $attribute) { - $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); - if (in_array($attribute_class, $provider_attributes, true)) { - $count++; - } - } - } - - return $count; - } - - /** - * PHPUnit's ParserChain ignores docblock annotations on a class or method that has any PHPUnit attribute. - * - * @internal - */ - public static function hasPhpUnitAttribute(ClassLike|ClassMethod $node, Aliases $aliases): bool - { - foreach ($node->attrGroups as $group) { - foreach ($group->attrs as $attribute) { - $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); - if (str_starts_with($attribute_class, 'PHPUnit\\Framework\\Attributes\\')) { - return true; - } - } - } - - return false; - } - - /** - * Mirrors PHPUnit's ParserChain: docblock annotations are only read when the method has no PHPUnit attribute. - * - * @return list "Class::method" per method dependency, null when the target is not a literal - */ - private static function getDependencyTargets(ClassMethod $method, Aliases $aliases, ClassLikeStorage $class): array - { - $has_phpunit_attribute = self::hasPhpUnitAttribute($method, $aliases); - $targets = []; - foreach ($method->getAttrGroups() as $group) { - foreach ($group->attrs as $attribute) { - $attribute_class = Type::getFQCLNFromString($attribute->name->toCodeString(), $aliases); - if (!isset(self::DEPENDS_ATTRIBUTES[$attribute_class])) { - continue; - } - - $args = self::attributeArguments($attribute); - - $is_external = self::DEPENDS_ATTRIBUTES[$attribute_class]; - $target_name = $args['methodName'] ?? $args[$is_external ? 1 : 0] ?? null; - $target_class = $is_external - ? self::classFromExpression($args['className'] ?? $args[0] ?? null, $aliases, $class) - : $class->name; - if ($target_name instanceof String_ && $target_name->value === 'class') { - continue; // PHPUnit treats this as a class dependency, which passes no value - } - - $targets[] = $target_class !== null && $target_name instanceof String_ - ? $target_class . '::' . $target_name->value - : null; - } - } - - if ($has_phpunit_attribute) { - return $targets; - } - - foreach (self::docblockSpecials($method)['depends'] ?? [] as [, $value]) { - $is_target = preg_match('/^(?:!?(?:clone|shallowClone)\s+)?(\S+)/', $value, $matches) === 1; - if (!$is_target || str_ends_with(strtolower($matches[1]), '::class')) { - continue; - } - - $target = $matches[1]; - $targets[] = str_contains($target, '::') ? ltrim($target, '\\') : $class->name . '::' . $target; - } - - return $targets; - } - - /** - * @return array named arguments by name, the others by position - * @psalm-mutation-free - * @internal - */ - public static function attributeArguments(Attribute $attribute): array - { - $args = []; - foreach ($attribute->args as $position => $arg) { - $args[$arg->name?->toString() ?? $position] = $arg->value; - } - - return $args; - } - - /** @internal */ - public static function classFromExpression( - Expr|null $expression, - Aliases $aliases, - ClassLikeStorage $class - ): string|null { - if ($expression instanceof String_) { - return ltrim($expression->value, '\\'); - } - - $is_class_fetch = $expression instanceof ClassConstFetch - && $expression->class instanceof Name - && $expression->name instanceof Identifier - && $expression->name->toLowerString() === 'class'; - if (!$is_class_fetch) { - return null; - } - - if ($expression->class->isSpecialClassName()) { - return $expression->class->toLowerString() === 'parent' ? $class->parent_class : $class->name; - } - - return Type::getFQCLNFromString($expression->class->toCodeString(), $aliases); - } - /** @return array> */ - private static function getAttributeSpecials(ClassMethod $method, Aliases $aliases): array - { - return array_filter([ - 'before' => self::attributeValue($method, $aliases, Before::class), - 'test' => self::attributeValue($method, $aliases, Test::class), - ], static fn(array|null $special): bool => $special !== null); - } - - /** @param array $suppressed_issues */ - private static function reportMalformedTags( - AfterClassLikeAnalysisEvent $event, - ClassLike|ClassMethod $node, - CodeLocation $location, - array $suppressed_issues, - ): void { - // Nothing is read from a docblock on PHPUnit 12, malformed or not - if (PhpUnitVersion::ignoresDocblockMetadata()) { - return; - } - - // Suppression keys are source offsets: those of the analysed class mean nothing in another file - $statements_source = $event->getStatementsSource(); - if ($location->file_path === $statements_source->getFilePath()) { - $suppressed_issues += $event->getClasslikeStorage()->suppressed_issues - + $statements_source->getSuppressedIssues(); - } - - foreach (DocblockSpecials::malformed($node) as ['tag' => $tag, 'line' => $line, 'problem' => $problem]) { - IssueBuffer::accepts(new InvalidPhpUnitAnnotation( - 'Annotation "@' . $tag . '" ' . $problem, - $location->setCommentLine($line), - ), $suppressed_issues); - } - } -} diff --git a/tests/acceptance/ProviderReturnTypes.feature b/tests/acceptance/ProviderReturnTypes.feature index 297f978..75cffd3 100644 --- a/tests/acceptance/ProviderReturnTypes.feature +++ b/tests/acceptance/ProviderReturnTypes.feature @@ -463,3 +463,57 @@ Feature: Data provider return types """ When I run Psalm Then I see no errors + + Scenario: An iterable provider with an object key type is still reported + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield new \stdClass => [1]; + } + + #[DataProvider('provide')] + public function testSomething(int $i): void { + $this->assertSame(1, $i); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | Line | + | InvalidReturnType | Providers must return iterable>, iterable[pure] provided | 10 | + And I see no other errors + + Scenario: An iterable provider without a key type and with non-array values is still reported + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase + { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 1; + } + + #[DataProvider('provide')] + public function testSomething(int $i): void { + $this->assertSame(1, $i); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | Line | + | InvalidReturnType | Providers must return iterable>, iterable[pure] provided | 10 | + And I see no other errors From d85804282f2d9e9ada4a4a213b83328e955db55d Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:17:03 +0100 Subject: [PATCH 10/24] fix(provider): resolve trait visibility of tests via appearing class #250 --- README.md | 2 +- src/Hooks/DataProviderNarrowingHandler.php | 9 +-- src/Hooks/TestCaseHandler.php | 36 ++++++++- src/TestMethod.php | 29 ------- tests/acceptance/DataProviders.feature | 94 +++++++++++++++++----- 5 files changed, 114 insertions(+), 56 deletions(-) delete mode 100644 src/TestMethod.php diff --git a/README.md b/README.md index 80fd665..e7e4f66 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ vendor/bin/psalm-plugin enable psalm/plugin-phpunit * Docblock annotations that PHPUnit ignores because they have no value or no space before it (`/** @dataProvider*/`, `@depends` alone on a line, `@group(slow)`) are reported as `InvalidPhpUnitAnnotation`, not as a missing data provider. This covers the class docblocks of the traits a test case uses directly, which PHPUnit merges into the test class. * Test parameters take the types their data providers return: with `@return iterable`, `testFoo(mixed $a, mixed $b)` sees `int $a` and `string $b`, so there is no `MixedAssignment`. Parameters with a default, variadic parameters and parameters that already have a narrower type keep their declared type. * Coverage targets are validated: a missing class, method or function in `#[CoversMethod]`, `#[UsesMethod]`, `#[CoversFunction]`, `#[UsesFunction]` or a docblock `@covers`, `@uses`, `@coversDefaultClass`, `@usesDefaultClass` is reported as `UndefinedClass`, `UndefinedMethod` or `UndefinedFunction`. -* Test methods, their data providers, and `TestCase` classes are not reported as unused code. +* Test methods, their data providers, and `TestCase` classes are not reported as unused code. As in PHPUnit, only public methods count as tests, for the provider validation above and for this. * Statements after `$this->expectException()` and its siblings are not reported as `UnusedFunctionCall`, `UnusedMethodCall` or `MissingThrowsDocblock`. * Exceptions declared by PHPUnit's own API (assertions, `createMock()`, `createStub()` and so on) are not reported as `MissingThrowsDocblock`. Exceptions in your `` stay ignored. * `assertSame(null, ...)`, `assertSame(true|false, ...)` and `assertSame(n, count(...))` are reported as `UnspecificNullAssertion`, `UnspecificBooleanAssertion` and `UnspecificCountAssertion` (at `errorLevel="1"`), suggesting `assertNull()`, `assertTrue()`/`assertFalse()` and `assertCount()`. diff --git a/src/Hooks/DataProviderNarrowingHandler.php b/src/Hooks/DataProviderNarrowingHandler.php index 1bd9596..d8e155c 100644 --- a/src/Hooks/DataProviderNarrowingHandler.php +++ b/src/Hooks/DataProviderNarrowingHandler.php @@ -9,7 +9,6 @@ use Psalm\Internal\MethodIdentifier; use Psalm\Internal\Type\TypeExpander; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; -use Psalm\PhpUnitPlugin\TestMethod; use Psalm\Plugin\EventHandler\BeforeStatementAnalysisInterface; use Psalm\Plugin\EventHandler\Event\BeforeStatementAnalysisEvent; use Psalm\Storage\ClassLikeStorage; @@ -74,7 +73,8 @@ public static function beforeStatementAnalysis(BeforeStatementAnalysisEvent $eve $method_id = (string) $method_analyzer->getMethodId(); $marked = (bool) $metadata['test']; - if (!self::isTest($codebase->classlike_storage_provider->get($self), $storage, $method_id, $marked)) { + $class = $codebase->classlike_storage_provider->get($self); + if (!self::isTest($codebase, $class, $storage, $method_id, $marked)) { return null; } @@ -144,10 +144,9 @@ private static function providedTypes( /** * PHPUnit runs public methods named test* or marked as tests. A trait method is a test under the name and with * the visibility the class imports it with. - * - * @psalm-mutation-free */ private static function isTest( + Codebase $codebase, ClassLikeStorage $class, MethodStorage $method, string $method_id, @@ -160,7 +159,7 @@ private static function isTest( } if ( - TestMethod::isPublic($class, $method, $name) + TestCaseHandler::isPublicMethod($codebase, $class, $method, $name) && ($marked || str_starts_with(self::casedName($class, $method, $name), 'test')) ) { return true; diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 89af85e..7e2bece 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -46,7 +46,6 @@ use Psalm\PhpUnitPlugin\Issue\InvalidDataProvider; use Psalm\PhpUnitPlugin\Issue\InvalidPhpUnitAnnotation; use Psalm\PhpUnitPlugin\PhpUnitVersion; -use Psalm\PhpUnitPlugin\TestMethod; use Psalm\Plugin\EventHandler\AfterClassLikeAnalysisInterface; use Psalm\Plugin\EventHandler\AfterClassLikeVisitInterface; use Psalm\Plugin\EventHandler\AfterCodebasePopulatedInterface; @@ -318,7 +317,7 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event // The class declaring an inherited test does not see the providers its descendants implement if (!$declaring_class_storage->is_trait && $declaring_class_storage->name !== $class_storage->name) { - if (TestMethod::isPublic($declaring_class_storage, $method_storage, $method_name_lc)) { + if (self::isPublicMethod($codebase, $class_storage, $method_storage, $method_name_lc)) { self::checkInheritedTestProviders( $codebase, $statements_source, @@ -350,7 +349,7 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event $method_storage->suppressed_issues + $declaring_class_storage->suppressed_issues, ); - $is_test = TestMethod::isPublic($class_storage, $method_storage, $method_name_lc) + $is_test = self::isPublicMethod($codebase, $class_storage, $method_storage, $method_name_lc) && (0 === strpos($method_name_lc, 'test') || isset($specials['test'])); if (!$is_test) { continue; // skip non-test methods @@ -1394,6 +1393,37 @@ private static function countProviderAttributes(ClassMethod $method, Aliases $al return $count; } + /** + * PHPUnit only runs public methods as tests, whatever their name or #[Test] says. A trait method has the + * visibility the class appearing with it imports it with, which a subclass inherits. A method the class declares + * itself keeps its own. + * + * @param lowercase-string $name_lc the name the class knows the method by + * + * @internal + */ + public static function isPublicMethod( + Codebase $codebase, + ClassLikeStorage $class, + MethodStorage $method, + string $name_lc + ): bool { + $declaring_id = $class->declaring_method_ids[$name_lc] ?? null; + $appearing_id = $class->appearing_method_ids[$name_lc] ?? null; + $visibility = $method->visibility; + + if ( + $declaring_id !== null + && $appearing_id !== null + && $codebase->classlike_storage_provider->get($declaring_id->fq_class_name)->is_trait + ) { + $visibility = $codebase->classlike_storage_provider->get($appearing_id->fq_class_name) + ->trait_visibility_map[$appearing_id->method_name] ?? $visibility; + } + + return $visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC; + } + /** * PHPUnit's ParserChain ignores docblock annotations on a class or method that has any PHPUnit attribute. * diff --git a/src/TestMethod.php b/src/TestMethod.php deleted file mode 100644 index 05e76f0..0000000 --- a/src/TestMethod.php +++ /dev/null @@ -1,29 +0,0 @@ -trait_visibility_map[$name_lc] ?? $method->visibility; - - return $visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC; - } -} diff --git a/tests/acceptance/DataProviders.feature b/tests/acceptance/DataProviders.feature index 074ce58..91e279d 100644 --- a/tests/acceptance/DataProviders.feature +++ b/tests/acceptance/DataProviders.feature @@ -1254,16 +1254,8 @@ Feature: Data providers final class MyTestCase extends TestCase { - /** - * @return list - * @psalm-pure - */ - public static function ints(): array { - return [[1]]; - } - #[Test] - #[DataProvider('ints')] + #[DataProvider('missing')] protected function helper(string $s): void { $this->assertSame('a', $s); } @@ -1321,15 +1313,7 @@ Feature: Data providers trait HelperTrait { - /** - * @return list - * @psalm-pure - */ - public static function ints(): array { - return [[1]]; - } - - #[DataProvider('ints')] + #[DataProvider('missing')] public function testHelper(string $s): void { $this->assertSame('a', $s); } @@ -1349,6 +1333,80 @@ Feature: Data providers When I run Psalm Then I see no errors + Scenario: A trait test method imported as public by an abstract parent is checked in a child + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + trait HelperTrait + { + #[DataProvider('provide')] + protected function testHelper(int $int): void { + $this->assertSame(1, $int); + } + } + + abstract class BaseTestCase extends TestCase + { + use HelperTrait { + testHelper as public; + } + + /** + * @return iterable> + * @psalm-pure + */ + abstract public function provide(); + } + + final class MyTestCase extends BaseTestCase + { + /** + * @return iterable> + * @psalm-pure + */ + #[\Override] + public function provide() { + yield [1]; + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidDataProvider | Data provider method NS\MyTestCase::provide must be static | + And I see no other errors + + Scenario: A public test of the class replaces a trait method imported as protected + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + trait HelperTrait + { + public function testSomething(): void { + $this->assertSame(1, 1); + } + } + + final class MyTestCase extends TestCase + { + use HelperTrait { + testSomething as protected; + } + + #[DataProvider('missing')] + public function testSomething(int $int): void { + $this->assertSame(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | UndefinedMethod | Provider method NS\MyTestCase::missing is not defined | + And I see no other errors + Scenario: An unused private test-named method is not marked as used by PHPUnit Given I have the following code """ From a76ca27ac748dd3adfd99d20d64d558f22daa5d7 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:18:38 +0100 Subject: [PATCH 11/24] fix(provider): keep null/false findings next to coercion #248 --- src/Hooks/TestCaseHandler.php | 25 +++++- tests/acceptance/Datasets.feature | 122 +++++++++++++++++++++++++++--- 2 files changed, 135 insertions(+), 12 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 173f477..b1cce16 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -641,9 +641,7 @@ static function ( $codebase, $potential_argument_type, $param_type, - false, - false, - $comparison, + union_comparison_result: $comparison, )) { // ok } elseif ($comparison->type_coerced && !$potential_argument_type->hasMixed()) { @@ -656,6 +654,27 @@ static function ( ? new Issue\MixedArgumentTypeCoercion($message, $provider_docblock_location) : new Issue\ArgumentTypeCoercion($message, $provider_docblock_location) ); + // Core reports these separately from the coercion, so they must not be hidden by it + $provided_by = ' by ' . $provider_name . ':(' . $provider_return_type_string . ')'; + if ($potential_argument_type->isNullable() && !$param_type->isNullable()) { + IssueBuffer::accepts(new Issue\PossiblyNullArgument( + $argument . ' of ' . $method_name + . ' cannot be null, possibly null value provided' . $provided_by, + $provider_docblock_location + )); + } + if ($potential_argument_type->isFalsable() + && !$param_type->isFalsable() + && !$param_type->hasBool() + && !$param_type->hasScalar() + ) { + IssueBuffer::accepts(new Issue\PossiblyFalseArgument( + $argument . ' of ' . $method_name + . ' cannot be false, possibly ' . $param_type->getId() . ' value expected' + . $provided_by, + $provider_docblock_location + )); + } } elseif ($codebase->canTypeBeContainedByType($potential_argument_type, $param_type)) { IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( $argument . ' of ' . $method_name diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index 527ee36..598e692 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -382,9 +382,7 @@ Feature: Datasets yield 'x' => ['a']; } - /** - * @param non-empty-string $s - */ + /** @param non-empty-string $s */ #[DataProvider('provide')] public function testSomething(string $s): void { $this->assertNotSame('', $s); @@ -411,9 +409,7 @@ Feature: Datasets yield 'x' => [null]; } - /** - * @param ?non-empty-string $s - */ + /** @param ?non-empty-string $s */ #[DataProvider('provide')] public function testSomething(?string $s): void { $this->assertNull($s); @@ -440,9 +436,7 @@ Feature: Datasets yield 'x' => ['s' => 'a']; } - /** - * @param non-empty-string $s - */ + /** @param non-empty-string $s */ #[DataProvider('provide')] public function testSomething(string $s): void { $this->assertNotSame('', $s); @@ -480,3 +474,113 @@ Feature: Datasets | Type | Message | | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]) | And I see no other errors + + Scenario: A possibly false provider value is still reported when the rest is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable|false}> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [[]]; + } + + /** @param list $a */ + #[DataProvider('provide')] + public function testSomething(array $a): void { + $this->assertCount(0, $a); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects list, but parent type array\|false provided by NS\MyTestCase::provide():(iterable[pure]\|false}>) | + | PossiblyFalseArgument | Argument 1 of NS\MyTestCase::testSomething cannot be false, possibly list value expected by NS\MyTestCase::provide():(iterable[pure]\|false}>) | + And I see no other errors + + Scenario: A possibly null provider value is still reported when the rest is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable|null}> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [[]]; + } + + /** @param list $a */ + #[DataProvider('provide')] + public function testSomething(array $a): void { + $this->assertCount(0, $a); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects list, but parent type array\|null provided by NS\MyTestCase::provide():(iterable[pure]\|null}>) | + | PossiblyNullArgument | Argument 1 of NS\MyTestCase::testSomething cannot be null, possibly null value provided by NS\MyTestCase::provide():(iterable[pure]\|null}>) | + And I see no other errors + + Scenario: A nullable list provider value is an argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable}> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [[]]; + } + + /** @param list $a */ + #[DataProvider('provide')] + public function testSomething(array $a): void { + $this->assertCount(0, $a); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects list, but parent type array provided by NS\MyTestCase::provide():(iterable[pure]}>) | + And I see no other errors + + Scenario: A mixed array value is a mixed argument type coercion + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable}> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => [[]]; + } + + /** @param array $a */ + #[DataProvider('provide')] + public function testSomething(array $a): void { + $this->assertCount(0, $a); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | MixedArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects array, but parent type array provided by NS\MyTestCase::provide():(iterable[pure]}>) | + And I see no other errors From 93b10a1fc45829bf66bd481b607020f379fdf04a Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:19:24 +0100 Subject: [PATCH 12/24] fix(provider): keep open list tails to arity and dependency bounds #247 --- src/Hooks/TestCaseHandler.php | 57 ++++++++++++++++--------------- tests/acceptance/Datasets.feature | 57 +++++++++++++++++-------------- 2 files changed, 60 insertions(+), 54 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index a9bb77d..130512c 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -743,18 +743,35 @@ static function ( // let's hope properties are sorted in array order $potential_argument_types = array_values($dataset_type->properties); - $params_count = count($method_storage->params); + // An open list supplies any number of further positions. A `mixed` tail only tells the + // arity: nothing is known about its values, so they are not checked. + $list_tail = $dataset_type->is_list ? $dataset_type->fallback_params[1] ?? null : null; + $typed_tail = $list_tail !== null && !$list_tail->hasMixed() ? $list_tail : null; + + // PHPUnit appends the return values of depended-upon tests right after the dataset, so the + // dataset certainly supplies only the positions that the required params need beyond them. + // A trailing variadic param could consume them as well. + $last_param = array_slice($method_storage->params, -1)[0] ?? null; + $tail_checked_until = count($method_storage->params); + if ($dependency_values > 0 && ($last_param === null || !$last_param->is_variadic)) { + $required_count = 0; + foreach ($method_storage->params as $param_offset => $param) { + if (!$param->is_optional) { + $required_count = $param_offset + 1; + } + } + $tail_checked_until = $required_count - $dependency_values; + } + foreach ($method_storage->params as $param_offset => $param) { $potential_argument_type = $potential_argument_types[$param_offset] ?? null; - if ($potential_argument_type === null) { - // an open list can supply any further position - $fallback_type = self::getPositionalFallbackType($codebase, $dataset_type, $param_offset); - if ($fallback_type !== null) { - // the return values of depended-upon tests follow the provider values - if (!$param->is_variadic && $param_offset >= $params_count - $dependency_values) { - break; - } - $potential_argument_type = $fallback_type; + if ($potential_argument_type === null && $list_tail !== null) { + if ($param->is_variadic) { + $potential_argument_type = $list_tail; + } elseif ($typed_tail !== null && $param_offset < $tail_checked_until) { + $potential_argument_type = $typed_tail; + } else { + continue; } } @@ -800,9 +817,8 @@ static function ( $param_offset ); } - $fallback_type = self::getPositionalFallbackType($codebase, $dataset_type, $param_offset); - if ($fallback_type !== null) { - $checkParam($fallback_type, $variadic_param_type, true, $param_offset); + if ($typed_tail !== null) { + $checkParam($typed_tail, $variadic_param_type, true, $param_offset); } break; } @@ -1002,21 +1018,6 @@ public function enterNode(Node $node) } } - /** The value type an unsealed dataset supplies at a position its known entries do not cover. */ - private static function getPositionalFallbackType( - Codebase $codebase, - Type\Atomic\TKeyedArray $dataset_type, - int $position - ): ?Type\Union { - if ($dataset_type->fallback_params === null) { - return null; - } - - [$key_type, $value_type] = $dataset_type->fallback_params; - - return $codebase->canTypeBeContainedByType(Type::getInt(false, $position), $key_type) ? $value_type : null; - } - /** @psalm-pure */ public static function hasStringKeys(Type\Atomic\TKeyedArray $dataset_type, bool $required_only = false): bool { diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index 218539e..8536e10 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -418,32 +418,6 @@ Feature: Datasets | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects string, int provided by NS\MyTestCase::provide():(iterable[pure]>) | And I see no other errors - Scenario: A sealed list dataset still cannot supply more parameters than it has entries - Given I have the following code - """ - use PHPUnit\Framework\Attributes\DataProvider; - - final class MyTestCase extends TestCase { - /** - * @return iterable - * @psalm-pure - */ - public static function provide(): iterable { - yield 'x' => ['a']; - } - - #[DataProvider('provide')] - public function testSomething(string $a, string $b): void { - $this->assertSame($a, $b); - } - } - """ - When I run Psalm - Then I see these errors - | Type | Message | - | TooFewArguments | Too few arguments for NS\MyTestCase::testSomething - expecting at least 2, but saw 1 provided by NS\MyTestCase::provide():(iterable[pure]) | - And I see no other errors - Scenario: A possibly empty list dataset is reported for a required first parameter only Given I have the following code """ @@ -565,3 +539,34 @@ Feature: Datasets """ When I run Psalm Then I see no errors + + Scenario: An open list dataset leaves optional parameters after the dependency params unchecked + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + use PHPUnit\Framework\Attributes\Depends; + + final class MyTestCase extends TestCase { + public function testOne(): int { + $this->assertTrue(true); + return 1; + } + + /** + * @return iterable> + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['a']; + } + + #[DataProvider('provide')] + #[Depends('testOne')] + public function testSecond(string $a, int $one, bool $unused = false): void { + $this->assertSame($a, (string) $one); + $this->assertFalse($unused); + } + } + """ + When I run Psalm + Then I see no errors From cb0517b742db41ca7779a5c0a33bd6dc3f6041e5 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:19:40 +0100 Subject: [PATCH 13/24] fix(missing-constructor): walk each used trait once #252 --- src/Hooks/TestCaseHandler.php | 30 ++++++++++++++++++++------- tests/acceptance/Initializers.feature | 4 ---- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 3178d74..3fa9581 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -175,19 +175,33 @@ public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event } } - /** Trait methods are not merged into the class yet when the class is visited, so its own flag misses them. */ - private static function hasInitializersFromTraits(Codebase $codebase, ClassLikeStorage $storage): bool - { + /** + * Trait methods are not merged into the class yet when the class is visited, so its own flag misses them. + * + * @param array $visited Psalm keeps circular trait edges, so each trait is walked once + */ + private static function hasInitializersFromTraits( + Codebase $codebase, + ClassLikeStorage $storage, + array &$visited = [], + ): bool { $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); if (($meta['hasInitializers'] ?? false) || isset($storage->trait_alias_map['setup'])) { return true; } - foreach ($storage->used_traits as $trait_name) { - if ( - $codebase->classlike_storage_provider->has($trait_name) - && self::hasInitializersFromTraits($codebase, $codebase->classlike_storage_provider->get($trait_name)) - ) { + foreach ($storage->used_traits as $trait_name_lc => $trait_name) { + if (isset($visited[$trait_name_lc]) || !$codebase->classlike_storage_provider->has($trait_name)) { + continue; + } + + $visited[$trait_name_lc] = true; + + if (self::hasInitializersFromTraits( + $codebase, + $codebase->classlike_storage_provider->get($trait_name), + $visited, + )) { return true; } } diff --git a/tests/acceptance/Initializers.feature b/tests/acceptance/Initializers.feature index 8639bfd..9e41c5f 100644 --- a/tests/acceptance/Initializers.feature +++ b/tests/acceptance/Initializers.feature @@ -423,10 +423,6 @@ Feature: MissingConstructor in stateful test cases final class MyTestCase extends TestCase { use State; - - public function testSomething(): void { - $this->assertSame(1, 1); - } } """ When I run Psalm From 9ec927f50dcd3a1afac2e410735faf460759ec17 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:21:08 +0100 Subject: [PATCH 14/24] style: apply phpcs fixes #248 --- src/Hooks/TestCaseHandler.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index b1cce16..b4cbbef 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -637,12 +637,14 @@ static function ( $param_type = $param_type->setPossiblyUndefined(true); } $comparison = new TypeComparisonResult(); - if (UnionTypeComparator::isContainedBy( - $codebase, - $potential_argument_type, - $param_type, - union_comparison_result: $comparison, - )) { + if ( + UnionTypeComparator::isContainedBy( + $codebase, + $potential_argument_type, + $param_type, + union_comparison_result: $comparison, + ) + ) { // ok } elseif ($comparison->type_coerced && !$potential_argument_type->hasMixed()) { $message = $argument . ' of ' . $method_name @@ -663,7 +665,8 @@ static function ( $provider_docblock_location )); } - if ($potential_argument_type->isFalsable() + if ( + $potential_argument_type->isFalsable() && !$param_type->isFalsable() && !$param_type->hasBool() && !$param_type->hasScalar() From 5775d502c9009b182d4d48387822f44a8456f6e1 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:21:13 +0100 Subject: [PATCH 15/24] style: apply phpcs fixes #252 --- src/Hooks/TestCaseHandler.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 3fa9581..89f59ed 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -197,11 +197,13 @@ private static function hasInitializersFromTraits( $visited[$trait_name_lc] = true; - if (self::hasInitializersFromTraits( - $codebase, - $codebase->classlike_storage_provider->get($trait_name), - $visited, - )) { + if ( + self::hasInitializersFromTraits( + $codebase, + $codebase->classlike_storage_provider->get($trait_name), + $visited, + ) + ) { return true; } } From 97fcded3437acecb0fd2ae2f6cb1f2f78b9229ed Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:26:54 +0100 Subject: [PATCH 16/24] fix(provider): expand self and aliases in provider checks per class #249 --- src/Hooks/TestCaseHandler.php | 62 +++++++++++++++++++++++-------- tests/acceptance/Datasets.feature | 30 +++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 93904cd..c0ea111 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -477,6 +477,13 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event continue; } + $provider_return_type = self::expandForMethod( + $codebase, + $provider_return_type, + $apparent_provider_method_name, + $provider['class'] ?? $class_storage->name, + ); + $provider_return_type_location = $codebase->getMethodReturnTypeLocation($provider_method_id); assert(null !== $provider_return_type_location); $provided_types[] = [ @@ -514,8 +521,13 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ]; } - // Test parameters may use type aliases, which a provider's datasets are never typed with - $params = self::expandParams($codebase, $method_storage->params, $class_storage); + // Expand parameter aliases before checking datasets + $params = self::expandParams( + $codebase, + $method_storage->params, + $class_storage, + $class_storage->name . '::' . $method_name_lc, + ); foreach ($provided_types as $provided) { $provider_name = $provided['name']; @@ -803,28 +815,48 @@ static function ( } } + /** + * `self` is the class the method appears in (the class importing a trait's method), `static` the analysed one. + * + * @param class-string $static_class + */ + private static function expandForMethod( + Codebase $codebase, + Type\Union $type, + string $apparent_method_name, + string $static_class + ): Type\Union { + $appearing_id = $codebase->getAppearingMethodId($apparent_method_name); + $self_class = $appearing_id === null ? $static_class : MethodIdentifier::wrap($appearing_id)->fq_class_name; + + return TypeExpander::expandUnion( + $codebase, + $type, + $self_class, + $static_class, + $codebase->classlike_storage_provider->get($self_class)->parent_class, + ); + } + /** * @param list $params * @return list */ - private static function expandParams(Codebase $codebase, array $params, ClassLikeStorage $class): array - { - $expanded = []; - foreach ($params as $param) { - $param = clone $param; + private static function expandParams( + Codebase $codebase, + array $params, + ClassLikeStorage $class, + string $apparent_method_name + ): array { + foreach ($params as $offset => $param) { if ($param->type !== null) { - $param->type = TypeExpander::expandUnion( - $codebase, - $param->type, - $class->name, - $class->name, - $class->parent_class, + $params[$offset] = $param->setType( + self::expandForMethod($codebase, $param->type, $apparent_method_name, $class->name), ); } - $expanded[] = $param; } - return $expanded; + return $params; } /** diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index c125768..22b8914 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -554,3 +554,33 @@ Feature: Datasets | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | And I see no other errors + + Scenario: self in a trait test and its provider means the class using the trait + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + trait ProvidesSelf { + /** + * @return list}> + * @psalm-pure + */ + public static function provide(): array { + return [[self::class]]; + } + + /** + * @param class-string $class + */ + #[DataProvider('provide')] + public function testSomething(string $class): void { + $this->assertSame(self::class, $class); + } + } + + final class MyTestCase extends TestCase { + use ProvidesSelf; + } + """ + When I run Psalm + Then I see no errors From a1962ebbfccadf5871220c91b24bbb2bcd0e1a08 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:31:13 +0100 Subject: [PATCH 17/24] fix(provider): accept any class name string when expanding types #249 --- src/Hooks/TestCaseHandler.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index c0ea111..e4d33b7 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -817,8 +817,6 @@ static function ( /** * `self` is the class the method appears in (the class importing a trait's method), `static` the analysed one. - * - * @param class-string $static_class */ private static function expandForMethod( Codebase $codebase, From 94bf8f96968604267bbb32b4a6da620b1a521660 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:58:27 +0100 Subject: [PATCH 18/24] test(provider): expect coercion for alias datasets after #248 #249 --- tests/acceptance/Datasets.feature | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index f970609..2db3da9 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -382,8 +382,8 @@ Feature: Datasets When I run Psalm Then I see these errors | Type | Message | Line | - | InvalidArgument | Argument 1 of NS\MyTestCase::testAlias expects array{host: string}, array provided by NS\MyTestCase::provide():(list}>) | 23 | - | InvalidArgument | Argument 1 of NS\MyTestCase::testShape expects array{host: string}, array provided by NS\MyTestCase::provide():(list}>) | 31 | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testAlias expects array{host: string}, but parent type array provided by NS\MyTestCase::provide():(list}>) | 23 | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testShape expects array{host: string}, but parent type array provided by NS\MyTestCase::provide():(list}>) | 31 | And I see no other errors Scenario: An imported type alias in a test parameter accepts a matching dataset @@ -462,8 +462,8 @@ Feature: Datasets When I run Psalm Then I see these errors | Type | Message | Line | - | InvalidArgument | Argument 1 ($options) of NS\MyTestCase::testNamed expects array{host: string}, array provided by NS\MyTestCase::named():(list}>) | 31 | - | InvalidArgument | Argument 1 of NS\MyTestCase::testLengthless expects array{host: string}, array provided by NS\MyTestCase::lengthless():(list>>) | 39 | + | ArgumentTypeCoercion | Argument 1 ($options) of NS\MyTestCase::testNamed expects array{host: string}, but parent type array provided by NS\MyTestCase::named():(list}>) | 31 | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testLengthless expects array{host: string}, but parent type array provided by NS\MyTestCase::lengthless():(list>>) | 39 | And I see no other errors Scenario: A test parameter typed with an imported type alias is narrowed to the dataset type @@ -527,8 +527,8 @@ Feature: Datasets When I run Psalm Then I see these errors | Type | Message | - | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | - | InvalidArgument | Argument 2 of NS\MyTestCase::testSomething expects array{host: string}, array provided by NS\MyTestCase::provide():(list, array}>) | + | ArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects array{host: string}, but parent type array provided by NS\MyTestCase::provide():(list, array}>) | + | ArgumentTypeCoercion | Argument 2 of NS\MyTestCase::testSomething expects array{host: string}, but parent type array provided by NS\MyTestCase::provide():(list, array}>) | And I see no other errors Scenario: self in a trait test and its provider means the class using the trait From 610c0694e2e4d20d04de84642184a4ee5809b7ad Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:05:06 +0100 Subject: [PATCH 19/24] test(provider): honour suppressions of argument coercions #248 --- tests/acceptance/Datasets.feature | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index 68750fd..9892b74 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -948,3 +948,30 @@ Feature: Datasets | Type | Message | | MixedArgumentTypeCoercion | Argument 1 of NS\MyTestCase::testSomething expects array, but parent type array provided by NS\MyTestCase::provide():(iterable[pure]}>) | And I see no other errors + + Scenario: An argument type coercion of the datasets can be suppressed on the test method + Given I have the following code + """ + use PHPUnit\Framework\Attributes\DataProvider; + + final class MyTestCase extends TestCase { + /** + * @return iterable + * @psalm-pure + */ + public static function provide(): iterable { + yield 'x' => ['x']; + } + + /** + * @psalm-suppress ArgumentTypeCoercion + * @param non-empty-string $s + */ + #[DataProvider('provide')] + public function testSomething(string $s): void { + $this->assertNotEmpty($s); + } + } + """ + When I run Psalm + Then I see no errors From b466cdf5de426abc90ba898f17ba017fa7046744 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:18:22 +0100 Subject: [PATCH 20/24] Reuse the computed specials to detect hook methods --- src/Hooks/TestCaseHandler.php | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index b368338..f4c976d 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -361,15 +361,13 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event self::reportMalformedTags($stmt_method, $method_storage->location, $test_suppressions); $is_test = 0 === strpos($method_name_lc, 'test') || isset($specials['test']); - if (!$is_test && !self::isHook($stmt_method, $method_aliases, $method_storage)) { - continue; // skip methods PHPUnit never calls + if ($is_test || self::isHook($specials, $method_storage)) { + $codebase->methodExists( + (string) $declaring_method_id, + calling_method_id: 'PHPUnit\Framework\TestSuite::run', + ); } - $codebase->methodExists( - (string) $declaring_method_id, - calling_method_id: 'PHPUnit\Framework\TestSuite::run', - ); - if (!$is_test) { continue; } @@ -1091,7 +1089,8 @@ private static function hasInitializers(ClassLikeStorage $storage, ClassLike $st private static function isBeforeInitializer(ClassMethod $method, Aliases $aliases): bool { - return isset(self::getSpecials($method, $aliases)['before']); + return isset(self::docblockSpecials($method)['before']) + || self::attributeValue($method, $aliases, Before::class) !== null; } /** @@ -1519,13 +1518,9 @@ public static function classFromExpression( return Type::getFQCLNFromString($expression->class->toCodeString(), $aliases); } - /** PHPUnit ignores a docblock tag on a method that has any PHPUnit attribute. */ - private static function isHook(ClassMethod $method, Aliases $aliases, MethodStorage $storage): bool + /** @param array> $specials */ + private static function isHook(array $specials, MethodStorage $storage): bool { - $specials = self::hasPhpUnitAttribute($method, $aliases) - ? self::getAttributeSpecials($method, $aliases) - : self::docblockSpecials($method); - // PHPUnit runs the class-level hooks only when they are static return isset($specials['before']) || isset($specials['after']) From dccbc07e8a82edfb0d3972cd03e32bf64b2e6ba8 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:18:22 +0100 Subject: [PATCH 21/24] Find initializers through the methods Psalm merged into the class Psalm has resolved parents, traits, aliases and insteadof into declaring_method_ids by AfterCodebasePopulated, so a setUp() not declared by TestCase or a #[Before] method of any declaring class is an initializer. This replaces the manual trait walk, the setUp alias special case and the descendant loop. --- src/Hooks/TestCaseHandler.php | 88 +++++++++-------------------------- 1 file changed, 22 insertions(+), 66 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 69ee8ed..71ba387 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -169,47 +169,29 @@ public static function afterCodebasePopulated(AfterCodebasePopulatedEvent $event foreach ($codebase->classlike_storage_provider->getAll() as $name => $storage) { if ( $codebase->classExtends($name, 'PHPUnit\Framework\TestCase') - && self::hasInitializersFromTraits($codebase, $storage) + && self::hasInitializers($codebase, $storage) ) { $storage->suppressed_issues[] = 'MissingConstructor'; - - foreach (self::getDescendants($codebase, $name) as $dependent_name) { - $dependent_storage = $codebase->classlike_storage_provider->get($dependent_name); - $dependent_storage->suppressed_issues[] = 'MissingConstructor'; - } } } } /** - * Trait methods are not merged into the class yet when the class is visited, so its own flag misses them. - * - * @param array $visited Psalm keeps circular trait edges, so each trait is walked once + * By now Psalm has merged the methods of parents and traits, with their aliases and exclusions, into the class. */ - private static function hasInitializersFromTraits( - Codebase $codebase, - ClassLikeStorage $storage, - array &$visited = [], - ): bool { - $meta = (array) ($storage->custom_metadata[__NAMESPACE__] ?? []); - if (($meta['hasInitializers'] ?? false) || isset($storage->trait_alias_map['setup'])) { - return true; - } - - foreach ($storage->used_traits as $trait_name_lc => $trait_name) { - if (isset($visited[$trait_name_lc]) || !$codebase->classlike_storage_provider->has($trait_name)) { + private static function hasInitializers(Codebase $codebase, ClassLikeStorage $storage): bool + { + foreach ($storage->declaring_method_ids as $name_lc => $method_id) { + if ($name_lc === 'setup') { + if (strtolower($method_id->fq_class_name) !== 'phpunit\framework\testcase') { + return true; + } continue; } - $visited[$trait_name_lc] = true; - - if ( - self::hasInitializersFromTraits( - $codebase, - $codebase->classlike_storage_provider->get($trait_name), - $visited, - ) - ) { + $declaring = $codebase->classlike_storage_provider->get($method_id->fq_class_name); + $meta = (array) ($declaring->custom_metadata[__NAMESPACE__] ?? []); + if (isset($meta['before'][$method_id->method_name])) { return true; } } @@ -217,29 +199,6 @@ private static function hasInitializersFromTraits( return false; } - /** - * @return string[] - * - * @psalm-mutation-free - */ - private static function getDescendants(Codebase $codebase, string $name): array - { - if (!$codebase->classlike_storage_provider->has($name)) { - return []; - } - - $storage = $codebase->classlike_storage_provider->get($name); - $ret = []; - - foreach ($storage->dependent_classlikes as $dependent => $_) { - if ($codebase->classExtends($dependent, $name)) { - $ret[] = $dependent; - $ret = array_merge($ret, self::getDescendants($codebase, $dependent)); - } - } - return $ret; - } - #[\Override] public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) { @@ -254,8 +213,9 @@ public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event) $class_storage->sealed_methods = false; } - if (self::hasInitializers($class_storage, $class_node, $aliases)) { - $class_storage->custom_metadata[__NAMESPACE__] = ['hasInitializers' => true]; + $before = self::getBeforeMethods($class_storage, $class_node, $aliases); + if ($before !== []) { + $class_storage->custom_metadata[__NAMESPACE__] = ['before' => $before]; } $file_path = $statements_source->getFilePath(); @@ -1109,22 +1069,18 @@ static function ($a, Type\Union $b) use ($codebase): Type\Union { } - private static function hasInitializers(ClassLikeStorage $storage, ClassLike $stmt, Aliases $aliases): bool + /** @return array */ + private static function getBeforeMethods(ClassLikeStorage $storage, ClassLike $stmt, Aliases $aliases): array { - if (isset($storage->methods['setup'])) { - return true; - } - + $before = []; foreach ($storage->methods as $method => $_) { $stmt_method = $stmt->getMethod($method); - if (!$stmt_method) { - continue; - } - if (self::isBeforeInitializer($stmt_method, $aliases)) { - return true; + if ($stmt_method && self::isBeforeInitializer($stmt_method, $aliases)) { + $before[$method] = true; } } - return false; + + return $before; } private static function isBeforeInitializer(ClassMethod $method, Aliases $aliases): bool From b236802b9bd79e80f47413c492d1f72d49c6d732 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:21:38 +0100 Subject: [PATCH 22/24] Build the provider argument messages once --- src/Hooks/TestCaseHandler.php | 48 ++++++++++++++--------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index e7b6970..e14f7bb 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -697,11 +697,16 @@ static function ( $provider_docblock_location, $test_suppressions ): void { - $argument = 'Argument ' . ($param_offset + 1) - . ($param_name === null ? '' : ' ($' . $param_name . ')'); + $subject = 'Argument ' . ($param_offset + 1) + . ($param_name === null ? '' : ' ($' . $param_name . ')') + . ' of ' . $method_name; + $by = ' by ' . $provider_name . ':(' . $provider_return_type_string . ')'; if ($is_optional) { $param_type = $param_type->setPossiblyUndefined(true); } + $param_id = $param_type->getId(); + $argument_id = $potential_argument_type->getId(); + $comparison = new TypeComparisonResult(); if ( UnionTypeComparator::isContainedBy( @@ -713,10 +718,8 @@ static function ( ) { // ok } elseif ($comparison->type_coerced && !$potential_argument_type->hasMixed()) { - $message = $argument . ' of ' . $method_name - . ' expects ' . $param_type->getId() . ', but parent type ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')'; + $message = $subject . ' expects ' . $param_id . ', but parent type ' . $argument_id + . ' provided' . $by; IssueBuffer::accepts( $comparison->type_coerced_from_mixed ? new Issue\MixedArgumentTypeCoercion($message, $provider_docblock_location) @@ -724,12 +727,10 @@ static function ( $test_suppressions, ); // Core reports these separately from the coercion, so they must not be hidden by it - $provided_by = ' by ' . $provider_name . ':(' . $provider_return_type_string . ')'; if ($potential_argument_type->isNullable() && !$param_type->isNullable()) { IssueBuffer::accepts(new Issue\PossiblyNullArgument( - $argument . ' of ' . $method_name - . ' cannot be null, possibly null value provided' . $provided_by, - $provider_docblock_location + $subject . ' cannot be null, possibly null value provided' . $by, + $provider_docblock_location, ), $test_suppressions); } if ( @@ -739,35 +740,24 @@ static function ( && !$param_type->hasScalar() ) { IssueBuffer::accepts(new Issue\PossiblyFalseArgument( - $argument . ' of ' . $method_name - . ' cannot be false, possibly ' . $param_type->getId() . ' value expected' - . $provided_by, - $provider_docblock_location + $subject . ' cannot be false, possibly ' . $param_id . ' value expected' . $by, + $provider_docblock_location, ), $test_suppressions); } } elseif ($codebase->canTypeBeContainedByType($potential_argument_type, $param_type)) { IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( - $argument . ' of ' . $method_name - . ' expects ' . $param_type->getId() . ', ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location + $subject . ' expects ' . $param_id . ', ' . $argument_id . ' provided' . $by, + $provider_docblock_location, ), $test_suppressions); } elseif ($potential_argument_type->possibly_undefined && !$is_optional) { IssueBuffer::accepts(new Issue\InvalidArgument( - $argument . ' of ' . $method_name - . ' is not optional, but possibly undefined ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location + $subject . ' is not optional, but possibly undefined ' . $argument_id . ' provided' . $by, + $provider_docblock_location, ), $test_suppressions); } else { IssueBuffer::accepts(new Issue\InvalidArgument( - $argument . ' of ' . $method_name - . ' expects ' . $param_type->getId() . ', ' - . $potential_argument_type->getId() . ' provided' - . ' by ' . $provider_name . ':(' . $provider_return_type_string . ')', - $provider_docblock_location + $subject . ' expects ' . $param_id . ', ' . $argument_id . ' provided' . $by, + $provider_docblock_location, ), $test_suppressions); } }; From 8d89598b0bd405519cfeab0009c29237222c788e Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:27:37 +0100 Subject: [PATCH 23/24] Share the self/static expansion of provider types with the narrowing handler --- src/Hooks/DataProviderNarrowingHandler.php | 13 +------------ src/Hooks/TestCaseHandler.php | 11 ++++++----- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/Hooks/DataProviderNarrowingHandler.php b/src/Hooks/DataProviderNarrowingHandler.php index d8e155c..9237d71 100644 --- a/src/Hooks/DataProviderNarrowingHandler.php +++ b/src/Hooks/DataProviderNarrowingHandler.php @@ -6,8 +6,6 @@ use Psalm\Codebase; use Psalm\Internal\Analyzer\MethodAnalyzer; -use Psalm\Internal\MethodIdentifier; -use Psalm\Internal\Type\TypeExpander; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; use Psalm\Plugin\EventHandler\BeforeStatementAnalysisInterface; use Psalm\Plugin\EventHandler\Event\BeforeStatementAnalysisEvent; @@ -205,16 +203,7 @@ private static function datasetType(Codebase $codebase, string $class, string $m return null; } - // A trait provider is a method of the class that uses it, so self is that class and not the trait - $appearing_id = $codebase->getAppearingMethodId($class . '::' . $method); - $self_class = $appearing_id === null ? $class : MethodIdentifier::wrap($appearing_id)->fq_class_name; - $return_type = TypeExpander::expandUnion( - $codebase, - $return_type, - $self_class, - $class, - $codebase->classlike_storage_provider->get($self_class)->parent_class, - ); + $return_type = TestCaseHandler::expandForMethod($codebase, $return_type, $class . '::' . $method, $class); if ($return_type->hasTemplate()) { return null; } diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index f55ada1..3d14239 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -584,11 +584,10 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ]; } - // Expand parameter aliases before checking datasets $params = self::expandParams( $codebase, $method_storage->params, - $class_storage, + $class_storage->name, $class_storage->name . '::' . $method_name_lc, ); @@ -958,8 +957,10 @@ static function ( /** * `self` is the class the method appears in (the class importing a trait's method), `static` the analysed one. + * + * @internal */ - private static function expandForMethod( + public static function expandForMethod( Codebase $codebase, Type\Union $type, string $apparent_method_name, @@ -984,13 +985,13 @@ private static function expandForMethod( private static function expandParams( Codebase $codebase, array $params, - ClassLikeStorage $class, + string $static_class, string $apparent_method_name ): array { foreach ($params as $offset => $param) { if ($param->type !== null) { $params[$offset] = $param->setType( - self::expandForMethod($codebase, $param->type, $apparent_method_name, $class->name), + self::expandForMethod($codebase, $param->type, $apparent_method_name, $static_class), ); } } From 5117aab852590ecbea170014eba61cfab752dc65 Mon Sep 17 00:00:00 2001 From: Alies Lapatsin <5278175+alies-dev@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:30:24 +0100 Subject: [PATCH 24/24] Use the required parameter count Psalm already records --- src/Hooks/TestCaseHandler.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Hooks/TestCaseHandler.php b/src/Hooks/TestCaseHandler.php index 6a1b46d..29648f7 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -820,13 +820,8 @@ static function ( $last_param = array_slice($method_storage->params, -1)[0] ?? null; $tail_checked_until = count($method_storage->params); if ($dependency_values > 0 && ($last_param === null || !$last_param->is_variadic)) { - $required_count = 0; - foreach ($method_storage->params as $param_offset => $param) { - if (!$param->is_optional) { - $required_count = $param_offset + 1; - } - } - $tail_checked_until = $required_count - $dependency_values; + $tail_checked_until = ($method_storage->required_param_count ?? $tail_checked_until) + - $dependency_values; } foreach ($method_storage->params as $param_offset => $param) {