Deprecate not overriding Type::closureToPHP() (#2960)

The default implementation of Type::closureToPhp() is incorrect as it forwards the unmodified value.
In 3.0, I want to change the default implementation to use the one from the ClosureToPHP trait that calls the type class.
Also adding the missing implementation for collection and hash.
Any type that does not override the closureToPhp will get a deprecation notice.

Author: GromNaN

UPGRADE-2.16.md

@@ -14,3 +14,16 @@ be supported in MongoDB ODM 3.0.
 Calling `Doctrine\ODM\MongoDB\Configuration::setProxyDir()` or
 `Doctrine\ODM\MongoDB\Configuration::getProxyDir()` is deprecated and triggers
 a deprecation notice when using native lazy objects.
+
+## Override `Type::closureToPHP()` for custom type classes
+
+The default implementation of `Doctrine\ODM\MongoDB\Types\Type::closureToPHP()`
+will change in MongoDB ODM 3.0 to call `convertToPHPValue()`. If you have custom
+type classes, use the `Doctrine\ODM\MongoDB\Types\ClosureToPHP` trait or
+implement `closureToPHP()`.
+
+## Deprecate `Type::closureToMongo()`
+
+The method `Doctrine\ODM\MongoDB\Types\Type::closureToMongo()` is not used,
+and will be removed in MongoDB ODM 3.0. Don't call this method, but use
+`convertToDatabaseValue()` instead.

src/Types/ClosureToPHP.php

@@ -6,6 +6,7 @@
 
 use function sprintf;
 
+/** This trait will be deprecated in 3.0 as this behavior will be used by default */
 trait ClosureToPHP
 {
     /** @return string Redirects to the method convertToPHPValue from child class */

src/Types/CollectionType.php

@@ -29,4 +29,9 @@ public function convertToPHPValue($value)
     {
         return $value !== null ? array_values($value) : null;
     }
+
+    public function closureToPHP(): string
+    {
+        return '$return = array_values($value);';
+    }
 }

src/Types/HashType.php

@@ -28,4 +28,9 @@ public function convertToPHPValue($value)
     {
         return $value !== null ? (array) $value : null;
     }
+
+    public function closureToPHP(): string
+    {
+        return '$return = (array) $value;';
+    }
 }

src/Types/KeyType.php

@@ -12,6 +12,8 @@
  */
 class KeyType extends Type
 {
+    use ClosureToPHP;
+
     /** @return MinKey|MaxKey|null */
     public function convertToDatabaseValue($value)
     {

src/Types/TimestampType.php

@@ -14,6 +14,8 @@
  */
 class TimestampType extends Type
 {
+    use ClosureToPHP;
+
     /** @return Timestamp|null */
     public function convertToDatabaseValue($value)
     {

src/Types/Type.php

@@ -15,6 +15,7 @@
 use function gettype;
 use function is_object;
 use function str_replace;
+use function trigger_deprecation;
 
 /**
  * The Type interface.
@@ -129,18 +130,26 @@ public function convertToPHPValue($value)
     /**
      * Get the PHP code equivalent to {@see convertToDatabaseValue()}, used in code generator.
      * Use variables $value for input and $return for output.
+     *
+     * @deprecated Since 2.16, will be removed in 3.0.
      */
     public function closureToMongo(): string
     {
+        trigger_deprecation('doctrine/mongodb-odm', '2.16', 'Type::closureToMongo() is deprecated and will be removed in 3.0.');
+
         return '$return = $value;';
     }
 
     /**
      * Get the PHP code equivalent to {@see convertToPHPValue()}, used in code generator.
      * Use variables $value for input and $return for output.
+     *
+     * @abstract The default implementation will change in 3.0.
      */
     public function closureToPHP(): string
     {
+        trigger_deprecation('doctrine/mongodb-odm', '2.16', 'The method Type::closureToPHP() will change its default implementation in 3.0 to use convertToPHPValue(). Override this method if you need custom behavior before upgrading to 3.0 or use the trait ClosureToPHP to get the upcoming behavior now.');
+
         return '$return = $value;';
     }
 

tests/Tests/CaptureDeprecationMessages.php

@@ -16,9 +16,13 @@ trait CaptureDeprecationMessages
      * This method can be replaced with expectUserDeprecationMessage() in PHPUnit 11+.
      * https://docs.phpunit.de/en/11.1/error-handling.html#expecting-deprecations-e-user-deprecated
      *
-     * @param list<string> $errors
+     * @param callable(): T $callable
+     * @param list<string>  $errors
+     *
+     * @return T
      *
      * @param-out list<string> $errors
+     * @template T
      */
     private function captureDeprecationMessages(callable $callable, ?array &$errors): mixed
     {

tests/Tests/Functional/CustomTypeTest.php

@@ -7,6 +7,7 @@
 use DateTime;
 use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
 use Doctrine\ODM\MongoDB\Tests\BaseTestCase;
+use Doctrine\ODM\MongoDB\Tests\CaptureDeprecationMessages;
 use Doctrine\ODM\MongoDB\Types\ClosureToPHP;
 use Doctrine\ODM\MongoDB\Types\Type;
 use Exception;
@@ -20,12 +21,15 @@
 
 class CustomTypeTest extends BaseTestCase
 {
+    use CaptureDeprecationMessages;
+
     public function setUp(): void
     {
         parent::setUp();
 
         Type::addType('date_collection', DateCollectionType::class);
         Type::addType(Language::class, LanguageType::class);
+        Type::addType('custom_type_without_closure_to_php', CustomTypeWithoutClosureToPHP::class);
     }
 
     #[After]
@@ -89,6 +93,16 @@ public function testTypeFromPHPVariable(): void
         $databaseValue = Type::convertPHPToDatabaseValue($lang);
         self::assertSame(['name' => 'French', 'code' => 'fr'], $databaseValue);
     }
+
+    public function testNotOverridingClosureToPHPIsDeprecated(): void
+    {
+        $type = Type::getType('custom_type_without_closure_to_php');
+
+        $code = $this->captureDeprecationMessages(static fn () => $type->closureToPHP(), $deprecations);
+
+        self::assertSame('$return = $value;', $code);
+        self::assertSame(['Since doctrine/mongodb-odm 2.16: The method Type::closureToPHP() will change its default implementation in 3.0 to use convertToPHPValue(). Override this method if you need custom behavior before upgrading to 3.0 or use the trait ClosureToPHP to get the upcoming behavior now.'], $deprecations);
+    }
 }
 
 class DateCollectionType extends Type
@@ -198,3 +212,7 @@ public function convertToPHPValue($value): ?Language
         return new Language($value['name'], $value['code']);
     }
 }
+
+class CustomTypeWithoutClosureToPHP extends Type
+{
+}

tests/Tests/Types/TypeTest.php

@@ -42,6 +42,16 @@ public function testConversion(string $typeName, mixed $phpValue, mixed $bsonVal
         self::assertSameTypeAndValue($bsonValue, $type->convertToDatabaseValue($phpValue));
     }
 
+    #[DataProvider('provideTypes')]
+    public function testConversionWithClosureToPHP(string $typeIdentifier, mixed $expectedValue, mixed $value = null): void
+    {
+        $value ??= $expectedValue;
+        $return = $this;
+        eval(Type::getType($typeIdentifier)->closureToPHP());
+
+        self::assertSameTypeAndValue($expectedValue, $return);
+    }
+
     public static function provideTypes(): array
     {
         return [