@dunglas could you make a code review and eventually merge the commit?

@nicolas-grekas Could you please make a review?

Instead of hacking that in the ObjectNormalizer, I think this should rather be a new Normalizer

@stof Good point. However, currently the normalizing process of stdClass is part of the same class -https://github.com/symfony/symfony/pull/52850/files#diff-397b4689ce56cc95e54911f6d78639bb3258a9ee9b36bfa06ec719e9635e6b1dR67

@stof I thought about it and think it should be part of the ObjectNormalizer.

1. it is an object and, at the same time, the simplest object.
2. Some bundles already use ObjectNormalizer to normalize properties of type stdClass, but it cannot be denormalized. e.g.https://github.com/dunglas/doctrine-json-odm -https://github.com/dunglas/doctrine-json-odm/blob/main/src/Bundle/Resources/config/services.xml

@stof@dunglas Could you please make a review and eventually merge it?

1. it is an object and, at the same time, the simplest object.

if we go that way,all core normalizers would be removed by bundling them into ObjectNormalizer, because BackedEnum or DateTime are also objects.

ObjectNormalizer is not about being the implementation to use for all objects. It is thegeneric implementation used for objects that don't have another normalizer registered with a higher priority to take over the normalization with specific logic.

2. Some bundles already use ObjectNormalizer to normalize properties of type stdClass, but it cannot be denormalized.

The JSON ODM does not use the ObjectNormalizer directly. It uses a Serializer with multiple normalizers registered in it. This means that they could totally register the new normalizer once it is available. They already did exactly that for the BackedEnumNormalizer and the UidNormalizer btw.

I'm with@stof here. Adding special behavior for a specific class usually means we're adding a new normalizer.

Agreed as well. TheObjectNormalizer is kind of big enough...

Plus, astdClass dedicated normalizer could be way faster than theObjectNormalizer one.
Therefore, the developers willing to normalizestdClass only will be able to use this instead of theObjectNormalizer one and have a performance boost, which is great!

We have this problem too:
#54613
Is somebody already working on this normalizer? Otherwise if I have some time, I will step in.

AKAIK, there is no ongoing work on that normalizer, so feel free to step in 🙂

Questions regarding "new normalizer". What about a class, that is a mixture of stdclass and object?

<?phpdeclare(strict_types =1);usestdClass;class Contactextends stdClass{publicstring$email ='';}

I am not sure if a new normalizer is the best solution here.
I am not an expert in the normalizer logic, but the "StdClassNormalizer" cannot work with logic of the "ObjectNormalizer" + "AbstractObjectNormalizer".
"ObjectNormalizer" is a final class, so a new "StdClassNormalizer" has a lot of duplicate code from the "ObjectNormalizer".

But I will kick in and give feedback

Hi..
I added a pull (7.2) for adding the support of stdclasses. I added this feature directly into the objectnormalizer.

The problem with an extra stdclass normalizer is the shared logic of stdclass objects and objects that extends stdclasses.
In the end, we would have a lot of duplicate code in the StdclassNormalizer + ObjectNormalizer. And putting everything in the AbstractObjectNormalizer would have impact on the other normalizers (GetSetMethodNormalizer, PropertyNormalizer).

It should be possible to extract the stdclass logic into a StdclassNormalizer and extend the ObjectNormalizer.
But I am not an expert of this normalizers, so if somebody with more inside .... feel free to change the pull.

Imho the ObjectNormalizer needs an overhaul, but this is an issue for later and not for now. Maybe, than it is possible to seperate the normalizers.

@dunglas
the coding style hates me, but the style problems are not within my code, but within existing code.
maybe execute coding style in another commit/issue.

What about acting directly onPropertyAccessor instead?

The actual behavior in PropertyAccessor is checking the following:$object instanceof \stdClass && property_exists($object, $property).
And it will work for dynamic properties if we only have checked$object instanceof \stdClass.

I'm still not sure why we need thatproperty_exists check. I've seen most of them have been introduced insymfony/property-access@2d75186, so maybe you can help us@alexandre-daubois?

The property_exists is used to check whether a property with a. exists in the object or not. It is an edge case, only possible when casting an array to an object with(object) (and maybe with ArrayObjects?). Ifproperty_exists returns false, then this means we are actually dealing with aproperty path, not an actual property of the object. Again, this truly is an edge-case 😄

@mtarld@alexandre-daubois
Thx for th info.... I always asked myself: why on earth was that property_exists added? ;-)