Loss of data for datetime fields if using a getter/setter

[brutal-factories]

Following #39 , support for multiple datetime deserialization formats was added, but there is a bug where if the field requires a setter, the temporary variable made to hold the DateTime object has the same name as the temporary that will be passed to the setter.

I've used the same script as in that PR, and fixed the mistake where start_date and end_date of the test data array were written in camelCase instead of snake_case. The issue appears when the Course::startDate or Course::endDate fields are set to private instead of public. the {{ date }} variable is the same as the {{ modelPath }}, and is unset by the end of the section

<?php

require_once 'vendor/autoload.php';

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;

use Doctrine\Common\Annotations\AnnotationReader;
use JMS\Serializer\Annotation as Serializer;
use Liip\MetadataParser\Builder;
use Liip\MetadataParser\ModelParser\VisibilityAwarePropertyAccessGuesser;
use Liip\MetadataParser\Parser;
use Liip\MetadataParser\RecursionChecker;
use Liip\MetadataParser\ModelParser\JMSParser;
use Liip\MetadataParser\ModelParser\LiipMetadataAnnotationParser;
use Liip\MetadataParser\ModelParser\PhpDocParser;
use Liip\MetadataParser\ModelParser\ReflectionParser;
use Liip\MetadataParser\Reducer\TakeBestReducer;
use Liip\Serializer\Configuration\GeneratorConfiguration;

class IndirectAccess {
    private ?string $name = null;
    private ?string $parent = null;

    public function getName(): ?string {
        return $this->name;
    }

    public function setName(?string $name): void {
        $this->name = $name;
    }

    public function getParent(): ?string {
        return $this->parent;
    }

    public function setParent(?string $parent): void {
        $this->parent = $parent;
    }
}

class StudentDated {
    private ?string $name = null;

    /**
     * @Serializer\Type("DateTimeInterface<'Y-m-d\TH:i:sP', 'UTC'>")
     */
    private ?DateTimeImmutable $createdAt = null;
    /**
     * @Serializer\Type("DateTimeInterface<'Y-m-d\TH:i:sP', 'UTC'>")
     */
    private ?DateTime $updatedAt = null;
}

class Student {
    public ?string $name = null;

    /**
     * Simple case
     * @Serializer\Type("DateTimeImmutable<'Y-m-d'>")
     */
    public ?DateTimeImmutable $createdAt = null;
    /**
      * Type-hint doesn't match JMS type information (probably not the best practices, but works)
     * @Serializer\Type("DateTimeInterface<'Y-m-d'>")
     */
    public ?Datetime $updatedAt = null;
}

class Course
{
    public ?string $name = null;
    /**
     * @Serializer\Type("ArrayCollection<Student>")
     */
    public ?Collection $students = null;
     /**
      * Multiple deserialization formats
      * Type-hint doesn't exactly match JMS type information
      * Timezone left for another PR
      * @Serializer\Type("DateTime<'Y-m-d\TH:i:sP', 'UTC', ['Y-m-d', 'Y-m-d\TH:i:sO']>")
      */
     private ?DateTimeInterface $startDate = null;
     /**
      * Multiple deserialization formats
      * Type-hint doesn't exactly match JMS type information
      * @Serializer\Type("DateTimeImmutable<'Y-m-d\TH:i:sP', 'UTC', ['Y-m-d', 'Y-m-d\TH:i:sO']>")
      */
     private ?DateTimeInterface $endDate = null;

    public function __construct()
    {
        $this->students = new ArrayCollection();
    }

    public function getStartDate(): ?DateTimeInterface
    {
        return $this->startDate;
    }

    public function setStartDate(?DateTimeInterface $startDate): void
    {
        $this->startDate = $startDate;
    }

    public function getEndDate(): ?DateTimeInterface
    {
        return $this->endDate;
    }

    public function setEndDate(?DateTimeInterface $endDate): void
    {
        $this->endDate = $endDate;
    }
}

$parser = new Parser([
    new ReflectionParser(),
    new JMSParser(new AnnotationReader()),
    new PhpDocParser(),
    new LiipMetadataAnnotationParser(new AnnotationReader()),
    new VisibilityAwarePropertyAccessGuesser(),
]);

$builder = new Builder($parser, new RecursionChecker());
$metadata = $builder->build(IndirectAccess::class, [new TakeBestReducer()]);
//$classes = [IndirectAccess::class];
$classes = [Student::class, Course::class];
$configuration = GeneratorConfiguration::createFomArray([
    'classes' => array_fill_keys($classes, []),
]);

@mkdir('./cached');
@mkdir('./cached/liip');
$cacheDirectory = 'cached/liip';
$deserializerGenerator = new \Liip\Serializer\DeserializerGenerator(new \Liip\Serializer\Template\Deserialization(), $classes, $cacheDirectory);
$deserializerGenerator->generate($builder);

$serializer = new \Liip\Serializer\Serializer($cacheDirectory);
//$indirectAccessesData = [
//    ['name' => 'student', 'parent' => 'course'],
//];
//var_dump(array_map(
//    fn($data) => $serializer->fromArray($data, IndirectAccess::class), $indirectAccessesData
//));
var_dump($serializer->fromArray([
    'name' => 'Advanced php - II',
    'students' => [
        ['name' => 'Bob', 'created_at' => '2020-01-01', 'updated_at' => '2023-05-09'],
        ['name' => 'Alice', 'created_at' => '2021-02-12', 'updated_at' => '2023-05-09'],
    ],
    'start_date' => '2021-05-07T23:52:45+0000',
    'end_date' => '2023-06-15',
], Course::class));

P.S.: First time I've made a PR with the tests first 🥳 but it's due to my own mistake😞

[brutal-factories]

The second to last commit of that PR also had a bug if i remember correctly, that PhpStan accidentally caught. Would it be weird to make a phpstan analysis of the generated code part of the automated tests?

[dbu]

thanks for digging into this and providing the fix.

using phpstan on the generated code sounds like an interesting idea indeed. if it does not give us lots of false positive, i am all for it. want to try a pull request?

i will fix the cs issues separately