barryvdh / laravel-ide-helper

IDE Helper for Laravel
MIT License
14.17k stars 1.16k forks source link

Always generate @property when a field is a database column #1280

Open vpratfr opened 2 years ago

vpratfr commented 2 years ago

Versions:

Description:

We have models with property mutators. The generated code looks fine when the mutators are both created (getter + setter).

However, the comment is not really correct when only one of them is included.

Mutators can be used for instance to clean up data when saving a field, but we do not need to implement the corresponding getter.

The IDE is fooled to consider that it is not safe to write/read from those properties whereas in reality it is.

Steps To Reproduce:

class Sample extends Model {

    protected $fillable = ['field_one', 'field_two', 'field_three', 'field_four'];

    // A field which only has a getter
    public function getFieldOneAttribute(): string {
        return Str::upper($this->attributes['field_one'] ?? '');
    }

    // A field which only has a setter
    public function setFieldTwoAttribute($value): string {
        $this->attributes['field_two'] = Str::lower($value ?? '');
    }

    // A field with both
    public function getFieldThreeAttribute(): string {
        return Str::upper($this->attributes['field_three'] ?? '');
    }

    public function setFieldThreeAttribute($value): string {
        $this->attributes['field_three'] = Str::upper($value ?? '');
    }
}

In that case, the following is generated:

 * @property-read $field_one
 * @property-write $field_two
 * @property $field_three
 * @property $field_four

PHPStorm will then issue an error if you do that:

$model->field_one = 'something'; // This is however correct as we want to write that value as it is to the DB
$something = $model->field_two; // This is however correct as we want to read that value as it is from the DB

Would it be possible to always have @property when the field with a getter/setter is also a database column? And keep the current behaviour when the implemented getter/setter is defining a new computed property (in which case, being more specific about -write or -read is indeed the expected behaviour)

vpratfr commented 2 years ago

Hmmm I think this is related to #1141

That seems to be working for our models existing in the landlord database. However, this bug appears when dealing with the models from the tenant database.

johnbacon commented 7 months ago

We appear to be having this issue as well, and have confirmed it can be caused by the same thing -- setting only one of an accessor or a mutator.