Use requestStack to access current request when needed.

[vlad-ghita]

Storing current request in constructor isn't a good idea.
In general, it works (eg: when request from browser or most tests).

But not always.
EG: when test extends WebTestCase and login is executed through $this->loginUser(), then container is built without an active request.
Thus, request is null in constructors ...

Proper fix implies storing $requestStack and accessing current request when required.

This PR fixes Canonical and FrontendMenu (these failed in my use-case, so far).

[bobdenotter]

Hi @vlad-ghita,

At first I though there might be danger of infinite recursion here?

    public function getRequest(): Request
    {
        // always try to use current request
        if ($this->request === null) {
            $this->request = $this->requestStack->getCurrentRequest() ?? Request::createFromGlobals();
            $this->init();
        }

        return $this->request;
    }

    public function init(): void
    {
        // Ensure in request cycle (even for override).
        if ($this->getRequest() === null || $this->getRequest()->getHost() === '') {
            return;
        }

But, that's not true, because init will only be called from getRequest. After which the Request is set.

PHPstan is complaining about it, though.. About it "will always evaluate to false". And i think it's correct. It looks like we can simplify it like this, by removing the bit that's "always false"

    public function init(): void
    {
        // Ensure in request cycle (even for override).
        if ($this->getRequest()->getHost() === '') {
            return;
        }

Would that work for you?

[bobdenotter]

Fix PHPStan's remark:

Strict comparison using === between Symfony\Component\HttpFoundation\Request and null will always evaluate to false.

Suggested change

	if ($this->getRequest() === null || $this->getRequest()->getHost() === '') {
	if ($this->getRequest()->getHost() === '') {

[vlad-ghita]

Agreed with possible infinite loop.
I aimed to keep init() for backwards compatibility.
But maybe we should simply ditch it.

[vlad-ghita]

See latest changes.
I added a request setter.
Please re-run checks.

My tests passed.

[bobdenotter]

Looks good now, thanks @vlad-ghita 👍

I'll merge in after tests are done!