diff --git a/extension.neon b/extension.neon index dcf43818..23ae52cf 100644 --- a/extension.neon +++ b/extension.neon @@ -53,13 +53,13 @@ services: class: PHPStan\Rules\PHPUnit\AnnotationHelper - - class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector + class: PHPStan\Rules\PHPUnit\TestMethodsHelper - - class: PHPStan\Rules\PHPUnit\TestMethodsHelper - factory: @PHPStan\Rules\PHPUnit\TestMethodsHelperFactory::create() + class: PHPStan\Rules\PHPUnit\PHPUnitVersion + factory: @PHPStan\Rules\PHPUnit\PHPUnitVersionDetector::createPHPUnitVersion() - - class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory + class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector - class: PHPStan\Rules\PHPUnit\DataProviderHelper diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index b468dfcb..d40d05e4 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -37,19 +37,19 @@ class DataProviderHelper private Parser $parser; - private bool $phpunit10OrNewer; + private PHPUnitVersion $PHPUnitVersion; public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - bool $phpunit10OrNewer + PHPUnitVersion $PHPUnitVersion ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; $this->parser = $parser; - $this->phpunit10OrNewer = $phpunit10OrNewer; + $this->PHPUnitVersion = $PHPUnitVersion; } /** @@ -65,7 +65,7 @@ public function getDataProviderMethods( { yield from $this->yieldDataProviderAnnotations($testMethod, $scope, $classReflection); - if (!$this->phpunit10OrNewer) { + if (!$this->PHPUnitVersion->supportsDataProviderAttribute()->yes()) { return; } @@ -156,7 +156,11 @@ public function processDataProvider( ->build(); } - if ($deprecationRulesInstalled && $this->phpunit10OrNewer && !$dataProviderMethodReflection->isStatic()) { + if ( + $deprecationRulesInstalled + && $this->PHPUnitVersion->requiresStaticDataProviders()->yes() + && !$dataProviderMethodReflection->isStatic() + ) { $errorBuilder = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be static in PHPUnit 10 and newer.', $dataProviderValue, diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 23a5f34a..33bbe22d 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -15,24 +15,24 @@ class DataProviderHelperFactory private Parser $parser; - private PHPUnitVersionDetector $PHPUnitVersionDetector; + private PHPUnitVersion $PHPUnitVersion; public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, Parser $parser, - PHPUnitVersionDetector $PHPUnitVersionDetector + PHPUnitVersion $PHPUnitVersion ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; $this->parser = $parser; - $this->PHPUnitVersionDetector = $PHPUnitVersionDetector; + $this->PHPUnitVersion = $PHPUnitVersion; } public function create(): DataProviderHelper { - return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); + return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersion); } } diff --git a/src/Rules/PHPUnit/PHPUnitVersion.php b/src/Rules/PHPUnit/PHPUnitVersion.php new file mode 100644 index 00000000..932deedd --- /dev/null +++ b/src/Rules/PHPUnit/PHPUnitVersion.php @@ -0,0 +1,41 @@ +majorVersion = $majorVersion; + } + + public function supportsDataProviderAttribute(): TrinaryLogic + { + if ($this->majorVersion === null) { + return TrinaryLogic::createMaybe(); + } + return TrinaryLogic::createFromBoolean($this->majorVersion >= 10); + } + + public function supportsTestAttribute(): TrinaryLogic + { + if ($this->majorVersion === null) { + return TrinaryLogic::createMaybe(); + } + return TrinaryLogic::createFromBoolean($this->majorVersion >= 10); + } + + public function requiresStaticDataProviders(): TrinaryLogic + { + if ($this->majorVersion === null) { + return TrinaryLogic::createMaybe(); + } + return TrinaryLogic::createFromBoolean($this->majorVersion >= 10); + } + +} diff --git a/src/Rules/PHPUnit/PHPUnitVersionDetector.php b/src/Rules/PHPUnit/PHPUnitVersionDetector.php index 841e69b9..f0e2c4b9 100644 --- a/src/Rules/PHPUnit/PHPUnitVersionDetector.php +++ b/src/Rules/PHPUnit/PHPUnitVersionDetector.php @@ -13,8 +13,6 @@ class PHPUnitVersionDetector { - private ?bool $is10OrNewer = null; - private ReflectionProvider $reflectionProvider; public function __construct(ReflectionProvider $reflectionProvider) @@ -22,13 +20,9 @@ public function __construct(ReflectionProvider $reflectionProvider) $this->reflectionProvider = $reflectionProvider; } - public function isPHPUnit10OrNewer(): bool + public function createPHPUnitVersion(): PHPUnitVersion { - if ($this->is10OrNewer !== null) { - return $this->is10OrNewer; - } - - $this->is10OrNewer = false; + $majorVersion = null; if ($this->reflectionProvider->hasClass(TestCase::class)) { $testCase = $this->reflectionProvider->getClass(TestCase::class); $file = $testCase->getFileName(); @@ -42,16 +36,13 @@ public function isPHPUnit10OrNewer(): bool $version = $json['extra']['branch-alias']['dev-main'] ?? null; if ($version !== null) { $majorVersion = (int) explode('.', $version)[0]; - if ($majorVersion >= 10) { - $this->is10OrNewer = true; - } } } } } } - return $this->is10OrNewer; + return new PHPUnitVersion($majorVersion); } } diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index 67b888e7..5eb274e0 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -16,15 +16,15 @@ final class TestMethodsHelper private FileTypeMapper $fileTypeMapper; - private bool $phpunit10OrNewer; + private PHPUnitVersion $PHPUnitVersion; public function __construct( FileTypeMapper $fileTypeMapper, - bool $phpunit10OrNewer + PHPUnitVersion $PHPUnitVersion ) { $this->fileTypeMapper = $fileTypeMapper; - $this->phpunit10OrNewer = $phpunit10OrNewer; + $this->PHPUnitVersion = $PHPUnitVersion; } /** @@ -63,7 +63,7 @@ public function getTestMethods(ClassReflection $classReflection, Scope $scope): } } - if (!$this->phpunit10OrNewer) { + if ($this->PHPUnitVersion->supportsTestAttribute()->no()) { continue; } diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php deleted file mode 100644 index 202f88cb..00000000 --- a/src/Rules/PHPUnit/TestMethodsHelperFactory.php +++ /dev/null @@ -1,28 +0,0 @@ -fileTypeMapper = $fileTypeMapper; - $this->PHPUnitVersionDetector = $PHPUnitVersionDetector; - } - - public function create(): TestMethodsHelper - { - return new TestMethodsHelper($this->fileTypeMapper, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); - } - -} diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index 647f830f..e3f80a3b 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -14,6 +14,7 @@ */ class DataProviderDataRuleTest extends RuleTestCase { + private int $phpunitVersion; protected function getRule(): Rule { @@ -24,13 +25,13 @@ protected function getRule(): Rule new DataProviderDataRule( new TestMethodsHelper( self::getContainer()->getByType(FileTypeMapper::class), - true + new PHPUnitVersion($this->phpunitVersion) ), new DataProviderHelper( $reflectionProvider, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), - true + new PHPUnitVersion($this->phpunitVersion) ), ), @@ -42,6 +43,8 @@ protected function getRule(): Rule public function testRule(): void { + $this->phpunitVersion = 10; + $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ [ 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', @@ -176,6 +179,8 @@ public function testRulePhp8(): void self::markTestSkipped(); } + $this->phpunitVersion = 10; + $this->analyse([__DIR__ . '/data/data-provider-data-named.php'], [ [ 'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.', @@ -203,6 +208,8 @@ public function testRulePhp8(): void public function testVariadicMethod(): void { + $this->phpunitVersion = 10; + $this->analyse([__DIR__ . '/data/data-provider-variadic-method.php'], [ [ 'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.', @@ -241,6 +248,8 @@ public function testVariadicMethod(): void public function testTrimmingArgs(): void { + $this->phpunitVersion = 10; + $this->analyse([__DIR__ . '/data/data-provider-trimming-args.php'], [ [ 'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.', diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 67f55ed4..2bf9d870 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -5,12 +5,15 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPStan\Type\FileTypeMapper; +use PHPUnit\Framework\Attributes\DataProvider; + /** * @extends RuleTestCase */ class DataProviderDeclarationRuleTest extends RuleTestCase { + private ?int $phpunitVersion; protected function getRule(): Rule { @@ -21,57 +24,97 @@ protected function getRule(): Rule $reflection, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), - true + new PHPUnitVersion($this->phpunitVersion) ), true, true ); } - public function testRule(): void + /** + * @dataProvider provideVersions + */ + #[DataProvider('provideVersions')] + public function testRule(?int $version): void { - $this->analyse([__DIR__ . '/data/data-provider-declaration.php'], [ - [ - '@dataProvider providebaz related method is used with incorrect case: provideBaz.', - 16, - ], - [ - '@dataProvider provideQux related method must be static in PHPUnit 10 and newer.', - 16, - ], - [ - '@dataProvider provideQuux related method must be public.', - 16, - ], - [ - '@dataProvider provideNonExisting related method not found.', - 70, - ], - [ - '@dataProvider NonExisting::provideNonExisting related class not found.', - 70, - ], - [ - '@dataProvider provideNonExisting related method not found.', - 85, - ], - [ - '@dataProvider provideNonExisting2 related method not found.', - 86, - ], - [ - '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', - 87, - ], - [ - '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', - 88, - ], - ]); + $this->phpunitVersion = $version; + + if ($version >= 10) { + $errors = [ + [ + '@dataProvider providebaz related method is used with incorrect case: provideBaz.', + 16, + ], + [ + '@dataProvider provideQux related method must be static in PHPUnit 10 and newer.', + 16, + ], + [ + '@dataProvider provideQuux related method must be public.', + 16, + ], + [ + '@dataProvider provideNonExisting related method not found.', + 70, + ], + [ + '@dataProvider NonExisting::provideNonExisting related class not found.', + 70, + ], + [ + '@dataProvider provideNonExisting related method not found.', + 85, + ], + [ + '@dataProvider provideNonExisting2 related method not found.', + 86, + ], + [ + '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', + 87, + ], + [ + '@dataProvider ExampleTestCase\\BarTestCase::providetootherclass related method is used with incorrect case: provideToOtherClass.', + 88, + ], + ]; + } else { + $errors = [ + [ + '@dataProvider providebaz related method is used with incorrect case: provideBaz.', + 16, + ], + [ + '@dataProvider provideQuux related method must be public.', + 16, + ], + [ + '@dataProvider provideNonExisting related method not found.', + 70, + ], + [ + '@dataProvider NonExisting::provideNonExisting related class not found.', + 70, + ], + ]; + } + + $this->analyse([__DIR__ . '/data/data-provider-declaration.php'], $errors); + } + + static public function provideVersions(): iterable + { + return [ + [null], + [9], + [10] + ]; } public function testFixDataProviderStatic(): void { + $this->phpunitVersion = 10; + $this->fix(__DIR__ . '/data/data-provider-static-fix.php', __DIR__ . '/data/data-provider-static-fix.php.fixed'); }