Decouple AnnotationDriver from doctrine/annotations

UPGRADE-2.3.md

@@ -2,3 +2,5 @@ UPGRADE FROM 2.2 to 2.3
 =======================
 
 * Deprecated using short namespace alias syntax in favor of ::class syntax.
+* Deprecated passing an annotation reader to `AnnotationDriver::__construct()`.
+* Deprecated not overriding `AnnotationDriver::isTransient()` in a child class.

lib/Doctrine/Persistence/Mapping/Driver/AnnotationDriver.php

@@ -3,8 +3,10 @@
 namespace Doctrine\Persistence\Mapping\Driver;
 
 use Doctrine\Common\Annotations\Reader;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\Persistence\Mapping\MappingException;
 use FilesystemIterator;
+use LogicException;
 use RecursiveDirectoryIterator;
 use RecursiveIteratorIterator;
 use RecursiveRegexIterator;
@@ -14,13 +16,17 @@
 use function array_merge;
 use function array_unique;
 use function assert;
+use function func_get_arg;
+use function func_num_args;
 use function get_class;
 use function get_declared_classes;
 use function in_array;
 use function is_dir;
+use function is_object;
 use function preg_match;
 use function preg_quote;
 use function realpath;
+use function sprintf;
 use function str_replace;
 use function strpos;
 
@@ -32,7 +38,9 @@ abstract class AnnotationDriver implements MappingDriver
     /**
      * The annotation reader.
      *
-     * @var Reader
+     * @deprecated Store the reader inside the child class instead.
+     *
+     * @var Reader|null
      */
     protected $reader;
 
@@ -68,6 +76,8 @@ abstract class AnnotationDriver implements MappingDriver
     /**
      * Name of the entity annotations as keys.
      *
+     * @deprecated
+     *
      * @var array<class-string, bool|int>
      */
     protected $entityAnnotationClasses = [];
@@ -76,12 +86,21 @@ abstract class AnnotationDriver implements MappingDriver
      * Initializes a new AnnotationDriver that uses the given AnnotationReader for reading
      * docblock annotations.
      *
-     * @param Reader               $reader The AnnotationReader to use, duck-typed.
-     * @param string|string[]|null $paths  One or multiple paths where mapping classes can be found.
+     * @param string|string[]|null $paths One or multiple paths where mapping classes can be found.
      */
-    public function __construct($reader, $paths = null)
+    public function __construct($paths = null)
     {
-        $this->reader = $reader;
+        if (is_object($paths)) {
+            Deprecation::trigger(
+                'doctrine/persistence',
+                'https://github.com/doctrine/persistence/pull/217',
+                'Passing an annotation reader to %s is deprecated. Store the reader in the child class instead and override isTransient().',
+                __METHOD__
+            );
+            $this->reader = $paths;
+            $paths        = func_num_args() > 1 ? func_get_arg(1) : null;
+        }
+
         if (! $paths) {
             return;
         }
@@ -136,10 +155,23 @@ public function getExcludePaths()
     /**
      * Retrieve the current annotation reader
      *
+     * @deprecated
+     *
      * @return Reader
      */
     public function getReader()
     {
+        Deprecation::trigger(
+            'doctrine/persistence',
+            'https://github.com/doctrine/persistence/pull/217',
+            '%s is deprecated without replacement.',
+            __METHOD__
+        );
+
+        if ($this->reader === null) {
+            throw new LogicException('The reader has not been initialized.');
+        }
+
         return $this->reader;
     }
 
@@ -176,6 +208,21 @@ public function setFileExtension($fileExtension)
      */
     public function isTransient($className)
     {
+        Deprecation::trigger(
+            'doctrine/persistence',
+            'https://github.com/doctrine/persistence/pull/217',
+            'Not overriding %s in %s is deprecated.',
+            __METHOD__,
+            static::class
+        );
+
+        if (! $this->reader) {
+            throw new LogicException(sprintf(
+                'The annotation reader has not been set. Override isTransient() in %s instead.',
+                static::class
+            ));
+        }
+
         $classAnnotations = $this->reader->getClassAnnotations(new ReflectionClass($className));
 
         foreach ($classAnnotations as $annot) {

phpcs.xml.dist

@@ -9,6 +9,8 @@
     <!-- Ignore warnings and show progress of the run -->
     <arg value="nps"/>
 
+    <config name="php_version" value="70100"/>
+
     <file>lib</file>
     <file>tests</file>
 

phpstan.neon

@@ -10,3 +10,12 @@ parameters:
 
     excludePaths:
         - tests/Doctrine/Tests/Persistence/Mapping/_files/Doctrine.Tests.Persistence.Mapping.PHPTestEntity.php
+
+    ignoreErrors:
+        # TODO: Remove in 3.0
+        -
+            message: "#^Call to function is_object\\(\\) with array\\<string\\>\\|string\\|null will always evaluate to false\\.$#"
+            path: lib/Doctrine/Persistence/Mapping/Driver/AnnotationDriver.php
+        -
+            message: "#^Parameter \\#1 \\$paths of class Doctrine\\\\Tests\\\\Persistence\\\\Mapping\\\\SimpleAnnotationDriver constructor expects array\\<string\\>\\|string\\|null, Doctrine\\\\Common\\\\Annotations\\\\AnnotationReader given\\.$#"
+            path: tests/Doctrine/Tests/Persistence/Mapping/AnnotationDriverTest.php

psalm.xml

@@ -22,6 +22,12 @@
                 <file name="lib/Doctrine/Persistence/Reflection/TypedNoDefaultReflectionProperty.php"/>
             </errorLevel>
         </RedundantCondition>
+        <InvalidArgument>
+            <errorLevel type="suppress">
+                <!-- TODO: Remove in 3.0 -->
+                <file name="tests/Doctrine/Tests/Persistence/Mapping/AnnotationDriverTest.php" />
+            </errorLevel>
+        </InvalidArgument>
         <InvalidNullableReturnType>
             <errorLevel type="suppress">
                 <!-- see https://github.com/vimeo/psalm/issues/5193 -->

tests/Doctrine/Tests/Persistence/Mapping/AnnotationDriverTest.php

@@ -3,6 +3,7 @@
 namespace Doctrine\Tests\Persistence\Mapping;
 
 use Doctrine\Common\Annotations\AnnotationReader;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\Entity;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\AnnotationDriver;
@@ -11,18 +12,44 @@
 
 class AnnotationDriverTest extends TestCase
 {
+    use VerifyDeprecations;
+
     public function testGetAllClassNames(): void
     {
-        $reader = new AnnotationReader();
-        $driver = new SimpleAnnotationDriver($reader, [__DIR__ . '/_files/annotation']);
+        $driver = new SimpleAnnotationDriver([__DIR__ . '/_files/annotation']);
+
+        self::assertSame([TestClass::class], $driver->getAllClassNames());
+    }
+
+    public function testGetAllClassNamesLegacy(): void
+    {
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/persistence/pull/217');
 
-        $classes = $driver->getAllClassNames();
+        $driver = new SimpleAnnotationDriver(new AnnotationReader(), [__DIR__ . '/_files/annotation']);
 
-        self::assertSame([TestClass::class], $classes);
+        self::assertSame([TestClass::class], $driver->getAllClassNames());
     }
 }
 
 class SimpleAnnotationDriver extends AnnotationDriver
+{
+    /**
+     * {@inheritDoc}
+     */
+    public function loadMetadataForClass($className, ClassMetadata $metadata): void
+    {
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isTransient($className): bool
+    {
+        return $className === Entity::class;
+    }
+}
+
+class LegacyAnnotationDriver extends AnnotationDriver
 {
     /** @var array<class-string, bool|int> */
     protected $entityAnnotationClasses = [Entity::class => true];