diff --git a/src/Hooks/DataProviderNarrowingHandler.php b/src/Hooks/DataProviderNarrowingHandler.php index 2e54d52..8bfcb46 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 a2a8b00..9543c0f 100644 --- a/src/Hooks/TestCaseHandler.php +++ b/src/Hooks/TestCaseHandler.php @@ -46,6 +46,7 @@ use Psalm\Internal\MethodIdentifier; use Psalm\Internal\Type\Comparator\TypeComparisonResult; use Psalm\Internal\Type\Comparator\UnionTypeComparator; +use Psalm\Internal\Type\TypeExpander; use Psalm\IssueBuffer; use Psalm\Issue; use Psalm\PhpUnitPlugin\Dataset\DatasetBinding; @@ -64,6 +65,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; @@ -483,6 +485,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[] = [ @@ -525,6 +534,13 @@ public static function afterStatementAnalysis(AfterClassLikeAnalysisEvent $event ]; } + $params = self::expandParams( + $codebase, + $method_storage->params, + $class_storage->name, + $class_storage->name . '::' . $method_name_lc, + ); + foreach ($provided_types as $provided) { $provider_name = $provided['name']; $provider_return_type = $provided['type']; @@ -718,13 +734,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]; @@ -736,7 +752,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( @@ -800,14 +816,14 @@ static function ( // 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); + $last_param = array_slice($params, -1)[0] ?? null; + $tail_checked_until = count($params); if ($dependency_values > 0 && ($last_param === null || !$last_param->is_variadic)) { $tail_checked_until = ($method_storage->required_param_count ?? $tail_checked_until) - $dependency_values; } - foreach ($method_storage->params as $param_offset => $param) { + foreach ($params as $param_offset => $param) { $potential_argument_type = $potential_argument_types[$param_offset] ?? null; if ($potential_argument_type === null && $list_tail !== null) { if ($param->is_variadic) { @@ -874,6 +890,50 @@ static function ( } } + /** + * `self` is the class the method appears in (the class importing a trait's method), `static` the analysed one. + * + * @internal + */ + public 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, + 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, $static_class), + ); + } + } + + return $params; + } + /** * Checks the providers of a test inherited from an ancestor that the class implements or overrides itself. * diff --git a/tests/acceptance/Datasets.feature b/tests/acceptance/Datasets.feature index 9892b74..b0ff459 100644 --- a/tests/acceptance/Datasets.feature +++ b/tests/acceptance/Datasets.feature @@ -975,3 +975,220 @@ Feature: Datasets """ When I run Psalm Then I see no 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 | + | 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 + 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 | + | 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 + 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 | + | 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 + 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