Require files loaded by the PHPDriver to return a Closure

Author: GromNaN

UPGRADE.md

@@ -21,6 +21,30 @@ The code base now has constants with type declarations. If you extend types
 from the library and override the constants, you will need to add compatible
 type declarations.
 
+## BC Break: Require files loaded by the `PHPDriver` to return a `Closure`
+
+If you use the `PHPDriver` for configuring metadata in PHP files, you must wrap 
+the code in a closure that is returned by the configuration file.
+
+Before:
+
+```php
+<?php // mappings/App.Entity.User.php
+
+$metadata->name = \App\Entity\User::class;
+```
+
+After:
+
+```php
+<?php // mappings/App.Entity.User.php
+
+use Doctrine\Persistence\Mapping\ClassMetadata;
+return function (ClassMetadata $metadata): void {
+    $metadata->name = \App\Entity\User::class;
+};
+```
+
 # Upgrade to 4.2
 
 ## Add `getFieldValue` and `setFieldValue` to `ClassMetadata` implementation

src/Mapping/Driver/PHPDriver.php

@@ -5,10 +5,8 @@
 namespace Doctrine\Persistence\Mapping\Driver;
 
 use Closure;
-use CompileError;
-use Doctrine\Deprecations\Deprecation;
 use Doctrine\Persistence\Mapping\ClassMetadata;
-use Error;
+use Doctrine\Persistence\Mapping\MappingException;
 
 /**
  * The PHPDriver includes php files which just populate ClassMetadataInfo
@@ -39,36 +37,18 @@ public function loadMetadataForClass(string $className, ClassMetadata $metadata)
      */
     protected function loadMappingFile(string $file): array
     {
-        try {
-            $callback = Closure::bind(static function (string $file): mixed {
-                $metadata = null;
+        $callback = Closure::bind(static function (string $file): mixed {
+            $metadata = null;
 
-                return include $file;
-            }, null, null)($file);
-        } catch (CompileError $e) {
-            throw $e;
-        } catch (Error) {
-            // Calling any method on $metadata=null will raise an Error
-            // Falling back to legacy behavior of expecting $metadata to be populated
-            $callback = null;
-        }
+            return include $file;
+        }, null, null)($file);
 
         if ($callback instanceof Closure) {
             $callback($this->metadata);
 
             return [$this->metadata->getName() => $this->metadata];
         }
 
-        Deprecation::trigger(
-            'doctrine/persistence',
-            'https://github.com/doctrine/persistence/pull/450',
-            'Not returning a Closure from a PHP mapping file is deprecated',
-        );
-
-        unset($callback);
-        $metadata = $this->metadata;
-        include $file;
-
-        return [$metadata->getName() => $metadata];
+        throw MappingException::phpFileMustReturnAClosure($file);
     }
 }

src/Mapping/MappingException.php

@@ -77,4 +77,12 @@ public static function classIsAnonymous(string $className): self
     {
         return new self(sprintf('Class "%s" is anonymous', $className));
     }
+
+    public static function phpFileMustReturnAClosure(string $fileName): self
+    {
+        return new self(sprintf(
+            'The PHP mapping file "%s" must return a Closure that receives the ClassMetadata instance as argument.',
+            $fileName,
+        ));
+    }
 }

tests/Mapping/PHPDriverTest.php

@@ -7,27 +7,23 @@
 use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\PHPDriver;
+use Doctrine\Persistence\Mapping\MappingException;
 use Error;
-use PHPUnit\Framework\Attributes\IgnoreDeprecations;
-use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\TestCase;
 
 class PHPDriverTest extends TestCase
 {
     use VerifyDeprecations;
 
-    /** @phpstan-param class-string $className */
-    #[IgnoreDeprecations]
-    #[TestWith([PHPTestEntity::class])]
-    #[TestWith([PHPTestEntityAssert::class])]
-    public function testLoadMetadata(string $className): void
+    public function testLoadMetadata(): void
     {
-        $metadata = $this->createMock(ClassMetadata::class);
-        $metadata->expects(self::once())->method('getFieldNames');
-        $driver = new PHPDriver([__DIR__ . '/_files']);
+        $metadata = $this->createStub(ClassMetadata::class);
+        $driver   = new PHPDriver([__DIR__ . '/_files']);
 
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/persistence/pull/450');
-        $driver->loadMetadataForClass($className, $metadata);
+        self::expectException(MappingException::class);
+        self::expectExceptionMessage('The PHP mapping file "' . __DIR__ . '/_files/Doctrine.Tests.Persistence.Mapping.PHPTestEntity.php" must return a Closure that receives the ClassMetadata instance as argument.');
+
+        $driver->loadMetadataForClass(PHPTestEntity::class, $metadata);
     }
 
     public function testLoadMetadataWithClosure(): void
@@ -66,10 +62,6 @@ class PHPTestEntity
 {
 }
 
-class PHPTestEntityAssert
-{
-}
-
 class PHPTestEntityClosure
 {
 }

tests/Mapping/_files/Doctrine.Tests.Persistence.Mapping.PHPTestEntity.php

@@ -1,5 +1,3 @@
 <?php
 
 declare(strict_types=1);
-
-$metadata->getFieldNames();