Dependency Injection in Cast Classes

[Eroha186]

Hey everyone! I'd like to discuss a small but annoying limitation in how Eloquent resolves custom cast classes.

The problem

When you register a custom cast in $casts, Laravel instantiates it like this (line 1882 in HasAttributes):

return new $castType(...$arguments);

The arguments here are just primitive strings that you pass through a colon in $casts:

protected $casts = [
    'type' => MyCast::class . ':some,args',
];

No container. This means that if your cast needs, for example, a repository, you can't just declare it in the constructor. You have to write app() directly inside the get() or set() method, which hides the dependency and complicates testing.

What I suggest.

One line in resolveCasterClass():

// now
return new $castType(...$arguments);

// I suggest
return count($arguments) > 0
    ? new $castType(...$arguments)  // the old behavior persists
    : app()->make($castType);       // container, if there are no arguments

Laravel already resolves dependencies through a container in Jobs, Listeners, Policies, and so on. Castes break out of this pattern for no apparent reason.

Backward compatibility

Full. If there are string arguments in $casts, the behavior does not change. The only case that changes is when there are no arguments.

What do you think? Are there any reasons why it's intentionally not done that way?

[shaedrich]

// now
return new $castType(...$arguments);

// I suggest
return count($arguments) > 0
    ? new $castType(...$arguments)  // the old behavior persists
    : app()->make($castType);       // container, if there are no arguments

Backward compatibility

Full. If there are string arguments in $casts, the behavior does not change. The only case that changes is when there are no arguments.

I don't see the reason for the above conditional. I imagine this distinction to be quite unintentional. The container can resolve classes with arguments just fine. If we were to change this, I'd say, we do it all the time, not just sometimes.

[Eroha186]

Yes, I agree with you. But in general, how do you like the concept of adding dependency injection to the caste class?

[shaedrich]

I personally never needed it but it doesn't harm to have it for those who need it, especially since it doesn't change anything for those who don't use dependency injection, except for those who will chime in to yell that dependency injection comes at a cost, yet this would be consistent with other parts of the framework, as you pointed out.