[Bug]: Better support for static analysis

DanielGSoftware commented 1 week ago

What happened?

Hello there! To start off, I want to say that this "bug" isn't that big of a deal. I believe it to be more relevant when using static analysis tools such as PHPStan and Psalm, which is exactly what we are now implementing in our project. So, I guess this bug report is kind of a discussion to see if it's something that is wanted to be "fixed", and if so, I have a proposed solution.

Alright, let me explain. Assume we want to use a column that extends the normal Column... let's say a ComponentColumn. I might implement this in a table like so:

ComponentColumn::make('Status', 'status')
    ->slot(fn ($foo) => //...),

All goes well. Let's say we also want to use attributes.

ComponentColumn::make('Status', 'status')
    ->component('badges.badge')

    // Let's set attributes
    ->attributes()
    ->slot(fn ($foo) => //...),

When running PHPStan on level 1 or higher, it raises the following error. Call to an undefined method Rappasoft\LaravelLivewireTables\Views\Column::slot(). When visiting the page this table would be present on, there is no error.

So what's going on here? It seems like we are calling slot() on... the Column class? But slot() exists in the ComponentColumnConfiguration trait, which is inherited by our ComponentColumn. The problem here is that the attributes() method, and many others, return self. This means that tools like static analysis see this as returning the class they were originally declared on (Component), and not any that extend this class (ComponentColumn). So

public function attributes(Closure $callback): self
{
    $this->attributesCallback = $callback;

    return $this;
}

Should return static

public function attributes(Closure $callback): static
{
    $this->attributesCallback = $callback;

    return $this;
}

I think that's all! Now, I would understand that this issue doesn't have a high priority, but I believe we are not the only ones using PHPStan and fixing this would be pretty neat. Let me know your thoughts, and if wanted, I'm pretty sure I can create a PR for this.

Package Version

3.2.5

PHP Version

8.3.x

Laravel Version

11.9.2

Theme

Tailwind 3.x

lrljoe commented 6 days ago

Firstly - thank you for asking the question! Always more than happy to take PRs, as is Anthony (rappasoft)

Unless it's a break-fix, we do require PHPStan Level 5 or higher to pass for v3.0+, and when I'm running PHPStan on Level 5, I'm not seeing that error.

We do have a couple of bits for skipping generic errors/known issues (purely so that we can focus on anything NEW that gets introduced)

Example from a new direct pull from the Master branch:

php vendor/bin/phpstan --level=5
Note: Using configuration file master/phpstan.neon.
⚠️  You're using a deprecated config option checkMissingIterableValueType ⚠️️

It's strongly recommended to remove it from your configuration file
and add the missing array typehints.

If you want to continue ignoring missing typehints from arrays,
add missingType.iterableValue error identifier to your ignoreErrors:

parameters:
        ignoreErrors:
                -
                        identifier: missingType.iterableValue

⚠️  You're using a deprecated config option checkGenericClassInNonGenericObjectType ⚠️️

It's strongly recommended to remove it from your configuration file
and add the missing generic typehints.

If you want to continue ignoring missing typehints from generics,
add missingType.generics error identifier to your ignoreErrors:

parameters:
        ignoreErrors:
                -
                        identifier: missingType.generics

 144/144 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors

Please do reach out on the Discord (feel free to send me a DM), so that I can try to replicate what you're experiencing.

DanielGSoftware commented 1 day ago

Hey Joe! Sorry for the little delay, I didn't get any notification of your reply, I'm gonna need to have to figure out why 😛 Anyway, it's correct that the PHPStan build of this package succeeds without any problem. That's because in source code, there is no instance similar to the one I described above. To verify that this error still happens if the code does exist, we can use the existing PetsTable and add the following:

public function columns(): array
{
    return [
        ComponentColumn::make('test')
          ->component('livewire-tables::components.test-component')
          ->attributes(function () {})
          ->slot(fn ($row) => 'test'),
    ];

and then we can run PHPStan on this specific file (by default files in tests dir are not included): vendor/bin/phpstan analyse tests/Http/Livewire/PetsTable.php Result:

In case you're somehow still not able to replicate the error, I will contact you through Discord 😄

rappasoft / laravel-livewire-tables