Curious about why Casters can not receive Null Values

[non-senses]

Versions:
Laravel 8
PHP 8.1
Laravel-Data ^1

Firstly, laravel-data is an incredibly useful package. I've been using it to replace Laravel's resources and requests.

For the record, I'm using v1 of the library; I wish I could upgrade, but unfortunately we're stuck with an old version of Laravel.

That being said, pretty often I face an issue that pertains 100% to a design choice you've made, that I can't fully understand.

Here's my example code:

Cast:

class NullToEmptyStringCaster implements Spatie\LaravelData\Casts\Cast
{
    public function cast(Spatie\LaravelData\Support\DataProperty $property, mixed $value): mixed
    {
        if(is_null($value)) {
            return '';
        }

        return (string) $value;
    }
}

Class

<?php
class Demo extends Spatie\LaravelData\Data
{
    public string $name;

    #[Spatie\LaravelData\Attributes\WithCast(NullToEmptyStringCaster::class)]
    public string $email;
}

And now, the code:

Skipping the key email seems to work:

> Demo::from(['name' => 'name']);
= Demo {#9319
    +name: "name",
    +email: "",
  }

However, including email as null, doesn't call the caster, nor uses the default value:

> Demo::from(['name' => 'name', 'email' => null]);

   TypeError  Cannot assign null to property Demo::$email of type string.

The behavior seems to come from this check:

        if ($value === null) {
            return $value;
        }

Given the same check appears in all versions, I assume you have a very good point on keeping that behavior. However, I think it removes an important functionality by avoiding the use of Casters to build values.

Tinkering with your code, I ended up with this check instead:

        if ($value === null && $property->isNullable()) {
            return $value;
        }

This generates the expected behavior in all cases:

1. if an attribute is nullable, and the passed value is null, then it will be set to null.
2. if an attribute is nullable, and the passed value is not null, it will be casted.
3. if an attribute is not nullable, and the passed value is null, then it will be casted.
4. if an attribute is not nullable, and the passed value is not null, then it will be casted.
5. if an attribute is not nullable, and the passed value is null, then it will be casted.

I though of creating a PR to include such isNullable check in the if; but I hesitated a lot because you've been insisting for 4 versions on keeping the same behavior. I think I might be missing a big point here, but wasn't able to figure out what it is.

I could theoretically fork the package and make the changes I need, but before doing so, I wanted to check with you if you have faced any problem that lead you to make the decision you made.

Thank you

Nicolás

[rubenvanassche]

The reason we're having this is not having to write the following line in every cast:

    public function cast(DataProperty $property, mixed $value, array $properties, CreationContext $context): DateTimeInterface|Uncastable
    {
        if($value === null){
            return null;
        }

        // ...
    }

In my opinion, if something is null it also shouldn't magically become something, null is null.

[linaspasv]

well it's opinionated... I guess the NULL cast behavior should come as an option. NULL for me is empty or "undefined", so it makes sense to convert NULL into object with default values defined in DTO.

This is why I have completely overridden the current behavior with:

class DataEloquentCast extends BaseDataEloquentCast
{
    public function get($model, string $key, $value, array $attributes): ?BaseData
    {
        if ($value === null) {
            $value = '{}';
        }

        return parent::get($model, $key, $value, $attributes);
    }
}