Possible improvement of the default requestGetAttributeMapping for "site"

[ervaude]

Hi, I just noticed, that $request->getAttribute('site') is by default configured to return a Site object. The attribute could, however, also hold an instance of NullSite, so I reconfigured the requestGetAttributeMapping for site in our projects to the SiteInterface.

parameters:
    typo3:
        requestGetAttributeMapping:
            site: TYPO3\CMS\Core\Site\Entity\SiteInterface

Maybe you want to adjust the default configuration? Note, that this would likely lead to some new error reports for people, so if you don't want to do it, just close this issue :)

Thanks for this great package, we use it a lot.

Cheers

[sascha-egerer]

You're right. I'm a bit scared to change it as we then have again a change that required a large set of code to be "guarded" in many projects. It would be pretty cool to be able to limit this to phpstan setups that your running with a high level but not for setups with low levels. I've created an issue in PHPStan to ask about such a "level based config file load support". https://github.com/phpstan/phpstan/issues/8887

[alexanderschnitzler]

The attribute could, however, also hold an instance of NullSite, so I reconfigured the requestGetAttributeMapping for site in our projects to the SiteInterface.

That's technically right but it shows how bad this new kind of API actually is. What I usually want from the site, is the router. And that's only accessible through the implementation, not the interface. Even worse: Router needs a Site as constructor argument, not a SiteInterface. So, where do you reliably get a Site from? Not just during runtime, but also during static code analysis?

I think the core needs to streamline things and this issue is just a symptom of a pain that isn't caused by phpstan.

Another example is the routing attribute. Early in the middleware stack it holds an instance of SiteRouteResult, later in the stack it's overridden with an instance of PageArguments. Just horrible. And this is not a rant. I just saw this issue and was working on the aspects, another (still new enough) api which is annoying. ^^

[sabbelasichon]

@sascha-egerer I would not change it now. Only with a new release. Everyone can configure it nonetheless.