From 7f1457f2fbea110c0a7be4a03747823f0575292f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 4 Sep 2024 22:52:21 +0200 Subject: [PATCH 01/50] Open 2.0.x --- .github/workflows/build.yml | 2 +- composer.json | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 270f0f51..0dc529dd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,7 +6,7 @@ on: pull_request: push: branches: - - "1.4.x" + - "2.0.x" jobs: lint: diff --git a/composer.json b/composer.json index 5b1ec505..5237f066 100644 --- a/composer.json +++ b/composer.json @@ -6,17 +6,16 @@ "MIT" ], "require": { - "php": "^7.2 || ^8.0", - "phpstan/phpstan": "^1.12" + "php": "^7.4 || ^8.0", + "phpstan/phpstan": "^2.0" }, "conflict": { "phpunit/phpunit": "<7.0" }, "require-dev": { - "nikic/php-parser": "^4.13.0", "php-parallel-lint/php-parallel-lint": "^1.2", - "phpstan/phpstan-strict-rules": "^1.5.1", - "phpunit/phpunit": "^9.5" + "phpstan/phpstan-strict-rules": "^2.0", + "phpunit/phpunit": "^9.6" }, "config": { "platform": { From 953195d722a2c38c5ee904cea31d0a91b4d8a784 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 4 Sep 2024 22:56:08 +0200 Subject: [PATCH 02/50] Stop testing PHP 7.2 and 7.3 --- .github/workflows/build.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0dc529dd..8d659b18 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,8 +16,6 @@ jobs: strategy: matrix: php-version: - - "7.2" - - "7.3" - "7.4" - "8.0" - "8.1" @@ -41,10 +39,6 @@ jobs: - name: "Install dependencies" run: "composer install --no-interaction --no-progress" - - name: "Downgrade PHPUnit" - if: matrix.php-version == '7.2' || matrix.php-version == '7.3' - run: "composer require --dev phpunit/phpunit:^7.5.20 --update-with-dependencies" - - name: "Lint" run: "make lint" @@ -94,8 +88,6 @@ jobs: fail-fast: false matrix: php-version: - - "7.2" - - "7.3" - "7.4" - "8.0" - "8.1" @@ -124,10 +116,6 @@ jobs: if: ${{ matrix.dependencies == 'highest' }} run: "composer update --no-interaction --no-progress" - - name: "Downgrade PHPUnit" - if: matrix.php-version == '7.2' || matrix.php-version == '7.3' - run: "composer require --dev phpunit/phpunit:^7.5.20 --update-with-dependencies" - - name: "Tests" run: "make tests" @@ -139,8 +127,6 @@ jobs: fail-fast: false matrix: php-version: - - "7.2" - - "7.3" - "7.4" - "8.0" - "8.1" @@ -171,9 +157,5 @@ jobs: if: ${{ matrix.dependencies == 'highest' }} run: "composer update --no-interaction --no-progress" - - name: "Downgrade PHPUnit" - if: matrix.php-version == '7.2' || matrix.php-version == '7.3' - run: "composer require --dev phpunit/phpunit:^7.5.20 --update-with-dependencies" - - name: "PHPStan" run: "make phpstan" From 3faa60573a32522772e7cda004003b15466e2b5b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 4 Sep 2024 22:56:23 +0200 Subject: [PATCH 03/50] Update build-cs --- .github/workflows/build.yml | 2 +- Makefile | 2 +- .../MockObjectTypeNodeResolverExtension.php | 3 +- src/Rules/PHPUnit/AnnotationHelper.php | 2 +- src/Rules/PHPUnit/AssertRuleHelper.php | 2 +- src/Rules/PHPUnit/ClassCoversExistsRule.php | 12 +- .../PHPUnit/ClassMethodCoversExistsRule.php | 18 +- src/Rules/PHPUnit/CoversHelper.php | 13 +- .../PHPUnit/DataProviderDeclarationRule.php | 13 +- src/Rules/PHPUnit/DataProviderHelper.php | 23 +- .../PHPUnit/DataProviderHelperFactory.php | 6 +- src/Rules/PHPUnit/MockMethodCallRule.php | 8 +- .../NoMissingSpaceInClassAnnotationRule.php | 3 +- .../NoMissingSpaceInMethodAnnotationRule.php | 3 +- .../PHPUnit/ShouldCallParentMethodsRule.php | 2 +- .../AssertFunctionTypeSpecifyingExtension.php | 7 +- .../AssertMethodTypeSpecifyingExtension.php | 7 +- ...ertStaticMethodTypeSpecifyingExtension.php | 7 +- .../AssertTypeSpecifyingExtensionHelper.php | 238 +++++++----------- .../MockBuilderDynamicReturnTypeExtension.php | 2 +- .../MockObjectDynamicReturnTypeExtension.php | 4 +- .../PHPUnit/ClassCoversExistsRuleTest.php | 2 +- .../ClassMethodCoversExistsRuleTest.php | 2 +- 23 files changed, 153 insertions(+), 228 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8d659b18..88543fb5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -56,7 +56,7 @@ jobs: with: repository: "phpstan/build-cs" path: "build-cs" - ref: "1.x" + ref: "2.x" - name: "Install PHP" uses: "shivammathur/setup-php@v2" diff --git a/Makefile b/Makefile index b01b1537..1ee557df 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ lint: .PHONY: cs-install cs-install: git clone https://github.com/phpstan/build-cs.git || true - git -C build-cs fetch origin && git -C build-cs reset --hard origin/1.x + git -C build-cs fetch origin && git -C build-cs reset --hard origin/2.x composer install --working-dir build-cs .PHONY: cs diff --git a/src/PhpDoc/PHPUnit/MockObjectTypeNodeResolverExtension.php b/src/PhpDoc/PHPUnit/MockObjectTypeNodeResolverExtension.php index 2d70b380..83f7b8b2 100644 --- a/src/PhpDoc/PHPUnit/MockObjectTypeNodeResolverExtension.php +++ b/src/PhpDoc/PHPUnit/MockObjectTypeNodeResolverExtension.php @@ -17,8 +17,7 @@ class MockObjectTypeNodeResolverExtension implements TypeNodeResolverExtension, TypeNodeResolverAwareExtension { - /** @var TypeNodeResolver */ - private $typeNodeResolver; + private TypeNodeResolver $typeNodeResolver; public function setTypeNodeResolver(TypeNodeResolver $typeNodeResolver): void { diff --git a/src/Rules/PHPUnit/AnnotationHelper.php b/src/Rules/PHPUnit/AnnotationHelper.php index 4335bc81..21623cab 100644 --- a/src/Rules/PHPUnit/AnnotationHelper.php +++ b/src/Rules/PHPUnit/AnnotationHelper.php @@ -56,7 +56,7 @@ public function processDocComment(Doc $docComment): array } $errors[] = RuleErrorBuilder::message( - 'Annotation "' . $matches['annotation'] . '" is invalid, "@' . $matches['property'] . '" should be followed by a space and a value.' + 'Annotation "' . $matches['annotation'] . '" is invalid, "@' . $matches['property'] . '" should be followed by a space and a value.', )->identifier('phpunit.invalidPhpDoc')->build(); } diff --git a/src/Rules/PHPUnit/AssertRuleHelper.php b/src/Rules/PHPUnit/AssertRuleHelper.php index 3ad79c00..442b0655 100644 --- a/src/Rules/PHPUnit/AssertRuleHelper.php +++ b/src/Rules/PHPUnit/AssertRuleHelper.php @@ -30,7 +30,7 @@ public static function isMethodOrStaticCallOnAssert(Node $node, Scope $scope): b 'static', 'parent', ], - true + true, ) ) { $calledOnType = new ObjectType($scope->getClassReflection()->getName()); diff --git a/src/Rules/PHPUnit/ClassCoversExistsRule.php b/src/Rules/PHPUnit/ClassCoversExistsRule.php index bbced022..f8d831c9 100644 --- a/src/Rules/PHPUnit/ClassCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassCoversExistsRule.php @@ -23,16 +23,14 @@ class ClassCoversExistsRule implements Rule /** * Covers helper. * - * @var CoversHelper */ - private $coversHelper; + private CoversHelper $coversHelper; /** * Reflection provider. * - * @var ReflectionProvider */ - private $reflectionProvider; + private ReflectionProvider $reflectionProvider; public function __construct( CoversHelper $coversHelper, @@ -62,7 +60,7 @@ public function processNode(Node $node, Scope $scope): array if (count($classCoversDefaultClasses) >= 2) { return [ RuleErrorBuilder::message(sprintf( - '@coversDefaultClass is defined multiple times.' + '@coversDefaultClass is defined multiple times.', ))->identifier('phpunit.coversDuplicate')->build(), ]; } @@ -75,7 +73,7 @@ public function processNode(Node $node, Scope $scope): array if (!$this->reflectionProvider->hasClass($className)) { $errors[] = RuleErrorBuilder::message(sprintf( '@coversDefaultClass references an invalid class %s.', - $className + $className, ))->identifier('phpunit.coversClass')->build(); } } @@ -83,7 +81,7 @@ public function processNode(Node $node, Scope $scope): array foreach ($classCovers as $covers) { $errors = array_merge( $errors, - $this->coversHelper->processCovers($node, $covers, null) + $this->coversHelper->processCovers($node, $covers, null), ); } diff --git a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php index 92cb1e2e..4bfdc417 100644 --- a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php @@ -25,16 +25,14 @@ class ClassMethodCoversExistsRule implements Rule /** * Covers helper. * - * @var CoversHelper */ - private $coversHelper; + private CoversHelper $coversHelper; /** * The file type mapper. * - * @var FileTypeMapper */ - private $fileTypeMapper; + private FileTypeMapper $fileTypeMapper; public function __construct( CoversHelper $coversHelper, @@ -65,9 +63,7 @@ public function processNode(Node $node, Scope $scope): array $classPhpDoc = $classReflection->getResolvedPhpDoc(); [$classCovers, $classCoversDefaultClasses] = $this->coversHelper->getCoverAnnotations($classPhpDoc); - $classCoversStrings = array_map(static function (PhpDocTagNode $covers): string { - return (string) $covers->value; - }, $classCovers); + $classCoversStrings = array_map(static fn (PhpDocTagNode $covers): string => (string) $covers->value, $classCovers); $docComment = $node->getDocComment(); if ($docComment === null) { @@ -83,7 +79,7 @@ public function processNode(Node $node, Scope $scope): array $classReflection->getName(), $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, $node->name->toString(), - $docComment->getText() + $docComment->getText(), ); [$methodCovers, $methodCoversDefaultClasses] = $this->coversHelper->getCoverAnnotations($methodPhpDoc); @@ -93,7 +89,7 @@ public function processNode(Node $node, Scope $scope): array if (count($methodCoversDefaultClasses) > 0) { $errors[] = RuleErrorBuilder::message(sprintf( '@coversDefaultClass defined on class method %s.', - $node->name + $node->name, ))->identifier('phpunit.covers')->build(); } @@ -101,13 +97,13 @@ public function processNode(Node $node, Scope $scope): array if (in_array((string) $covers->value, $classCoversStrings, true)) { $errors[] = RuleErrorBuilder::message(sprintf( 'Class already @covers %s so the method @covers is redundant.', - $covers->value + $covers->value, ))->identifier('phpunit.coversDuplicate')->build(); } $errors = array_merge( $errors, - $this->coversHelper->processCovers($node, $covers, $coversDefaultClass) + $this->coversHelper->processCovers($node, $covers, $coversDefaultClass), ); } diff --git a/src/Rules/PHPUnit/CoversHelper.php b/src/Rules/PHPUnit/CoversHelper.php index 55dbe8de..40ae561e 100644 --- a/src/Rules/PHPUnit/CoversHelper.php +++ b/src/Rules/PHPUnit/CoversHelper.php @@ -20,9 +20,8 @@ class CoversHelper /** * Reflection provider. * - * @var ReflectionProvider */ - private $reflectionProvider; + private ReflectionProvider $reflectionProvider; public function __construct(ReflectionProvider $reflectionProvider) { @@ -48,12 +47,12 @@ public function getCoverAnnotations(?ResolvedPhpDocBlock $phpDoc): array foreach ($phpDocNodes as $docNode) { $covers = array_merge( $covers, - $docNode->getTagsByName('@covers') + $docNode->getTagsByName('@covers'), ); $coversDefaultClasses = array_merge( $coversDefaultClasses, - $docNode->getTagsByName('@coversDefaultClass') + $docNode->getTagsByName('@coversDefaultClass'), ); } @@ -100,14 +99,14 @@ public function processCovers( if ($class->isInterface()) { $errors[] = RuleErrorBuilder::message(sprintf( '@covers value %s references an interface.', - $fullName + $fullName, ))->identifier('phpunit.coversInterface')->build(); } if (isset($method) && $method !== '' && !$class->hasMethod($method)) { $errors[] = RuleErrorBuilder::message(sprintf( '@covers value %s references an invalid method.', - $fullName + $fullName, ))->identifier('phpunit.coversMethod')->build(); } } elseif (isset($method) && $this->reflectionProvider->hasFunction(new Name($method, []), null)) { @@ -118,7 +117,7 @@ public function processCovers( $error = RuleErrorBuilder::message(sprintf( '@covers value %s references an invalid %s.', $fullName, - $isMethod ? 'method' : 'class or function' + $isMethod ? 'method' : 'class or function', ))->identifier(sprintf('phpunit.covers%s', $isMethod ? 'Method' : '')); if (strpos($className, '\\') === false) { diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php index 37c586de..cc44eeb4 100644 --- a/src/Rules/PHPUnit/DataProviderDeclarationRule.php +++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php @@ -17,23 +17,20 @@ class DataProviderDeclarationRule implements Rule /** * Data provider helper. * - * @var DataProviderHelper */ - private $dataProviderHelper; + private DataProviderHelper $dataProviderHelper; /** * When set to true, it reports data provider method with incorrect name case. * - * @var bool */ - private $checkFunctionNameCase; + private bool $checkFunctionNameCase; /** * When phpstan-deprecation-rules is installed, it reports deprecated usages. * - * @var bool */ - private $deprecationRulesInstalled; + private bool $deprecationRulesInstalled; public function __construct( DataProviderHelper $dataProviderHelper, @@ -70,8 +67,8 @@ public function processNode(Node $node, Scope $scope): array $dataProviderMethodName, $lineNumber, $this->checkFunctionNameCase, - $this->deprecationRulesInstalled - ) + $this->deprecationRulesInstalled, + ), ); } diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 9d1b160a..ad354fa5 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -28,19 +28,16 @@ class DataProviderHelper /** * Reflection provider. * - * @var ReflectionProvider */ - private $reflectionProvider; + private ReflectionProvider $reflectionProvider; /** * The file type mapper. * - * @var FileTypeMapper */ - private $fileTypeMapper; + private FileTypeMapper $fileTypeMapper; - /** @var bool */ - private $phpunit10OrNewer; + private bool $phpunit10OrNewer; public function __construct( ReflectionProvider $reflectionProvider, @@ -69,7 +66,7 @@ public function getDataProviderMethods( $classReflection->getName(), $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, $node->name->toString(), - $docComment->getText() + $docComment->getText(), ); foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); @@ -122,7 +119,7 @@ private function getDataProviderAnnotations(?ResolvedPhpDocBlock $phpDoc): array foreach ($phpDocNodes as $docNode) { $annotations = array_merge( $annotations, - $docNode->getTagsByName('@dataProvider') + $docNode->getTagsByName('@dataProvider'), ); } @@ -145,7 +142,7 @@ public function processDataProvider( return [ RuleErrorBuilder::message(sprintf( '@dataProvider %s related class not found.', - $dataProviderValue + $dataProviderValue, )) ->line($lineNumber) ->identifier('phpunit.dataProviderClass') @@ -159,7 +156,7 @@ public function processDataProvider( return [ RuleErrorBuilder::message(sprintf( '@dataProvider %s related method not found.', - $dataProviderValue + $dataProviderValue, )) ->line($lineNumber) ->identifier('phpunit.dataProviderMethod') @@ -173,7 +170,7 @@ public function processDataProvider( $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method is used with incorrect case: %s.', $dataProviderValue, - $dataProviderMethodReflection->getName() + $dataProviderMethodReflection->getName(), )) ->line($lineNumber) ->identifier('method.nameCase') @@ -183,7 +180,7 @@ public function processDataProvider( if (!$dataProviderMethodReflection->isPublic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be public.', - $dataProviderValue + $dataProviderValue, )) ->line($lineNumber) ->identifier('phpunit.dataProviderPublic') @@ -193,7 +190,7 @@ public function processDataProvider( if ($deprecationRulesInstalled && $this->phpunit10OrNewer && !$dataProviderMethodReflection->isStatic()) { $errors[] = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be static in PHPUnit 10 and newer.', - $dataProviderValue + $dataProviderValue, )) ->line($lineNumber) ->identifier('phpunit.dataProviderStatic') diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 7fc8af0f..9b965e2c 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -14,11 +14,9 @@ class DataProviderHelperFactory { - /** @var ReflectionProvider */ - private $reflectionProvider; + private ReflectionProvider $reflectionProvider; - /** @var FileTypeMapper */ - private $fileTypeMapper; + private FileTypeMapper $fileTypeMapper; public function __construct(ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper) { diff --git a/src/Rules/PHPUnit/MockMethodCallRule.php b/src/Rules/PHPUnit/MockMethodCallRule.php index f93ef167..b6f8932a 100644 --- a/src/Rules/PHPUnit/MockMethodCallRule.php +++ b/src/Rules/PHPUnit/MockMethodCallRule.php @@ -54,9 +54,7 @@ public function processNode(Node $node, Scope $scope): array ) && !$type->hasMethod($method)->yes() ) { - $mockClasses = array_filter($type->getObjectClassNames(), static function (string $class): bool { - return $class !== MockObject::class && $class !== Stub::class; - }); + $mockClasses = array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class && $class !== Stub::class); if (count($mockClasses) === 0) { continue; } @@ -64,7 +62,7 @@ public function processNode(Node $node, Scope $scope): array $errors[] = RuleErrorBuilder::message(sprintf( 'Trying to mock an undefined method %s() on class %s.', $method, - implode('&', $mockClasses) + implode('&', $mockClasses), ))->identifier('phpunit.mockMethod')->build(); continue; } @@ -82,7 +80,7 @@ public function processNode(Node $node, Scope $scope): array $errors[] = RuleErrorBuilder::message(sprintf( 'Trying to mock an undefined method %s() on class %s.', $method, - implode('|', $classNames) + implode('|', $classNames), ))->identifier('phpunit.mockMethod')->build(); } diff --git a/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php b/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php index 89e3e8ff..c8fd3a14 100644 --- a/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php +++ b/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php @@ -17,9 +17,8 @@ class NoMissingSpaceInClassAnnotationRule implements Rule /** * Covers helper. * - * @var AnnotationHelper */ - private $annotationHelper; + private AnnotationHelper $annotationHelper; public function __construct(AnnotationHelper $annotationHelper) { diff --git a/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php b/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php index 77577206..a44fc539 100644 --- a/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php +++ b/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php @@ -17,9 +17,8 @@ class NoMissingSpaceInMethodAnnotationRule implements Rule /** * Covers helper. * - * @var AnnotationHelper */ - private $annotationHelper; + private AnnotationHelper $annotationHelper; public function __construct(AnnotationHelper $annotationHelper) { diff --git a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php index 917d0bf9..4035dcc9 100644 --- a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php +++ b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php @@ -56,7 +56,7 @@ public function processNode(Node $node, Scope $scope): array if (!$hasParentCall) { return [ RuleErrorBuilder::message( - sprintf('Missing call to parent::%s() method.', $methodName) + sprintf('Missing call to parent::%s() method.', $methodName), )->identifier('phpunit.callParent')->build(), ]; } diff --git a/src/Type/PHPUnit/Assert/AssertFunctionTypeSpecifyingExtension.php b/src/Type/PHPUnit/Assert/AssertFunctionTypeSpecifyingExtension.php index 31805a3f..f5a5a745 100644 --- a/src/Type/PHPUnit/Assert/AssertFunctionTypeSpecifyingExtension.php +++ b/src/Type/PHPUnit/Assert/AssertFunctionTypeSpecifyingExtension.php @@ -17,8 +17,7 @@ class AssertFunctionTypeSpecifyingExtension implements FunctionTypeSpecifyingExtension, TypeSpecifierAwareExtension { - /** @var TypeSpecifier */ - private $typeSpecifier; + private TypeSpecifier $typeSpecifier; public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void { @@ -33,7 +32,7 @@ public function isFunctionSupported( { return AssertTypeSpecifyingExtensionHelper::isSupported( $this->trimName($functionReflection->getName()), - $node->getArgs() + $node->getArgs(), ); } @@ -48,7 +47,7 @@ public function specifyTypes( $this->typeSpecifier, $scope, $this->trimName($functionReflection->getName()), - $node->getArgs() + $node->getArgs(), ); } diff --git a/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php b/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php index 6307f244..753c8b89 100644 --- a/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php +++ b/src/Type/PHPUnit/Assert/AssertMethodTypeSpecifyingExtension.php @@ -14,8 +14,7 @@ class AssertMethodTypeSpecifyingExtension implements MethodTypeSpecifyingExtension, TypeSpecifierAwareExtension { - /** @var TypeSpecifier */ - private $typeSpecifier; + private TypeSpecifier $typeSpecifier; public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void { @@ -35,7 +34,7 @@ public function isMethodSupported( { return AssertTypeSpecifyingExtensionHelper::isSupported( $methodReflection->getName(), - $node->getArgs() + $node->getArgs(), ); } @@ -50,7 +49,7 @@ public function specifyTypes( $this->typeSpecifier, $scope, $functionReflection->getName(), - $node->getArgs() + $node->getArgs(), ); } diff --git a/src/Type/PHPUnit/Assert/AssertStaticMethodTypeSpecifyingExtension.php b/src/Type/PHPUnit/Assert/AssertStaticMethodTypeSpecifyingExtension.php index 54da4457..ec0dad14 100644 --- a/src/Type/PHPUnit/Assert/AssertStaticMethodTypeSpecifyingExtension.php +++ b/src/Type/PHPUnit/Assert/AssertStaticMethodTypeSpecifyingExtension.php @@ -14,8 +14,7 @@ class AssertStaticMethodTypeSpecifyingExtension implements StaticMethodTypeSpecifyingExtension, TypeSpecifierAwareExtension { - /** @var TypeSpecifier */ - private $typeSpecifier; + private TypeSpecifier $typeSpecifier; public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void { @@ -35,7 +34,7 @@ public function isStaticMethodSupported( { return AssertTypeSpecifyingExtensionHelper::isSupported( $methodReflection->getName(), - $node->getArgs() + $node->getArgs(), ); } @@ -50,7 +49,7 @@ public function specifyTypes( $this->typeSpecifier, $scope, $functionReflection->getName(), - $node->getArgs() + $node->getArgs(), ); } diff --git a/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php b/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php index 49512575..cdc6ff34 100644 --- a/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php +++ b/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php @@ -32,13 +32,13 @@ class AssertTypeSpecifyingExtensionHelper { /** @var Closure[] */ - private static $resolvers; + private static ?array $resolvers = null; /** * Those can specify types correctly, but would produce always-true issue * @var string[] */ - private static $resolversCausingAlwaysTrue = ['ContainsOnlyInstancesOf', 'ContainsEquals', 'Contains']; + private static array $resolversCausingAlwaysTrue = ['ContainsOnlyInstancesOf', 'ContainsEquals', 'Contains']; /** * @param Arg[] $args @@ -101,7 +101,7 @@ public static function specifyTypes( $scope, $expression, TypeSpecifierContext::createTruthy(), - $bypassAlwaysTrueIssue ? new Expr\BinaryOp\BooleanAnd($expression, new Expr\Variable('nonsense')) : null + $bypassAlwaysTrueIssue ? new Expr\BinaryOp\BooleanAnd($expression, new Expr\Variable('nonsense')) : null, ); } @@ -136,95 +136,57 @@ private static function getExpressionResolvers(): array { if (self::$resolvers === null) { self::$resolvers = [ - 'Count' => static function (Scope $scope, Arg $expected, Arg $actual): Identical { - return new Identical( + 'Count' => static fn (Scope $scope, Arg $expected, Arg $actual): Identical => new Identical( + $expected->value, + new FuncCall(new Name('count'), [$actual]), + ), + 'NotCount' => static fn (Scope $scope, Arg $expected, Arg $actual): BooleanNot => new BooleanNot( + new Identical( $expected->value, - new FuncCall(new Name('count'), [$actual]) - ); - }, - 'NotCount' => static function (Scope $scope, Arg $expected, Arg $actual): BooleanNot { - return new BooleanNot( - new Identical( - $expected->value, - new FuncCall(new Name('count'), [$actual]) - ) - ); - }, - 'InstanceOf' => static function (Scope $scope, Arg $class, Arg $object): Instanceof_ { - return new Instanceof_( - $object->value, - $class->value - ); - }, - 'Same' => static function (Scope $scope, Arg $expected, Arg $actual): Identical { - return new Identical( - $expected->value, - $actual->value - ); - }, - 'True' => static function (Scope $scope, Arg $actual): Identical { - return new Identical( - $actual->value, - new ConstFetch(new Name('true')) - ); - }, - 'False' => static function (Scope $scope, Arg $actual): Identical { - return new Identical( - $actual->value, - new ConstFetch(new Name('false')) - ); - }, - 'Null' => static function (Scope $scope, Arg $actual): Identical { - return new Identical( - $actual->value, - new ConstFetch(new Name('null')) - ); - }, - 'Empty' => static function (Scope $scope, Arg $actual): Expr\BinaryOp\BooleanOr { - return new Expr\BinaryOp\BooleanOr( - new Instanceof_($actual->value, new Name(EmptyIterator::class)), - new Expr\BinaryOp\BooleanOr( - new Expr\BinaryOp\BooleanAnd( - new Instanceof_($actual->value, new Name(Countable::class)), - new Identical(new FuncCall(new Name('count'), [new Arg($actual->value)]), new LNumber(0)) - ), - new Expr\Empty_($actual->value) - ) - ); - }, - 'IsArray' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_array'), [$actual]); - }, - 'IsBool' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_bool'), [$actual]); - }, - 'IsCallable' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_callable'), [$actual]); - }, - 'IsFloat' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_float'), [$actual]); - }, - 'IsInt' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_int'), [$actual]); - }, - 'IsIterable' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_iterable'), [$actual]); - }, - 'IsNumeric' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_numeric'), [$actual]); - }, - 'IsObject' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_object'), [$actual]); - }, - 'IsResource' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_resource'), [$actual]); - }, - 'IsString' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_string'), [$actual]); - }, - 'IsScalar' => static function (Scope $scope, Arg $actual): FuncCall { - return new FuncCall(new Name('is_scalar'), [$actual]); - }, + new FuncCall(new Name('count'), [$actual]), + ), + ), + 'InstanceOf' => static fn (Scope $scope, Arg $class, Arg $object): Instanceof_ => new Instanceof_( + $object->value, + $class->value, + ), + 'Same' => static fn (Scope $scope, Arg $expected, Arg $actual): Identical => new Identical( + $expected->value, + $actual->value, + ), + 'True' => static fn (Scope $scope, Arg $actual): Identical => new Identical( + $actual->value, + new ConstFetch(new Name('true')), + ), + 'False' => static fn (Scope $scope, Arg $actual): Identical => new Identical( + $actual->value, + new ConstFetch(new Name('false')), + ), + 'Null' => static fn (Scope $scope, Arg $actual): Identical => new Identical( + $actual->value, + new ConstFetch(new Name('null')), + ), + 'Empty' => static fn (Scope $scope, Arg $actual): Expr\BinaryOp\BooleanOr => new Expr\BinaryOp\BooleanOr( + new Instanceof_($actual->value, new Name(EmptyIterator::class)), + new Expr\BinaryOp\BooleanOr( + new Expr\BinaryOp\BooleanAnd( + new Instanceof_($actual->value, new Name(Countable::class)), + new Identical(new FuncCall(new Name('count'), [new Arg($actual->value)]), new LNumber(0)), + ), + new Expr\Empty_($actual->value), + ), + ), + 'IsArray' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_array'), [$actual]), + 'IsBool' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_bool'), [$actual]), + 'IsCallable' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_callable'), [$actual]), + 'IsFloat' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_float'), [$actual]), + 'IsInt' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_int'), [$actual]), + 'IsIterable' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_iterable'), [$actual]), + 'IsNumeric' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_numeric'), [$actual]), + 'IsObject' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_object'), [$actual]), + 'IsResource' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_resource'), [$actual]), + 'IsString' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_string'), [$actual]), + 'IsScalar' => static fn (Scope $scope, Arg $actual): FuncCall => new FuncCall(new Name('is_scalar'), [$actual]), 'InternalType' => static function (Scope $scope, Arg $type, Arg $value): ?FuncCall { $typeNames = $scope->getType($type->value)->getConstantStrings(); if (count($typeNames) !== 1) { @@ -286,61 +248,49 @@ private static function getExpressionResolvers(): array new Name($functionName), [ $value, - ] - ); - }, - 'ArrayHasKey' => static function (Scope $scope, Arg $key, Arg $array): Expr { - return new Expr\BinaryOp\BooleanOr( - new Expr\BinaryOp\BooleanAnd( - new Expr\Instanceof_($array->value, new Name('ArrayAccess')), - new Expr\MethodCall($array->value, 'offsetExists', [$key]) - ), - new FuncCall(new Name('array_key_exists'), [$key, $array]) - ); - }, - 'ObjectHasAttribute' => static function (Scope $scope, Arg $property, Arg $object): FuncCall { - return new FuncCall(new Name('property_exists'), [$object, $property]); - }, - 'ObjectHasProperty' => static function (Scope $scope, Arg $property, Arg $object): FuncCall { - return new FuncCall(new Name('property_exists'), [$object, $property]); - }, - 'Contains' => static function (Scope $scope, Arg $needle, Arg $haystack): Expr { - return new Expr\BinaryOp\BooleanOr( - new Expr\Instanceof_($haystack->value, new Name('Traversable')), - new FuncCall(new Name('in_array'), [$needle, $haystack, new Arg(new ConstFetch(new Name('true')))]) - ); - }, - 'ContainsEquals' => static function (Scope $scope, Arg $needle, Arg $haystack): Expr { - return new Expr\BinaryOp\BooleanOr( - new Expr\Instanceof_($haystack->value, new Name('Traversable')), - new Expr\BinaryOp\BooleanAnd( - new Expr\BooleanNot(new Expr\Empty_($haystack->value)), - new FuncCall(new Name('in_array'), [$needle, $haystack, new Arg(new ConstFetch(new Name('false')))]) - ) - ); - }, - 'ContainsOnlyInstancesOf' => static function (Scope $scope, Arg $className, Arg $haystack): Expr { - return new Expr\BinaryOp\BooleanOr( - new Expr\Instanceof_($haystack->value, new Name('Traversable')), - new Identical( - $haystack->value, - new FuncCall(new Name('array_filter'), [ - $haystack, - new Arg(new Expr\Closure([ - 'static' => true, - 'params' => [ - new Param(new Expr\Variable('_')), - ], - 'stmts' => [ - new Stmt\Return_( - new FuncCall(new Name('is_a'), [new Arg(new Expr\Variable('_')), $className]) - ), - ], - ])), - ]) - ) + ], ); }, + 'ArrayHasKey' => static fn (Scope $scope, Arg $key, Arg $array): Expr => new Expr\BinaryOp\BooleanOr( + new Expr\BinaryOp\BooleanAnd( + new Expr\Instanceof_($array->value, new Name('ArrayAccess')), + new Expr\MethodCall($array->value, 'offsetExists', [$key]), + ), + new FuncCall(new Name('array_key_exists'), [$key, $array]), + ), + 'ObjectHasAttribute' => static fn (Scope $scope, Arg $property, Arg $object): FuncCall => new FuncCall(new Name('property_exists'), [$object, $property]), + 'ObjectHasProperty' => static fn (Scope $scope, Arg $property, Arg $object): FuncCall => new FuncCall(new Name('property_exists'), [$object, $property]), + 'Contains' => static fn (Scope $scope, Arg $needle, Arg $haystack): Expr => new Expr\BinaryOp\BooleanOr( + new Expr\Instanceof_($haystack->value, new Name('Traversable')), + new FuncCall(new Name('in_array'), [$needle, $haystack, new Arg(new ConstFetch(new Name('true')))]), + ), + 'ContainsEquals' => static fn (Scope $scope, Arg $needle, Arg $haystack): Expr => new Expr\BinaryOp\BooleanOr( + new Expr\Instanceof_($haystack->value, new Name('Traversable')), + new Expr\BinaryOp\BooleanAnd( + new Expr\BooleanNot(new Expr\Empty_($haystack->value)), + new FuncCall(new Name('in_array'), [$needle, $haystack, new Arg(new ConstFetch(new Name('false')))]), + ), + ), + 'ContainsOnlyInstancesOf' => static fn (Scope $scope, Arg $className, Arg $haystack): Expr => new Expr\BinaryOp\BooleanOr( + new Expr\Instanceof_($haystack->value, new Name('Traversable')), + new Identical( + $haystack->value, + new FuncCall(new Name('array_filter'), [ + $haystack, + new Arg(new Expr\Closure([ + 'static' => true, + 'params' => [ + new Param(new Expr\Variable('_')), + ], + 'stmts' => [ + new Stmt\Return_( + new FuncCall(new Name('is_a'), [new Arg(new Expr\Variable('_')), $className]), + ), + ], + ])), + ]), + ), + ), ]; } diff --git a/src/Type/PHPUnit/MockBuilderDynamicReturnTypeExtension.php b/src/Type/PHPUnit/MockBuilderDynamicReturnTypeExtension.php index 5390b11c..166a9038 100644 --- a/src/Type/PHPUnit/MockBuilderDynamicReturnTypeExtension.php +++ b/src/Type/PHPUnit/MockBuilderDynamicReturnTypeExtension.php @@ -27,7 +27,7 @@ public function isMethodSupported(MethodReflection $methodReflection): bool 'getMockForAbstractClass', 'getMockForTrait', ], - true + true, ); } diff --git a/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php b/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php index 4f74fe68..626f168c 100644 --- a/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php +++ b/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php @@ -31,9 +31,7 @@ public function isMethodSupported(MethodReflection $methodReflection): bool public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type { $type = $scope->getType($methodCall->var); - $mockClasses = array_values(array_filter($type->getObjectClassNames(), static function (string $class): bool { - return $class !== MockObject::class; - })); + $mockClasses = array_values(array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class)); if (count($mockClasses) !== 1) { return new ObjectType(InvocationMocker::class); diff --git a/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php b/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php index 69a7de2f..2a835eaf 100644 --- a/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php +++ b/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php @@ -17,7 +17,7 @@ protected function getRule(): Rule return new ClassCoversExistsRule( new CoversHelper($reflection), - $reflection + $reflection, ); } diff --git a/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php b/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php index b886b460..45e8b1f0 100644 --- a/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php +++ b/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php @@ -18,7 +18,7 @@ protected function getRule(): Rule return new ClassMethodCoversExistsRule( new CoversHelper($reflection), - self::getContainer()->getByType(FileTypeMapper::class) + self::getContainer()->getByType(FileTypeMapper::class), ); } From 4d861e0843cd1f8eccacfac14e24a8629280a887 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 13 Sep 2024 14:47:01 +0200 Subject: [PATCH 04/50] Fix after TypeSpecifier BC break --- .../PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php b/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php index cdc6ff34..04def4e3 100644 --- a/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php +++ b/src/Type/PHPUnit/Assert/AssertTypeSpecifyingExtensionHelper.php @@ -101,8 +101,7 @@ public static function specifyTypes( $scope, $expression, TypeSpecifierContext::createTruthy(), - $bypassAlwaysTrueIssue ? new Expr\BinaryOp\BooleanAnd($expression, new Expr\Variable('nonsense')) : null, - ); + )->setRootExpr($bypassAlwaysTrueIssue ? new Expr\BinaryOp\BooleanAnd($expression, new Expr\Variable('nonsense')) : $expression); } /** From 09e2d3b470bdda02824c626735153dfd962e3f29 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 24 Sep 2024 18:07:03 +0200 Subject: [PATCH 05/50] Uncover everything behind the bleedingEdge flag --- rules.neon | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/rules.neon b/rules.neon index 8dc7056b..023e11c1 100644 --- a/rules.neon +++ b/rules.neon @@ -2,28 +2,18 @@ rules: - PHPStan\Rules\PHPUnit\AssertSameBooleanExpectedRule - PHPStan\Rules\PHPUnit\AssertSameNullExpectedRule - PHPStan\Rules\PHPUnit\AssertSameWithCountRule + - PHPStan\Rules\PHPUnit\ClassCoversExistsRule + - PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule - PHPStan\Rules\PHPUnit\MockMethodCallRule + - PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule + - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule services: - - class: PHPStan\Rules\PHPUnit\ClassCoversExistsRule - - class: PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule arguments: checkFunctionNameCase: %checkFunctionNameCase% deprecationRulesInstalled: %deprecationRulesInstalled% - - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - - class: PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - -conditionalTags: - PHPStan\Rules\PHPUnit\ClassCoversExistsRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% - PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% - PHPStan\Rules\PHPUnit\DataProviderDeclarationRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% - PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% + tags: + - phpstan.rules.rule From 3cc855474263ad6220dfa49167cbea34ca1dd300 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 14 Oct 2024 05:16:27 +0200 Subject: [PATCH 06/50] Fixes after PHPStan update --- tests/Type/PHPUnit/data/assert-function.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Type/PHPUnit/data/assert-function.php b/tests/Type/PHPUnit/data/assert-function.php index 10fb4176..851e07b0 100644 --- a/tests/Type/PHPUnit/data/assert-function.php +++ b/tests/Type/PHPUnit/data/assert-function.php @@ -38,7 +38,7 @@ public function assertInstanceOfWorksWithTemplate($o, $class): void public function arrayHasNumericKey(array $a, \ArrayAccess $b): void { assertArrayHasKey(0, $a); - assertType('array&hasOffset(0)', $a); + assertType('non-empty-array&hasOffset(0)', $a); assertArrayHasKey(0, $b); assertType('ArrayAccess', $b); @@ -47,7 +47,7 @@ public function arrayHasNumericKey(array $a, \ArrayAccess $b): void { public function arrayHasStringKey(array $a, \ArrayAccess $b): void { assertArrayHasKey('key', $a); - assertType("array&hasOffset('key')", $a); + assertType("non-empty-array&hasOffset('key')", $a); assertArrayHasKey('key', $b); assertType("ArrayAccess", $b); From 4b6ad7fab8683ff4efd7887ba26ef8ee171c7475 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 12 Nov 2024 13:48:00 +0100 Subject: [PATCH 07/50] Fix --- stubs/Assert.stub | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stubs/Assert.stub b/stubs/Assert.stub index 01df95a1..d9ccd12b 100644 --- a/stubs/Assert.stub +++ b/stubs/Assert.stub @@ -5,7 +5,7 @@ namespace PHPUnit\Framework; abstract class Assert { /** - * @phpstan-assert list $array + * @phpstan-assert list $array * * @throws ExpectationFailedException */ From 10880dad5190e788634a62ebb211802669382f3d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 17 Dec 2024 18:22:01 +0100 Subject: [PATCH 08/50] Implement AssertEqualsIsDiscouragedRule (#216) --- README.md | 1 + composer.json | 2 +- rules.neon | 7 ++ .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 64 +++++++++++++++++++ .../AssertEqualsIsDiscouragedRuleTest.php | 38 +++++++++++ .../data/assert-equals-is-discouraged.php | 37 +++++++++++ 6 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php create mode 100644 tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php diff --git a/README.md b/README.md index 205cbe4b..526085b9 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ It also contains this strict framework-specific rules (can be enabled separately * Check that you are not using `assertSame()` with `false` as expected value. `assertFalse()` should be used instead. * Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead. * Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead. +* Check that you are not using `assertEquals()` with same types (`assertSame()` should be used) ## How to document mock objects in phpDocs? diff --git a/composer.json b/composer.json index 5237f066..3453041e 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.0" + "phpstan/phpstan": "^2.0.4" }, "conflict": { "phpunit/phpunit": "<7.0" diff --git a/rules.neon b/rules.neon index 023e11c1..84b71498 100644 --- a/rules.neon +++ b/rules.neon @@ -9,6 +9,10 @@ rules: - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule +conditionalTags: + PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: + phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] + services: - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule @@ -17,3 +21,6 @@ services: deprecationRulesInstalled: %deprecationRulesInstalled% tags: - phpstan.rules.rule + + - + class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php new file mode 100644 index 00000000..7aea39a5 --- /dev/null +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -0,0 +1,64 @@ + + */ +class AssertEqualsIsDiscouragedRule implements Rule +{ + + public function getNodeType(): string + { + return CallLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + return []; + } + + if (count($node->getArgs()) < 2) { + return []; + } + if (!$node->name instanceof Node\Identifier || strtolower($node->name->name) !== 'assertequals') { + return []; + } + + $leftType = TypeCombinator::removeNull($scope->getType($node->getArgs()[0]->value)); + $rightType = TypeCombinator::removeNull($scope->getType($node->getArgs()[1]->value)); + + if ($leftType->isConstantScalarValue()->yes()) { + $leftType = $leftType->generalize(GeneralizePrecision::lessSpecific()); + } + if ($rightType->isConstantScalarValue()->yes()) { + $rightType = $rightType->generalize(GeneralizePrecision::lessSpecific()); + } + + if ( + ($leftType->isScalar()->yes() && $rightType->isScalar()->yes()) + && ($leftType->isSuperTypeOf($rightType)->yes()) + && ($rightType->isSuperTypeOf($leftType)->yes()) + ) { + return [ + RuleErrorBuilder::message( + 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type', + )->identifier('phpunit.assertEquals')->build(), + ]; + } + + return []; + } + +} diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php new file mode 100644 index 00000000..e739ee49 --- /dev/null +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -0,0 +1,38 @@ + + */ +final class AssertEqualsIsDiscouragedRuleTest extends RuleTestCase +{ + + private const ERROR_MESSAGE = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type'; + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [ + [self::ERROR_MESSAGE, 19], + [self::ERROR_MESSAGE, 22], + [self::ERROR_MESSAGE, 23], + [self::ERROR_MESSAGE, 24], + [self::ERROR_MESSAGE, 25], + [self::ERROR_MESSAGE, 26], + [self::ERROR_MESSAGE, 27], + [self::ERROR_MESSAGE, 28], + [self::ERROR_MESSAGE, 29], + [self::ERROR_MESSAGE, 30], + [self::ERROR_MESSAGE, 32], + ]); + } + + protected function getRule(): Rule + { + return new AssertEqualsIsDiscouragedRule(); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php new file mode 100644 index 00000000..727408d1 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php @@ -0,0 +1,37 @@ +assertSame(5, $integer); + static::assertSame(5, $integer); + + $this->assertEquals('', $string); + $this->assertEquals(null, $string); + static::assertEquals(null, $string); + static::assertEquals($nullableString, $string); + $this->assertEquals(2, $integer); + $this->assertEquals(2.2, $float); + static::assertEquals((int) '2', (int) $string); + $this->assertEquals(true, $boolean); + $this->assertEquals($string, $string); + $this->assertEquals($integer, $integer); + $this->assertEquals($boolean, $boolean); + $this->assertEquals($float, $float); + $this->assertEquals($null, $null); + $this->assertEquals((string) new Exception(), (string) new Exception()); + $this->assertEquals([], []); + $this->assertEquals(new Exception(), new Exception()); + static::assertEquals(new Exception(), new Exception()); + } +} From e32ac656788a5bf3dedda89e6a2cad5643bf1a18 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 19 Dec 2024 09:27:11 +0100 Subject: [PATCH 09/50] Support assertNotEquals in AssertEqualsIsDiscouragedRule --- src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php | 6 +++++- tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php | 3 +++ tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index 7aea39a5..f4fc89c0 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -10,6 +10,7 @@ use PHPStan\Type\GeneralizePrecision; use PHPStan\Type\TypeCombinator; use function count; +use function in_array; use function strtolower; /** @@ -32,7 +33,10 @@ public function processNode(Node $node, Scope $scope): array if (count($node->getArgs()) < 2) { return []; } - if (!$node->name instanceof Node\Identifier || strtolower($node->name->name) !== 'assertequals') { + if ( + !$node->name instanceof Node\Identifier + || !in_array(strtolower($node->name->name), ['assertequals', 'assertnotequals'], true) + ) { return []; } diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php index e739ee49..f1d34d0e 100644 --- a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -27,6 +27,9 @@ public function testRule(): void [self::ERROR_MESSAGE, 29], [self::ERROR_MESSAGE, 30], [self::ERROR_MESSAGE, 32], + [self::ERROR_MESSAGE, 37], + [self::ERROR_MESSAGE, 38], + [self::ERROR_MESSAGE, 39], ]); } diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php index 727408d1..7f4d80e1 100644 --- a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php @@ -33,5 +33,11 @@ public function dummyTest(string $string, int $integer, bool $boolean, float $fl $this->assertEquals([], []); $this->assertEquals(new Exception(), new Exception()); static::assertEquals(new Exception(), new Exception()); + + $this->assertNotEquals($string, $string); + $this->assertNotEquals($integer, $integer); + $this->assertNotEquals($boolean, $boolean); + $this->assertNotSame(5, $integer); + static::assertNotSame(5, $integer); } } From d09e152f403c843998d7a52b5d87040c937525dd Mon Sep 17 00:00:00 2001 From: PrinsFrank <25006490+PrinsFrank@users.noreply.github.com> Date: Wed, 22 Jan 2025 14:07:38 +0100 Subject: [PATCH 10/50] Fix error message for "assertNotEquals" usage referencing "assertSame" and "assertEquals" --- README.md | 1 + .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 7 ++++- .../AssertEqualsIsDiscouragedRuleTest.php | 31 ++++++++++--------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 526085b9..34693790 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ It also contains this strict framework-specific rules (can be enabled separately * Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead. * Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead. * Check that you are not using `assertEquals()` with same types (`assertSame()` should be used) +* Check that you are not using `assertNotEquals()` with same types (`assertNotSame()` should be used) ## How to document mock objects in phpDocs? diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index f4fc89c0..bd685dd7 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -11,6 +11,7 @@ use PHPStan\Type\TypeCombinator; use function count; use function in_array; +use function sprintf; use function strtolower; /** @@ -57,7 +58,11 @@ public function processNode(Node $node, Scope $scope): array ) { return [ RuleErrorBuilder::message( - 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type', + sprintf( + 'You should use %s() instead of %s(), because both values are scalars of the same type', + strtolower($node->name->name) === 'assertnotequals' ? 'assertNotSame' : 'assertSame', + $node->name->name, + ), )->identifier('phpunit.assertEquals')->build(), ]; } diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php index f1d34d0e..568b5b6f 100644 --- a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -11,25 +11,26 @@ final class AssertEqualsIsDiscouragedRuleTest extends RuleTestCase { - private const ERROR_MESSAGE = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type'; + private const ERROR_MESSAGE_EQUALS = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type'; + private const ERROR_MESSAGE_NOT_EQUALS = 'You should use assertNotSame() instead of assertNotEquals(), because both values are scalars of the same type'; public function testRule(): void { $this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [ - [self::ERROR_MESSAGE, 19], - [self::ERROR_MESSAGE, 22], - [self::ERROR_MESSAGE, 23], - [self::ERROR_MESSAGE, 24], - [self::ERROR_MESSAGE, 25], - [self::ERROR_MESSAGE, 26], - [self::ERROR_MESSAGE, 27], - [self::ERROR_MESSAGE, 28], - [self::ERROR_MESSAGE, 29], - [self::ERROR_MESSAGE, 30], - [self::ERROR_MESSAGE, 32], - [self::ERROR_MESSAGE, 37], - [self::ERROR_MESSAGE, 38], - [self::ERROR_MESSAGE, 39], + [self::ERROR_MESSAGE_EQUALS, 19], + [self::ERROR_MESSAGE_EQUALS, 22], + [self::ERROR_MESSAGE_EQUALS, 23], + [self::ERROR_MESSAGE_EQUALS, 24], + [self::ERROR_MESSAGE_EQUALS, 25], + [self::ERROR_MESSAGE_EQUALS, 26], + [self::ERROR_MESSAGE_EQUALS, 27], + [self::ERROR_MESSAGE_EQUALS, 28], + [self::ERROR_MESSAGE_EQUALS, 29], + [self::ERROR_MESSAGE_EQUALS, 30], + [self::ERROR_MESSAGE_EQUALS, 32], + [self::ERROR_MESSAGE_NOT_EQUALS, 37], + [self::ERROR_MESSAGE_NOT_EQUALS, 38], + [self::ERROR_MESSAGE_NOT_EQUALS, 39], ]); } From 17bbfd3532b399d57ba9f9d788711e79c487b593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Mirtes?= Date: Tue, 28 Jan 2025 10:24:59 +0100 Subject: [PATCH 11/50] Update LICENSE --- LICENSE | 1 + 1 file changed, 1 insertion(+) diff --git a/LICENSE b/LICENSE index d0053746..52fba1e2 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,7 @@ MIT License Copyright (c) 2016 Ondřej Mirtes +Copyright (c) 2025 PHPStan s.r.o. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From 0f857bfcea0d73d7dc4b506c40c8c2a5fffc3191 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 18 Mar 2025 10:49:50 +0100 Subject: [PATCH 12/50] Introduce phpstan-deprecation-rules --- composer.json | 1 + phpstan.neon | 1 + src/Rules/PHPUnit/ClassCoversExistsRule.php | 2 +- src/Rules/PHPUnit/ClassMethodCoversExistsRule.php | 2 +- src/Rules/PHPUnit/DataProviderDeclarationRule.php | 2 +- src/Rules/PHPUnit/DataProviderHelper.php | 6 +++--- src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php | 2 +- src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php | 2 +- src/Rules/PHPUnit/ShouldCallParentMethodsRule.php | 2 +- 9 files changed, 11 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 3453041e..212c518f 100644 --- a/composer.json +++ b/composer.json @@ -14,6 +14,7 @@ }, "require-dev": { "php-parallel-lint/php-parallel-lint": "^1.2", + "phpstan/phpstan-deprecation-rules": "^2.0", "phpstan/phpstan-strict-rules": "^2.0", "phpunit/phpunit": "^9.6" }, diff --git a/phpstan.neon b/phpstan.neon index 2b8fa1ae..7c96296a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,6 +2,7 @@ includes: - extension.neon - rules.neon - vendor/phpstan/phpstan-strict-rules/rules.neon + - vendor/phpstan/phpstan-deprecation-rules/rules.neon - phar://phpstan.phar/conf/bleedingEdge.neon - phpstan-baseline.neon diff --git a/src/Rules/PHPUnit/ClassCoversExistsRule.php b/src/Rules/PHPUnit/ClassCoversExistsRule.php index f8d831c9..a36317ef 100644 --- a/src/Rules/PHPUnit/ClassCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassCoversExistsRule.php @@ -50,7 +50,7 @@ public function processNode(Node $node, Scope $scope): array { $classReflection = $node->getClassReflection(); - if (!$classReflection->isSubclassOf(TestCase::class)) { + if (!$classReflection->is(TestCase::class)) { return []; } diff --git a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php index 4bfdc417..dd328f83 100644 --- a/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php +++ b/src/Rules/PHPUnit/ClassMethodCoversExistsRule.php @@ -56,7 +56,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - if (!$classReflection->isSubclassOf(TestCase::class)) { + if (!$classReflection->is(TestCase::class)) { return []; } diff --git a/src/Rules/PHPUnit/DataProviderDeclarationRule.php b/src/Rules/PHPUnit/DataProviderDeclarationRule.php index cc44eeb4..1983493c 100644 --- a/src/Rules/PHPUnit/DataProviderDeclarationRule.php +++ b/src/Rules/PHPUnit/DataProviderDeclarationRule.php @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array { $classReflection = $scope->getClassReflection(); - if ($classReflection === null || !$classReflection->isSubclassOf(TestCase::class)) { + if ($classReflection === null || !$classReflection->is(TestCase::class)) { return []; } diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index ad354fa5..aad678e3 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -76,7 +76,7 @@ public function getDataProviderMethods( } $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); - $dataProviderMethod[] = $node->getLine(); + $dataProviderMethod[] = $node->getStartLine(); yield $dataProviderValue => $dataProviderMethod; } @@ -257,7 +257,7 @@ private function parseDataProviderExternalAttribute(Attribute $attribute): ?arra sprintf('%s::%s', $className, $methodNameArg->value) => [ $dataProviderClassReflection, $methodNameArg->value, - $attribute->getLine(), + $attribute->getStartLine(), ], ]; } @@ -279,7 +279,7 @@ private function parseDataProviderAttribute(Attribute $attribute, ClassReflectio $methodNameArg->value => [ $classReflection, $methodNameArg->value, - $attribute->getLine(), + $attribute->getStartLine(), ], ]; } diff --git a/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php b/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php index c8fd3a14..a2fc39f1 100644 --- a/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php +++ b/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php @@ -33,7 +33,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { $classReflection = $scope->getClassReflection(); - if ($classReflection === null || $classReflection->isSubclassOf(TestCase::class) === false) { + if ($classReflection === null || $classReflection->is(TestCase::class) === false) { return []; } diff --git a/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php b/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php index a44fc539..906e60b1 100644 --- a/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php +++ b/src/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRule.php @@ -33,7 +33,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { $classReflection = $scope->getClassReflection(); - if ($classReflection === null || $classReflection->isSubclassOf(TestCase::class) === false) { + if ($classReflection === null || $classReflection->is(TestCase::class) === false) { return []; } diff --git a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php index 4035dcc9..bfd31690 100644 --- a/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php +++ b/src/Rules/PHPUnit/ShouldCallParentMethodsRule.php @@ -33,7 +33,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - if (!$scope->getClassReflection()->isSubclassOf(TestCase::class)) { + if (!$scope->getClassReflection()->is(TestCase::class)) { return []; } From 855b82c4e70c0274ca099585d1dcf026a5485a2d Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 14:57:01 +0100 Subject: [PATCH 13/50] Remove config.platform --- composer.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/composer.json b/composer.json index 212c518f..60ba1e7a 100644 --- a/composer.json +++ b/composer.json @@ -19,9 +19,6 @@ "phpunit/phpunit": "^9.6" }, "config": { - "platform": { - "php": "7.4.6" - }, "sort-packages": true }, "extra": { From 846d1612c1f1f589d3ffdbf683d5bc82b60ffd0c Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 14:58:54 +0100 Subject: [PATCH 14/50] Test multiple PHPUnit versions --- .github/workflows/build.yml | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 88543fb5..07de1083 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -97,6 +97,21 @@ jobs: dependencies: - "lowest" - "highest" + phpunit-version: + - "^9.5" + - "^10.5" + - "^11.5" + exclude: + - php-version: "7.4" + phpunit-version: "^10.5" + - php-version: "8.0" + phpunit-version: "^10.5" + - php-version: "7.4" + phpunit-version: "^11.5" + - php-version: "8.0" + phpunit-version: "^11.5" + - php-version: "8.1" + phpunit-version: "^11.5" steps: - name: "Checkout" @@ -108,6 +123,9 @@ jobs: coverage: "none" php-version: "${{ matrix.php-version }}" + - name: "Require specific PHPUnit version" + run: "composer require --dev phpunit/phpunit:${{ matrix.phpunit-version }}" + - name: "Install lowest dependencies" if: ${{ matrix.dependencies == 'lowest' }} run: "composer update --prefer-lowest --no-interaction --no-progress" @@ -136,6 +154,21 @@ jobs: dependencies: - "lowest" - "highest" + phpunit-version: + - "^9.5" + - "^10.5" + - "^11.5" + exclude: + - php-version: "7.4" + phpunit-version: "^10.5" + - php-version: "8.0" + phpunit-version: "^10.5" + - php-version: "7.4" + phpunit-version: "^11.5" + - php-version: "8.0" + phpunit-version: "^11.5" + - php-version: "8.1" + phpunit-version: "^11.5" steps: - name: "Checkout" @@ -149,6 +182,9 @@ jobs: extensions: mbstring tools: composer:v2 + - name: "Require specific PHPUnit version" + run: "composer require --dev phpunit/phpunit:${{ matrix.phpunit-version }}" + - name: "Install lowest dependencies" if: ${{ matrix.dependencies == 'lowest' }} run: "composer update --prefer-lowest --no-interaction --no-progress" From 342b6c18973fd1205b67ad59a83a316cfb2ee45e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:05:58 +0100 Subject: [PATCH 15/50] Data providers must be static --- .../PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php | 6 +++--- .../PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php index 2d43f8bc..4405c4d6 100644 --- a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php @@ -9,14 +9,14 @@ class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase { /** @return mixed[] */ - public function dataFileAsserts(): iterable + public static function dataFileAsserts(): iterable { if (function_exists('PHPUnit\\Framework\\assertInstanceOf')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/assert-function.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/assert-function.php'); } if (function_exists('PHPUnit\\Framework\\assertObjectHasProperty')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/assert-function-9.6.11.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/assert-function-9.6.11.php'); } return []; diff --git a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php index e1841e0b..0b36c2b9 100644 --- a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php @@ -8,9 +8,9 @@ class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase { /** @return mixed[] */ - public function dataFileAsserts(): iterable + public static function dataFileAsserts(): iterable { - yield from $this->gatherAssertTypes(__DIR__ . '/data/assert-method.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/assert-method.php'); } /** From 630aa9981ec9ca809ab2e2f6e69f18bc790ae6aa Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:09:31 +0100 Subject: [PATCH 16/50] Remove deprecated assert --- tests/Type/PHPUnit/data/assert-function.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/Type/PHPUnit/data/assert-function.php b/tests/Type/PHPUnit/data/assert-function.php index 851e07b0..0fabf59e 100644 --- a/tests/Type/PHPUnit/data/assert-function.php +++ b/tests/Type/PHPUnit/data/assert-function.php @@ -53,12 +53,6 @@ public function arrayHasStringKey(array $a, \ArrayAccess $b): void assertType("ArrayAccess", $b); } - public function objectHasAttribute(object $a): void - { - assertObjectHasAttribute('property', $a); - assertType("object&hasProperty(property)", $a); - } - public function testEmpty($a): void { assertEmpty($a); From 19f8059bdc64a6234c45483f46a3e034c7dd06c2 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:15:12 +0100 Subject: [PATCH 17/50] Do not generate code coverage --- phpunit.xml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index 8f71615a..420ef740 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -12,20 +12,6 @@ failOnWarning="true" xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xml" > - - - ./src - - - - - - - tests From eb88670836752d7f4a231512f4da407f24cf458a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:23:33 +0100 Subject: [PATCH 18/50] Always install nikic/php-parser v5 --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 60ba1e7a..be6b3bd7 100644 --- a/composer.json +++ b/composer.json @@ -13,6 +13,7 @@ "phpunit/phpunit": "<7.0" }, "require-dev": { + "nikic/php-parser": "^5", "php-parallel-lint/php-parallel-lint": "^1.2", "phpstan/phpstan-deprecation-rules": "^2.0", "phpstan/phpstan-strict-rules": "^2.0", From 1a07095989e956832e1fd0f36eb422c1ab8ca725 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 23 Mar 2025 23:01:32 +0100 Subject: [PATCH 19/50] Add non regression test for #222 --- phpstan-baseline.neon | 5 +++ tests/Rules/Methods/CallMethodsRuleTest.php | 34 +++++++++++++++++++++ tests/Rules/Methods/data/bug-222.php | 34 +++++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 tests/Rules/Methods/CallMethodsRuleTest.php create mode 100644 tests/Rules/Methods/data/bug-222.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 53fb96b8..f56b3cfb 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -14,3 +14,8 @@ parameters: message: "#^Accessing PHPStan\\\\Rules\\\\Comparison\\\\ImpossibleCheckTypeMethodCallRule\\:\\:class is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#" count: 1 path: tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php + + - + message: "#^Accessing PHPStan\\\\Rules\\\\Methods\\\\CallMethodsRule\\:\\:class is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#" + count: 1 + path: tests/Rules/Methods/CallMethodsRuleTest.php diff --git a/tests/Rules/Methods/CallMethodsRuleTest.php b/tests/Rules/Methods/CallMethodsRuleTest.php new file mode 100644 index 00000000..99d572f1 --- /dev/null +++ b/tests/Rules/Methods/CallMethodsRuleTest.php @@ -0,0 +1,34 @@ + + */ +class CallMethodsRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return self::getContainer()->getByType(CallMethodsRule::class); + } + + public function testBug222(): void + { + $this->analyse([__DIR__ . '/data/bug-222.php'], []); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../../extension.neon', + ]; + } + +} diff --git a/tests/Rules/Methods/data/bug-222.php b/tests/Rules/Methods/data/bug-222.php new file mode 100644 index 00000000..d07ca146 --- /dev/null +++ b/tests/Rules/Methods/data/bug-222.php @@ -0,0 +1,34 @@ +expects($this->exactly(1)) + ->method('get') + ->with(24) + ->willReturn('24'); + + $mockService + ->method('get') + ->with(24) + ->willReturn('24'); + + $mockService + ->expects($this->exactly(1)) + ->method('get') + ->willReturn('24'); + + $mockService + ->method('get') + ->willReturn('24'); + } + +} From 40fbbc1e6fe722f9c244b90b14c097c39bc77989 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 24 Mar 2025 02:28:06 +0000 Subject: [PATCH 20/50] chore(deps): update metcalfc/changelog-generator action to v4.5.0 --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b1a669a9..be6cad08 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,7 @@ jobs: - name: Generate changelog id: changelog - uses: metcalfc/changelog-generator@v4.3.1 + uses: metcalfc/changelog-generator@v4.5.0 with: myToken: ${{ secrets.PHPSTAN_BOT_TOKEN }} From bf031aeef3eae57052c8275553d4e688d07a3b99 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:26:26 +0100 Subject: [PATCH 21/50] Test PHPUnit v12 --- .github/workflows/build.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 07de1083..65f351a3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -101,6 +101,7 @@ jobs: - "^9.5" - "^10.5" - "^11.5" + - "^12.0.9" exclude: - php-version: "7.4" phpunit-version: "^10.5" @@ -112,6 +113,14 @@ jobs: phpunit-version: "^11.5" - php-version: "8.1" phpunit-version: "^11.5" + - php-version: "7.4" + phpunit-version: "^12.0.9" + - php-version: "8.0" + phpunit-version: "^12.0.9" + - php-version: "8.1" + phpunit-version: "^12.0.9" + - php-version: "8.2" + phpunit-version: "^12.0.9" steps: - name: "Checkout" @@ -158,6 +167,7 @@ jobs: - "^9.5" - "^10.5" - "^11.5" + - "^12.0.9" exclude: - php-version: "7.4" phpunit-version: "^10.5" @@ -169,6 +179,14 @@ jobs: phpunit-version: "^11.5" - php-version: "8.1" phpunit-version: "^11.5" + - php-version: "7.4" + phpunit-version: "^12.0.9" + - php-version: "8.0" + phpunit-version: "^12.0.9" + - php-version: "8.1" + phpunit-version: "^12.0.9" + - php-version: "8.2" + phpunit-version: "^12.0.9" steps: - name: "Checkout" From 1f36fc548cf0af5db4ca4d12891f82d4cc7bcc96 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 24 Mar 2025 14:53:55 +0100 Subject: [PATCH 22/50] Use DataProvider attribute --- phpstan.neon | 5 +++++ .../PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php | 2 ++ .../Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php | 2 ++ 3 files changed, 9 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index 7c96296a..57379453 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -9,3 +9,8 @@ includes: parameters: excludePaths: - tests/*/data/* + ignoreErrors: + - + message: '#^Attribute class PHPUnit\\Framework\\Attributes\\DataProvider does not exist\.$#' + identifier: attribute.notFound + reportUnmatched: false diff --git a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php index 4405c4d6..40140174 100644 --- a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Type\PHPUnit; use PHPStan\Testing\TypeInferenceTestCase; +use PHPUnit\Framework\Attributes\DataProvider; use function function_exists; class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase @@ -26,6 +27,7 @@ public static function dataFileAsserts(): iterable * @dataProvider dataFileAsserts * @param mixed ...$args */ + #[DataProvider('dataFileAsserts')] public function testFileAsserts( string $assertType, string $file, diff --git a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php index 0b36c2b9..8c6ebb8b 100644 --- a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Type\PHPUnit; use PHPStan\Testing\TypeInferenceTestCase; +use PHPUnit\Framework\Attributes\DataProvider; class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase { @@ -17,6 +18,7 @@ public static function dataFileAsserts(): iterable * @dataProvider dataFileAsserts * @param mixed ...$args */ + #[DataProvider('dataFileAsserts')] public function testFileAsserts( string $assertType, string $file, From 4d2b44bdba358f0a1990247f8b59f6c31185e600 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 24 Mar 2025 15:10:24 +0100 Subject: [PATCH 23/50] InvocationMocker class no longer exists --- extension.neon | 9 --- src/Rules/PHPUnit/MockMethodCallRule.php | 65 ++++++++++++------- ...cationMockerDynamicReturnTypeExtension.php | 30 --------- .../MockObjectDynamicReturnTypeExtension.php | 43 ------------ stubs/InvocationMocker.stub | 13 ---- .../Rules/PHPUnit/MockMethodCallRuleTest.php | 10 +-- 6 files changed, 43 insertions(+), 127 deletions(-) delete mode 100644 src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php delete mode 100644 src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php delete mode 100644 stubs/InvocationMocker.stub diff --git a/extension.neon b/extension.neon index 8de21f54..117c9922 100644 --- a/extension.neon +++ b/extension.neon @@ -12,7 +12,6 @@ parameters: - stubs/Assert.stub - stubs/AssertionFailedError.stub - stubs/ExpectationFailedException.stub - - stubs/InvocationMocker.stub - stubs/MockBuilder.stub - stubs/MockObject.stub - stubs/Stub.stub @@ -42,18 +41,10 @@ services: class: PHPStan\Type\PHPUnit\Assert\AssertStaticMethodTypeSpecifyingExtension tags: - phpstan.typeSpecifier.staticMethodTypeSpecifyingExtension - - - class: PHPStan\Type\PHPUnit\InvocationMockerDynamicReturnTypeExtension - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - class: PHPStan\Type\PHPUnit\MockBuilderDynamicReturnTypeExtension tags: - phpstan.broker.dynamicMethodReturnTypeExtension - - - class: PHPStan\Type\PHPUnit\MockObjectDynamicReturnTypeExtension - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - class: PHPStan\Rules\PHPUnit\CoversHelper - diff --git a/src/Rules/PHPUnit/MockMethodCallRule.php b/src/Rules/PHPUnit/MockMethodCallRule.php index b6f8932a..e953d18e 100644 --- a/src/Rules/PHPUnit/MockMethodCallRule.php +++ b/src/Rules/PHPUnit/MockMethodCallRule.php @@ -5,9 +5,10 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use PHPStan\Analyser\Scope; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use PHPUnit\Framework\MockObject\Builder\InvocationMocker; +use PHPStan\Type\Type; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\Stub; use function array_filter; @@ -47,44 +48,58 @@ public function processNode(Node $node, Scope $scope): array $method = $constantString->getValue(); $type = $scope->getType($node->var); - if ( - ( - in_array(MockObject::class, $type->getObjectClassNames(), true) - || in_array(Stub::class, $type->getObjectClassNames(), true) - ) - && !$type->hasMethod($method)->yes() - ) { - $mockClasses = array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class && $class !== Stub::class); - if (count($mockClasses) === 0) { - continue; - } - - $errors[] = RuleErrorBuilder::message(sprintf( - 'Trying to mock an undefined method %s() on class %s.', - $method, - implode('&', $mockClasses), - ))->identifier('phpunit.mockMethod')->build(); + $error = $this->checkCallOnType($type, $method); + if ($error !== null) { + $errors[] = $error; continue; } - $mockedClassObject = $type->getTemplateType(InvocationMocker::class, 'TMockedClass'); - if ($mockedClassObject->hasMethod($method)->yes()) { + if (!$node->var instanceof MethodCall) { continue; } - $classNames = $mockedClassObject->getObjectClassNames(); - if (count($classNames) === 0) { + if (!$node->var->name instanceof Node\Identifier) { continue; } - $errors[] = RuleErrorBuilder::message(sprintf( + if ($node->var->name->toLowerString() !== 'expects') { + continue; + } + + $varType = $scope->getType($node->var->var); + $error = $this->checkCallOnType($varType, $method); + if ($error === null) { + continue; + } + + $errors[] = $error; + } + + return $errors; + } + + private function checkCallOnType(Type $type, string $method): ?IdentifierRuleError + { + if ( + ( + in_array(MockObject::class, $type->getObjectClassNames(), true) + || in_array(Stub::class, $type->getObjectClassNames(), true) + ) + && !$type->hasMethod($method)->yes() + ) { + $mockClasses = array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class && $class !== Stub::class); + if (count($mockClasses) === 0) { + return null; + } + + return RuleErrorBuilder::message(sprintf( 'Trying to mock an undefined method %s() on class %s.', $method, - implode('|', $classNames), + implode('&', $mockClasses), ))->identifier('phpunit.mockMethod')->build(); } - return $errors; + return null; } } diff --git a/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php b/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php deleted file mode 100644 index 44764f62..00000000 --- a/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php +++ /dev/null @@ -1,30 +0,0 @@ -getName() !== 'getMatcher'; - } - - public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type - { - return $scope->getType($methodCall->var); - } - -} diff --git a/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php b/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php deleted file mode 100644 index 626f168c..00000000 --- a/src/Type/PHPUnit/MockObjectDynamicReturnTypeExtension.php +++ /dev/null @@ -1,43 +0,0 @@ -getName() === 'expects'; - } - - public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type - { - $type = $scope->getType($methodCall->var); - $mockClasses = array_values(array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class)); - - if (count($mockClasses) !== 1) { - return new ObjectType(InvocationMocker::class); - } - - return new GenericObjectType(InvocationMocker::class, [new ObjectType($mockClasses[0])]); - } - -} diff --git a/stubs/InvocationMocker.stub b/stubs/InvocationMocker.stub deleted file mode 100644 index c58719f5..00000000 --- a/stubs/InvocationMocker.stub +++ /dev/null @@ -1,13 +0,0 @@ - @@ -28,14 +27,11 @@ public function testRule(): void 'Trying to mock an undefined method doBadThing() on class MockMethodCall\Bar.', 20, ], - ]; - - if (interface_exists('PHPUnit\Framework\MockObject\Builder\InvocationStubber')) { - $expectedErrors[] = [ + [ 'Trying to mock an undefined method doBadThing() on class MockMethodCall\Bar.', 36, - ]; - } + ], + ]; $this->analyse([__DIR__ . '/data/mock-method-call.php'], $expectedErrors); } From 0aef32ff3a4e5f01b94d2373aae586aea0af6ed8 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 26 Mar 2025 13:43:47 +0100 Subject: [PATCH 24/50] Improve logic MockMethodCallRule to search for method even on wrong type --- src/Rules/PHPUnit/MockMethodCallRule.php | 18 ++++---- .../Rules/PHPUnit/MockMethodCallRuleTest.php | 5 +++ tests/Rules/PHPUnit/data/bug-227.php | 42 +++++++++++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/bug-227.php diff --git a/src/Rules/PHPUnit/MockMethodCallRule.php b/src/Rules/PHPUnit/MockMethodCallRule.php index e953d18e..6c3b0dc4 100644 --- a/src/Rules/PHPUnit/MockMethodCallRule.php +++ b/src/Rules/PHPUnit/MockMethodCallRule.php @@ -48,7 +48,7 @@ public function processNode(Node $node, Scope $scope): array $method = $constantString->getValue(); $type = $scope->getType($node->var); - $error = $this->checkCallOnType($type, $method); + $error = $this->checkCallOnType($scope, $type, $method); if ($error !== null) { $errors[] = $error; continue; @@ -67,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array } $varType = $scope->getType($node->var->var); - $error = $this->checkCallOnType($varType, $method); + $error = $this->checkCallOnType($scope, $varType, $method); if ($error === null) { continue; } @@ -78,14 +78,16 @@ public function processNode(Node $node, Scope $scope): array return $errors; } - private function checkCallOnType(Type $type, string $method): ?IdentifierRuleError + private function checkCallOnType(Scope $scope, Type $type, string $method): ?IdentifierRuleError { + $methodReflection = $scope->getMethodReflection($type, $method); + if ($methodReflection !== null) { + return null; + } + if ( - ( - in_array(MockObject::class, $type->getObjectClassNames(), true) - || in_array(Stub::class, $type->getObjectClassNames(), true) - ) - && !$type->hasMethod($method)->yes() + in_array(MockObject::class, $type->getObjectClassNames(), true) + || in_array(Stub::class, $type->getObjectClassNames(), true) ) { $mockClasses = array_filter($type->getObjectClassNames(), static fn (string $class): bool => $class !== MockObject::class && $class !== Stub::class); if (count($mockClasses) === 0) { diff --git a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php index a7f73085..284b2820 100644 --- a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php +++ b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php @@ -36,6 +36,11 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/mock-method-call.php'], $expectedErrors); } + public function testBug227(): void + { + $this->analyse([__DIR__ . '/data/bug-227.php'], []); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/bug-227.php b/tests/Rules/PHPUnit/data/bug-227.php new file mode 100644 index 00000000..18a4d4a6 --- /dev/null +++ b/tests/Rules/PHPUnit/data/bug-227.php @@ -0,0 +1,42 @@ +tsfe = $this->getMockBuilder(Foo::class) + ->onlyMethods(['addCacheTags', 'getLanguage']) + ->disableOriginalConstructor() + ->getMock(); + $this->tsfe->method('getLanguage')->willReturn('aaa'); + } + + public function testSometest(): void + { + $this->tsfe->expects(self::once())->method('addCacheTags'); + } +} From 6b92469f8a7995e626da3aa487099617b8dfa260 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 26 Mar 2025 13:47:06 +0100 Subject: [PATCH 25/50] Fix build --- tests/Rules/PHPUnit/MockMethodCallRuleTest.php | 4 ++++ tests/Rules/PHPUnit/data/bug-227.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php index 284b2820..f7e89c7a 100644 --- a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php +++ b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -38,6 +39,9 @@ public function testRule(): void public function testBug227(): void { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped('Test requires PHP 8.0.'); + } $this->analyse([__DIR__ . '/data/bug-227.php'], []); } diff --git a/tests/Rules/PHPUnit/data/bug-227.php b/tests/Rules/PHPUnit/data/bug-227.php index 18a4d4a6..1efe6c1c 100644 --- a/tests/Rules/PHPUnit/data/bug-227.php +++ b/tests/Rules/PHPUnit/data/bug-227.php @@ -1,4 +1,4 @@ -= 8.0 namespace Bug227; From d35895e0388e9aa822af00e1dfd4f8086fd8938d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 7 Apr 2025 03:35:39 +0000 Subject: [PATCH 26/50] chore(deps): update metcalfc/changelog-generator action to v4.6.2 --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index be6cad08..b8c96d48 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,7 @@ jobs: - name: Generate changelog id: changelog - uses: metcalfc/changelog-generator@v4.5.0 + uses: metcalfc/changelog-generator@v4.6.2 with: myToken: ${{ secrets.PHPSTAN_BOT_TOKEN }} From 22c6949f5c0d4d3d343fbd42d2bd75472089d135 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 6 Jun 2025 10:34:24 +0200 Subject: [PATCH 27/50] Make `phpunit.dataProviderStatic` auto-fixable --- composer.json | 2 +- extension.neon | 2 ++ src/Rules/PHPUnit/DataProviderHelper.php | 28 +++++++++++++++++-- .../PHPUnit/DataProviderHelperFactory.php | 12 ++++++-- .../DataProviderDeclarationRuleTest.php | 7 ++++- .../PHPUnit/data/data-provider-static-fix.php | 21 ++++++++++++++ .../data/data-provider-static-fix.php.fixed | 21 ++++++++++++++ 7 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/data-provider-static-fix.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-static-fix.php.fixed diff --git a/composer.json b/composer.json index be6b3bd7..3c07436d 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.0.4" + "phpstan/phpstan": "^2.1.18" }, "conflict": { "phpunit/phpunit": "<7.0" diff --git a/extension.neon b/extension.neon index 117c9922..28900524 100644 --- a/extension.neon +++ b/extension.neon @@ -54,6 +54,8 @@ services: factory: @PHPStan\Rules\PHPUnit\DataProviderHelperFactory::create() - class: PHPStan\Rules\PHPUnit\DataProviderHelperFactory + arguments: + parser: @defaultAnalysisParser conditionalTags: PHPStan\PhpDoc\PHPUnit\MockObjectTypeNodeResolverExtension: diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index aad678e3..338d7167 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -2,12 +2,15 @@ namespace PHPStan\Rules\PHPUnit; +use PhpParser\Modifiers; use PhpParser\Node\Attribute; use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\NodeFinder; use PHPStan\Analyser\Scope; +use PHPStan\Parser\Parser; use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use PHPStan\Reflection\ClassReflection; @@ -37,16 +40,20 @@ class DataProviderHelper */ private FileTypeMapper $fileTypeMapper; + private Parser $parser; + private bool $phpunit10OrNewer; public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, + Parser $parser, bool $phpunit10OrNewer ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; + $this->parser = $parser; $this->phpunit10OrNewer = $phpunit10OrNewer; } @@ -188,13 +195,28 @@ public function processDataProvider( } if ($deprecationRulesInstalled && $this->phpunit10OrNewer && !$dataProviderMethodReflection->isStatic()) { - $errors[] = RuleErrorBuilder::message(sprintf( + $errorBuilder = RuleErrorBuilder::message(sprintf( '@dataProvider %s related method must be static in PHPUnit 10 and newer.', $dataProviderValue, )) ->line($lineNumber) - ->identifier('phpunit.dataProviderStatic') - ->build(); + ->identifier('phpunit.dataProviderStatic'); + + $dataProviderMethodReflectionDeclaringClass = $dataProviderMethodReflection->getDeclaringClass(); + if ($dataProviderMethodReflectionDeclaringClass->getFileName() !== null) { + $stmts = $this->parser->parseFile($dataProviderMethodReflectionDeclaringClass->getFileName()); + $nodeFinder = new NodeFinder(); + /** @var ClassMethod|null $methodNode */ + $methodNode = $nodeFinder->findFirst($stmts, static fn ($node) => $node instanceof ClassMethod && $node->name->toString() === $dataProviderMethodReflection->getName()); + if ($methodNode !== null) { + $errorBuilder->fixNode($methodNode, static function (ClassMethod $methodNode) { + $methodNode->flags |= Modifiers::STATIC; + + return $methodNode; + }); + } + } + $errors[] = $errorBuilder->build(); } return $errors; diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 9b965e2c..a0768c8a 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\PHPUnit; +use PHPStan\Parser\Parser; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; use PHPUnit\Framework\TestCase; @@ -18,10 +19,17 @@ class DataProviderHelperFactory private FileTypeMapper $fileTypeMapper; - public function __construct(ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper) + private Parser $parser; + + public function __construct( + ReflectionProvider $reflectionProvider, + FileTypeMapper $fileTypeMapper, + Parser $parser + ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; + $this->parser = $parser; } public function create(): DataProviderHelper @@ -49,7 +57,7 @@ public function create(): DataProviderHelper } } - return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $phpUnit10OrNewer); + return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $phpUnit10OrNewer); } } diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 18cc12b3..cbe40bf1 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -17,7 +17,7 @@ protected function getRule(): Rule $reflection = $this->createReflectionProvider(); return new DataProviderDeclarationRule( - new DataProviderHelper($reflection, self::getContainer()->getByType(FileTypeMapper::class),true), + new DataProviderHelper($reflection, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), true), true, true ); @@ -65,6 +65,11 @@ public function testRule(): void ]); } + public function testFixDataProviderStatic(): void + { + $this->fix(__DIR__ . '/data/data-provider-static-fix.php', __DIR__ . '/data/data-provider-static-fix.php.fixed'); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/data-provider-static-fix.php b/tests/Rules/PHPUnit/data/data-provider-static-fix.php new file mode 100644 index 00000000..1f8005a2 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-static-fix.php @@ -0,0 +1,21 @@ + Date: Sun, 13 Jul 2025 13:29:09 +0200 Subject: [PATCH 28/50] Check isFirstClassCallable before accessing getArgs --- src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php | 4 ++++ src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php | 4 ++++ src/Rules/PHPUnit/AssertSameNullExpectedRule.php | 4 ++++ src/Rules/PHPUnit/AssertSameWithCountRule.php | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index bd685dd7..7257eec5 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -31,6 +31,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->isFirstClassCallable()) { + return []; + } + if (count($node->getArgs()) < 2) { return []; } diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index 308f5147..9abbd75a 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -27,6 +27,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->isFirstClassCallable()) { + return []; + } + if (count($node->getArgs()) < 2) { return []; } diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index 363fa578..83807ec9 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -27,6 +27,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->isFirstClassCallable()) { + return []; + } + if (count($node->getArgs()) < 2) { return []; } diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 3c3ada4b..9e051cc8 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -28,6 +28,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->isFirstClassCallable()) { + return []; + } + if (count($node->getArgs()) < 2) { return []; } From 9a9b161baee88a5f5c58d816943cff354ff233dc Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 13 Jul 2025 12:42:18 +0200 Subject: [PATCH 29/50] Make AssertEqualsIsDiscouragedRule auto-fixable --- .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 17 +++++++++++-- .../AssertEqualsIsDiscouragedRuleTest.php | 5 ++++ .../assert-equals-is-discouraged-fixable.php | 24 +++++++++++++++++++ ...rt-equals-is-discouraged-fixable.php.fixed | 24 +++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php create mode 100644 tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php.fixed diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index 7257eec5..b03f0830 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -60,14 +60,27 @@ public function processNode(Node $node, Scope $scope): array && ($leftType->isSuperTypeOf($rightType)->yes()) && ($rightType->isSuperTypeOf($leftType)->yes()) ) { + $correctName = strtolower($node->name->name) === 'assertnotequals' ? 'assertNotSame' : 'assertSame'; return [ RuleErrorBuilder::message( sprintf( 'You should use %s() instead of %s(), because both values are scalars of the same type', - strtolower($node->name->name) === 'assertnotequals' ? 'assertNotSame' : 'assertSame', + $correctName, $node->name->name, ), - )->identifier('phpunit.assertEquals')->build(), + )->identifier('phpunit.assertEquals') + ->fixNode($node, static function (CallLike $node) use ($correctName) { + if ($node instanceof Node\Expr\MethodCall) { + $node->name = new Node\Identifier($correctName); + } + + if ($node instanceof Node\Expr\StaticCall) { + $node->name = new Node\Identifier($correctName); + } + + return $node; + }) + ->build(), ]; } diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php index 568b5b6f..040c1eee 100644 --- a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -34,6 +34,11 @@ public function testRule(): void ]); } + public function testFix(): void + { + $this->fix(__DIR__ . '/data/assert-equals-is-discouraged-fixable.php', __DIR__ . '/data/assert-equals-is-discouraged-fixable.php.fixed'); + } + protected function getRule(): Rule { return new AssertEqualsIsDiscouragedRule(); diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php new file mode 100644 index 00000000..5c0c993b --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php @@ -0,0 +1,24 @@ +assertEquals('', $s); + $this->assertNotEquals('', $t); + } + + public function doFoo2(string $s, string $t): void + { + self::assertEquals('', $s); + self::assertNotEquals('', $t); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php.fixed new file mode 100644 index 00000000..9217e3e1 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged-fixable.php.fixed @@ -0,0 +1,24 @@ +assertSame('', $s); + $this->assertNotSame('', $t); + } + + public function doFoo2(string $s, string $t): void + { + self::assertSame('', $s); + self::assertNotSame('', $t); + } + +} From 1d7eb7e5d8e7daefb033df1beaa497dfeed72c04 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 7 Aug 2025 11:00:11 +0000 Subject: [PATCH 30/50] chore(deps): update eomm/why-don-t-you-tweet action to v2 --- .github/workflows/release-tweet.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-tweet.yml b/.github/workflows/release-tweet.yml index 09b39ded..d81f34ca 100644 --- a/.github/workflows/release-tweet.yml +++ b/.github/workflows/release-tweet.yml @@ -10,7 +10,7 @@ jobs: tweet: runs-on: ubuntu-latest steps: - - uses: Eomm/why-don-t-you-tweet@v1 + - uses: Eomm/why-don-t-you-tweet@v2 if: ${{ !github.event.repository.private }} with: # GitHub event payload From b1df1f52d17f04ad6670fd2b6ffbda99d0b63bb6 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 18 Aug 2025 02:01:02 +0000 Subject: [PATCH 31/50] chore(deps): update actions/checkout action to v5 --- .github/workflows/build.yml | 10 +++++----- .github/workflows/create-tag.yml | 2 +- .github/workflows/release.yml | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 65f351a3..28ab39c6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -25,7 +25,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: "Install PHP" uses: "shivammathur/setup-php@v2" @@ -49,10 +49,10 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: "Checkout build-cs" - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: "phpstan/build-cs" path: "build-cs" @@ -124,7 +124,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: "Install PHP" uses: "shivammathur/setup-php@v2" @@ -190,7 +190,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: "Install PHP" uses: "shivammathur/setup-php@v2" diff --git a/.github/workflows/create-tag.yml b/.github/workflows/create-tag.yml index a8535014..fd918164 100644 --- a/.github/workflows/create-tag.yml +++ b/.github/workflows/create-tag.yml @@ -21,7 +21,7 @@ jobs: runs-on: "ubuntu-latest" steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: fetch-depth: 0 token: ${{ secrets.PHPSTAN_BOT_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b8c96d48..ed7e51ad 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,7 +14,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Generate changelog id: changelog From 9ec3fa951a29922c8f7d456fc04a7ccc9bb2ae81 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Tue, 2 Sep 2025 18:14:57 +0200 Subject: [PATCH 32/50] Modernize code examples in README.md --- README.md | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 34693790..c86df268 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ This extension provides following features: * `createMock()`, `getMockForAbstractClass()` and `getMockFromWsdl()` methods return an intersection type (see the [detailed explanation of intersection types](https://phpstan.org/blog/union-types-vs-intersection-types)) of the mock object and the mocked class so that both methods from the mock object (like `expects`) and from the mocked class are available on the object. * `getMock()` called on `MockBuilder` is also supported. -* Interprets `Foo|PHPUnit_Framework_MockObject_MockObject` in phpDoc so that it results in an intersection type instead of a union type. +* Interprets `Foo|MockObject` in phpDoc so that it results in an intersection type instead of a union type. * Defines early terminating method calls for the `PHPUnit\Framework\TestCase` class to prevent undefined variable errors. * Specifies types of expressions passed to various `assert` methods like `assertInstanceOf`, `assertTrue`, `assertInternalType` etc. * Combined with PHPStan's level 4, it points out always-true and always-false asserts like `assertTrue(true)` etc. @@ -27,18 +27,15 @@ It also contains this strict framework-specific rules (can be enabled separately ## How to document mock objects in phpDocs? -If you need to configure the mock even after you assign it to a property or return it from a method, you should add `PHPUnit_Framework_MockObject_MockObject` to the phpDoc: +If you need to configure the mock even after you assign it to a property or return it from a method, you should add `\PHPUnit\Framework\MockObject\MockObject` to the type: ```php -/** - * @return Foo&PHPUnit_Framework_MockObject_MockObject - */ -private function createFooMock() +private function createFooMock(): Foo&\PHPUnit\Framework\MockObject\MockObject { return $this->createMock(Foo::class); } -public function testSomething() +public function testSomething(): void { $fooMock = $this->createFooMock(); $fooMock->method('doFoo')->will($this->returnValue('test')); @@ -46,22 +43,33 @@ public function testSomething() } ``` -Please note that the correct syntax for intersection types is `Foo&PHPUnit_Framework_MockObject_MockObject`. `Foo|PHPUnit_Framework_MockObject_MockObject` is also supported, but only for ecosystem and legacy reasons. +If you cannot use native intersection types yet, you can use PHPDoc instead. + +```php +/** + * @return Foo&\PHPUnit\Framework\MockObject\MockObject + */ +private function createFooMock(): Foo +{ + return $this->createMock(Foo::class); +} +``` + +Please note that the correct syntax for intersection types is `Foo&\PHPUnit\Framework\MockObject\MockObject`. `Foo|\PHPUnit\Framework\MockObject\MockObject` is also supported, but only for ecosystem and legacy reasons. If the mock is fully configured and only the methods of the mocked class are supposed to be called on the value, it's fine to typehint only the mocked class: ```php -/** @var Foo */ -private $foo; +private Foo $foo; -protected function setUp() +protected function setUp(): void { $fooMock = $this->createMock(Foo::class); $fooMock->method('doFoo')->will($this->returnValue('test')); $this->foo = $fooMock; } -public function testSomething() +public function testSomething(): void { $this->foo->doFoo(); // $this->foo->method() and expects() can no longer be called From c54a26903d4a34b6d2a13b004f505ba340aabcf4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 24 Sep 2025 13:42:52 +0200 Subject: [PATCH 33/50] Narrow too wide return type --- src/Rules/PHPUnit/DataProviderHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 338d7167..5f65656f 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -285,7 +285,7 @@ private function parseDataProviderExternalAttribute(Attribute $attribute): ?arra } /** - * @return array|null + * @return array|null */ private function parseDataProviderAttribute(Attribute $attribute, ClassReflection $classReflection): ?array { From 39950c70b6a4faa36567b83b8c4b32f889c68e03 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 14 Oct 2025 12:27:41 +0200 Subject: [PATCH 34/50] Preparational refactoring for DataProviderDataRule --- extension.neon | 3 + phpstan.neon | 2 + src/Rules/PHPUnit/DataProviderHelper.php | 141 ++++++++++++------ .../PHPUnit/DataProviderHelperFactory.php | 37 +---- src/Rules/PHPUnit/PHPUnitVersionDetector.php | 57 +++++++ 5 files changed, 163 insertions(+), 77 deletions(-) create mode 100644 src/Rules/PHPUnit/PHPUnitVersionDetector.php diff --git a/extension.neon b/extension.neon index 28900524..70831227 100644 --- a/extension.neon +++ b/extension.neon @@ -49,6 +49,9 @@ services: class: PHPStan\Rules\PHPUnit\CoversHelper - class: PHPStan\Rules\PHPUnit\AnnotationHelper + - + class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector + - class: PHPStan\Rules\PHPUnit\DataProviderHelper factory: @PHPStan\Rules\PHPUnit\DataProviderHelperFactory::create() diff --git a/phpstan.neon b/phpstan.neon index 57379453..7f64d1c0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,8 @@ includes: - phpstan-baseline.neon parameters: + reportUnmatchedIgnoredErrors: false + excludePaths: - tests/*/data/* ignoreErrors: diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 5f65656f..64137c76 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\PHPUnit; +use PhpParser\Comment\Doc; use PhpParser\Modifiers; use PhpParser\Node\Attribute; use PhpParser\Node\Expr\ClassConstFetch; @@ -19,25 +20,19 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\FileTypeMapper; +use ReflectionMethod; use function array_merge; use function count; use function explode; +use function method_exists; use function preg_match; use function sprintf; class DataProviderHelper { - /** - * Reflection provider. - * - */ private ReflectionProvider $reflectionProvider; - /** - * The file type mapper. - * - */ private FileTypeMapper $fileTypeMapper; private Parser $parser; @@ -58,56 +53,23 @@ public function __construct( } /** + * @param ReflectionMethod|ClassMethod $node + * * @return iterable */ public function getDataProviderMethods( Scope $scope, - ClassMethod $node, + $node, ClassReflection $classReflection ): iterable { - $docComment = $node->getDocComment(); - if ($docComment !== null) { - $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( - $scope->getFile(), - $classReflection->getName(), - $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, - $node->name->toString(), - $docComment->getText(), - ); - foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { - $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); - if ($dataProviderValue === null) { - // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule - continue; - } - - $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); - $dataProviderMethod[] = $node->getStartLine(); - - yield $dataProviderValue => $dataProviderMethod; - } - } + yield from $this->yieldDataProviderAnnotations($node, $scope, $classReflection); if (!$this->phpunit10OrNewer) { return; } - foreach ($node->attrGroups as $attrGroup) { - foreach ($attrGroup->attrs as $attr) { - $dataProviderMethod = null; - if ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataprovider') { - $dataProviderMethod = $this->parseDataProviderAttribute($attr, $classReflection); - } elseif ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataproviderexternal') { - $dataProviderMethod = $this->parseDataProviderExternalAttribute($attr); - } - if ($dataProviderMethod === null) { - continue; - } - - yield from $dataProviderMethod; - } - } + yield from $this->yieldDataProviderAttributes($node, $classReflection); } /** @@ -306,4 +268,91 @@ private function parseDataProviderAttribute(Attribute $attribute, ClassReflectio ]; } + /** + * @param ReflectionMethod|ClassMethod $node + * + * @return iterable + */ + private function yieldDataProviderAttributes($node, ClassReflection $classReflection): iterable + { + if ( + $node instanceof ReflectionMethod + ) { + /** @phpstan-ignore function.alreadyNarrowedType */ + if (!method_exists($node, 'getAttributes')) { + return; + } + + foreach ($node->getAttributes('PHPUnit\Framework\Attributes\DataProvider') as $attr) { + $args = $attr->getArguments(); + if (count($args) !== 1) { + continue; + } + + $startLine = $node->getStartLine(); + if ($startLine === false) { + $startLine = -1; + } + + yield [$classReflection, $args[0], $startLine]; + } + + return; + } + + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + $dataProviderMethod = null; + if ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataprovider') { + $dataProviderMethod = $this->parseDataProviderAttribute($attr, $classReflection); + } elseif ($attr->name->toLowerString() === 'phpunit\\framework\\attributes\\dataproviderexternal') { + $dataProviderMethod = $this->parseDataProviderExternalAttribute($attr); + } + if ($dataProviderMethod === null) { + continue; + } + + yield from $dataProviderMethod; + } + } + } + + /** + * @param ReflectionMethod|ClassMethod $node + * + * @return iterable + */ + private function yieldDataProviderAnnotations($node, Scope $scope, ClassReflection $classReflection): iterable + { + $docComment = $node->getDocComment(); + if ($docComment === null || $docComment === false) { + return; + } + + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $node instanceof ClassMethod ? $node->name->toString() : $node->getName(), + $docComment instanceof Doc ? $docComment->getText() : $docComment, + ); + foreach ($this->getDataProviderAnnotations($methodPhpDoc) as $annotation) { + $dataProviderValue = $this->getDataProviderAnnotationValue($annotation); + if ($dataProviderValue === null) { + // Missing value is already handled in NoMissingSpaceInMethodAnnotationRule + continue; + } + + $startLine = $node->getStartLine(); + if ($startLine === false) { + $startLine = -1; + } + + $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); + $dataProviderMethod[] = $startLine; + + yield $dataProviderValue => $dataProviderMethod; + } + } + } diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index a0768c8a..23a5f34a 100644 --- a/src/Rules/PHPUnit/DataProviderHelperFactory.php +++ b/src/Rules/PHPUnit/DataProviderHelperFactory.php @@ -5,12 +5,6 @@ use PHPStan\Parser\Parser; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\FileTypeMapper; -use PHPUnit\Framework\TestCase; -use function dirname; -use function explode; -use function file_get_contents; -use function is_file; -use function json_decode; class DataProviderHelperFactory { @@ -21,43 +15,24 @@ class DataProviderHelperFactory private Parser $parser; + private PHPUnitVersionDetector $PHPUnitVersionDetector; + public function __construct( ReflectionProvider $reflectionProvider, FileTypeMapper $fileTypeMapper, - Parser $parser + Parser $parser, + PHPUnitVersionDetector $PHPUnitVersionDetector ) { $this->reflectionProvider = $reflectionProvider; $this->fileTypeMapper = $fileTypeMapper; $this->parser = $parser; + $this->PHPUnitVersionDetector = $PHPUnitVersionDetector; } public function create(): DataProviderHelper { - $phpUnit10OrNewer = false; - if ($this->reflectionProvider->hasClass(TestCase::class)) { - $testCase = $this->reflectionProvider->getClass(TestCase::class); - $file = $testCase->getFileName(); - if ($file !== null) { - $phpUnitRoot = dirname($file, 3); - $phpUnitComposer = $phpUnitRoot . '/composer.json'; - if (is_file($phpUnitComposer)) { - $composerJson = @file_get_contents($phpUnitComposer); - if ($composerJson !== false) { - $json = json_decode($composerJson, true); - $version = $json['extra']['branch-alias']['dev-main'] ?? null; - if ($version !== null) { - $majorVersion = (int) explode('.', $version)[0]; - if ($majorVersion >= 10) { - $phpUnit10OrNewer = true; - } - } - } - } - } - } - - return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $phpUnit10OrNewer); + return new DataProviderHelper($this->reflectionProvider, $this->fileTypeMapper, $this->parser, $this->PHPUnitVersionDetector->isPHPUnit10OrNewer()); } } diff --git a/src/Rules/PHPUnit/PHPUnitVersionDetector.php b/src/Rules/PHPUnit/PHPUnitVersionDetector.php new file mode 100644 index 00000000..841e69b9 --- /dev/null +++ b/src/Rules/PHPUnit/PHPUnitVersionDetector.php @@ -0,0 +1,57 @@ +reflectionProvider = $reflectionProvider; + } + + public function isPHPUnit10OrNewer(): bool + { + if ($this->is10OrNewer !== null) { + return $this->is10OrNewer; + } + + $this->is10OrNewer = false; + if ($this->reflectionProvider->hasClass(TestCase::class)) { + $testCase = $this->reflectionProvider->getClass(TestCase::class); + $file = $testCase->getFileName(); + if ($file !== null) { + $phpUnitRoot = dirname($file, 3); + $phpUnitComposer = $phpUnitRoot . '/composer.json'; + if (is_file($phpUnitComposer)) { + $composerJson = @file_get_contents($phpUnitComposer); + if ($composerJson !== false) { + $json = json_decode($composerJson, true); + $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; + } + +} From 61860a671c7ab5fb3891ea730a31cf1ac669a3e9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 26 Oct 2025 09:38:27 +0100 Subject: [PATCH 35/50] Implement DataProviderDataRule --- composer.json | 2 +- extension.neon | 7 + rules.neon | 6 + src/Rules/PHPUnit/DataProviderDataRule.php | 246 +++++++++ src/Rules/PHPUnit/TestMethodsHelper.php | 94 ++++ .../PHPUnit/TestMethodsHelperFactory.php | 28 + .../PHPUnit/DataProviderDataRuleTest.php | 273 +++++++++ .../DataProviderDeclarationRuleTest.php | 7 +- .../PHPUnit/data/data-provider-data-named.php | 109 ++++ .../Rules/PHPUnit/data/data-provider-data.php | 519 ++++++++++++++++++ .../data/data-provider-trimming-args.php | 32 ++ .../data/data-provider-variadic-method.php | 61 ++ 12 files changed, 1382 insertions(+), 2 deletions(-) create mode 100644 src/Rules/PHPUnit/DataProviderDataRule.php create mode 100644 src/Rules/PHPUnit/TestMethodsHelper.php create mode 100644 src/Rules/PHPUnit/TestMethodsHelperFactory.php create mode 100644 tests/Rules/PHPUnit/DataProviderDataRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-data-named.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-data.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-trimming-args.php create mode 100644 tests/Rules/PHPUnit/data/data-provider-variadic-method.php diff --git a/composer.json b/composer.json index 3c07436d..39d7a030 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.1.18" + "phpstan/phpstan": "^2.1.32" }, "conflict": { "phpunit/phpunit": "<7.0" diff --git a/extension.neon b/extension.neon index 70831227..c6ce9bf4 100644 --- a/extension.neon +++ b/extension.neon @@ -49,9 +49,16 @@ services: class: PHPStan\Rules\PHPUnit\CoversHelper - class: PHPStan\Rules\PHPUnit\AnnotationHelper + - class: PHPStan\Rules\PHPUnit\PHPUnitVersionDetector + - + class: PHPStan\Rules\PHPUnit\TestMethodsHelper + factory: @PHPStan\Rules\PHPUnit\TestMethodsHelperFactory::create() + - + class: PHPStan\Rules\PHPUnit\TestMethodsHelperFactory + - class: PHPStan\Rules\PHPUnit\DataProviderHelper factory: @PHPStan\Rules\PHPUnit\DataProviderHelperFactory::create() diff --git a/rules.neon b/rules.neon index 84b71498..63e10b47 100644 --- a/rules.neon +++ b/rules.neon @@ -13,6 +13,9 @@ conditionalTags: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] + PHPStan\Rules\PHPUnit\DataProviderDataRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% + services: - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule @@ -24,3 +27,6 @@ services: - class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule + + - + class: PHPStan\Rules\PHPUnit\DataProviderDataRule diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php new file mode 100644 index 00000000..e241053e --- /dev/null +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -0,0 +1,246 @@ + + */ +class DataProviderDataRule implements Rule +{ + + private TestMethodsHelper $testMethodsHelper; + + private DataProviderHelper $dataProviderHelper; + + public function __construct( + TestMethodsHelper $testMethodsHelper, + DataProviderHelper $dataProviderHelper + ) + { + $this->testMethodsHelper = $testMethodsHelper; + $this->dataProviderHelper = $dataProviderHelper; + } + + public function getNodeType(): string + { + return Node::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ( + !$node instanceof Node\Stmt\Return_ + && !$node instanceof Node\Expr\Yield_ + && !$node instanceof Node\Expr\YieldFrom + ) { + return []; + } + + if ($scope->getFunction() === null) { + return []; + } + if ($scope->isInAnonymousFunction()) { + return []; + } + + $arraysTypes = $this->buildArrayTypesFromNode($node, $scope); + if ($arraysTypes === []) { + return []; + } + + $method = $scope->getFunction(); + $classReflection = $scope->getClassReflection(); + if ( + $classReflection === null + || !$classReflection->is(TestCase::class) + ) { + return []; + } + + $testsWithProvider = []; + $testMethods = $this->testMethodsHelper->getTestMethods($classReflection, $scope); + foreach ($testMethods as $testMethod) { + foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [, $providerMethodName]) { + if ($providerMethodName === $method->getName()) { + $testsWithProvider[] = $testMethod; + continue 2; + } + } + } + + if (count($testsWithProvider) === 0) { + return []; + } + + $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); + if (count($testsWithProvider) > 1) { + foreach ($testsWithProvider as $testMethod) { + if ($testMethod->isVariadic()) { + $maxNumberOfParameters = PHP_INT_MAX; + break; + } + + $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters()); + } + } + + foreach ($testsWithProvider as $testMethod) { + $numberOfParameters = $testMethod->getNumberOfParameters(); + + foreach ($arraysTypes as [$startLine, $arraysType]) { + $args = $this->arrayItemsToArgs($arraysType, $maxNumberOfParameters); + if ($args === null) { + continue; + } + + if ( + !$testMethod->isVariadic() + && $numberOfParameters !== $maxNumberOfParameters + ) { + $args = array_slice($args, 0, $numberOfParameters); + } + + $scope->invokeNodeCallback(new Node\Expr\MethodCall( + new TypeExpr(new ObjectType($classReflection->getName())), + $testMethod->getName(), + $args, + ['startLine' => $startLine], + )); + } + } + + return []; + } + + /** + * @return array + */ + private function arrayItemsToArgs(Type $array, int $numberOfParameters): ?array + { + $args = []; + + $constArrays = $array->getConstantArrays(); + if ($constArrays !== [] && count($constArrays) === 1) { + $keyTypes = $constArrays[0]->getKeyTypes(); + $valueTypes = $constArrays[0]->getValueTypes(); + } elseif ($array->isArray()->yes()) { + $keyTypes = []; + $valueTypes = []; + for ($i = 0; $i < $numberOfParameters; ++$i) { + $keyTypes[$i] = $array->getIterableKeyType(); + $valueTypes[$i] = $array->getIterableValueType(); + } + } else { + return null; + } + + foreach ($valueTypes as $i => $valueType) { + $key = $keyTypes[$i]->getConstantStrings(); + if (count($key) > 1) { + return null; + } + + if (count($key) === 0) { + $arg = new Node\Arg(new TypeExpr($valueType)); + $args[] = $arg; + continue; + + } + + $arg = new Node\Arg( + new TypeExpr($valueType), + false, + false, + [], + new Node\Identifier($key[0]->getValue()), + ); + $args[] = $arg; + } + + return $args; + } + + /** + * @param Node\Stmt\Return_|Node\Expr\Yield_|Node\Expr\YieldFrom $node + * + * @return list + */ + private function buildArrayTypesFromNode(Node $node, Scope $scope): array + { + $arraysTypes = []; + + // special case for providers only containing static data, so we get more precise error lines + if ( + ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) + || ($node instanceof Node\Expr\YieldFrom && $node->expr instanceof Node\Expr\Array_) + ) { + foreach ($node->expr->items as $item) { + if (!$item->value instanceof Node\Expr\Array_) { + $arraysTypes = []; + break; + } + + $constArrays = $scope->getType($item->value)->getConstantArrays(); + if ($constArrays === []) { + $arraysTypes = []; + break; + } + + foreach ($constArrays as $constArray) { + $arraysTypes[] = [$item->value->getStartLine(), $constArray]; + } + } + + if ($arraysTypes !== []) { + return $arraysTypes; + } + } + + // general case with less precise error message lines + if ($node instanceof Node\Stmt\Return_ || $node instanceof Node\Expr\YieldFrom) { + if ($node->expr === null) { + return []; + } + + $exprType = $scope->getType($node->expr); + $exprConstArrays = $exprType->getConstantArrays(); + foreach ($exprConstArrays as $constArray) { + foreach ($constArray->getValueTypes() as $valueType) { + foreach ($valueType->getConstantArrays() as $constValueArray) { + $arraysTypes[] = [$node->getStartLine(), $constValueArray]; + } + } + } + + if ($arraysTypes === []) { + foreach ($exprType->getIterableValueType()->getArrays() as $arrayType) { + $arraysTypes[] = [$node->getStartLine(), $arrayType]; + } + } + } elseif ($node instanceof Node\Expr\Yield_) { + if ($node->value === null) { + return []; + } + + $exprType = $scope->getType($node->value); + foreach ($exprType->getConstantArrays() as $constValueArray) { + $arraysTypes[] = [$node->getStartLine(), $constValueArray]; + } + } + + return $arraysTypes; + } + +} diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php new file mode 100644 index 00000000..d0984442 --- /dev/null +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -0,0 +1,94 @@ +fileTypeMapper = $fileTypeMapper; + $this->phpunit10OrNewer = $phpunit10OrNewer; + } + + /** + * @return array + */ + public function getTestMethods(ClassReflection $classReflection, Scope $scope): array + { + $testMethods = []; + foreach ($classReflection->getNativeReflection()->getMethods() as $reflectionMethod) { + if (!$reflectionMethod->isPublic()) { + continue; + } + + if (str_starts_with(strtolower($reflectionMethod->getName()), 'test')) { + $testMethods[] = $reflectionMethod; + continue; + } + + $docComment = $reflectionMethod->getDocComment(); + if ($docComment !== false) { + $methodPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $reflectionMethod->getName(), + $docComment, + ); + + if ($this->hasTestAnnotation($methodPhpDoc)) { + $testMethods[] = $reflectionMethod; + continue; + } + } + + if (!$this->phpunit10OrNewer) { + continue; + } + + $testAttributes = $reflectionMethod->getAttributes('PHPUnit\Framework\Attributes\Test'); // @phpstan-ignore argument.type + if ($testAttributes === []) { + continue; + } + + $testMethods[] = $reflectionMethod; + } + + return $testMethods; + } + + private function hasTestAnnotation(?ResolvedPhpDocBlock $phpDoc): bool + { + if ($phpDoc === null) { + return false; + } + + $phpDocNodes = $phpDoc->getPhpDocNodes(); + + foreach ($phpDocNodes as $docNode) { + $tags = $docNode->getTagsByName('@test'); + if ($tags !== []) { + return true; + } + } + + return false; + } + +} diff --git a/src/Rules/PHPUnit/TestMethodsHelperFactory.php b/src/Rules/PHPUnit/TestMethodsHelperFactory.php new file mode 100644 index 00000000..202f88cb --- /dev/null +++ b/src/Rules/PHPUnit/TestMethodsHelperFactory.php @@ -0,0 +1,28 @@ +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 new file mode 100644 index 00000000..fa3aa9fc --- /dev/null +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -0,0 +1,273 @@ + + */ +class DataProviderDataRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $reflectionProvider = $this->createReflectionProvider(); + + /** @var list> $rules */ + $rules = [ + new DataProviderDataRule( + new TestMethodsHelper( + self::getContainer()->getByType(FileTypeMapper::class), + true + ), + new DataProviderHelper( + $reflectionProvider, + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + true + ), + + ), + self::getContainer()->getByType(CallMethodsRule::class) /** @phpstan-ignore phpstanApi.classConstant */ + ]; + + return new CompositeRule($rules); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/data-provider-data.php'], [ + [ + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, int given.', + 24, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\FooTest::testWithAttribute() expects string, false given.', + 28, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, int given.', + 51, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\BarTest::testWithAnnotation() expects string, false given.', + 55, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, int given.', + 80, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldTest::myTestMethod() expects string, false given.', + 86, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, int given.', + 112, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromTest::myTestMethod() expects string, false given.', + 116, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 3 parameters, 2 required.', + 141, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCount::testFoo() invoked with 1 parameter, 2 required.', + 146, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 3 parameters, 2 required.', + 177, + ], + [ + 'Method DataProviderDataTest\DifferentArgumentCountWithReusedDataprovider::testFoo() invoked with 1 parameter, 2 required.', + 182, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\UnionTypeReturnTest::testFoo() expects string, int given.', + 216, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, int given.', + 236, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\YieldFromExpr::testFoo() expects string, true given.', + 238, + ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic::testBar() expects int, string given.', + 295, + ], + [ + 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic::testFoo() expects string, int given.', + 296, + ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestInvalidVariadic2::testBar() expects int, string given.', + 317, + ], + [ + 'Parameter #2 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', + 317, + ], + [ + 'Parameter #4 ...$moreS of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects int, string given.', + 317, + ], + [ + 'Parameter #1 $s of method DataProviderDataTest\TestInvalidVariadic2::testFoo() expects string, int given.', + 318, + ], + [ + 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testBar() expects int, int|string given.', + 362, + ], + [ + 'Parameter #1 $i of method DataProviderDataTest\TestArrayIterator::testFoo() expects int, int|string given.', + 362, + ], + [ + 'Parameter #1 $s1 of method DataProviderDataTest\TestArrayIterator::testFooBar() expects string, int|string given.', + 362, + ], + [ + 'Parameter #1 $si of method DataProviderDataTest\TestWrongTypedIterable::testBar() expects int, string given.', + 380, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, int given.', + 407, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\AbstractBaseTest::testWithAttribute() expects string, false given.', + 411, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionTypeReturnTest::testFoo() expects string, int given.', + 446, + ], + [ + 'Method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() invoked with 3 parameters, 2 required.', + 484, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayDifferentLengthUnionTypeReturnTest::testFoo() expects string, int given.', + 484, + ], + [ + 'Parameter #2 $input of method DataProviderDataTest\ConstantArrayUnionWithDifferentValueTypeReturnTest::testFoo() expects string, int|string given.', + 517, + ], + ]); + } + + public function testRulePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped(); + } + + $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 + ], + ]); + } + + + public function testVariadicMethod(): void + { + $this->analyse([__DIR__ . '/data/data-provider-variadic-method.php'], [ + [ + 'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.', + 12, + ], + [ + 'Parameter #1 $a of method DataProviderVariadicMethod\FooTest::testProvide() expects int, string given.', + 13, + ], + [ + 'Method DataProviderVariadicMethod\FooTest::testProvide2() invoked with 1 parameter, at least 2 required.', + 13, + ], + [ + 'Parameter #1 $a of method DataProviderVariadicMethod\FooTest::testProvide2() expects int, string given.', + 13, + ], + [ + 'Parameter #2 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide() expects string, int given.', + 15, + ], + [ + 'Parameter #3 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide() expects string, int given.', + 15, + ], + [ + 'Parameter #2 $two of method DataProviderVariadicMethod\FooTest::testProvide2() expects string, int given.', + 15, + ], + [ + 'Parameter #3 ...$rest of method DataProviderVariadicMethod\FooTest::testProvide2() expects string, int given.', + 15, + ], + ]); + } + + public function testTrimmingArgs(): void + { + $this->analyse([__DIR__ . '/data/data-provider-trimming-args.php'], [ + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.', + 12, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide2() invoked with 2 parameters, 1 required.', + 12, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide() invoked with 2 parameters, 1 required.', + 13, + ], + [ + 'Method DataProviderTrimmingArgs\FooTest::testProvide2() invoked with 2 parameters, 1 required.', + 13, + ], + ]); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../../extension.neon', + ]; + } +} diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index cbe40bf1..67f55ed4 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -17,7 +17,12 @@ protected function getRule(): Rule $reflection = $this->createReflectionProvider(); return new DataProviderDeclarationRule( - new DataProviderHelper($reflection, self::getContainer()->getByType(FileTypeMapper::class), self::getContainer()->getService('defaultAnalysisParser'), true), + new DataProviderHelper( + $reflection, + self::getContainer()->getByType(FileTypeMapper::class), + self::getContainer()->getService('defaultAnalysisParser'), + true + ), true, true ); diff --git a/tests/Rules/PHPUnit/data/data-provider-data-named.php b/tests/Rules/PHPUnit/data/data-provider-data-named.php new file mode 100644 index 00000000..bea3e50e --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-data-named.php @@ -0,0 +1,109 @@ + 'Hello World', + "expectedResult" => " Hello World \n" + ] + ]; + + if (rand(0,1)) { + $arr = [ + [ + "input" => 123, + "expectedResult" => " Hello World \n" + ] + ]; + } + if (rand(0,1)) { + $arr = [ + [ + "input" => false, + "expectedResult" => " Hello World \n" + ] + ]; + } + + return $arr; + } +} + + +class TestWrongOffsetNameArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} + + +class TestWrongTypeInArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} + + +class TestValidArrayShapeIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable + */ + public function data(): iterable + { + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-data.php b/tests/Rules/PHPUnit/data/data-provider-data.php new file mode 100644 index 00000000..d684df79 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-data.php @@ -0,0 +1,519 @@ +moreData(); + + yield [ + 'Hello World', + true, + ]; + } + + /** + * @return array{array{'Hello World', 123}} + */ + private function moreData(): array + { + return [ + [ + 'Hello World', + 123, + ] + ]; + } +} + +class TestValidVariadic extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(string $s): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, string ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", "foo", "bar"], + ["hi", "ho"], + ["nope"] + ]; + } +} + +class TestInvalidVariadic extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, string ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", "foo", "bar"], + [123] + ]; + } +} + + +class TestInvalidVariadic2 extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(string $s, int ...$moreS): void + { + } + + public function aProvider(): iterable + { + return [ + ["hello", "world", 5, "bar"], + [123] + ]; + } +} + +class TestTypedIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable> + */ + public function data(): iterable + { + } +} + +class TestArrayIterator extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $i): void + { + } + + /** @dataProvider aProvider */ + public function testFoo(int $i, string $si): void + { + } + + /** @dataProvider aProvider */ + public function testFooBar(string $s1, string $s2): void + { + } + + public function aProvider(): iterable + { + return new \ArrayIterator([ + [1], + [2, "hello"], + ["no"], + ["no", "yes"], + ]); + } +} + +class TestWrongTypedIterable extends TestCase +{ + /** @dataProvider aProvider */ + public function testBar(int $si): void + { + } + + public function aProvider(): iterable + { + return $this->data(); + } + + /** + * @return iterable> + */ + public function data(): iterable + { + } +} + + +abstract class AbstractBaseTest extends TestCase +{ + + #[DataProvider('aProvider')] + public function testWithAttribute(string $expectedResult, string $input): void + { + } + + static public function aProvider(): array + { + return [ + [ + 'Hello World', + " Hello World \n", + ], + [ + 'Hello World', + 123, + ], + [ + 'Hello World', + false, + ], + ]; + } +} + + +class ConstantArrayUnionTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 123 + ] + ]; + } else { + $arr = [ + [ + 'Hello World', + " Hello World \n" + ] + ]; + } + + return $arr; + } +} + +class ConstantArrayDifferentLengthUnionTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 123 + ] + ]; + } elseif (rand(0,1)) { + $arr = [ + [ + 'Hello World', + 'Hello World', + ] + ]; + } else { + $arr = [ + [ + 'Hello World', + " Hello World \n", + " Too much \n", + ] + ]; + } + + return $arr; + } +} + +class ConstantArrayUnionWithDifferentValueTypeReturnTest extends TestCase +{ + + /** @dataProvider aProvider */ + public function testFoo(string $expectedResult, string $input): void + { + } + + public function aProvider(): array + { + if (rand(0,1)) { + $arr = [ + [ + 'Hellooo', + ' World', + ] + ]; + } else { + $a = rand(0,1) ? 'Hello' : 'World'; + $b = rand(0,1) ? " Hello World \n" : 123; + + $arr = [ + [ + $a, + $b + ] + ]; + } + + return $arr; + } +} diff --git a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php new file mode 100644 index 00000000..60fc7cf4 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php @@ -0,0 +1,32 @@ + Date: Sun, 26 Oct 2025 17:08:45 +0100 Subject: [PATCH 36/50] Fix memory consumption in DataProviderDataRule --- src/Rules/PHPUnit/DataProviderDataRule.php | 23 ++++++++----- .../data/data-provider-trimming-args.php | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index e241053e..45a7fa16 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -84,15 +84,20 @@ public function processNode(Node $node, Scope $scope): array return []; } - $maxNumberOfParameters = $testsWithProvider[0]->getNumberOfParameters(); - if (count($testsWithProvider) > 1) { - foreach ($testsWithProvider as $testMethod) { - if ($testMethod->isVariadic()) { - $maxNumberOfParameters = PHP_INT_MAX; - break; - } + $maxNumberOfParameters = null; + foreach ($testsWithProvider as $testMethod) { + $num = $testMethod->getNumberOfParameters(); + if ($testMethod->isVariadic()) { + $num = PHP_INT_MAX; + } + if ($maxNumberOfParameters === null) { + $maxNumberOfParameters = $num; + continue; + } - $maxNumberOfParameters = max($maxNumberOfParameters, $testMethod->getNumberOfParameters()); + $maxNumberOfParameters = max($maxNumberOfParameters, $num); + if ($num === PHP_INT_MAX) { + break; } } @@ -100,7 +105,7 @@ public function processNode(Node $node, Scope $scope): array $numberOfParameters = $testMethod->getNumberOfParameters(); foreach ($arraysTypes as [$startLine, $arraysType]) { - $args = $this->arrayItemsToArgs($arraysType, $maxNumberOfParameters); + $args = $this->arrayItemsToArgs($arraysType, $numberOfParameters); if ($args === null) { continue; } diff --git a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php index 60fc7cf4..231d3f52 100644 --- a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php +++ b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php @@ -30,3 +30,37 @@ public function testProvide2(int $i): void } } + +class BarTest extends TestCase +{ + + /** + * @return array> + */ + public function getData(): array + { + return []; + } + + public function dataProvide(): array + { + return $this->getData(); + } + + /** + * @dataProvider dataProvide + */ + public function testProvide(string ...$arg): void + { + + } + + /** + * @dataProvider dataProvide + */ + public function testProvide2(string $arg): void + { + + } + +} From 12676a75bce0124ed355e6dd189ccd996cdcbbe8 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 26 Oct 2025 20:46:49 +0100 Subject: [PATCH 37/50] One more test --- .../PHPUnit/DataProviderDataRuleTest.php | 4 +++ .../data/data-provider-trimming-args.php | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index fa3aa9fc..647f830f 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -258,6 +258,10 @@ public function testTrimmingArgs(): void 'Method DataProviderTrimmingArgs\FooTest::testProvide2() invoked with 2 parameters, 1 required.', 13, ], + [ + 'Parameter #6 ...$m of method DataProviderTrimmingArgs\BazTest::testProvide() expects int, string given.', + 90, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php index 231d3f52..dcfcd84d 100644 --- a/tests/Rules/PHPUnit/data/data-provider-trimming-args.php +++ b/tests/Rules/PHPUnit/data/data-provider-trimming-args.php @@ -64,3 +64,31 @@ public function testProvide2(string $arg): void } } + +class BazTest extends TestCase +{ + + /** + * @dataProvider dataProvide + */ + public function testProvide(int $i, int $j, int $k, int ...$m): void + { + + } + + /** + * @dataProvider dataProvide + */ + public function testProvide2(int $i, int $j, int $k, int $m, int $n): void + { + + } + + public function dataProvide(): array + { + return [ + [1, 2, 3, 4, 5, 'foo'], + ]; + } + +} From 53e33fc365ad23de4fbbde1acab40ea36980b64d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 26 Oct 2025 21:26:50 +0100 Subject: [PATCH 38/50] Setup mutation testing --- .github/workflows/build.yml | 68 +++++++++++++++++++++++++++++++++++++ phpstan.neon | 2 ++ phpunit.xml | 3 +- 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 28ab39c6..f4f92b51 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,6 +8,10 @@ on: branches: - "2.0.x" +concurrency: + group: build-${{ github.head_ref || github.run_id }} # will be canceled on subsequent pushes in pull requests but not branches + cancel-in-progress: true + jobs: lint: name: "Lint" @@ -213,3 +217,67 @@ jobs: - name: "PHPStan" run: "make phpstan" + + mutation-testing: + name: "Mutation Testing" + runs-on: "ubuntu-latest" + needs: ["tests", "static-analysis"] + + strategy: + fail-fast: false + matrix: + php-version: + - "8.2" + - "8.3" + - "8.4" + operating-system: [ubuntu-latest] + + steps: + - name: "Checkout" + uses: actions/checkout@v5 + + - name: "Checkout build-infection" + uses: actions/checkout@v5 + with: + repository: "phpstan/build-infection" + path: "build-infection" + ref: "1.x" + + - uses: ./build-infection/.github/actions/setup-php + with: + php-version: "${{ matrix.php-version }}" + extensions: mbstring + + - name: "Install dependencies" + run: "composer install --no-interaction --no-progress" + + - name: "Install build-infection dependencies" + working-directory: "build-infection" + run: "composer install --no-interaction --no-progress" + + - name: "Configure infection" + run: | + php build-infection/bin/infection-config.php \ + > infection.json5 + cat infection.json5 | jq + + - name: "Cache Result cache" + uses: actions/cache@v4 + with: + path: ./tmp + key: "result-cache-v1-${{ matrix.php-version }}-${{ github.run_id }}" + restore-keys: | + result-cache-v1-${{ matrix.php-version }}- + + - name: "Run infection" + run: | + git fetch --depth=1 origin $GITHUB_BASE_REF + infection \ + --git-diff-base=origin/$GITHUB_BASE_REF \ + --git-diff-lines \ + --ignore-msi-with-no-mutations \ + --min-msi=100 \ + --min-covered-msi=100 \ + --log-verbosity=all \ + --debug \ + --logger-text=php://stdout diff --git a/phpstan.neon b/phpstan.neon index 7f64d1c0..a88579c4 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -9,6 +9,8 @@ includes: parameters: reportUnmatchedIgnoredErrors: false + resultCachePath: tmp/resultCache.php + excludePaths: - tests/*/data/* ignoreErrors: diff --git a/phpunit.xml b/phpunit.xml index 420ef740..2ccef9fc 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -10,7 +10,8 @@ beStrictAboutTodoAnnotatedTests="true" failOnRisky="true" failOnWarning="true" - xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xml" + xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" + executionOrder="random" > From 11f3ada682f2e260c5ff422de17fefbbe8ee0496 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 27 Oct 2025 07:07:13 +0100 Subject: [PATCH 39/50] Fix mutation testing on dev branch (#244) --- .github/workflows/build.yml | 20 ++++++++++++++++---- Makefile | 4 ++-- phpstan.neon | 1 + 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f4f92b51..50334926 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -261,8 +261,13 @@ jobs: > infection.json5 cat infection.json5 | jq - - name: "Cache Result cache" - uses: actions/cache@v4 + - name: "Determine default branch" + id: default-branch + run: | + echo "name=$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')" >> $GITHUB_OUTPUT + + - name: "Restore result cache" + uses: actions/cache/restore@v4 with: path: ./tmp key: "result-cache-v1-${{ matrix.php-version }}-${{ github.run_id }}" @@ -271,9 +276,9 @@ jobs: - name: "Run infection" run: | - git fetch --depth=1 origin $GITHUB_BASE_REF + git fetch --depth=1 origin ${{ steps.default-branch.outputs.name }} infection \ - --git-diff-base=origin/$GITHUB_BASE_REF \ + --git-diff-base=origin/${{ steps.default-branch.outputs.name }} \ --git-diff-lines \ --ignore-msi-with-no-mutations \ --min-msi=100 \ @@ -281,3 +286,10 @@ jobs: --log-verbosity=all \ --debug \ --logger-text=php://stdout + + - name: "Save result cache" + uses: actions/cache/save@v4 + if: ${{ !cancelled() }} + with: + path: ./tmp + key: "result-cache-v1-${{ matrix.php-version }}-${{ github.run_id }}" diff --git a/Makefile b/Makefile index 1ee557df..8cec0a5b 100644 --- a/Makefile +++ b/Makefile @@ -26,8 +26,8 @@ cs-fix: .PHONY: phpstan phpstan: - php vendor/bin/phpstan analyse -l 8 -c phpstan.neon src tests + php vendor/bin/phpstan analyse -c phpstan.neon src tests .PHONY: phpstan-generate-baseline phpstan-generate-baseline: - php vendor/bin/phpstan analyse -l 8 -c phpstan.neon src tests -b phpstan-baseline.neon + php vendor/bin/phpstan analyse -c phpstan.neon src tests -b phpstan-baseline.neon diff --git a/phpstan.neon b/phpstan.neon index a88579c4..4441aa97 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,7 @@ includes: - phpstan-baseline.neon parameters: + level: 8 reportUnmatchedIgnoredErrors: false resultCachePath: tmp/resultCache.php From 1fbc321eb8f4ddd719f9f31348edcc54510c5e94 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 27 Oct 2025 07:23:29 +0100 Subject: [PATCH 40/50] move analyse-paths into phpstan.neon --- Makefile | 4 ++-- phpstan.neon | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8cec0a5b..c58ca06e 100644 --- a/Makefile +++ b/Makefile @@ -26,8 +26,8 @@ cs-fix: .PHONY: phpstan phpstan: - php vendor/bin/phpstan analyse -c phpstan.neon src tests + php vendor/bin/phpstan analyse -c phpstan.neon .PHONY: phpstan-generate-baseline phpstan-generate-baseline: - php vendor/bin/phpstan analyse -c phpstan.neon src tests -b phpstan-baseline.neon + php vendor/bin/phpstan analyse -c phpstan.neon -b phpstan-baseline.neon diff --git a/phpstan.neon b/phpstan.neon index 4441aa97..7b35ce80 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,6 +12,10 @@ parameters: resultCachePath: tmp/resultCache.php + paths: + - src + - tests + excludePaths: - tests/*/data/* ignoreErrors: From 5c89d749a03a8efc333b097407ba9f99692f589f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 27 Oct 2025 12:04:54 +0100 Subject: [PATCH 41/50] Ignore missingType.iterableValue for data-providers --- extension.neon | 7 +++ rules.neon | 2 +- src/Rules/PHPUnit/DataProviderDataRule.php | 6 +- src/Rules/PHPUnit/DataProviderHelper.php | 8 +-- src/Rules/PHPUnit/TestMethodsHelper.php | 5 ++ .../DataProviderReturnTypeIgnoreExtension.php | 56 +++++++++++++++++++ ...aProviderReturnTypeIgnoreExtensionTest.php | 38 +++++++++++++ .../data/data-provider-iterable-value.neon | 6 ++ .../data/data-provider-iterable-value.php | 39 +++++++++++++ 9 files changed, 157 insertions(+), 10 deletions(-) create mode 100644 src/Type/PHPUnit/DataProviderReturnTypeIgnoreExtension.php create mode 100644 tests/Type/PHPUnit/DataProviderReturnTypeIgnoreExtensionTest.php create mode 100644 tests/Type/PHPUnit/data/data-provider-iterable-value.neon create mode 100644 tests/Type/PHPUnit/data/data-provider-iterable-value.php diff --git a/extension.neon b/extension.neon index c6ce9bf4..dcf43818 100644 --- a/extension.neon +++ b/extension.neon @@ -1,6 +1,7 @@ parameters: phpunit: convertUnionToIntersectionType: true + checkDataProviderData: %featureToggles.bleedingEdge% additionalConstructors: - PHPUnit\Framework\TestCase::setUp earlyTerminatingMethodCalls: @@ -24,6 +25,7 @@ parameters: parametersSchema: phpunit: structure([ convertUnionToIntersectionType: bool() + checkDataProviderData: bool(), ]) services: @@ -67,6 +69,11 @@ services: arguments: parser: @defaultAnalysisParser + - + class: PHPStan\Type\PHPUnit\DataProviderReturnTypeIgnoreExtension + conditionalTags: PHPStan\PhpDoc\PHPUnit\MockObjectTypeNodeResolverExtension: phpstan.phpDoc.typeNodeResolverExtension: %phpunit.convertUnionToIntersectionType% + PHPStan\Type\PHPUnit\DataProviderReturnTypeIgnoreExtension: + phpstan.ignoreErrorExtension: %phpunit.checkDataProviderData% diff --git a/rules.neon b/rules.neon index 63e10b47..8272f47a 100644 --- a/rules.neon +++ b/rules.neon @@ -14,7 +14,7 @@ conditionalTags: phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] PHPStan\Rules\PHPUnit\DataProviderDataRule: - phpstan.rules.rule: %featureToggles.bleedingEdge% + phpstan.rules.rule: %phpunit.checkDataProviderData% services: - diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 45a7fa16..6f250fee 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -8,7 +8,6 @@ use PHPStan\Rules\Rule; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; -use PHPUnit\Framework\TestCase; use function array_slice; use function count; use function max; @@ -62,10 +61,7 @@ public function processNode(Node $node, Scope $scope): array $method = $scope->getFunction(); $classReflection = $scope->getClassReflection(); - if ( - $classReflection === null - || !$classReflection->is(TestCase::class) - ) { + if ($classReflection === null) { return []; } diff --git a/src/Rules/PHPUnit/DataProviderHelper.php b/src/Rules/PHPUnit/DataProviderHelper.php index 64137c76..b468dfcb 100644 --- a/src/Rules/PHPUnit/DataProviderHelper.php +++ b/src/Rules/PHPUnit/DataProviderHelper.php @@ -53,23 +53,23 @@ public function __construct( } /** - * @param ReflectionMethod|ClassMethod $node + * @param ReflectionMethod|ClassMethod $testMethod * * @return iterable */ public function getDataProviderMethods( Scope $scope, - $node, + $testMethod, ClassReflection $classReflection ): iterable { - yield from $this->yieldDataProviderAnnotations($node, $scope, $classReflection); + yield from $this->yieldDataProviderAnnotations($testMethod, $scope, $classReflection); if (!$this->phpunit10OrNewer) { return; } - yield from $this->yieldDataProviderAttributes($node, $classReflection); + yield from $this->yieldDataProviderAttributes($testMethod, $classReflection); } /** diff --git a/src/Rules/PHPUnit/TestMethodsHelper.php b/src/Rules/PHPUnit/TestMethodsHelper.php index d0984442..67b888e7 100644 --- a/src/Rules/PHPUnit/TestMethodsHelper.php +++ b/src/Rules/PHPUnit/TestMethodsHelper.php @@ -6,6 +6,7 @@ use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\Reflection\ClassReflection; use PHPStan\Type\FileTypeMapper; +use PHPUnit\Framework\TestCase; use ReflectionMethod; use function str_starts_with; use function strtolower; @@ -31,6 +32,10 @@ public function __construct( */ public function getTestMethods(ClassReflection $classReflection, Scope $scope): array { + if (!$classReflection->is(TestCase::class)) { + return []; + } + $testMethods = []; foreach ($classReflection->getNativeReflection()->getMethods() as $reflectionMethod) { if (!$reflectionMethod->isPublic()) { diff --git a/src/Type/PHPUnit/DataProviderReturnTypeIgnoreExtension.php b/src/Type/PHPUnit/DataProviderReturnTypeIgnoreExtension.php new file mode 100644 index 00000000..be6af678 --- /dev/null +++ b/src/Type/PHPUnit/DataProviderReturnTypeIgnoreExtension.php @@ -0,0 +1,56 @@ +testMethodsHelper = $testMethodsHelper; + $this->dataProviderHelper = $dataProviderHelper; + } + + public function shouldIgnore(Error $error, Node $node, Scope $scope): bool + { + if ($error->getIdentifier() !== 'missingType.iterableValue') { + return false; + } + + if (!$scope->isInClass()) { + return false; + } + $classReflection = $scope->getClassReflection(); + + $methodReflection = $scope->getFunction(); + if ($methodReflection === null) { + return false; + } + + $testMethods = $this->testMethodsHelper->getTestMethods($classReflection, $scope); + foreach ($testMethods as $testMethod) { + foreach ($this->dataProviderHelper->getDataProviderMethods($scope, $testMethod, $classReflection) as [, $providerMethodName]) { + if ($providerMethodName === $methodReflection->getName()) { + return true; + } + } + } + + return false; + } + +} diff --git a/tests/Type/PHPUnit/DataProviderReturnTypeIgnoreExtensionTest.php b/tests/Type/PHPUnit/DataProviderReturnTypeIgnoreExtensionTest.php new file mode 100644 index 00000000..fb5b927a --- /dev/null +++ b/tests/Type/PHPUnit/DataProviderReturnTypeIgnoreExtensionTest.php @@ -0,0 +1,38 @@ + + */ +class DataProviderReturnTypeIgnoreExtensionTest extends RuleTestCase { + protected function getRule(): Rule + { + /** @phpstan-ignore phpstanApi.classConstant */ + $rule = self::getContainer()->getByType(MissingMethodReturnTypehintRule::class); + + return $rule; + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/data-provider-iterable-value.php'], [ + [ + 'Method DataProviderIterableValueTest\Foo::notADataProvider() return type has no value type specified in iterable type iterable.', + 32, + 'See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type' + ], + ]); + } + + static public function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/data/data-provider-iterable-value.neon' + ]; + } +} diff --git a/tests/Type/PHPUnit/data/data-provider-iterable-value.neon b/tests/Type/PHPUnit/data/data-provider-iterable-value.neon new file mode 100644 index 00000000..eed12a5b --- /dev/null +++ b/tests/Type/PHPUnit/data/data-provider-iterable-value.neon @@ -0,0 +1,6 @@ +parameters: + phpunit: + checkDataProviderData: true + +includes: + - ../../../../extension.neon diff --git a/tests/Type/PHPUnit/data/data-provider-iterable-value.php b/tests/Type/PHPUnit/data/data-provider-iterable-value.php new file mode 100644 index 00000000..613d3b14 --- /dev/null +++ b/tests/Type/PHPUnit/data/data-provider-iterable-value.php @@ -0,0 +1,39 @@ + Date: Tue, 28 Oct 2025 07:28:21 +0100 Subject: [PATCH 42/50] Assert*Rules: Do cheap checks first (#247) --- src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php | 13 ++++++++----- src/Rules/PHPUnit/AssertRuleHelper.php | 3 --- src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php | 12 +++++++----- src/Rules/PHPUnit/AssertSameNullExpectedRule.php | 12 +++++++----- src/Rules/PHPUnit/AssertSameWithCountRule.php | 13 +++++++------ 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index b03f0830..c4c23887 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -27,17 +27,16 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + if (!$node instanceof Node\Expr\MethodCall && ! $node instanceof Node\Expr\StaticCall) { return []; } - - if ($node->isFirstClassCallable()) { + if (count($node->getArgs()) < 2) { return []; } - - if (count($node->getArgs()) < 2) { + if ($node->isFirstClassCallable()) { return []; } + if ( !$node->name instanceof Node\Identifier || !in_array(strtolower($node->name->name), ['assertequals', 'assertnotequals'], true) @@ -45,6 +44,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + return []; + } + $leftType = TypeCombinator::removeNull($scope->getType($node->getArgs()[0]->value)); $rightType = TypeCombinator::removeNull($scope->getType($node->getArgs()[1]->value)); diff --git a/src/Rules/PHPUnit/AssertRuleHelper.php b/src/Rules/PHPUnit/AssertRuleHelper.php index 442b0655..ecaec91d 100644 --- a/src/Rules/PHPUnit/AssertRuleHelper.php +++ b/src/Rules/PHPUnit/AssertRuleHelper.php @@ -11,9 +11,6 @@ class AssertRuleHelper { - /** - * @phpstan-assert-if-true Node\Expr\MethodCall|Node\Expr\StaticCall $node - */ public static function isMethodOrStaticCallOnAssert(Node $node, Scope $scope): bool { if ($node instanceof Node\Expr\MethodCall) { diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index 9abbd75a..fd76f7c4 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -23,15 +23,13 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + if (!$node instanceof Node\Expr\MethodCall && ! $node instanceof Node\Expr\StaticCall) { return []; } - - if ($node->isFirstClassCallable()) { + if (count($node->getArgs()) < 2) { return []; } - - if (count($node->getArgs()) < 2) { + if ($node->isFirstClassCallable()) { return []; } if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { @@ -43,6 +41,10 @@ public function processNode(Node $node, Scope $scope): array return []; } + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + return []; + } + if ($expectedArgumentValue->name->toLowerString() === 'true') { return [ RuleErrorBuilder::message('You should use assertTrue() instead of assertSame() when expecting "true"')->identifier('phpunit.assertTrue')->build(), diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index 83807ec9..3362f443 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -23,21 +23,23 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + if (!$node instanceof Node\Expr\MethodCall && ! $node instanceof Node\Expr\StaticCall) { return []; } - - if ($node->isFirstClassCallable()) { + if (count($node->getArgs()) < 2) { return []; } - - if (count($node->getArgs()) < 2) { + if ($node->isFirstClassCallable()) { return []; } if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { return []; } + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + return []; + } + $expectedArgumentValue = $node->getArgs()[0]->value; if (!($expectedArgumentValue instanceof ConstFetch)) { return []; diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 9e051cc8..0e6d83cf 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -24,23 +24,24 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + if (!$node instanceof Node\Expr\MethodCall && ! $node instanceof Node\Expr\StaticCall) { + return []; + } + if (count($node->getArgs()) < 2) { return []; } - if ($node->isFirstClassCallable()) { return []; } - - if (count($node->getArgs()) < 2) { + if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { return []; } - if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { + + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; } $right = $node->getArgs()[1]->value; - if ( $right instanceof Node\Expr\FuncCall && $right->name instanceof Node\Name From 758d343c748bf09c834f03a89a3e5168ba250e50 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 29 Oct 2025 07:28:51 +0100 Subject: [PATCH 43/50] Refactor PHPUnitVersionDetector to ease different major version checks --- extension.neon | 8 +- src/Rules/PHPUnit/DataProviderHelper.php | 14 +- .../PHPUnit/DataProviderHelperFactory.php | 8 +- src/Rules/PHPUnit/PHPUnitVersion.php | 41 ++++++ src/Rules/PHPUnit/PHPUnitVersionDetector.php | 15 +-- src/Rules/PHPUnit/TestMethodsHelper.php | 8 +- .../PHPUnit/TestMethodsHelperFactory.php | 28 ---- .../PHPUnit/DataProviderDataRuleTest.php | 13 +- .../DataProviderDeclarationRuleTest.php | 123 ++++++++++++------ 9 files changed, 159 insertions(+), 99 deletions(-) create mode 100644 src/Rules/PHPUnit/PHPUnitVersion.php delete mode 100644 src/Rules/PHPUnit/TestMethodsHelperFactory.php 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'); } From 033f6906f7abc5acbe3c81b642ad9e363e1bb4a0 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 29 Oct 2025 09:37:09 +0100 Subject: [PATCH 44/50] PHPUnit 9.x/10.x does not at all support named arguments from data-providers --- src/Rules/PHPUnit/DataProviderDataRule.php | 9 +- src/Rules/PHPUnit/PHPUnitVersion.php | 8 ++ .../PHPUnit/DataProviderDataRuleTest.php | 126 ++++++++++++++---- .../PHPUnit/data/data-provider-named-args.php | 32 +++++ 4 files changed, 144 insertions(+), 31 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/data-provider-named-args.php diff --git a/src/Rules/PHPUnit/DataProviderDataRule.php b/src/Rules/PHPUnit/DataProviderDataRule.php index 6f250fee..ce994676 100644 --- a/src/Rules/PHPUnit/DataProviderDataRule.php +++ b/src/Rules/PHPUnit/DataProviderDataRule.php @@ -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 @@ -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( diff --git a/src/Rules/PHPUnit/PHPUnitVersion.php b/src/Rules/PHPUnit/PHPUnitVersion.php index 932deedd..b7259a84 100644 --- a/src/Rules/PHPUnit/PHPUnitVersion.php +++ b/src/Rules/PHPUnit/PHPUnitVersion.php @@ -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); + } + } diff --git a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php index e3f80a3b..cca88e7f 100644 --- a/tests/Rules/PHPUnit/DataProviderDataRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDataRuleTest.php @@ -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 */ class DataProviderDataRuleTest extends RuleTestCase { - private int $phpunitVersion; + private ?int $phpunitVersion; protected function getRule(): Rule { $reflectionProvider = $this->createReflectionProvider(); + $phpunitVersion = new PHPUnitVersion($this->phpunitVersion); /** @var list> $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 */ ]; @@ -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); } @@ -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+ + } + 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[] */ diff --git a/tests/Rules/PHPUnit/data/data-provider-named-args.php b/tests/Rules/PHPUnit/data/data-provider-named-args.php new file mode 100644 index 00000000..094ea3b1 --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-named-args.php @@ -0,0 +1,32 @@ +assertTrue(true); + } + + public static function dataProvider(): iterable + { + yield 'even' => [ + 'int' => 50, + 'string' => 'abc', + ]; + + yield 'odd' => [ + 'string' => 'def', + 'int' => 51, + ]; + } +} + From 450bfc08e2e5d00b214110de14168d4ebd210cfb Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 24 Oct 2025 09:12:33 +0200 Subject: [PATCH 45/50] Added assertArrayHasKey() expectations --- tests/Type/PHPUnit/data/assert-function.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Type/PHPUnit/data/assert-function.php b/tests/Type/PHPUnit/data/assert-function.php index 0fabf59e..f7a27d94 100644 --- a/tests/Type/PHPUnit/data/assert-function.php +++ b/tests/Type/PHPUnit/data/assert-function.php @@ -11,7 +11,6 @@ use function PHPUnit\Framework\assertNotCount; use function PHPUnit\Framework\assertEmpty; use function PHPUnit\Framework\assertInstanceOf; -use function PHPUnit\Framework\assertObjectHasAttribute; class Foo { @@ -53,6 +52,12 @@ public function arrayHasStringKey(array $a, \ArrayAccess $b): void assertType("ArrayAccess", $b); } + public function arrayHasExprKey(int $index, array $a): void + { + assertArrayHasKey($index, $a); + assertType("non-empty-array", $a); + } + public function testEmpty($a): void { assertEmpty($a); From 36cb9a621f7e7c3ee667dd7f60be6614a2f1664b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 2 Nov 2025 09:23:04 +0100 Subject: [PATCH 46/50] Make AssertSameBooleanExpectedRule auto-fixable --- .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 8 +---- .../PHPUnit/AssertSameBooleanExpectedRule.php | 33 +++++++++++++++++-- .../AssertSameBooleanExpectedRuleTest.php | 5 +++ .../assert-same-boolean-expected-fixable.php | 21 ++++++++++++ ...rt-same-boolean-expected-fixable.php.fixed | 21 ++++++++++++ 5 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php create mode 100644 tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php.fixed diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index c4c23887..ed6470e2 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -73,13 +73,7 @@ public function processNode(Node $node, Scope $scope): array ), )->identifier('phpunit.assertEquals') ->fixNode($node, static function (CallLike $node) use ($correctName) { - if ($node instanceof Node\Expr\MethodCall) { - $node->name = new Node\Identifier($correctName); - } - - if ($node instanceof Node\Expr\StaticCall) { - $node->name = new Node\Identifier($correctName); - } + $node->name = new Node\Identifier($correctName); return $node; }) diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index fd76f7c4..f185bdf7 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -47,17 +47,46 @@ public function processNode(Node $node, Scope $scope): array if ($expectedArgumentValue->name->toLowerString() === 'true') { return [ - RuleErrorBuilder::message('You should use assertTrue() instead of assertSame() when expecting "true"')->identifier('phpunit.assertTrue')->build(), + RuleErrorBuilder::message('You should use assertTrue() instead of assertSame() when expecting "true"') + ->identifier('phpunit.assertTrue') + ->fixNode($node, static function (CallLike $node) { + $node->name = new Node\Identifier('assertTrue'); + $node->args = self::rewriteArgs($node->args); + + return $node; + }) + ->build(), ]; } if ($expectedArgumentValue->name->toLowerString() === 'false') { return [ - RuleErrorBuilder::message('You should use assertFalse() instead of assertSame() when expecting "false"')->identifier('phpunit.assertFalse')->build(), + RuleErrorBuilder::message('You should use assertFalse() instead of assertSame() when expecting "false"') + ->identifier('phpunit.assertFalse') + ->fixNode($node, static function (CallLike $node) { + $node->name = new Node\Identifier('assertFalse'); + $node->args = self::rewriteArgs($node->args); + + return $node; + }) + ->build(), ]; } return []; } + /** + * @param array $args + * @return list + */ + private static function rewriteArgs(array $args): array + { + $newArgs = []; + for ($i = 1; $i < count($args); $i++) { + $newArgs[] = $args[$i]; + } + return $newArgs; + } + } diff --git a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php index 1fe31df9..6dacd685 100644 --- a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php @@ -42,6 +42,11 @@ public function testRule(): void ]); } + public function testFix(): void + { + $this->fix(__DIR__ . '/data/assert-same-boolean-expected-fixable.php', __DIR__ . '/data/assert-same-boolean-expected-fixable.php.fixed'); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php b/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php new file mode 100644 index 00000000..5d5a3ba4 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php @@ -0,0 +1,21 @@ +assertSame(true, $this->returnBool()); + self::assertSame(false, $this->returnBool()); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php.fixed new file mode 100644 index 00000000..d0bb802a --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-boolean-expected-fixable.php.fixed @@ -0,0 +1,21 @@ +assertTrue($this->returnBool()); + self::assertFalse($this->returnBool()); + } + +} From a4abfc11b1486ad1aff02661c3fd3982e1f904d7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 4 Nov 2025 17:05:44 +0100 Subject: [PATCH 47/50] Use TypeSystem in AssertSameBooleanExpectedRule --- .../data/assert-same-boolean-expected.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php b/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php index dccd2ceb..d6f7f14a 100644 --- a/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php +++ b/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php @@ -75,6 +75,26 @@ public function testNonLowercase(): void \PHPUnit\Framework\Assert::assertSame(False, 'foo'); } + public function testMaybeTrueFalse(): void + { + $a = rand(0, 1) ? true : 'foo'; + \PHPUnit\Framework\Assert::assertSame($a, 'foo'); + $a = rand(0, 1) ? false : 'foo'; + \PHPUnit\Framework\Assert::assertSame($a, 'foo'); + } + + public function testConstMaybeTrueFalse(): void + { + if ( + !defined('MY_TEST_CONST') + ) { + return; + } + if (MY_TEST_CONST !== true && MY_TEST_CONST !== false) { + return; + } + \PHPUnit\Framework\Assert::assertSame(MY_TEST_CONST, 'foo'); + } } const PHPSTAN_PHPUNIT_TRUE = true; From 8532ceb49e02884cbd8ae7da63d62224c8eb44f5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 3 Nov 2025 15:01:09 +0100 Subject: [PATCH 48/50] Make AssertSameNullExpectedRule auto-fixable --- .../PHPUnit/AssertSameNullExpectedRule.php | 23 ++++++++++++++++++- .../AssertSameNullExpectedRuleTest.php | 5 ++++ .../assert-same-null-expected-fixable.php | 22 ++++++++++++++++++ ...ssert-same-null-expected-fixable.php.fixed | 22 ++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php create mode 100644 tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php.fixed diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index 3362f443..f6032efb 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -47,11 +47,32 @@ public function processNode(Node $node, Scope $scope): array if ($expectedArgumentValue->name->toLowerString() === 'null') { return [ - RuleErrorBuilder::message('You should use assertNull() instead of assertSame(null, $actual).')->identifier('phpunit.assertNull')->build(), + RuleErrorBuilder::message('You should use assertNull() instead of assertSame(null, $actual).') + ->identifier('phpunit.assertNull') + ->fixNode($node, static function (CallLike $node) { + $node->name = new Node\Identifier('assertNull'); + $node->args = self::rewriteArgs($node->args); + + return $node; + }) + ->build(), ]; } return []; } + /** + * @param array $args + * @return list + */ + private static function rewriteArgs(array $args): array + { + $newArgs = []; + for ($i = 1; $i < count($args); $i++) { + $newArgs[] = $args[$i]; + } + return $newArgs; + } + } diff --git a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php index 1e802dc0..e29096d9 100644 --- a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php @@ -34,6 +34,11 @@ public function testRule(): void ]); } + public function testFix(): void + { + $this->fix(__DIR__ . '/data/assert-same-null-expected-fixable.php', __DIR__ . '/data/assert-same-null-expected-fixable.php.fixed'); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php b/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php new file mode 100644 index 00000000..f95fadd6 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php @@ -0,0 +1,22 @@ +assertSame(null, 'a'); + + \PHPUnit\Framework\Assert::assertSame($this->returnNull(), 'foo'); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php.fixed new file mode 100644 index 00000000..a3c91042 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-null-expected-fixable.php.fixed @@ -0,0 +1,22 @@ +assertNull('a'); + + \PHPUnit\Framework\Assert::assertSame($this->returnNull(), 'foo'); + } + +} From d935297c98d40c2756fc44e571010a6196239ea7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 11 Nov 2025 07:54:22 +0100 Subject: [PATCH 49/50] Fix "Unexpected input(s) 'extensions', valid inputs are ['php-version', 'php-extensions', 'build-infection-path']" --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 50334926..9299e527 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -246,7 +246,7 @@ jobs: - uses: ./build-infection/.github/actions/setup-php with: php-version: "${{ matrix.php-version }}" - extensions: mbstring + php-extensions: mbstring - name: "Install dependencies" run: "composer install --no-interaction --no-progress" From 2fe9fbeceaf76dd1ebaa7bbbb25e2fb5e59db2fe Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 11 Nov 2025 07:26:10 +0100 Subject: [PATCH 50/50] Fix AssertSameWithCountRule for recursive count() --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 67 ++++++++++++++----- .../PHPUnit/AssertSameWithCountRuleTest.php | 8 +++ .../Rules/PHPUnit/data/assert-same-count.php | 26 ++++++- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 0e6d83cf..2a5a7651 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -8,8 +8,12 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\TrinaryLogic; +use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; use function count; +use const COUNT_NORMAL; /** * @implements Rule @@ -42,11 +46,7 @@ public function processNode(Node $node, Scope $scope): array } $right = $node->getArgs()[1]->value; - if ( - $right instanceof Node\Expr\FuncCall - && $right->name instanceof Node\Name - && $right->name->toLowerString() === 'count' - ) { + if (self::isCountFunctionCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') ->identifier('phpunit.assertCount') @@ -54,24 +54,59 @@ public function processNode(Node $node, Scope $scope): array ]; } + if (self::isCountableMethodCall($right, $scope)) { + return [ + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') + ->identifier('phpunit.assertCount') + ->build(), + ]; + } + + return []; + } + + /** + * @phpstan-assert-if-true Node\Expr\FuncCall $expr + */ + private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool + { + return $expr instanceof Node\Expr\FuncCall + && $expr->name instanceof Node\Name + && $expr->name->toLowerString() === 'count' + && count($expr->getArgs()) >= 1 + && self::isNormalCount($expr, $scope->getType($expr->getArgs()[0]->value), $scope)->yes(); + } + + /** + * @phpstan-assert-if-true Node\Expr\MethodCall $expr + */ + private static function isCountableMethodCall(Node\Expr $expr, Scope $scope): bool + { if ( - $right instanceof Node\Expr\MethodCall - && $right->name instanceof Node\Identifier - && $right->name->toLowerString() === 'count' - && count($right->getArgs()) === 0 + $expr instanceof Node\Expr\MethodCall + && $expr->name instanceof Node\Identifier + && $expr->name->toLowerString() === 'count' + && count($expr->getArgs()) === 0 ) { - $type = $scope->getType($right->var); + $type = $scope->getType($expr->var); if ((new ObjectType(Countable::class))->isSuperTypeOf($type)->yes()) { - return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') - ->identifier('phpunit.assertCount') - ->build(), - ]; + return true; } } - return []; + return false; + } + + private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $countedType, Scope $scope): TrinaryLogic + { + if (count($countFuncCall->getArgs()) === 1) { + $isNormalCount = TrinaryLogic::createYes(); + } else { + $mode = $scope->getType($countFuncCall->getArgs()[1]->value); + $isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($countedType->getIterableValueType()->isArray()->negate()); + } + return $isNormalCount; } } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 32f564d6..dfb940cf 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -31,6 +31,14 @@ public function testRule(): void 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).', 30, ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 40, + ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 45, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/assert-same-count.php b/tests/Rules/PHPUnit/data/assert-same-count.php index 73df333d..bc40eed7 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count.php +++ b/tests/Rules/PHPUnit/data/assert-same-count.php @@ -30,6 +30,30 @@ public function testAssertSameWithCountMethodForCountableVariableIsNotOK() $this->assertSame(5, $foo->bar->count()); } + public function testRecursiveCount($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); // OK + } + + public function testNormalCount($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_NORMAL)); + } + + public function testImplicitNormalCount($mode) + { + $this->assertSame(5, count([1, 2, 3], $mode)); + } + + public function testUnknownCountable($x, $mode) + { + $this->assertSame(5, count($x, $mode)); // OK + } + + public function testUnknownCountMode($x, $mode) + { + $this->assertSame(5, count([1, 2, 3, $x], $mode)); // OK + } } class Bar implements \Countable { @@ -37,4 +61,4 @@ public function count(): int { return 1; } -}; +}