Type error in `UnionHandler`

[dmaicher]

Q	A
Bug report?	yes
Feature request?	yeso
BC Break report?	yes
RFC?	no

Steps required to reproduce the problem

jms/serializer        3.31.1 Library for (de-)serializing data of any complexity; supports XML, and JSON.
jms/serializer-bundle 5.5.1  Allows you to easily serialize, and deserialize data of any complexity

$serializer = SerializerBuilder::create()->build();
$serializer->serialize(new class() {
    public array|int $foo = [1, 2, 3];
}, 'json');

Expected Result

• Should work fine

Actual Result

TypeError:
get_class(): Argument #1 ($object) must be of type object, array given

  at vendor/jms/serializer/src/Handler/UnionHandler.php:56

[scyzoryck]

It looks like arrays are not supported in current implementation at all. I will take a look at it. Looks like @simPod already started some work around that. https://github.com/schmittjoh/serializer/pull/1552

[mermshaus]

I can confirm this issue. Unfortunately, it breaks existing code. For the time being, we had to pin jms/serializer to v3.30.0 because of this. It might be a good idea to release a new version reverting these changes until the feature supports arrays. (Or, as I see the issue with that, to specify a workaround for disabling the new feature.)

#1552 isn’t working on this, I think. The MR is about “unions” (not really) in arrays, not arrays in unions.

[goetas]

Unfortunately, it breaks existing code

@scyzoryck would it make sense to revert the union types thing that is causing this error?

from my understanding, https://github.com/schmittjoh/serializer/pull/1546 is the guilty one, right?

[scyzoryck]

We also I think have 2 MRs on top of #1546 so it might need some additional reverts. As an example: #1563 Current issues with Union Types:

• High prio: Support for true and false types. MR: #1569 - we need to add some
• High prio: Support for array type - I believe I can finish the basic support (fix the issue from this issue), #1552 may add complex types later.
• Medium/Low prio: It looks like there might be some issues with DepthExclusionStrategy for Union types, but I guess it was not working in the past at all.

Soo, personally I would go with fixing the issues, and going with the fix release this week with support for missing types.

[goetas]

Soo, personally I would go with fixing the issues, and going with the fix release this week with support for missing types.

:+1:

[scyzoryck]

@dmaicher could you test newest dev-master version, please? :) If it will look good for you I will make the new fix release.

[dmaicher]

@scyzoryck with my example code mentioned in the issue description I get

PHP Warning: Array to string conversion in /var/www/app/symfony/vendor/jms/serializer/src/Handler/UnionHandler.php on line 129

because in my case int is checked first it seems.

Maybe testPrimitive can be adjusted like this

    private function testPrimitive(mixed $data, string $type, string $format): bool
    {
        if ($type === 'array') {
            return is_array($data);
        }

        if (!is_scalar($data)) {
            return false;
        }

        switch ($type) {
            case 'integer':
            case 'int':
                return (string) (int) $data === (string) $data;

            case 'double':
            case 'float':
                return (string) (float) $data === (string) $data;

            case 'bool':
            case 'boolean':
                return (string) (bool) $data === (string) $data;

            case 'string':
                return is_string($data);
        }

        return false;
    }

Then the warning is gone in my case.

Another issue I noticed:

If I change my property to public array|int|string $foo with string value "666" then its serialized into an integer {"foo":666}. Should be {"foo":"666"} though?

[scyzoryck]

Sooo, it looks like we need a bit more development on Unions.

I would propose the next steps:

• add enableUnionSupport method similar to add enableEnumSupport - to bring back the state from the 3.30.x as a default one
• continue work on the UnionSupport as a experimental one.

@goetas does it sounds good for you?

If I change my property to public array|int|string $foo with string value "666" then its serialized into an integer {"foo":666}. Should be {"foo":"666"} though?

Yes, I think you are right. I think we have "type recognition" logic to determine the correct type - soo "666" is parsed as 666 during both - serialization and deserialization at the moment. So it seems to be similar issue as #1573 #1575 - FYI @fabianerni

IMO we should have the following logic for the type choosing:

Serialization:

• Keep the types all the time

Deserialization:

• If the type is supported - keep the type - as example: "666" will be kept as string
• If the type is not supported - try to guess the type - as example: if property do not support string, but it supports int|bool "1" will be deserialized as int.

[goetas]

@scyzoryck adding enableUnionSupport makes sense, it seems that there is much more work than expected to have it working properly.

It would be good to revert https://github.com/schmittjoh/JMSSerializerBundle/pull/954 as well

[scyzoryck]

Hi @dmaicher!

The fix has been published to dev-master branch. Could you test it, please?

Best, Marcin.

[dmaicher]

Thanks a lot for the update @scyzoryck. I tested it on my app and the problem is indeed fixed 👍