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/sibling_classes_with_same_helper_body.php.inc b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/sibling_classes_with_same_helper_body.php.inc new file mode 100644 index 00000000..7eae4967 --- /dev/null +++ b/rules-tests/PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector/Fixture/sibling_classes_with_same_helper_body.php.inc @@ -0,0 +1,68 @@ +helper(); + } + + private function helper() + { + $value = 1; + } +} + +final class SiblingHelperAsserting extends TestCase +{ + public function testHelper() + { + $this->helper(); + } + + private function helper() + { + $this->assertTrue(true); + } +} + +?> +----- +helper(); + } + + private function helper() + { + $value = 1; + } +} + +final class SiblingHelperAsserting extends TestCase +{ + public function testHelper() + { + $this->helper(); + } + + private function helper() + { + $this->assertTrue(true); + } +} + +?> 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/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 @@ +betterStandardPrinter->prettyPrint([$classMethod])); + // the same body means a different method per class, e.g. "$this->helper()" or "parent::test()" + $cacheHash = md5( + $this->betterStandardPrinter->prettyPrint([$classMethod]) . $this->resolveOwnClassName($classMethod) + ); if (isset($this->containsAssertCallByClassMethod[$cacheHash])) { return $this->containsAssertCallByClassMethod[$cacheHash]; @@ -83,7 +91,7 @@ public function containsAssertCall(ClassMethod $classMethod): bool // A. try "->assert" shallow search first for performance $hasDirectAssertOrMockCall = $this->hasDirectAssertOrMockCall($classMethod); if ($hasDirectAssertOrMockCall) { - $this->containsAssertCallByClassMethod[$cacheHash] = $hasDirectAssertOrMockCall; + $this->containsAssertCallByClassMethod[$cacheHash] = true; return true; } @@ -203,22 +211,64 @@ 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 resolveOwnClassName(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; + } + + // PHPUnit itself has no asserting test methods, no need to parse its huge classes + if ($parentClassReflection->getName() === PHPUnitClassName::TEST_CASE) { + 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(); } }