diff --git a/.github/workflows/code_analysis.yaml b/.github/workflows/code_analysis.yaml index 5a7c5c1e..d8b630f1 100644 --- a/.github/workflows/code_analysis.yaml +++ b/.github/workflows/code_analysis.yaml @@ -44,6 +44,10 @@ jobs: name: 'Bench abstract classes' run: php bench/does_not_perform_assertion_abstract_class_bench.php + - + name: 'Bench cache' + run: php bench/does_not_perform_assertion_cache_bench.php + name: ${{ matrix.actions.name }} runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/bench/does_not_perform_assertion_cache_bench.php b/bench/does_not_perform_assertion_cache_bench.php new file mode 100644 index 00000000..0e85cf98 --- /dev/null +++ b/bench/does_not_perform_assertion_cache_bench.php @@ -0,0 +1,131 @@ + \$item + {$statement}, \$items);\n"; + } + + $methods .= " protected function helper{$helper}(array \$items): array\n {\n{$statements} return \$items;\n }\n\n"; + } + + $writeClass( + $parentClassName, + "abstract class {$parentClassName} extends \PHPUnit\Framework\TestCase\n{\n{$methods}}\n" + ); + + for ($test = 0; $test < TESTS_PER_PARENT; ++$test) { + $className = sprintf('CacheParent%dTest%d', $parent, $test); + + $methods = " public function testRestart(): void\n {\n parent::setUp();\n }\n\n"; + for ($testMethod = 0; $testMethod < TEST_METHODS; ++$testMethod) { + $methods .= " public function testSomething{$testMethod}(): void\n {\n \$value = \$this->helper{$testMethod}([1, 2, 3]);\n }\n\n"; + } + + $testFilePaths[] = $writeClass( + $className, + "final class {$className} extends {$parentClassName}\n{\n{$methods}}\n" + ); + } +} + +$rectorConfig = (new LazyContainerFactory())->create(); +$rectorConfig->boot(); + +/** @var TestingParser $testingParser */ +$testingParser = $rectorConfig->make(TestingParser::class); + +/** @var BetterNodeFinder $betterNodeFinder */ +$betterNodeFinder = $rectorConfig->make(BetterNodeFinder::class); + +$rector = $rectorConfig->make(AddDoesNotPerformAssertionToNonAssertingTestRector::class); + +// parse test files up front, parent files are left for the rule to resolve +$classMethods = []; +foreach ($testFilePaths as $testFilePath) { + $stmts = $testingParser->parseFileToDecoratedNodes($testFilePath); + $classMethods = [...$classMethods, ...$betterNodeFinder->findInstanceOf($stmts, ClassMethod::class)]; +} + +$cpuSeconds = static function (): float { + $usage = getrusage(); + return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000 + + $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000; +}; + +$wallStart = microtime(true); +$cpuStart = $cpuSeconds(); + +$changedCount = 0; +foreach ($classMethods as $classMethod) { + if ($rector->refactor($classMethod) instanceof ClassMethod) { + ++$changedCount; + } +} + +$cpu = $cpuSeconds() - $cpuStart; +$wall = microtime(true) - $wallStart; + +array_map('unlink', glob($directory . '/*.php')); +@rmdir($directory); + +printf('test classes: %d, test methods: %d, changed: %d%s', count($testFilePaths), count($classMethods), $changedCount, PHP_EOL); +printf('CPU: %.4f s (%.3f ms/method)%s', $cpu, $cpu / count($classMethods) * 1000, PHP_EOL); +printf('wall: %.4f s%s', $wall, PHP_EOL); + +if ($cpu > MAX_CPU_SECONDS) { + printf('CPU %.4f s is over the %.1f s limit%s', $cpu, MAX_CPU_SECONDS, PHP_EOL); + exit(1); +} diff --git a/src/NodeAnalyzer/AssertCallAnalyzer.php b/src/NodeAnalyzer/AssertCallAnalyzer.php index 678321ad..54bbafd2 100644 --- a/src/NodeAnalyzer/AssertCallAnalyzer.php +++ b/src/NodeAnalyzer/AssertCallAnalyzer.php @@ -19,7 +19,6 @@ use Rector\NodeTypeResolver\NodeTypeResolver; use Rector\PhpParser\AstResolver; use Rector\PhpParser\Node\BetterNodeFinder; -use Rector\PhpParser\Printer\BetterStandardPrinter; use Rector\PHPUnit\Enum\PHPUnitClassName; use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType; @@ -27,6 +26,8 @@ final class AssertCallAnalyzer { private const int MAX_NESTED_METHOD_CALL_LEVEL = 5; + private const string CONTAINS_ASSERT_CALL_ATTRIBUTE = 'contains_assert_call'; + /** * @see https://docs.phpunit.de/en/12.4/assertions.html */ @@ -45,11 +46,6 @@ final class AssertCallAnalyzer 'should', ]; - /** - * @var array - */ - private array $containsAssertCallByClassMethod = []; - /** * This should prevent segfaults while going too deep into to parsed code. Without it, it might end-up with segfault */ @@ -57,7 +53,6 @@ final class AssertCallAnalyzer public function __construct( private readonly AstResolver $astResolver, - private readonly BetterStandardPrinter $betterStandardPrinter, private readonly BetterNodeFinder $betterNodeFinder, private readonly NodeNameResolver $nodeNameResolver, private readonly NodeTypeResolver $nodeTypeResolver, @@ -79,25 +74,22 @@ public function containsAssertCall(ClassMethod $classMethod): bool return false; } - // 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]; + // cached per node, as the same body means a different method per class, e.g. "$this->helper()" + $containsAssertCall = $classMethod->getAttribute(self::CONTAINS_ASSERT_CALL_ATTRIBUTE); + if (is_bool($containsAssertCall)) { + return $containsAssertCall; } // A. try "->assert" shallow search first for performance $hasDirectAssertOrMockCall = $this->hasDirectAssertOrMockCall($classMethod); if ($hasDirectAssertOrMockCall) { - $this->containsAssertCallByClassMethod[$cacheHash] = true; + $classMethod->setAttribute(self::CONTAINS_ASSERT_CALL_ATTRIBUTE, true); return true; } // B. look for nested calls $hasNestedAssertOrMockCall = $this->hasNestedAssertCall($classMethod); - $this->containsAssertCallByClassMethod[$cacheHash] = $hasNestedAssertOrMockCall; + $classMethod->setAttribute(self::CONTAINS_ASSERT_CALL_ATTRIBUTE, $hasNestedAssertOrMockCall); return $hasNestedAssertOrMockCall; } finally { @@ -216,7 +208,7 @@ private function resolveClassMethodFromCall(StaticCall|MethodCall $call): ?Class return null; } - $className = $this->resolveClassName($call); + $className = $this->resolveClassName($call, $methodName); if ($className === null) { return null; } @@ -224,17 +216,7 @@ private function resolveClassMethodFromCall(StaticCall|MethodCall $call): ?Class 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 + private function resolveClassName(StaticCall|MethodCall $call, string $methodName): ?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 @@ -243,12 +225,21 @@ private function resolveClassName(StaticCall|MethodCall $call): ?string return null; } + if (! $parentClassReflection->hasNativeMethod($methodName)) { + return null; + } + + // the method can be inherited from any ancestor + $declaringClassName = $parentClassReflection->getNativeMethod($methodName) + ->getDeclaringClass() + ->getName(); + // PHPUnit itself has no asserting test methods, no need to parse its huge classes - if ($parentClassReflection->getName() === PHPUnitClassName::TEST_CASE) { + if ($declaringClassName === PHPUnitClassName::TEST_CASE) { return null; } - return $parentClassReflection->getName(); + return $declaringClassName; } $objectType = $call instanceof MethodCall @@ -262,9 +253,9 @@ private function resolveClassName(StaticCall|MethodCall $call): ?string return $objectType->getClassName(); } - private function resolveClassReflection(Node $node): ?ClassReflection + private function resolveClassReflection(StaticCall $staticCall): ?ClassReflection { - $scope = $node->getAttribute(AttributeKey::SCOPE); + $scope = $staticCall->getAttribute(AttributeKey::SCOPE); if (! $scope instanceof Scope) { return null; }