[9.x] Adds feature to include attribute return type as part of output of model:show command

[tcampbPPU]

I really enjoy the new artisan model:show command, I find this very useful!

I have worked on a similar project modeltyper where we had similar functionality, but to produce an output for Typescript. 1 Thing I added in that package which it looks like is missing here is the ability to also include the Return Type for the Model Attributes / Accessors.

Given now that there are 2 primary ways of creating these now given the traditional style and with the new Attribute class the developer needs to includes these to be "discoverable" like so:

Note Using the Traditional Style

<?php
 
namespace App\Models;
 
use Illuminate\Database\Eloquent\Model;
 
class User extends Model
{
     public function getFirstNameAttribute($value): string
    {
        return ucfirst($value);
    }
}

Then using the newest style:

Note Using the Newest Style. Notice the short function can have a return type

<?php
 
namespace App\Models;
 
use Illuminate\Database\Eloquent\Casts\Attribute;
use Illuminate\Database\Eloquent\Model;
 
class User extends Model
{
    protected function firstName(): Attribute
    {
        return Attribute::make(
            get: fn ($value): string => ucfirst($value),
        );
    }
}

Now both can produce an output like so:

Attributes ........................................ type / cast  
id increments, unique .................... bigint unsigned / int  
name fillable ............................. string(255)  
created_at nullable ...................... datetime / datetime  
updated_at nullable .......................datetime / datetime  
first_name ................................. string / attribute 

[jessarcher]

Hey @tcampbPPU, thanks for the PR!

It looks like the code only applies to "virtual" attributes/accessors (i.e. those that don't have a matching database column behind them) but in your example above the attributes/accessors appear to be for actual columns (otherwise, what are you calling ucfirst() on?).

If we were going to do this, I think the return type would need to be displayed after the attribute/accessor cast. E.g.

first_name .................... attribute(string)

or

first_name ..................... attribute:string

That way we could still show the column type if it exists as well.

Ultimately I don't think it's worth it though. You'd still need to dig into the model to see what the cast actually does. I think it's enough that we show that there is a cast.

I could see value in adding the return type to the JSON output though, but only if there was a tool that specifically needed that information.

[tcampbPPU]

The return type should work for both virtual and non-virtual attributes/accessors, from my example I was just trying to keep it as simple as possible using the examples from the Laravel Docs.

I like and fine with the idea of appending the return type at the end

accessor(string)
attribute(string)

But yes, regardless the Reflection API would need you (as the developer) to include the return type to include these, if they don't include them then that is fine too, that value will be null and not be present in the output but still show as either accessor or attribute.

However for myself I find that little extra detail, when I do include the return types very helpful in the future.

[tcampbPPU]

Like for example your Model could have:

Which there would be no DB column for...

protected function fooBar(): Attribute
{
    return Attribute::make(
        get: fn (): string => 'foo bar',
    );
}

and you will still get the ouput

foo_bar ............................................................... string / attribute  

[tcampbPPU]

Having them listed after will totally work, if that makes the most sense

And not adding the return type, would just shown as normal

[tcampbPPU]

Also to your other point I believe if you are naming an Attribute the same as an existing DB Column the Column would take priority, meaning it would use its already declared types, that's what the

->reject(fn ($cast, $name) => collect($columns)->has($name))

is doing I think.

So the attributes type would not be included from the method, since the DB should be used in that case.

[tcampbPPU]

Hi @taylorotwell,

Since you marked this PR as draft did you want me to use @jessarcher suggestion and display the return type after the attribute/accessor?

[tcampbPPU]

Any update on what to do here? I have both options working:

first_name ..................... string / attribute # or acessor depending on what was used to create the attribute

or

first_name ..................... attribute:string

As mentioned this should work for both virtual and non-virtual attributes and will only produce that extra "type" output if the developer includes the return type, otherwise it will continue to work as it does now.

[jessarcher]

I'm confused as to how this will do anything for non-virtual attributes.

The PR is only updating getVirtualAttributes() and not getAttributes() as well.

[tcampbPPU]

maybe I am wording it wrong but it should work with both attributes back by DB and non backed by DB. Say you have model with name column and you have attribute to get first letter

  protected function firstLetter(): Attribute
  {
      return Attribute::make(
          get: fn (): string => $this->name[0]
      );
  }

or made up / non existing / non back db value like:

protected function fooBar(): Attribute
{
    return Attribute::make(
        get: fn (): string => 'foo bar',
    );
}

The getVirtualAttributes method gets all the methods of the model return collect($class->getMethods()) that alone does not filter backed db column attributes and non-backed db column attributes

[jessarcher]

Okay, I see the misunderstanding now.

When I talk about non-virtual attributes, I'm referring to attributes with the same name as a database column.

For example, the following attribute that customises the name column on Laravel's default User model:

protected function name(): Attribute
{
    return Attribute::make(
        get: fn ($value): string => strtoupper($name)
    );
}

With this PR, I'd have expected model:show to look like this:

name fillable ............ string / attribute:string

But at the moment it looks like this:

name fillable ................... string / attribute

[tcampbPPU]

Why would you expect the type to be both listed before and after?

Within the getVirtualAttributes there is that check I belive to use what the database has provided

->reject(fn ($cast, $name) => collect($columns)->has($name))

Should that no longer be used and check both the database column type, and attribute method return type if provided?

[tcampbPPU]

Should that no longer be used and check both the database column type, and attribute method return type if provided?

If that is the case I can make that work if needed

[tcampbPPU]

yup I think I can make this work and my project model looks a little like

<?php

namespace App\Models;

/**
 * App\Models\Project
 *
 * @property int $id
 * @property int $user_id
 * @property int|null $team_id
 * @property string $name
 */
class Project extends Model
{
    protected $fillable = [
        'user_id',
        'team_id',
        'name',
    ];

    protected function name(): Attribute
    {
        return Attribute::make(
            get: fn (): string => strtoupper($this->name)
        );
    }
}

let me just clean it up a little

[jessarcher]

Why would you expect the type to be both listed before and after?

The type that's listed before is the database column type, the type that's listed after is the return type of the attribute. They won't always be the same. Imagine a json column with an attribute that returns a value object. Or an integer column that maps to some sort of status string or an object.

It seems inconsistent to include the attribute return type for a virtual attribute, but not for a regular attribute.

An alternative could be to display the return type alongside the column type, but only when it's different from the column type:

E.g.

status .............. integer:string / attribute

And for virtual attributes, it would just be the return type, like you originally had.

This could lead to confusion when there is no return type specified as it might imply that the return type is the same as the column type, which might not actually be true.

I'm still not sold on this feature but will leave that up to Taylor.

Do you actually need the output in the CLI or are you mostly just wanting it in the JSON output to generate TypeScript definitions?

[tcampbPPU]

Do you actually need the output in the CLI or are you mostly just wanting it in the JSON output to generate TypeScript definitions?

I feel not having this feels incomplete, if the information is there why not use it? If the purpose of this command as a whole is to display info about the model and we know the attribute/accessor return type but don't include it then I must go back into the Model file and look.

I have updated this to include the return type for the following scenarios To start off I have Project model let say it has:

• id
• user_id
• name

I have Attribute that looks like:

protected function name(): Attribute
{
    return Attribute::make(
        get: fn (): string => strtoupper($this->name)
    );
}

I should get:

Choose to not include return type:

If I create Accessor:

  public function getNameAttribute(): string
  {
      return strtoupper($this->name);
  }

I should get:

If you where to include both I belive whatever order you listed protected function name() or public function getNameAttribute() the later would be used last regardless it would still include the type if needed

[jessarcher]

I feel not having this feels incomplete, if the information is there why not use it? If the purpose of this command as a whole is to display info about the model and we know the attribute/accessor return type but don't include it then I must go back into the Model file and look.

Fair enough. For me, showing that it has an accessor/attribute is enough for an overview. If I want to know more about what will be returned then I'll dig into the code. Showing the return type helps, but I'm still probably going to dive into the code anyway to see what actual transformation is being done. There was a lot of extra stuff I wanted to display in this command, but I didn't want the output to be too full-on.

I definitely see the value in including this in the JSON output as that could be used for type definitions. I would probably want to see the return type in a separate property of the JSON output though, rather than being tacked onto the cast property.

If you where to include both I belive whatever order you listed protected function name() or public function getNameAttribute() the later would be used last regardless it would still include the type if needed

If both exist, Laravel will use the accessor, so I would be making sure this does the same:

https://github.com/laravel/framework/blob/bb91bf7c0883707dfef34a20a87e95a4f75e6c29/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L1980-L1989

model:show already prioritises accessors over attributes if both exist.

[driesvints]

@tcampbPPU @jessarcher anything that should be followed up here? Or should we close this PR?

[driesvints]

Closing this for now. Feel free to resend if you wan to continue work on this.