Support closure returned by .php files in the PHPDriver

UPGRADE.md

@@ -16,6 +16,31 @@ The interface `Doctrine\Persistence\Mapping\ClassMetadata` has two new methods:
 
 Not implementing these methods is deprecated. They will be required in 5.0.
 
+## Deprecated modifying `$metadata` in PHP mapping files
+
+Relying on the `$metadata` variable directly in PHP mapping files is deprecated.
+Instead, 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.0
 
 ## BC Break: Removed `StaticReflectionService`

composer.json

@@ -21,6 +21,7 @@
     ],
     "require": {
         "php": "^8.1",
+        "doctrine/deprecations": "^1",
         "doctrine/event-manager": "^1 || ^2",
         "psr/cache": "^1.0 || ^2.0 || ^3.0"
     },

docs/en/reference/index.rst

@@ -258,10 +258,15 @@ mapping metadata.
 .. code-block:: php
 
     use App\Model\User;
+    use Doctrine\Persistence\Mapping\ClassMetadata;
+
+    return function (ClassMetadata $metadata): void {
+        $metadata->name = User::class;
+
+        // ...
 
-    $metadata->name = User::class;
+    };
 
-    // ...
 
 StaticPHPDriver
 --------------
@@ -284,6 +289,8 @@ Your class in ``App\Model\User`` would look like the following.
 
     namespace App\Model;
 
+    use Doctrine\Persistence\Mapping\ClassMetadata;
+
     final class User
     {
         // ...

phpstan.neon

@@ -14,7 +14,7 @@ parameters:
         - tests/Persistence/Mapping/_files/colocated/Foo.mphp
 
     excludePaths:
-        - tests/Persistence/Mapping/_files/Doctrine.Tests.Persistence.Mapping.PHPTestEntity.php
+        - tests/Persistence/Mapping/_files/Doctrine.*.php
 
     ignoreErrors:
         - '#Variable property access on \$this\(Doctrine\\Persistence\\Reflection\\TypedNoDefaultReflectionProperty\)\.#'

src/Persistence/Mapping/Driver/PHPDriver.php

@@ -4,7 +4,11 @@
 
 namespace Doctrine\Persistence\Mapping\Driver;
 
+use Closure;
+use CompileError;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\Persistence\Mapping\ClassMetadata;
+use Error;
 
 /**
  * The PHPDriver includes php files which just populate ClassMetadataInfo
@@ -35,6 +39,33 @@ public function loadMetadataForClass(string $className, ClassMetadata $metadata)
      */
     protected function loadMappingFile(string $file): array
     {
+        try {
+            $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;
+        }
+
+        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;
 

tests/Persistence/Mapping/PHPDriverTest.php

@@ -4,22 +4,80 @@
 
 namespace Doctrine\Tests\Persistence\Mapping;
 
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\PHPDriver;
 use Doctrine\Tests\DoctrineTestCase;
+use Error;
+use PHPUnit\Framework\Attributes\IgnoreDeprecations;
+use PHPUnit\Framework\Attributes\TestWith;
 
 class PHPDriverTest extends DoctrineTestCase
 {
-    public function testLoadMetadata(): void
+    use VerifyDeprecations;
+
+    /** @phpstan-param class-string $className */
+    #[IgnoreDeprecations]
+    #[TestWith([PHPTestEntity::class])]
+    #[TestWith([PHPTestEntityAssert::class])]
+    public function testLoadMetadata(string $className): void
+    {
+        $metadata = $this->createMock(ClassMetadata::class);
+        $metadata->expects(self::once())->method('getFieldNames');
+        $driver = new PHPDriver([__DIR__ . '/_files']);
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/persistence/pull/450');
+        $driver->loadMetadataForClass($className, $metadata);
+    }
+
+    public function testLoadMetadataWithClosure(): void
     {
         $metadata = $this->createMock(ClassMetadata::class);
         $metadata->expects(self::once())->method('getFieldNames');
 
         $driver = new PHPDriver([__DIR__ . '/_files']);
-        $driver->loadMetadataForClass(PHPTestEntity::class, $metadata);
+        $driver->loadMetadataForClass(PHPTestEntityClosure::class, $metadata);
+    }
+
+    public function testLoadMetadataClosureNotBoundToObject(): void
+    {
+        $metadata = $this->createMock(ClassMetadata::class);
+        $driver   = new PHPDriver([__DIR__ . '/_files']);
+
+        $this->expectException(Error::class);
+        $this->expectExceptionMessage('Using $this when not in object context');
+
+        $driver->loadMetadataForClass(PHPTestIncorrectUseThis::class, $metadata);
+    }
+
+    public function testLoadMetadataClosureNotBoundToClass(): void
+    {
+        $metadata = $this->createMock(ClassMetadata::class);
+        $driver   = new PHPDriver([__DIR__ . '/_files']);
+
+        $this->expectException(Error::class);
+        $this->expectExceptionMessage('Cannot use "static" in the global scope');
+
+        $driver->loadMetadataForClass(PHPTestIncorrectUseStatic::class, $metadata);
     }
 }
 
 class PHPTestEntity
 {
 }
+
+class PHPTestEntityAssert
+{
+}
+
+class PHPTestEntityClosure
+{
+}
+
+class PHPTestIncorrectUseThis
+{
+}
+
+class PHPTestIncorrectUseStatic
+{
+}

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

@@ -0,0 +1,8 @@
+<?php
+
+declare(strict_types=1);
+
+use Doctrine\Persistence\Mapping\ClassMetadata;
+
+assert($metadata instanceof ClassMetadata);
+$metadata->getFieldNames();

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

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+use Doctrine\Persistence\Mapping\ClassMetadata;
+
+return static function (ClassMetadata $metadata): void {
+    $metadata->getFieldNames();
+};

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

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+use Doctrine\Persistence\Mapping\ClassMetadata;
+
+return static function (ClassMetadata $metadata): void {
+    $metadata->isIdentifier(static::class);
+};

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

@@ -0,0 +1,7 @@
+<?php
+
+declare(strict_types=1);
+
+return function (): void {
+    $this->setGlobalBasename('global-mapping');
+};