diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/non_asserting_set_up.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/non_asserting_set_up.php.inc new file mode 100644 index 00000000..4273e653 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/non_asserting_set_up.php.inc @@ -0,0 +1,42 @@ +value = 1; + } + + public function testSomething() + { + $value = 5; + } +} + +?> +----- +value = 1; + } + + #[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions] + public function testSomething() + { + $value = 5; + } +} + +?> diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/parent_test_call_without_assert.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/parent_test_call_without_assert.php.inc new file mode 100644 index 00000000..1235c78c --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/parent_test_call_without_assert.php.inc @@ -0,0 +1,32 @@ + +----- + diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_after_annotation.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_after_annotation.php.inc new file mode 100644 index 00000000..6219c981 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_after_annotation.php.inc @@ -0,0 +1,21 @@ +assertTrue(true); + } + + public function testSomething() + { + $value = 5; + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_before_attribute.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_before_attribute.php.inc new file mode 100644 index 00000000..2f549b94 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_before_attribute.php.inc @@ -0,0 +1,20 @@ +assertTrue(true); + } + + public function testSomething() + { + $value = 5; + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_own_post_conditions.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_own_post_conditions.php.inc new file mode 100644 index 00000000..fe6a9772 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_own_post_conditions.php.inc @@ -0,0 +1,18 @@ +assertTrue(true); + } + + public function testSomething() + { + $value = 5; + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_set_up.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_set_up.php.inc new file mode 100644 index 00000000..f2e13004 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_set_up.php.inc @@ -0,0 +1,18 @@ +assertTrue(true); + } + + public function testSomething() + { + $value = 5; + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_tear_down_in_parent.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_tear_down_in_parent.php.inc new file mode 100644 index 00000000..6a42306f --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_asserting_tear_down_in_parent.php.inc @@ -0,0 +1,13 @@ +expectUserDeprecationMessage('Since foo 1.0: Bar is deprecated.'); + + @trigger_error('Since foo 1.0: Bar is deprecated.', E_USER_DEPRECATED); + } + + public function testExpectUserDeprecationMessageMatches() + { + $this->expectUserDeprecationMessageMatches('#^Since foo 1\.0#'); + + @trigger_error('Since foo 1.0: Bar is deprecated.', E_USER_DEPRECATED); + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_parent_test_call_with_assert.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_parent_test_call_with_assert.php.inc new file mode 100644 index 00000000..db1ec5c6 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/skip_parent_test_call_with_assert.php.inc @@ -0,0 +1,13 @@ +assertTrue(true); + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingPostConditions.php b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingPostConditions.php new file mode 100644 index 00000000..9cb7da7a --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingPostConditions.php @@ -0,0 +1,15 @@ +assertTrue(true); + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingTearDown.php b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingTearDown.php new file mode 100644 index 00000000..caf91155 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithAssertingTearDown.php @@ -0,0 +1,15 @@ +assertTrue(true); + } +} diff --git a/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithNonAssertingMethod.php b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithNonAssertingMethod.php new file mode 100644 index 00000000..b405a118 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Source/AbstractClassWithNonAssertingMethod.php @@ -0,0 +1,15 @@ +lifecycleMethodAssertAnalyzer->hasAssertingLifecycleMethod($classMethod)) { + return true; + } + return $this->mockedVariableAnalyzer->containsMockAsUsedVariable($classMethod); } diff --git a/src/Enum/PHPUnitAttribute.php b/src/Enum/PHPUnitAttribute.php index cca04295..7a93fece 100644 --- a/src/Enum/PHPUnitAttribute.php +++ b/src/Enum/PHPUnitAttribute.php @@ -33,4 +33,12 @@ final class PHPUnitAttribute public const string ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS = 'PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations'; public const string DATA_PROVIDER = 'PHPUnit\Framework\Attributes\DataProvider'; + + public const string BEFORE = 'PHPUnit\Framework\Attributes\Before'; + + public const string AFTER = 'PHPUnit\Framework\Attributes\After'; + + public const string PRE_CONDITION = 'PHPUnit\Framework\Attributes\PreCondition'; + + public const string POST_CONDITION = 'PHPUnit\Framework\Attributes\PostCondition'; } diff --git a/src/NodeAnalyzer/AssertCallAnalyzer.php b/src/NodeAnalyzer/AssertCallAnalyzer.php index 02db1f97..218d7ce6 100644 --- a/src/NodeAnalyzer/AssertCallAnalyzer.php +++ b/src/NodeAnalyzer/AssertCallAnalyzer.php @@ -10,8 +10,12 @@ use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; use PhpParser\Node\Stmt\ClassMethod; +use PHPStan\Analyser\Scope; +use PHPStan\Reflection\ClassReflection; use PHPStan\Type\TypeWithClassName; +use Rector\Enum\ObjectReference; use Rector\NodeNameResolver\NodeNameResolver; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\NodeTypeResolver; use Rector\PhpParser\AstResolver; use Rector\PhpParser\Node\BetterNodeFinder; @@ -36,6 +40,7 @@ final class AssertCallAnalyzer 'expectException', 'setExpectedException', 'expectOutput', + 'expectUserDeprecationMessage', 'should', ]; @@ -73,7 +78,12 @@ public function containsAssertCall(ClassMethod $classMethod): bool return false; } - $cacheHash = md5($this->betterStandardPrinter->prettyPrint([$classMethod])); + // the same body can behave differently per class, e.g. "parent::test()" + $cacheHash = md5( + $this->betterStandardPrinter->prettyPrint([$classMethod]) . $this->resolveClassMethodClassName( + $classMethod + ) + ); if (isset($this->containsAssertCallByClassMethod[$cacheHash])) { return $this->containsAssertCallByClassMethod[$cacheHash]; @@ -202,22 +212,59 @@ private function hasNestedAssertCall(ClassMethod $classMethod): bool private function resolveClassMethodFromCall(StaticCall|MethodCall $call): ?ClassMethod { - if ($call instanceof MethodCall) { - $objectType = $this->nodeTypeResolver->getType($call->var); - } else { - // StaticCall - $objectType = $this->nodeTypeResolver->getType($call->class); + $methodName = $this->nodeNameResolver->getName($call->name); + if ($methodName === null) { + return null; + } + + $className = $this->resolveClassName($call); + if ($className === null) { + return null; + } + + return $this->astResolver->resolveClassMethod($className, $methodName); + } + + private function resolveClassMethodClassName(ClassMethod $classMethod): string + { + $classReflection = $this->resolveClassReflection($classMethod); + if (! $classReflection instanceof ClassReflection) { + return ''; + } + + return $classReflection->getName(); + } + + private function resolveClassName(StaticCall|MethodCall $call): ?string + { + if ($call instanceof StaticCall && $this->nodeNameResolver->isName($call->class, ObjectReference::PARENT)) { + // "parent" resolves to a union of all ancestors in type resolver, so use the direct parent class instead + $parentClassReflection = $this->resolveClassReflection($call)?->getParentClass(); + if (! $parentClassReflection instanceof ClassReflection) { + return null; + } + + return $parentClassReflection->getName(); } + $objectType = $call instanceof MethodCall + ? $this->nodeTypeResolver->getType($call->var) + : $this->nodeTypeResolver->getType($call->class); + if (! $objectType instanceof TypeWithClassName) { return null; } - $methodName = $this->nodeNameResolver->getName($call->name); - if ($methodName === null) { + return $objectType->getClassName(); + } + + private function resolveClassReflection(Node $node): ?ClassReflection + { + $scope = $node->getAttribute(AttributeKey::SCOPE); + if (! $scope instanceof Scope) { return null; } - return $this->astResolver->resolveClassMethod($objectType->getClassName(), $methodName); + return $scope->getClassReflection(); } } diff --git a/src/NodeAnalyzer/LifecycleMethodAssertAnalyzer.php b/src/NodeAnalyzer/LifecycleMethodAssertAnalyzer.php new file mode 100644 index 00000000..7329e3fd --- /dev/null +++ b/src/NodeAnalyzer/LifecycleMethodAssertAnalyzer.php @@ -0,0 +1,144 @@ + + */ + private array $assertingByClassName = []; + + public function __construct( + private readonly ReflectionResolver $reflectionResolver, + private readonly AstResolver $astResolver, + private readonly AssertCallAnalyzer $assertCallAnalyzer, + private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer, + private readonly PhpDocInfoFactory $phpDocInfoFactory, + ) { + } + + public function hasAssertingLifecycleMethod(ClassMethod $classMethod): bool + { + $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); + if (! $classReflection instanceof ClassReflection) { + return false; + } + + $className = $classReflection->getName(); + $this->assertingByClassName[$className] ??= $this->resolveHasAssertingLifecycleMethod($classReflection); + + return $this->assertingByClassName[$className]; + } + + private function resolveHasAssertingLifecycleMethod(ClassReflection $classReflection): bool + { + // only the most derived override of a hook is called by PHPUnit, parents are reached via "parent::" calls + $seenHookMethodNames = []; + + for ( + $currentClassReflection = $classReflection; + $currentClassReflection instanceof ClassReflection; + $currentClassReflection = $currentClassReflection->getParentClass() + ) { + // do not analyze PHPUnit itself + if ($currentClassReflection->getName() === PHPUnitClassName::TEST_CASE) { + return false; + } + + $class = $this->astResolver->resolveClassFromClassReflection($currentClassReflection); + if (! $class instanceof Class_) { + continue; + } + + $hookMethodNamesOfClass = []; + + foreach ($class->getMethods() as $hookCandidate) { + $hookMethodName = $this->resolveHookMethodName($hookCandidate); + if ($hookMethodName !== null) { + $hookMethodNamesOfClass[] = $hookMethodName; + } + + if (! $this->isRelevantHook($hookCandidate, $hookMethodName, $seenHookMethodNames)) { + continue; + } + + $this->assertCallAnalyzer->resetNesting(); + if ($this->assertCallAnalyzer->containsAssertCall($hookCandidate)) { + return true; + } + } + + $seenHookMethodNames = array_merge($seenHookMethodNames, $hookMethodNamesOfClass); + } + + return false; + } + + private function resolveHookMethodName(ClassMethod $classMethod): ?string + { + $methodName = $classMethod->name->toString(); + + foreach (self::HOOK_METHOD_NAMES as $hookMethodName) { + if (strcasecmp($hookMethodName, $methodName) === 0) { + return $hookMethodName; + } + } + + return null; + } + + /** + * @param string[] $seenHookMethodNames + */ + private function isRelevantHook(ClassMethod $classMethod, ?string $hookMethodName, array $seenHookMethodNames): bool + { + if ($hookMethodName !== null && ! in_array($hookMethodName, $seenHookMethodNames, true)) { + return true; + } + + // all methods marked as hooks run, also when overridden + if ($this->phpAttributeAnalyzer->hasPhpAttributes($classMethod, self::HOOK_ATTRIBUTES)) { + return true; + } + + return $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod) + ->hasByNames(self::HOOK_ANNOTATIONS); + } +}