Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/Rules/PHPUnit/DataProviderDataRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ class DataProviderDataRule implements Rule

private DataProviderHelper $dataProviderHelper;

private PHPUnitVersion $PHPUnitVersion;

public function __construct(
TestMethodsHelper $testMethodsHelper,
DataProviderHelper $dataProviderHelper
DataProviderHelper $dataProviderHelper,
PHPUnitVersion $PHPUnitVersion
)
{
$this->testMethodsHelper = $testMethodsHelper;
$this->dataProviderHelper = $dataProviderHelper;
$this->PHPUnitVersion = $PHPUnitVersion;
}

public function getNodeType(): string
Expand Down Expand Up @@ -153,11 +157,10 @@ private function arrayItemsToArgs(Type $array, int $numberOfParameters): ?array
return null;
}

if (count($key) === 0) {
if (count($key) === 0 || !$this->PHPUnitVersion->supportsNamedArgumentsInDataProvider()->yes()) {
$arg = new Node\Arg(new TypeExpr($valueType));
$args[] = $arg;
continue;

}

$arg = new Node\Arg(
Expand Down
8 changes: 8 additions & 0 deletions src/Rules/PHPUnit/PHPUnitVersion.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,12 @@ public function requiresStaticDataProviders(): TrinaryLogic
return TrinaryLogic::createFromBoolean($this->majorVersion >= 10);
}

public function supportsNamedArgumentsInDataProvider(): TrinaryLogic
{
if ($this->majorVersion === null) {
return TrinaryLogic::createMaybe();
}
return TrinaryLogic::createFromBoolean($this->majorVersion >= 11);
}

}
126 changes: 98 additions & 28 deletions tests/Rules/PHPUnit/DataProviderDataRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,36 @@
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStan\Type\FileTypeMapper;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\TestWith;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<CompositeRule>
*/
class DataProviderDataRuleTest extends RuleTestCase
{
private int $phpunitVersion;
private ?int $phpunitVersion;

protected function getRule(): Rule
{
$reflectionProvider = $this->createReflectionProvider();
$phpunitVersion = new PHPUnitVersion($this->phpunitVersion);

/** @var list<Rule<Node>> $rules */
$rules = [
new DataProviderDataRule(
new TestMethodsHelper(
self::getContainer()->getByType(FileTypeMapper::class),
new PHPUnitVersion($this->phpunitVersion)
$phpunitVersion
),
new DataProviderHelper(
$reflectionProvider,
self::getContainer()->getByType(FileTypeMapper::class),
self::getContainer()->getService('defaultAnalysisParser'),
new PHPUnitVersion($this->phpunitVersion)
$phpunitVersion
),

$phpunitVersion,
),
self::getContainer()->getByType(CallMethodsRule::class) /** @phpstan-ignore phpstanApi.classConstant */
];
Expand Down Expand Up @@ -173,36 +177,64 @@ public function testRule(): void
]);
}

public function testRulePhp8(): void

/**
* @dataProvider provideNamedArgumentPHPUnitVersions
*/
#[DataProvider('provideNamedArgumentPHPUnitVersions')]
public function testRulePhp8(?int $phpunitVersion): void
{
if (PHP_VERSION_ID < 80000) {
self::markTestSkipped();
}

$this->phpunitVersion = 10;
$this->phpunitVersion = $phpunitVersion;

$this->analyse([__DIR__ . '/data/data-provider-data-named.php'], [
[
'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.',
44
],
[
'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, false given.',
44
],
[
'Unknown parameter $wrong in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().',
58
],
[
'Missing parameter $si (int) in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().',
58
],
[
'Parameter $si of method DataProviderDataTestPhp8\TestWrongTypeInArrayShapeIterable::testBar() expects int, string given.',
79
],
]);
if ($phpunitVersion >= 11) {
$errors = [
[
'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.',
44
],
[
'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, false given.',
44
],
[
'Unknown parameter $wrong in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().',
58
],
[
'Missing parameter $si (int) in call to method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar().',
58
],
[
'Parameter $si of method DataProviderDataTestPhp8\TestWrongTypeInArrayShapeIterable::testBar() expects int, string given.',
79
],
];
} else {
$errors = [
[
'Parameter #1 $expectedResult of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.',
44
],
[
'Parameter #1 $expectedResult of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, false given.',
44
],
[
'Parameter #1 $si of method DataProviderDataTestPhp8\TestWrongOffsetNameArrayShapeIterable::testBar() expects int, string given.',
58
],
[
'Parameter #1 $si of method DataProviderDataTestPhp8\TestWrongTypeInArrayShapeIterable::testBar() expects int, string given.',
79
],
];
}

$this->analyse([__DIR__ . '/data/data-provider-data-named.php'], $errors);
}


Expand Down Expand Up @@ -274,6 +306,44 @@ public function testTrimmingArgs(): void
]);
}

static public function provideNamedArgumentPHPUnitVersions(): iterable
{
yield [null]; // unknown phpunit version

if (PHP_VERSION_ID >= 80100) {
yield [10]; // PHPUnit 10.x requires PHP 8.1+
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm.. maybe we should even reflect this in PHPUnitVersion.

that way we could turn a maybe into a no depending on the php runtime version?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it but probably no.

The only PHPUnit versions that support named arguments (in the sense they do not do array_values() on ...$args also only support PHP 8+.

The only wrong thing that can happen I think is when we're not successfully detecting PHPUnit version, and the user is running PHPUnit 11.5+ - we will produce Arg without names so possibly the result is going to be wrong.

But I don't think a condition about PHP version belongs to the logic. Because it's unrelated - you can run PHPUnit 9.x on PHP 8 as well...

}
if (PHP_VERSION_ID >= 80200) {
yield [11]; // PHPUnit 11.x requires PHP 8.2+
}
}

/**
* @dataProvider provideNamedArgumentPHPUnitVersions
*/
#[DataProvider('provideNamedArgumentPHPUnitVersions')]
public function testNamedArgumentsInDataProviders(?int $phpunitVersion): void
{
$this->phpunitVersion = $phpunitVersion;

if ($phpunitVersion >= 11) {
$errors = [];
} else {
$errors = [
[
'Parameter #1 $int of method DataProviderNamedArgs\FooTest::testFoo() expects int, string given.',
26
],
[
'Parameter #2 $string of method DataProviderNamedArgs\FooTest::testFoo() expects string, int given.',
26
],
];
}

$this->analyse([__DIR__ . '/data/data-provider-named-args.php'], $errors);
}

/**
* @return string[]
*/
Expand Down
32 changes: 32 additions & 0 deletions tests/Rules/PHPUnit/data/data-provider-named-args.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace DataProviderNamedArgs;

class FooTest extends \PHPUnit\Framework\TestCase
{

/**
* @dataProvider dataProvider
*/
public function testFoo(
int $int,
string $string
): void
{
$this->assertTrue(true);
}

public static function dataProvider(): iterable
{
yield 'even' => [
'int' => 50,
'string' => 'abc',
];

yield 'odd' => [
'string' => 'def',
'int' => 51,
];
}
}

Loading