Hint model properties

[AJenbo]

This is helpful for project that run static analyzers on there code, or simply like auto-completion in there IDE.

[AJenbo]

1. I haven't encountered issues with it yet
2. I thought I would start with this one and see how open this project is to these changes.

[AJenbo]

Looking in to things a bit more I think that there is a fault in the application I'm working on where the wrong model has been hinted for this relation so that explains the incorrect properties that I added by mistake. I'll correct them but I'm a bit sure if you guys are just not interested in this change or have issues with the mistakes I made as a first time contributor to this project?

[AJenbo]

Ok, I have adjusted the properties according to the migration found here: https://github.com/spatie/laravel-permission/blob/main/database/migrations/create_permission_tables.php.stub#L28

Please let me know if there are any other adjustments you would like me to make to this PR for it to be ready for merging.

[AJenbo]

@erikn69 since you edited your comment I'm guessing that maybe you also want me to add hints to Role for this PR to be ready? Well in any case I have done so now.

P.s. I'm not here for Hacktoberfest if that is what is causing the terse environment, I can sympathies as I manage a project as well where we had to deal with some less then helpful contributions.

[erikn69]

I am not a staff member, i only helping reviewing, I have nothing against the changes, I just want they are as complete as possible

[AJenbo]

Ok, I saw that you where a regular contributor and didn't know how things are organized in this project. The feedback is still appreciated.

[parallels999]

Is it possible to avoid unnecessary use?

 * @property ?\Illuminate\Support\Carbon $created_at
 * @property ?\Illuminate\Support\Carbon $updated_at

[AJenbo]

You could do that. But I don't think they are unnecessary, you could remove all cases of use as they are all non critical:

<?php

/**
 * @property int                         $id
 * @property string                      $name
 * @property string                      $guard_name
 * @property ?\Illuminate\Support\Carbon $created_at
 * @property ?\Illuminate\Support\Carbon $updated_at
 */
class Role extends \Illuminate\Database\Eloquent\Model implements \Spatie\Permission\Contracts\Role
{
}

Doing so generally degrade readability imo.

[parallels999]

Doing so generally degrade readability

if you leave all that unnecessary and pointless space, it affects readability, look at this:

laravel-permission/src/Traits/HasRoles.php

Lines 65 to 69 in 2ec9bef

	* Scope the model query to certain roles only.
	*
	* @param \Illuminate\Database\Eloquent\Builder $query
	* @param string|int|array|\Spatie\Permission\Contracts\Role|\Illuminate\Support\Collection $roles
	* @param string $guard

[AJenbo]

To me It's hard to distinguish what is a pipe and what is a backslash in that block, also I can't even read the name of the second parameter as it is off screen.

The spaces are not pointless, they are there to align the properties to allow it to be read as a list.

What is the issue with having a potentially extra import line? Calling things unnecessary and pointless is loaded language and doesn't help understanding.

[euoia]

I was going to create a similar PR myself.

I sometimes log the name of the permission, but need to disable phpstan on that line otherwise it complains that name does not exist on permission:

    public function deletePermission($name)
    {
        /** @var Permission $permission */
        $permission = Permission::findByName($name);
        if ($permission !== null) {
            $permission->delete();

            // See https://github.com/spatie/laravel-permission/pull/2230
            // @phpstan-ignore-next-line
            $this->info("Permission deleted: {$permission->name}");
            $this->hasChanges = true;
        }
    }

[jnoordsij]

This would be great to have!

I think currently the best workaround for those wanting to fix this locally is adding a stub file with these 'magic properties' listed. This way you don't need to add manual ignores for each line and can still automatically check for typos in your property-accessing code.

[AJenbo]

@patinthehat what is the best way to get a review from the maintainers?

[angeljqv]

what is the best way to get a review from the maintainers?

The best way is follow preferred coding patterns for the package

[AJenbo]

The best way is follow preferred coding patterns for the package

@angeljqv could you please point me to where the issue is so that I can adjust my PR? This is my first contribution to this project so I'm not familiar with it.

[AJenbo]

@angeljqv Thanks, it was getting hard to see what was the opinion outsiders and what was the project style. I have updated the PR please let me know if anything else needs adjusting.

[angeljqv]

@freekmurze @patinthehat @drbyte ping

[drbyte]

Apologies for the long delay in merging.
Many thanks for your contribution!