orchestral/testbench

PHPStan warnings when using $this->app

Closed this issue · 6 comments

pionl commented
  • Testbench Version: 7.13.0
  • Laravel Version: 9.40.1
  • PHP Version: 8.1.10
  • Database Driver & Version: none

Description:

Hi,

Changes from this commit 3eceaa407543884e812e40f2f2ebcbf6c20e4bfe922c6ee5bb9319c02b10ccfa introduces PHPStan warnings in user code when using $this->app in test case (due the fact null was added).

I've mitigated this by providing own test case and adding this method that makes PHPStan happy (I'm not using the property anymore).

    public function app(): Application
    {
        if ($this->app === null) {
            throw new LogicException('App not initialized');
        }

        return $this->app;
    }

There are probably 3 ways to fix this:

  • make $app private and provide method to access it
  • change to non null phpdoc and check if app is set using temporary property (not ideal?).
  • ignore phpstan null warnings within testbench core (not ideal?)

Steps To Reproduce:

  • Use the package
  • Add $this->app->XX in your test
  • Run PHPStan on max level

Thank you for the package and have a great day,

Why Testbench have to ignore phpstan error when your package expect to run phpstan at max level without any issue?

pionl commented

Well I don't understand your response :) I think it would be great to provide sensible option when using $this->app and PHPStan (due the fact, that test case will setup $this->app before any consumer code uses it).

Thats all. If you don't think it is important and consumers should handle it by self then OK.

Thanks,

Consumer should handle it, $this->app is null between __construct until seTup and null again between tearDown till __destruct.

pionl commented

Yes, thats why app() method can help make the code safe (when we always want Application in our tests), due the change you have to do this (which will introduce more problems in consumer code):

if ($this->app !== null) {
     $this->app->make(XX)
}

Instead of providing safe way to access it:

$this->app()->make(XX);

This is not great DX. But, my point was just letting you know that this was introduced.

Have a great day,

Why don't you use the available app()?

pionl commented

Because i prefer to use more object oriented approach then global function (maybe to safe some nano seconds 😅). In rare occasions I can swap $this->app without breaking facades etc (which I do not use at this moment). Thank you for your time