From 72a6721c9b64b3e4c9db55abbc38f790b318267e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 13 Dec 2024 11:48:43 +0100 Subject: [PATCH 01/21] Use more specific node-type --- src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php | 6 +++--- src/Rules/PHPUnit/AssertSameNullExpectedRule.php | 6 +++--- src/Rules/PHPUnit/AssertSameWithCountRule.php | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) 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 From 10880dad5190e788634a62ebb211802669382f3d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 17 Dec 2024 18:22:01 +0100 Subject: [PATCH 02/21] 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 205cbe4..526085b 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 5237f06..3453041 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 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..7aea39a --- /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 0000000..e739ee4 --- /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 0000000..727408d --- /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 03/21] 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 7aea39a..f4fc89c 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 e739ee4..f1d34d0 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 727408d..7f4d80e 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 04/21] 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 526085b..3469379 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 f4fc89c..bd685dd 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 f1d34d0..568b5b6 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 05/21] Update LICENSE --- LICENSE | 1 + 1 file changed, 1 insertion(+) 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 From 0f857bfcea0d73d7dc4b506c40c8c2a5fffc3191 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 18 Mar 2025 10:49:50 +0100 Subject: [PATCH 06/21] 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 3453041..212c518 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 2b8fa1a..7c96296 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 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..aad678e 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 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 []; } From 855b82c4e70c0274ca099585d1dcf026a5485a2d Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 14:57:01 +0100 Subject: [PATCH 07/21] Remove config.platform --- composer.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/composer.json b/composer.json index 212c518..60ba1e7 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 08/21] 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 88543fb..07de108 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 09/21] 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 2d43f8b..4405c4d 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 e1841e0..0b36c2b 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 10/21] 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 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); From 19f8059bdc64a6234c45483f46a3e034c7dd06c2 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:15:12 +0100 Subject: [PATCH 11/21] Do not generate code coverage --- phpunit.xml | 14 -------------- 1 file changed, 14 deletions(-) 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 From eb88670836752d7f4a231512f4da407f24cf458a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 23 Mar 2025 15:23:33 +0100 Subject: [PATCH 12/21] Always install nikic/php-parser v5 --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 60ba1e7..be6b3bd 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 13/21] 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 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/tests/Rules/Methods/CallMethodsRuleTest.php b/tests/Rules/Methods/CallMethodsRuleTest.php new file mode 100644 index 0000000..99d572f --- /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 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'); + } + +} 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 14/21] 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 b1a669a..be6cad0 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 15/21] 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 07de108..65f351a 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 16/21] 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 7c96296..5737945 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 4405c4d..4014017 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 0b36c2b..8c6ebb8 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 17/21] 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 8de21f5..117c992 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 b6f8932..e953d18 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 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 @@ - @@ -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 18/21] 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 e953d18..6c3b0dc 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 a7f7308..284b282 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 0000000..18a4d4a --- /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 19/21] 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 284b282..f7e89c7 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 18a4d4a..1efe6c1 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 20/21] 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 be6cad0..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.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 21/21] 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 be6b3bd..3c07436 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 117c992..2890052 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 aad678e..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; } @@ -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 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/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/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 @@ +