larastan / larastan

⚗️ Adds code analysis to Laravel improving developer productivity and code quality.
MIT License
5.52k stars 414 forks source link

`phpstan analyse` hangs assigning a model property in a trait #2075

Closed oli-laban closed 2 weeks ago

oli-laban commented 2 weeks ago

Description

I have installed Larastan into an existing project but I'm unable to run phpstan analyse as it hangs indefinitely at a particular line.

If I uninstall Larastan, the command runs as expected, albeit with lots of errors as I have made no changes to appease PHPStan yet.

I've done nothing to configure it yet other than copy/paste the phpstan.neon from the readme.

Laravel code where the issue was found

The offending file is a simple model trait:

<?php

namespace App\Models\Concerns;

use Illuminate\Database\Eloquent\Model;

trait HasUpdatedByColumn
{
    protected static function bootHasUpdatedByColumn(): void
    {
        static::creating(self::setUpdatedByColumn(...));
        static::updating(self::setUpdatedByColumn(...));
    }

    protected static function setUpdatedByColumn(Model $model): void
    {
        /** @noinspection PhpPossiblePolymorphicInvocationInspection */
        $model->updated_by = 'Web';
    }
}

In particular, the $model->updated_by = 'Web'; line. If I comment that out, PHPStan doesn't hang. Ignoring the line makes no difference, it has to be commented out.

Ouput

> sail bin phpstan analyse -vvv --debug
Note: Using configuration file /var/www/html/phpstan.neon.
Result cache not used because of debug mode.
/var/www/html/app/Casts/ArgbColorCast.php
--- consumed 24 MB, total 106.5 MB, took 2.19 s
/var/www/html/app/Providers/AppServiceProvider.php
--- consumed 22 MB, total 128.5 MB, took 3.55 s
/var/www/html/app/Providers/AwsServiceProvider.php
--- consumed 4 MB, total 132.5 MB, took 0.64 s
/var/www/html/app/Providers/TenancyServiceProvider.php
--- consumed 18 MB, total 150.5 MB, took 3.12 s
/var/www/html/app/Providers/DatabaseServiceProvider.php
--- consumed 0 B, total 150.5 MB, took 0.46 s
/var/www/html/app/Providers/FakerServiceProvider.php
--- consumed 6 MB, total 156.5 MB, took 0.90 s
/var/www/html/app/Enums/ChannelAccessFlags.php
--- consumed 0 B, total 156.5 MB, took 0.02 s
/var/www/html/app/Enums/Concerns/FromName.php
--- consumed 0 B, total 156.5 MB, took 0.08 s
/var/www/html/app/Models/Venue.php

Where Venue is the first model it encounters that includes this trait.

As you can see, there's not much to go on from the output. If you can give me some direction in order to provide more useful information/logs, I'm happy to do so.

Thanks.

calebdw commented 2 weeks ago

Here is my UpdatedByTrait:

trait UpdatedByTrait
{
    public static function bootUpdatedByTrait(): void
    {
        static::registerModelEvent('saving', function ($model): void {
            $model->updated_by = Auth::id();
        });
    }

    /** @return BelongsTo<User,$this> */
    public function updatedByUser(): BelongsTo
    {
        return $this->belongsTo(User::class, 'updated_by');
    }
}
oli-laban commented 2 weeks ago

@calebdw Thanks, it no longer hangs on that file and that's a better way to do it anyway 😄 It seems I can't type hint Model in the closure though as that reintroduces the issue (not the end of the world).

However, it's immediately moved to hanging on another file now, again while interacting with a model property:

trait CachesAttributes
{
    protected function cacheKey(string $attribute): string
    {
        return sprintf(
            '%s/%s-%s:%s',
            $this->getTable(),
            $this->getKey(),
            $attribute,
            $this->last_updated->timestamp, // <-- This line
        );
    }

    protected function remember(string $attribute, Closure $callback, ?int $ttl = null): mixed
    {
        return Cache::remember($this->cacheKey($attribute), $ttl ?? 1440, $callback);
    }
}

Which may point to a wider issue.

calebdw commented 2 weeks ago

It's because you're trying to access database properties on a general Model instance---you need to use the public api for that

In the second case, that should work, but it's probably hanging because it's trying to parse all of your schema dumps / migrations to check if the property exists. Just let it do its thing---if it still takes to long then you'll probably need to look into squashing your migrations

oli-laban commented 2 weeks ago

@calebdw

It's because you're trying to access database properties on a general Model instance---you need to use the public api for that

Yeah I understand that I was likely going to get an error there, but not hang the entire command.

In the second case, that should work, but it's probably hanging because it's trying to parse all of your schema dumps / migrations to check if the property exists. Just let it do its thing---if it still takes to long then you'll probably need to look into squashing your migrations

You're right, it was some particularly large schemas. The Laravel app doesn't handle the database migrations but I'm hijacking the SchemaState stuff to clone hosted test DBs. disableSchemaScan solved the issue, thanks for pointing me in that direction!

calebdw commented 2 weeks ago

Yeah I understand that I was likely going to get an error there, but not hang the entire command.

You would have received an error after all the migration scanning was completed

oli-laban commented 2 weeks ago

@calebdw Ah yeah, I hadn't clocked that the original file was the same issue. Cheers for your help.