Introduced an intermediate singleton registry to memoize docblocks

sebastianbergmann · Sep 7, 2019 · c38b65c · c38b65c
1 parent e2c581a
commit c38b65c
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 90 deletions.
diff --git a/src/Annotation/Registry.php b/src/Annotation/Registry.php
@@ -0,0 +1,90 @@
+<?php declare(strict_types=1);
+/*
+ * This file is part of PHPUnit.
+ *
+ * (c) Sebastian Bergmann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+namespace PHPUnit\Annotation;
+
+use PHPUnit\Util\Exception;
+
+/**
+ * Yes, this is a singleton registry. The reason why this can actually be a singleton
+ * without having kittens die is that reflection information (and therefore docblock
+ * information) is static within a single process.
+ *
+ * @internal This class is part of PHPUnit internals, an not intended
+ *           for downstream usage
+ *
+ * @TODO test me (srsly!)
+ */
+final class Registry
+{
+    /** @var self|null */
+    private static $instance;
+
+    /** @var array<string, DocBlock> indexed by class name */
+    private $classDocBlocks = [];
+
+    /** @var array<string, array<string, DocBlock>> indexed by class name and method name */
+    private $methodDocBlocks = [];
+
+    private function __construct()
+    {
+    }
+
+    public static function singleton() : self
+    {
+        return self::$instance
+            ?? self::$instance = new self();
+    }
+
+    /**
+     * @throws Exception
+     * @psalm-param class-string $class
+     */
+    public function forClassName(string $class) : DocBlock
+    {
+        if (\array_key_exists($class, $this->classDocBlocks)) {
+            return $this->classDocBlocks[$class];
+        }
+
+        try {
+            $reflection = new \ReflectionClass($class);
+        } catch (\ReflectionException $e) {
+            throw new Exception(
+                $e->getMessage(),
+                (int) $e->getCode(),
+                $e
+            );
+        }
+
+        return $this->classDocBlocks[$class] = DocBlock::ofClass($reflection);
+    }
+
+    /**
+     * @throws Exception
+     * @psalm-param class-string $className
+     */
+    public function forMethod(string $class, string $method) : DocBlock
+    {
+        if (isset($this->methodDocBlocks[$class][$method])) {
+            return $this->methodDocBlocks[$class][$method];
+        }
+
+        try {
+            $reflection = new \ReflectionMethod($class, $method);
+        } catch (\ReflectionException $e) {
+            throw new Exception(
+                $e->getMessage(),
+                (int) $e->getCode(),
+                $e
+            );
+        }
+
+        return $this->methodDocBlocks[$class][$method] = DocBlock::ofFunction($reflection);
+    }
+}
diff --git a/src/Util/Test.php b/src/Util/Test.php
@@ -9,14 +9,11 @@
  */
 namespace PHPUnit\Util;
 
-use PharIo\Version\VersionConstraintParser;
-use PHPUnit\Annotation\DocBlock;
+use PHPUnit\Annotation\Registry;
 use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\CodeCoverageException;
 use PHPUnit\Framework\InvalidCoversTargetException;
-use PHPUnit\Framework\InvalidDataProviderException;
 use PHPUnit\Framework\SelfDescribing;
-use PHPUnit\Framework\SkippedTestError;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Framework\Warning;
 use PHPUnit\Runner\Version;
@@ -150,30 +147,12 @@ public static function requiresCodeCoverageDataCollection(TestCase $test): bool
      */
     public static function getRequirements(string $className, string $methodName): array
     {
-        try {
-            $reflector = new \ReflectionClass($className);
-        } catch (\ReflectionException $e) {
-            throw new Exception(
-                $e->getMessage(),
-                (int) $e->getCode(),
-                $e
-            );
-        }
-
-        try {
-            $method = $reflector->getMethod($methodName);
-        } catch (\ReflectionException $e) {
-            throw new Exception(
-                $e->getMessage(),
-                (int) $e->getCode(),
-                $e
-            );
-        }
-
         return self::mergeThingsRecursivelyBecausePHPsArrayMergeRecursiveIsUselessBurningGarbage(
-            DocBlock::ofClass($reflector)
+            Registry::singleton()
+                ->forClassName($className)
                 ->requirements(),
-            DocBlock::ofFunction($method)
+            Registry::singleton()
+                ->forMethod($className, $methodName)
                 ->requirements()
         );
     }
@@ -313,17 +292,8 @@ public static function getMissingRequirements(string $className, string $methodN
      */
     public static function getExpectedException(string $className, string $methodName)
     {
-        try {
-            $reflector = new \ReflectionMethod($className, $methodName);
-        } catch (\ReflectionException $e) {
-            throw new Exception(
-                $e->getMessage(),
-                (int) $e->getCode(),
-                $e
-            );
-        }
-
-        return DocBlock::ofFunction($reflector)
+        return Registry::singleton()
+            ->forMethod($className, $methodName)
             ->expectedException();
     }
 
@@ -334,17 +304,8 @@ public static function getExpectedException(string $className, string $methodNam
      */
     public static function getProvidedData(string $className, string $methodName): ?array
     {
-        try {
-            $reflector = new \ReflectionMethod($className, $methodName);
-        } catch (\ReflectionException $e) {
-            throw new Exception(
-                $e->getMessage(),
-                (int) $e->getCode(),
-                $e
-            );
-        }
-
-        return DocBlock::ofFunction($reflector)
+        return Registry::singleton()
+            ->forMethod($className, $methodName)
             ->getProvidedData();
     }
 
@@ -353,54 +314,29 @@ public static function getProvidedData(string $className, string $methodName): ?
      */
     public static function parseTestMethodAnnotations(string $className, ?string $methodName = ''): array
     {
-        if (!isset(self::$annotationCache[$className])) {
-            try {
-                $class = new \ReflectionClass($className);
-            } catch (\ReflectionException $e) {
-                throw new Exception(
-                    $e->getMessage(),
-                    (int) $e->getCode(),
-                    $e
-                );
-            }
-
-            self::$annotationCache[$className] = DocBlock::ofClass($class)
-                ->parseSymbolAnnotations();
-        }
-
-        $cacheKey = $className . '::' . $methodName;
-
-        if ($methodName !== null && !isset(self::$annotationCache[$cacheKey])) {
-            self::$annotationCache[$cacheKey] = [];
-
-            try {
-                $method                           = new \ReflectionMethod($className, $methodName);
-                self::$annotationCache[$cacheKey] = DocBlock::ofFunction($method)
-                    ->parseSymbolAnnotations();
-            } catch (\ReflectionException $e) {
-                self::$annotationCache[$cacheKey] = [];
-            }
+        if ($methodName === null) {
+            return [
+                'class' => Registry::singleton()
+                    ->forClassName($className)
+                    ->parseSymbolAnnotations(),
+                'method' => null,
+            ];
         }
 
         return [
-            'class'  => self::$annotationCache[$className],
-            'method' => $methodName !== null ? self::$annotationCache[$cacheKey] : [],
+            'class' => Registry::singleton()
+                ->forClassName($className)
+                ->parseSymbolAnnotations(),
+            'method' => Registry::singleton()
+                ->forMethod($className, $methodName)
+                ->parseSymbolAnnotations(),
         ];
     }
 
     public static function getInlineAnnotations(string $className, string $methodName): array
     {
-        try {
-            $method = new \ReflectionMethod($className, $methodName);
-        } catch (\ReflectionException $e) {
-            throw new Exception(
-                $e->getMessage(),
-                (int) $e->getCode(),
-                $e
-            );
-        }
-
-        return DocBlock::ofFunction($method)
+        return Registry::singleton()
+            ->forMethod($className, $methodName)
             ->getInlineAnnotations();
     }
 
@@ -550,7 +486,8 @@ public static function getHookMethods(string $className): array
                         continue;
                     }
 
-                    $docBlock = DocBlock::ofFunction($method);
+                    $docBlock = Registry::singleton()
+                        ->forMethod($className, $method->getName());
 
                     if ($method->isStatic()) {
                         if ($docBlock->isHookToBeExecutedBeforeClass()) {
@@ -591,7 +528,12 @@ public static function isTestMethod(\ReflectionMethod $method): bool
 
         return \array_key_exists(
             'test',
-            DocBlock::ofFunction($method)
+            Registry::singleton()
+                ->forMethod(
+                    $method->getDeclaringClass()
+                        ->getName(),
+                    $method->getName()
+                )
                 ->parseSymbolAnnotations()
         );
     }