Handle array value casting

[nlemoine]

Hello,

This is more a question than a bug report.

Are array shapes supposed to be casted? To be more accurate:

<?php

class Dto
{
    public float $age;
}
$source = new Dto();
$target = $autoMapper->map(['age' => '10'], $source);

assert($target->age === 10.0); // ✅

class DtoArray
{
    /**
     * @var array<float>
     */
    public array $age;
}
$source = new DtoArray();
$target = $autoMapper->map(['age' => ['10']], $source);

assert($target->age === [10.0]); // ❌
// $target->age === ['10']

Would it be possible to handle array docblock types?

[Korbeil]

Hey @nlemoine and thanks for reaching us about your issue !

I added your case in my local test suite and reproduced it. I do think this is not even a behavior we do "on purpose", I think this is something we have because we don't add the declare(strict_types=1); on top of the generated mappers.

Also, at the moment we do not add it, but since #180 you can have it in your mappers (it's a configuration that is false by default). When I added it onto your example I had the following result:

$ ./vendor/bin/phpunit --filter=testIssue195
PHPUnit 9.6.21 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 00:00.021, Memory: 14.00 MB

There was 1 error:

1) AutoMapper\Tests\AutoMapperTest::testIssue195
TypeError: Cannot assign string to property AutoMapper\Tests\Dto::$age of type float

/home/bleduc/Sites/korbeil/automapper/tests/cache/Mapper_array_AutoMapper_Tests_Dto.php:22
/home/bleduc/Sites/korbeil/automapper/src/AutoMapper.php:116
/home/bleduc/Sites/korbeil/automapper/tests/AutoMapperTest.php:1612

Since this is a configuration I would like to flip from false to true by default on next major release, your behavior won't even work anymore 😕

I would say this is not something we want to support (array value casting) since we don't even support the "normal" casting and it's just a coincidence it is working at the moment.
@joelwurtz @nikophil what do you think about this issue ?

Baptiste

[MrMeshok]

Hi everyone, I did have a thought about this issue earlier but didn't find the time to work on implementation. I think in theory we can extract array shape from doc block and create anonymous function to cast array values. It would look something like this:

$cast = static fn (float $value) => $value;
$values = [];
foreach ($value['age'] ?? [] as $key => $value_1) {
    $values[] = $cast($value_1);
}

And its works with either strictTypes setting, if we have it disabled value get casted, if it's enabled we get an error and we know something wrong with the source. Also example where it would behavior the same:

class DtoArray
{
    /**
     * @var array<float>
     */
    public array $age;
}

$source = ['age' => [10]];
$target = AutoMapper::create(new Configuration(strictTypes: true))->map($source, DtoArray::class);
assert($target->age === [10.0]); // It's allowed to cast int to float with declare(strict_types=1)

[nlemoine]

Salut @Korbeil !

Thanks for taking the time to provide such detailed feedback 🙂

Having the ability to cast weakly typed values would be a nice addition (maybe opt in, with a enableFlexibleCasting configuration setting, similar to Valinor). There are situations where you have to map poorly typed sources. However, I would totally understand if you think that's out of this library's scope.

Feel free to close the issue if this is an unwanted feature.

[joelwurtz]

In fact we already do casting, if you say that the source may have a string it should do it.

In your case you set array<float> in the source and so we consider this is always the case and don't do any casting.

However if you set array<string|float> we should generate something like this :

if (is_string($value['age'])) {
    $result->age = (float) $value['age'];
} elseif (is_float($value['age'])) {
    $result->age = $value['age'];
}

[joelwurtz]

Hum, but in fact since it's coming from an array we cannot set the source type for that, so it's not possible to have this code generated.

We may need something to force the type of a source or target property instead of assuming it's the same when mapping to / from an array

[nlemoine]

@joelwurtz Indeed, something to enforce (flexible casting) / ensure (strict casting) types would be nice!