diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 88543fb..65f351a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -97,6 +97,30 @@ jobs: dependencies: - "lowest" - "highest" + phpunit-version: + - "^9.5" + - "^10.5" + - "^11.5" + - "^12.0.9" + 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" + - 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" @@ -108,6 +132,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 +163,30 @@ jobs: dependencies: - "lowest" - "highest" + phpunit-version: + - "^9.5" + - "^10.5" + - "^11.5" + - "^12.0.9" + 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" + - 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" @@ -149,6 +200,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" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b1a669a..b8c96d4 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.6.2 with: myToken: ${{ secrets.PHPSTAN_BOT_TOKEN }} diff --git a/LICENSE b/LICENSE index d005374..52fba1e 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 diff --git a/README.md b/README.md index 205cbe4..3469379 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,8 @@ 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) +* Check that you are not using `assertNotEquals()` with same types (`assertNotSame()` should be used) ## How to document mock objects in phpDocs? diff --git a/composer.json b/composer.json index 5237f06..3c07436 100644 --- a/composer.json +++ b/composer.json @@ -7,20 +7,19 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.0" + "phpstan/phpstan": "^2.1.18" }, "conflict": { "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", "phpunit/phpunit": "^9.6" }, "config": { - "platform": { - "php": "7.4.6" - }, "sort-packages": true }, "extra": { diff --git a/extension.neon b/extension.neon index 8de21f5..2890052 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 - @@ -63,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/phpstan-baseline.neon b/phpstan-baseline.neon index 53fb96b..f56b3cf 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/phpstan.neon b/phpstan.neon index 2b8fa1a..5737945 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,9 +2,15 @@ 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 parameters: excludePaths: - tests/*/data/* + ignoreErrors: + - + message: '#^Attribute class PHPUnit\\Framework\\Attributes\\DataProvider does not exist\.$#' + identifier: attribute.notFound + reportUnmatched: false diff --git a/phpunit.xml b/phpunit.xml index 8f71615..420ef74 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -12,20 +12,6 @@ failOnWarning="true" xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xml" > - - - ./src - - - - - - - tests diff --git a/rules.neon b/rules.neon index 023e11c..84b7149 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 0000000..bd685dd --- /dev/null +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -0,0 +1,73 @@ + + */ +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 + || !in_array(strtolower($node->name->name), ['assertequals', 'assertnotequals'], true) + ) { + 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( + 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(), + ]; + } + + return []; + } + +} diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index 969f7b3..308f514 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -3,22 +3,22 @@ namespace PHPStan\Rules\PHPUnit; use PhpParser\Node; +use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\ConstFetch; -use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function count; /** - * @implements Rule + * @implements Rule */ class AssertSameBooleanExpectedRule implements Rule { public function getNodeType(): string { - return NodeAbstract::class; + return CallLike::class; } public function processNode(Node $node, Scope $scope): array diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index a2d0cfa..363fa57 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -3,22 +3,22 @@ namespace PHPStan\Rules\PHPUnit; use PhpParser\Node; +use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\ConstFetch; -use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use function count; /** - * @implements Rule + * @implements Rule */ class AssertSameNullExpectedRule implements Rule { public function getNodeType(): string { - return NodeAbstract::class; + return CallLike::class; } public function processNode(Node $node, Scope $scope): array diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 38fdf9a..3c3ada4 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -4,7 +4,7 @@ use Countable; use PhpParser\Node; -use PhpParser\NodeAbstract; +use PhpParser\Node\Expr\CallLike; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -12,14 +12,14 @@ use function count; /** - * @implements Rule + * @implements Rule */ class AssertSameWithCountRule implements Rule { public function getNodeType(): string { - return NodeAbstract::class; + return CallLike::class; } public function processNode(Node $node, Scope $scope): array diff --git a/src/Rules/PHPUnit/ClassCoversExistsRule.php b/src/Rules/PHPUnit/ClassCoversExistsRule.php index f8d831c..a36317e 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 4bfdc41..dd328f8 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 cc44eeb..1983493 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 ad354fa..338d716 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; } @@ -76,7 +83,7 @@ public function getDataProviderMethods( } $dataProviderMethod = $this->parseDataProviderAnnotationValue($scope, $dataProviderValue); - $dataProviderMethod[] = $node->getLine(); + $dataProviderMethod[] = $node->getStartLine(); yield $dataProviderValue => $dataProviderMethod; } @@ -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; @@ -257,7 +279,7 @@ private function parseDataProviderExternalAttribute(Attribute $attribute): ?arra sprintf('%s::%s', $className, $methodNameArg->value) => [ $dataProviderClassReflection, $methodNameArg->value, - $attribute->getLine(), + $attribute->getStartLine(), ], ]; } @@ -279,7 +301,7 @@ private function parseDataProviderAttribute(Attribute $attribute, ClassReflectio $methodNameArg->value => [ $classReflection, $methodNameArg->value, - $attribute->getLine(), + $attribute->getStartLine(), ], ]; } diff --git a/src/Rules/PHPUnit/DataProviderHelperFactory.php b/src/Rules/PHPUnit/DataProviderHelperFactory.php index 9b965e2..a0768c8 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/src/Rules/PHPUnit/MockMethodCallRule.php b/src/Rules/PHPUnit/MockMethodCallRule.php index b6f8932..6c3b0dc 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,60 @@ 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($scope, $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($scope, $varType, $method); + if ($error === null) { + continue; + } + + $errors[] = $error; + } + + return $errors; + } + + 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) + ) { + $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/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php b/src/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRule.php index c8fd3a1..a2fc39f 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 a44fc53..906e60b 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 4035dcc..bfd3169 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 []; } diff --git a/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php b/src/Type/PHPUnit/InvocationMockerDynamicReturnTypeExtension.php deleted file mode 100644 index 44764f6..0000000 --- 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 626f168..0000000 --- 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 c58719f..0000000 --- a/stubs/InvocationMocker.stub +++ /dev/null @@ -1,13 +0,0 @@ - + */ +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 0000000..d07ca14 --- /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'); + } + +} diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php new file mode 100644 index 0000000..568b5b6 --- /dev/null +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -0,0 +1,42 @@ + + */ +final class AssertEqualsIsDiscouragedRuleTest extends RuleTestCase +{ + + 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_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], + ]); + } + + protected function getRule(): Rule + { + return new AssertEqualsIsDiscouragedRule(); + } + +} diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 18cc12b..cbe40bf 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/MockMethodCallRuleTest.php b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php index c9c33e6..f7e89c7 100644 --- a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php +++ b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php @@ -4,7 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; -use function interface_exists; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -28,18 +28,23 @@ 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); } + public function testBug227(): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped('Test requires PHP 8.0.'); + } + $this->analyse([__DIR__ . '/data/bug-227.php'], []); + } + /** * @return string[] */ 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 0000000..7f4d80e --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php @@ -0,0 +1,43 @@ +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()); + + $this->assertNotEquals($string, $string); + $this->assertNotEquals($integer, $integer); + $this->assertNotEquals($boolean, $boolean); + $this->assertNotSame(5, $integer); + static::assertNotSame(5, $integer); + } +} diff --git a/tests/Rules/PHPUnit/data/bug-227.php b/tests/Rules/PHPUnit/data/bug-227.php new file mode 100644 index 0000000..1efe6c1 --- /dev/null +++ b/tests/Rules/PHPUnit/data/bug-227.php @@ -0,0 +1,42 @@ += 8.0 + +namespace Bug227; + +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use stdClass; + +class Foo +{ + + public function addCacheTags(array $tags) + { + + } + + public function getLanguage(): stdClass + { + + } + +} + +class SomeTest extends TestCase +{ + + protected MockObject|Foo $tsfe; + + protected function setUp(): void + { + $this->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'); + } +} 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 0000000..1f8005a --- /dev/null +++ b/tests/Rules/PHPUnit/data/data-provider-static-fix.php @@ -0,0 +1,21 @@ +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 []; @@ -26,6 +27,7 @@ public 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 e1841e0..8c6ebb8 100644 --- a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php @@ -3,20 +3,22 @@ namespace PHPStan\Type\PHPUnit; use PHPStan\Testing\TypeInferenceTestCase; +use PHPUnit\Framework\Attributes\DataProvider; 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'); } /** * @dataProvider dataFileAsserts * @param mixed ...$args */ + #[DataProvider('dataFileAsserts')] public function testFileAsserts( string $assertType, string $file, diff --git a/tests/Type/PHPUnit/data/assert-function.php b/tests/Type/PHPUnit/data/assert-function.php index 851e07b..0fabf59 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);