driftingly / rector-laravel

Rector upgrades rules for Laravel
http://getrector.org
MIT License
579 stars 54 forks source link

[Idea] UnifyModelDatesWithCastsRector: Make rule configurable for date casting #123

Open mcampbell508 opened 1 year ago

mcampbell508 commented 1 year ago

First of all, this project looks fantastic. I used this rule on a codebase and it covered 90% of my needs.

However, this is just a proposal but could there be a way to add configuration to this rule, so that you can cast specific properties to values that the end user defines?

I.e something that is not just datetime?

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->ruleWithConfiguration(UnifyModelDatesWithCastsRector::class, [
        new UnifyModelDateWithCast('App\Models\User', 'date_of_birth', 'immutable_date'), 
        new UnifyModelDateWithCast('App\Models\AnotherModel', 'another_column', 'immutable_datetime'), 
        new UnifyModelDateWithCast('App\Models\YetAnotherModel', 'another_column', 'date'), 
    ]);
};

and UnifyModelDateWithCast would be something like:

<?php

namespace App\Namespace;

class UnifyModelDateWithCast
{
    public function __construct(
        private readonly string $modelNamespace, 
        private readonly string $column, 
        private readonly string $castToValue, 
    ) {
    }
}

UnifyModelDateWithCast was just off the top of my head, and could be named more appropriately.

and everything else not specified here using the config will be set as datetime, as the current behaviour. Also, by default if there is no configuration, all casting will be done as datetime, as the current behaviour.

Desired outcome of above

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

 class User extends Model
 {
     protected $casts = [
-        'age' => 'integer',
+        'age' => 'integer', 'date_of_birth' => 'immutable_date',
     ];
-
-    protected $dates = ['date_of_birth'];
 }
namespace App\Models;

use Illuminate\Database\Eloquent\Model;

 class AnotherModel extends Model
 {
     protected $casts = [
-        'age' => 'integer',
+        'age' => 'integer', 'another_column' => 'immutable_datetime',
     ];
-
-    protected $dates = ['another_column'];
 }
namespace App\Models;

use Illuminate\Database\Eloquent\Model;

 class YetAnotherModel extends Model
 {
     protected $casts = [
-        'age' => 'integer',
+        'age' => 'integer', 'another_column' => 'date',
     ];
-
-    protected $dates = ['another_column'];
 }

Ideally, looking at this https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L750-L784, the accepted values should be anything that is:

I've just gotten into learning about Rector, so could attempt to make a PR for the above, if it was desired behaviour of this project.

I am also not sure of how possible, all of this would be.

Again, all just an idea, which may or may not be progressed upon or even desired.

GeniJaho commented 7 months ago

Cool idea 😁

The rule UnifyModelDatesWithCastsRector is meant to be run just once per project, and in fact is part of the rules for upgrading to Laravel 10, since the $dates property was removed.

A simpler change that we can do to the rule is to allow casting to something other than datetime, but again, this would only be helpful if you're still using the $dates property in new day-to-day code. In that case, a custom rule would be a better choice in my opinion.