chore: Setup PHPStan Level 5

[SychO9]

Closes #3559

Changes proposed in this pull request:

• Sets up PHPStan infrastructure.
• Adapts Docblocks and certain backend typings.
• Fixes some issues caught by phpstan.

Reviewers should focus on:

• Pay most attention to codebase edits.
• Generally they are just minor docblock improvements.
• The most notable change is the replacement of \Illuminate\Database\ConnectionInterface by \Illuminate\Database\Connection. Ideally we would stick to injecting the interface as that is more correct to separate the implementation from the contract. But in most of our codebase, we use methods that only exist within the implementation and not the interface. We could just leave it and fool phpstan with docblocks, but I thought it would be better to be explicit about what we expect injected. Thoughts are very welcome about this especially.

Necessity

• [ ] Has the problem that is being solved here been clearly explained?
• [ ] If applicable, have various options for solving this problem been considered?
• [ ] For core PRs, does this need to be in core, or could it be in an extension?
• [ ] Are we willing to maintain this for years / potentially forever?

Confirmed

• [ ] Frontend changes: tested on a local Flarum installation.
• [ ] Backend changes: tests are green (run composer test).
• [ ] Core developer confirmed locally this works as intended.
• [ ] Tests have been added, or are not appropriate here.

[SychO9]

(@luceos gonna reply here globally here about some of the issues raised here)

Database connection interface that's the thing with ConnectionInterface, I dislike having to inject the actual implementation, but we either do that because that's what our code actually expects because it uses methods exclusive to the implementation or we hack around it.

// The interface
interface ConnectionInterface { ... }

// Illuminate implementation
abstract class Connection extends ConnectionInterface
{
    public function getSchemaBuilder(); // doesn't exist in the interface
}

// Solution 1: actually inject the implementation because that's what we actually expect.
class Example
{
    public function __construct(Connection $db) { ... }

    public function example()
    {
        $this->db->getSchemaBuilder()->...
    }
}

// Solution 2: hack around it everywhere we expect implementation-specific instances.
class Example
{
    public function __construct(ConnetionInterface $db) { ... }

    public function example()
    {
        /** @var Connection */
        $db = $this->db;

        $db->getSchemaBuilder()->...
    }
}

---

Filesystem vs Cloud (https://github.com/flarum/framework/pull/3553#discussion_r933761636, https://github.com/flarum/framework/pull/3553#discussion_r933761734) Illuminate's filesystem Factory contract disk method returns a Filesystem contract, not Cloud. And since PHP doesn't have generics, we're gonna have to hack it with a docblock one way or the other.

---

self vs static PHPStan complains about using static as it can be unsafe when extending and lead to fatal errors. I'm personally not a fan of using static unless it really makes sense. Otherwise, we could add a global ignore for phpstan's error about the static keyword.

---

exists() vs get() I understand the mentioned performance gain, but I believe that is outside the scope of this PR. We only change has to exists and others because the older methods are deprecated.

[tankerkiller125]

@SychO9 in regards to generics PHPStan has a way to work around that issue https://phpstan.org/blog/generics-in-php-using-phpdocs

NVM, I just saw your mention of the docblock 🤦

[luceos]

Thanks for the clarification. I hope this doesn't bite is in the behind in the long run 💪

[SychO9]

I do think it might be better to use stubs to tell phpstan that the interface does have the methods even though that's not the case.

Same for the filesystem, perhaps using a stub to change the return type to Cloud might be better.

Not sure yet, need to consider this more.

[SychO9]

New changes:

• Reverts to injecting ConnectionInterface instead of Connection and ignores related errors, while our codebase expects the actual implementation and it would make sense to inject it instead. I feel like it would be more sane to keep the interface instead.
• Uses a stub to modify the Filesystem contract to have the disk return type changed to Cloud. That's what our codebase expects everywhere.
• Also revets static to self changes and ignores errors. I think this needs more discussion in the long run, while we do this for the sake of extensibility, it just seems to be used anywhere and everywhere, even in private API.

[luceos]

• Also revets static to self changes and ignores errors. I think this needs more discussion in the long run, while we do this for the sake of extensibility, it just seems to be used anywhere and everywhere, even in private API.

You mean you reverted self to static?

I'm okay with this if that's the case 👍 Thanks for considering my argumentation.

[SychO9]

yup self to staticmy bad