Larastan: Properties not inferred correctly from database migrations

[DSpeichert]

• Larastan Version: 0.5.2
• --level used: 5

Description

 ------ ------------------------------------------------------- 
  Line   DatabaseNotificationPolicy.php                         
 ------ ------------------------------------------------------- 
  34     Access to an undefined property App\Models\User::$id.  
  34     Access to an undefined property App\Models\User::$id.  
 ------ ------------------------------------------------------- 

Laravel code where the issue was found

    /**
     * Determine whether the user can view the model.
     *
     * @param  \App\Models\User                               $user
     * @param  \Illuminate\Notifications\DatabaseNotification $notification
     * @return mixed
     */
    public function view(User $user, DatabaseNotification $notification)
    {
        return $notification->notifiable instanceof User
            and $user->id == $notification->notifiable->id;
    }

It is my understanding that with #435 merged, the properties found in database migrations should be automatically detected, as the tests in that PR suggests.

Am I missing something here? Is there a way to trace what properties larastan "knows" for a given model?

[szepeviktor]

Thank you for your report.
How does your User model/migration look like?

[szepeviktor]

It is best to use === for comparison :)

[DSpeichert]

I have multiple migrations that modify the table later (application grew over time).
The first one that creates it looks like this:

    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('users', function (Blueprint $table) {
            $table->increments('id');
            $table->timestamps();
            $table->softDeletes();
            $table->string('email');
            $table->string('email_cc')->nullable();
            $table->string('email_previous')->nullable();
            $table->boolean('email_confirmed')->default(0);
            $table->string('email_code')->nullable();
            $table->string('password');
            $table->rememberToken();
            $table->string('old_password')->nullable();
            $table->string('first_name');
            $table->string('last_name');
            $table->string('company')->nullable();
            $table->string('address1')->nullable();
            $table->string('address2')->nullable();
            $table->string('post_code', 10)->nullable();
            $table->string('city', 50)->nullable();
            $table->string('country_code', 2)->nullable();
            $table->string('phone', 14)->nullable();
            $table->string('phone_previous')->nullable();
            $table->boolean('phone_confirmed')->default(0);
            $table->string('phone_code')->nullable();
            $table->string('vat_id', 20)->nullable();
            $table->string('registration_ip')->nullable();
            $table->double('balance', 8, 2)->default(0.00);
            $table->text('admin_notes')->nullable();
            $table->integer('referred_by')->unsigned()->nullable()->index('referred_by');
            $table->boolean('is_locked')->default(0);
            $table->boolean('is_admin')->default(0);
            $table->boolean('is_online')->default(0);

            $table->foreign('referred_by')->references('id')->on('users')->onUpdate('SET NULL')->onDelete('SET NULL');
        });
    }

[szepeviktor]

I think larastan understands increments()

https://github.com/nunomaduro/larastan/blob/8a992b4c6ab1b4254b23bd1e36acb641b37d6993/src/Properties/SchemaAggregator.php#L216

[canvural]

Does it not work for any property or just in the code sample you gave?

[DSpeichert]

Doesn't seem to work in any property for any model, although I can't be sure with hundreds of errors being shown.

[canvural]

Do you have the migration files in the default place? That is at the project root database/migrations folder.

[DSpeichert]

Yes, they're in the default place and generally conform to the default way of doing migrations.

[canvural]

Ok, then properties should be working.

You can run PHPStan just on one file, where you have some property access on models. So we can be sure if it's not working totally or not.

[DSpeichert]

Yes, I did that and it wasn't working. Is there a more verbose mode or some debug mode I can use to see into where possibly the migration-parsing code may not be working properly on my codebase?

[canvural]

Can you show what wasn't working? Any error messages or code samples?

There is no really easy way to debug it. You can try to debug the code in ModelPropertyExtension in IDE with xdebug maybe. Just pass the --debug and --xdebug options to PHPStan.

[azurinspire]

@DSpeichert do you have models in default location on app folder or some where else? For me it seems that properties are working on default Laravel project but in one larger project, which is using different folder and namespace structure as default Laravel, I get errors for all properties.

[canvural]

Model's namespace does not matter. It doesn't even have to have App namespace. It can be MyApp\Nested\Folder\Models\User and it'd be fine.

What errors are you getting?

[azurinspire]

Access to an undefined property Domain\Skills\Models\Course::$id.

The problem is in project which was originally Laravel 5.8, updated to Laravel 7 now. In pristine Laravel 7 project I don't get this error.

[canvural]

There are 2 main conditions now for type checking properties.

1. Model class have to extend Illuminate\Database\Eloquent\Model
2. Migrations should be in the default place. That is now at the app root path database/migrations

And under the hood, we just call getTable on the model to get the table name and try to match that table name in the migrations to find its properties.

Can you confirm these in your case?

[azurinspire]

Both of those are true in my case.

[canvural]

Then I am out of ideas. You have to debug it more yourself. And I will be glad if I can help in anyway.

You can start by putting a dump($this->tables) statement here. So we can see if the migrations are read correctly and how does it look like.

[azurinspire]

It's this import in migration file: use Illuminate\Support\Facades\Schema;

If it is there, no errors, if it is missing, errors appear. It seems that older Laravel versions didn't add this import because there is alias for it so it's not obligatory. Laravel 7 seems to add it automatically now.

---edit
make:model -m has probably added that always, my missing ones were probably in reverse engineered migrations created by a script.

[canvural]

Thank you! That sounds helpful! I'll check how we can solve it.

[canvural]

@azurinspire The issue mentioned is fixed in 0.5.3

@DSpeichert Can you try 0.5.3 and see if it fixes the issue?

[DSpeichert]

@canvural yes, it appears now it's working properly! Thank you!