[Serializer] Restrictions

[evrinoma]

Symfony version(s) affected

6.4

Description

Hi everyone,
I'd like to ask why Symfony Serializer has name restrictions. Here are two code examples, and they give me two different results

I would suggest that it's incorrect behavior and completely undocumented. I'm feeling a bit sad about switching from JMS Serializer.
thanks in advance.

How to reproduce

<?php
namespace App\Dtos;
class Project
{
    private ProjectData $projectData;

//    public function __construct(ProjectData $projectData)
//    {
//        $this->projectData = $projectData;
//    }

    public function setProjectData(ProjectData $projectData)
    {
        $this->projectData = $projectData;
    }

    public function getProjectData(): ProjectData
    {
        return $this->projectData;
    }
}

<?php
namespace App\Dtos;
use Symfony\Component\Serializer\Attribute\SerializedName;
class ProjectArray
{
    #[SerializedName('projectDto')]
    private array $projectsDto = [];

    public function setProjectsDto(array $projectsData)
    {
        $this->projectsDto = $projectsData;
    }

//    public function __construct(array $projectData)
//    {
//        $this->projectData = $projectData;
//    }

    public function getProjectsDto(): array
    {
        return $this->projectsDto;
    }

    public function addProjectsDto(ProjectData $projectDto): void
    {
        $this->projectsDto[] = $projectDto;
    }
}

        $data = new ProjectArray();
        $data->addProjectsDto(new ProjectData(123));
        $data->addProjectsDto(new ProjectData(456));
         $ser = $this->serializer->serialize(
            $data,
            JsonEncoder::FORMAT
        );
        $object = $this->serializer->deserialize(
            $ser,
            ProjectArray::class,
            JsonEncoder::FORMAT
        );

//$ser is '{"projectDto":[{"projectId":123},{"projectId":456}]}'

//  $object  is \App\Dtos\ProjectArray::__set_state(array(
//    'projectsDto' =>  array (
//         0 =>  array ( 'projectId' => 123, ),
//         1 =>  array ( 'projectId' => 456,  ),
//       ),
// ))

In the second example, I would like to rename 'projectsDto' to 'projectsData' along with its associated methods.

<?php
namespace App\Dtos;
class Project
{
    private ProjectData $projectData;

//    public function __construct(ProjectData $projectData)
//    {
//        $this->projectData = $projectData;
//    }

    public function setProjectData(ProjectData $projectData)
    {
        $this->projectData = $projectData;
    }

    public function getProjectData(): ProjectData
    {
        return $this->projectData;
    }
}

<?php
namespace App\Dtos;
use Symfony\Component\Serializer\Attribute\SerializedName;
class ProjectArray
{

    #[SerializedName('projectData')]
    private array $projectsData = [];

    public function setProjectsData(array $projectsData)
    {
        $this->projectsData = $projectsData;
    }

//    public function __construct(array $projectData)
//    {
//        $this->projectData = $projectData;
//    }

    public function getProjectsData(): array
    {
        return $this->projectsData;
    }

    public function addProjectsData(ProjectData $projectData): void
    {
        $this->projectsData[] = $projectData;
    }
}

        $data = new ProjectArray();
        $data->addProjectsData(new ProjectData(123));
        $data->addProjectsData(new ProjectData(456));

         $ser = $this->serializer->serialize(
            $data,
            JsonEncoder::FORMAT
        );

        $object = $this->serializer->deserialize(
            $ser,
            ProjectArray::class,
            JsonEncoder::FORMAT
        );

//$ser is '{"projectDto":[{"projectId":123},{"projectId":456}]}'
//$object  is \App\Dtos\ProjectArray::__set_state(array(
//   'projectsData' =>  array (
//    0 =>  \App\Dtos\ProjectData::__set_state(array(
//       'projectId' => 123,
//    )),
//    1 => \App\Dtos\ProjectData::__set_state(array(
//       'projectId' => 456,
//    )),
// ),
//))

Possible Solution

No response

Additional Context

No response

[EugeneKwasny]

Thank you @evrinoma for creating this bug report! This indeed looks
like a bug. I reproduced the bug in the "main" branch of
https://github.com/EugeneKwasny/issue-5475-bug-repoducer

Status: Reviewed

[krzysztof-ciszewski]

I figured out why this happens. The type in the projectsData example is correctly detected as ProjectData class because the property name is considered plural. When that happens the type extractor looks for the method with add prefix first and from that it detects the correct type. With projectsDto the name is considered singular, so it first looks for method with set prefix and since it's array it deserializes into a simple array.

To fix this you just have to add a phpdoc typehint to your property

    #[SerializedName('projectDto')]
    /**
    * @var \App\Dtos\ProjectData[]
    */
    private array $projectsDto = [];

I am not sure if this is considered a bug. In my opinion it is, we should always check for add methods first since they are most likely to have a type hint in the argument, but I might be missing something. Looking for advice here.

Depending on this being considered a bug I can either create a fix or update the docs and mention this case.

[evrinoma]

I'm afraid your assumption seems to be incorrect, and it won't work as expected. Could you please take a closer look at the code, not just at the ReflectionExtractor, but also at this problematic part of the EnglishInflector. In this file, variable names are hardcoded in reverse, and data is present in the list, but not dto. In common, It seems odd if I were to use German and name a variable projektDaten, and then the serializer behaves differently. It appears to be a suboptimal solution that contradicts the principles of KISS (Keep It Simple, Stupid). Why not simply allow users to specify the data type for a specific field via context? Why use reflection to study fields and analyze field and method names? Speed and simplicity are crucial for serializers, and if we have to struggle with every variable name, then what's the point of having it? It feels like a feeble imitation of artificial intelligence. More than that now it's slower than oldest JMS serializer.

[krzysztof-ciszewski]

I agree with you that behavior should not be affected by property naming, but this is a pretty niche edge case. To encounter this you have to not use the docblocks, which(I hope) is pretty rare.

I'm afraid your assumption seems to be incorrect, and it won't work as expected

I have tested it with the docblock and it works, I can make a PR to the bug-reproducer repo if you're interested.

It is possible to add support to setting types via context or configuration, but I feel like that would get rejected due to the design differences.