RuntimeException during deserializing variable defined in trait

[max-voloshin]

Hi!

There is a problem in such case:

<?php

use Ivory\Serializer\Format;
use Ivory\Serializer\Serializer;

require_once __DIR__ . '/vendor/autoload.php';

trait Foo
{
    /**
     * @var int
     */
    protected $variable = 0;
}

class Bar
{
    use Foo;
}

$serializer = new Serializer();

$object = $serializer->deserialize('{"variable":5}', Bar::class, Format::JSON);

var_dump($object);

Expected: something like

class Bar#145 (1) {
  protected $variable =>
  int(5)
}

Actual:

PHP Fatal error:  Uncaught RuntimeException: You must define the type of the Bar:variable.

Composer dependencies:

• "egeloen/serializer": "@stable"
• "symfony/property-info": "@stable"
• "symfony/property-access": "@stable"
• "doctrine/annotations": "@stable"
• "phpdocumentor/reflection": "@stable"

Should it be reported here as a bug or maybe in some of the above dependencies?

[egeloen]

@max-voloshin Thanks for your feedback! :)

I need to investigate what's going on with trait but the bug should defitively be there as it is a problem related to metadata autodiscovering.

[egeloen]

So, after quickly looking to it, it seems there is an issue in the property type guessing as the property is well discovered but the @var int is not guessed by the ReflectionClassMetadataLoader. This is weird because such use case is present in the test suite (not using trait)... Can you debug what is guessed here for your Bar::variable property?

[max-voloshin]

Yeah, the type isn't resolved due to https://github.com/symfony/symfony/issues/23365, thank you for the hint!

[max-voloshin]

@egeloen I've tested my script with "symfony/property-info": "dev-master" and "phpdocumentor/reflection": "*", and can confirm that this problem is already resolved there.

But ivory serializer should be prepared for newer versions of symfony/property-info and phpdocumentor/reflection.

I've made hotfix for that:

diff --git a/src/Mapping/Factory/ClassMetadataFactory.php b/src/Mapping/Factory/ClassMetadataFactory.php
index 944d5c4..6a3a5fd 100644
--- a/src/Mapping/Factory/ClassMetadataFactory.php
+++ b/src/Mapping/Factory/ClassMetadataFactory.php
@@ -19,6 +19,7 @@ use Ivory\Serializer\Mapping\Loader\ChainClassMetadataLoader;
 use Ivory\Serializer\Mapping\Loader\ClassMetadataLoaderInterface;
 use Ivory\Serializer\Mapping\Loader\ReflectionClassMetadataLoader;
 use phpDocumentor\Reflection\ClassReflector;
+use phpDocumentor\Reflection\DocBlockFactoryInterface;
 use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
 use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
@@ -54,7 +55,9 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory
             if (class_exists(PropertyInfoExtractor::class)) {
                 $extractors = $typeExtractors = [new ReflectionExtractor()];

-                if (class_exists(ClassReflector::class)) {
+                if (class_exists(ClassReflector::class)
+                    or interface_exists(DocBlockFactoryInterface::class)
+                ) {
                     array_unshift($typeExtractors, new PhpDocExtractor());
                 }

Is this appropriate fix for this issue?

[egeloen]

Hum, why do you want to check for the interface existence? If the ClassReflector exists, then, the PhpDocExtractor can be used safely regardless the components minor/patch versions or am I missing something?

[max-voloshin]

Hum, why do you want to check for the interface existence?

I follow similar source in Symfony

If the ClassReflector exists, then, the PhpDocExtractor can be used safely regardless the components minor/patch versions or am I missing something?

ClassReflector doesn't exist in newer version of phpdocumentor/reflection and fresh version of PhpDocExtractor works with DocBlockFactoryInterface in the __construct

[egeloen]

If in a specific version, the class exists and in the next version the class does not exist anymore and has been "replaced" by the interface, then I agree it's the way to go :)

[max-voloshin]

PR: #24