Refactor PHPUnitVersionDetector to ease different major version checks

staabm · web-flow · commit 758d343c748b · 2025-10-29T07:28:51.000+01:00
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,41 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\TrinaryLogic;
+
+class PHPUnitVersion
+{
+
+	private ?int $majorVersion;
+
+	public function __construct(?int $majorVersion)
+	{
+		$this->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
@@ -13,22 +13,16 @@
 class PHPUnitVersionDetector
 {
 
-	private ?bool $is10OrNewer = null;
-
 	private ReflectionProvider $reflectionProvider;
 
 	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
@@ -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
diff --git 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

Original file line number	Diff line number	Diff line change
`@@ -53,13 +53,13 @@ services:`
`53`	`53`	`class: PHPStan\Rules\PHPUnit\AnnotationHelper`
`54`	`54`
`55`	`55`	`-`
`56`		`- class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector`
	`56`	`+ class: PHPStan\Rules\PHPUnit\TestMethodsHelper`
`57`	`57`
`58`	`58`	`-`
`59`		`- class: PHPStan\Rules\PHPUnit\TestMethodsHelper`
`60`		`- factory: @PHPStan\Rules\PHPUnit\TestMethodsHelperFactory::create()`
	`59`	`+ class: PHPStan\Rules\PHPUnit\PHPUnitVersion`
	`60`	`+ factory: @PHPStan\Rules\PHPUnit\PHPUnitVersionDetector::createPHPUnitVersion()`
`61`	`61`	`-`
`62`		`- class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory`
	`62`	`+ class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector`
`63`	`63`
`64`	`64`	`-`
`65`	`65`	`class: PHPStan\Rules\PHPUnit\DataProviderHelper`
Original file line number	Diff line number	Diff line change
`@@ -15,24 +15,24 @@ class DataProviderHelperFactory`
`15`	`15`
`16`	`16`	`private Parser $parser;`
`17`	`17`
`18`		`- private PHPUnitVersionDetector $PHPUnitVersionDetector;`
	`18`	`+ private PHPUnitVersion $PHPUnitVersion;`
`19`	`19`
`20`	`20`	`public function __construct(`
`21`	`21`	`ReflectionProvider $reflectionProvider,`
`22`	`22`	`FileTypeMapper $fileTypeMapper,`
`23`	`23`	`Parser $parser,`
`24`		`- PHPUnitVersionDetector $PHPUnitVersionDetector`
	`24`	`+ PHPUnitVersion $PHPUnitVersion`
`25`	`25`	`)`
`26`	`26`	`{`
`27`	`27`	`$this->reflectionProvider = $reflectionProvider;`
`28`	`28`	`$this->fileTypeMapper = $fileTypeMapper;`
`29`	`29`	`$this->parser = $parser;`
`30`		`- $this->PHPUnitVersionDetector = $PHPUnitVersionDetector;`
	`30`	`+ $this->PHPUnitVersion = $PHPUnitVersion;`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`public function create(): DataProviderHelper`
`34`	`34`	`{`
`35`		`- return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer());`
	`35`	`+ return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersion);`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,22 +13,16 @@`
`13`	`13`	`class PHPUnitVersionDetector`
`14`	`14`	`{`
`15`	`15`
`16`		`- private ?bool $is10OrNewer = null;`
`17`		`-`
`18`	`16`	`private ReflectionProvider $reflectionProvider;`
`19`	`17`
`20`	`18`	`public function __construct(ReflectionProvider $reflectionProvider)`
`21`	`19`	`{`
`22`	`20`	`$this->reflectionProvider = $reflectionProvider;`
`23`	`21`	`}`
`24`	`22`
`25`		`- public function isPHPUnit10OrNewer(): bool`
	`23`	`+ public function createPHPUnitVersion(): PHPUnitVersion`
`26`	`24`	`{`
`27`		`- if ($this->is10OrNewer !== null) {`
`28`		`- return $this->is10OrNewer;`
`29`		`- }`
`30`		`-`
`31`		`- $this->is10OrNewer = false;`
	`25`	`+ $majorVersion = null;`
`32`	`26`	`if ($this->reflectionProvider->hasClass(TestCase::class)) {`
`33`	`27`	`$testCase = $this->reflectionProvider->getClass(TestCase::class);`
`34`	`28`	`$file = $testCase->getFileName();`
`@@ -42,16 +36,13 @@ public function isPHPUnit10OrNewer(): bool`
`42`	`36`	`$version = $json['extra']['branch-alias']['dev-main'] ?? null;`
`43`	`37`	`if ($version !== null) {`
`44`	`38`	`$majorVersion = (int) explode('.', $version)[0];`
`45`		`- if ($majorVersion >= 10) {`
`46`		`- $this->is10OrNewer = true;`
`47`		`- }`
`48`	`39`	`}`
`49`	`40`	`}`
`50`	`41`	`}`
`51`	`42`	`}`
`52`	`43`	`}`
`53`	`44`
`54`		`- return $this->is10OrNewer;`
	`45`	`+ return new PHPUnitVersion($majorVersion);`
`55`	`46`	`}`
`56`	`47`
`57`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,15 +16,15 @@ final class TestMethodsHelper`
`16`	`16`
`17`	`17`	`private FileTypeMapper $fileTypeMapper;`
`18`	`18`
`19`		`- private bool $phpunit10OrNewer;`
	`19`	`+ private PHPUnitVersion $PHPUnitVersion;`
`20`	`20`
`21`	`21`	`public function __construct(`
`22`	`22`	`FileTypeMapper $fileTypeMapper,`
`23`		`- bool $phpunit10OrNewer`
	`23`	`+ PHPUnitVersion $PHPUnitVersion`
`24`	`24`	`)`
`25`	`25`	`{`
`26`	`26`	`$this->fileTypeMapper = $fileTypeMapper;`
`27`		`- $this->phpunit10OrNewer = $phpunit10OrNewer;`
	`27`	`+ $this->PHPUnitVersion = $PHPUnitVersion;`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
`@@ -63,7 +63,7 @@ public function getTestMethods(ClassReflection $classReflection, Scope $scope):`
`63`	`63`	`}`
`64`	`64`	`}`
`65`	`65`
`66`		`- if (!$this->phpunit10OrNewer) {`
	`66`	`+ if ($this->PHPUnitVersion->supportsTestAttribute()->no()) {`
`67`	`67`	`continue;`
`68`	`68`	`}`
`69`	`69`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@`
`14`	`14`	`*/`
`15`	`15`	`class DataProviderDataRuleTest extends RuleTestCase`
`16`	`16`	`{`
	`17`	`+ private int $phpunitVersion;`
`17`	`18`
`18`	`19`	`protected function getRule(): Rule`
`19`	`20`	`{`
`@@ -24,13 +25,13 @@ protected function getRule(): Rule`
`24`	`25`	`new DataProviderDataRule(`
`25`	`26`	`new TestMethodsHelper(`
`26`	`27`	`self::getContainer()->getByType(FileTypeMapper::class),`
`27`		`- true`
	`28`	`+ new PHPUnitVersion($this->phpunitVersion)`
`28`	`29`	`),`
`29`	`30`	`new DataProviderHelper(`
`30`	`31`	`$reflectionProvider,`
`31`	`32`	`self::getContainer()->getByType(FileTypeMapper::class),`
`32`	`33`	`self::getContainer()->getService('defaultAnalysisParser'),`
`33`		`- true`
	`34`	`+ new PHPUnitVersion($this->phpunitVersion)`
`34`	`35`	`),`
`35`	`36`
`36`	`37`	`),`
`@@ -42,6 +43,8 @@ protected function getRule(): Rule`
`42`	`43`
`43`	`44`	`public function testRule(): void`
`44`	`45`	`{`
	`46`	`+ $this->phpunitVersion = 10;`
	`47`	`+`
`45`	`48`	`$this->analyse([__DIR__ . '/data/data-provider-data.php'], [`
`46`	`49`	`[`
`47`	`50`	`'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.',`
`@@ -176,6 +179,8 @@ public function testRulePhp8(): void`
`176`	`179`	`self::markTestSkipped();`
`177`	`180`	`}`
`178`	`181`
	`182`	`+ $this->phpunitVersion = 10;`
	`183`	`+`
`179`	`184`	`$this->analyse([__DIR__ . '/data/data-provider-data-named.php'], [`
`180`	`185`	`[`
`181`	`186`	`'Parameter $input of method DataProviderDataTestPhp8\NamedArgsInProvider::testFoo() expects string, int given.',`
`@@ -203,6 +208,8 @@ public function testRulePhp8(): void`
`203`	`208`
`204`	`209`	`public function testVariadicMethod(): void`
`205`	`210`	`{`
	`211`	`+ $this->phpunitVersion = 10;`
	`212`	`+`
`206`	`213`	`$this->analyse([__DIR__ . '/data/data-provider-variadic-method.php'], [`
`207`	`214`	`[`
`208`	`215`	`'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.',`
`@@ -241,6 +248,8 @@ public function testVariadicMethod(): void`
`241`	`248`
`242`	`249`	`public function testTrimmingArgs(): void`
`243`	`250`	`{`
	`251`	`+ $this->phpunitVersion = 10;`
	`252`	`+`
`244`	`253`	`$this->analyse([__DIR__ . '/data/data-provider-trimming-args.php'], [`
`245`	`254`	`[`
`246`	`255`	`'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.',`