Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle array value casting #195

Open
nlemoine opened this issue Oct 8, 2024 · 3 comments
Open

Handle array value casting #195

nlemoine opened this issue Oct 8, 2024 · 3 comments
Labels
question Further information is requested

Comments

@nlemoine
Copy link

nlemoine commented Oct 8, 2024

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
Copy link
Member

Korbeil commented Oct 25, 2024

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

@Korbeil Korbeil added the question Further information is requested label Oct 25, 2024
@MrMeshok
Copy link
Contributor

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants