phpstan / phpstan-symfony

Symfony extension for PHPStan
MIT License
709 stars 89 forks source link

Session/SessionInterface inconsistency #323

Closed VincentLanglet closed 1 year ago

VincentLanglet commented 1 year ago

Hi @ondrejmirtes,

After the merge of https://github.com/phpstan/phpstan-symfony/commit/012305dab7a08f4e1a7158e01d7b4bccfbc6fa31 (cf https://github.com/phpstan/phpstan-symfony/pull/233)

The return type of Request::getSession is Session instead of SessionInterface. This was not really true be could be considered as a good help for developers since

1) This is not the case after the merge of https://github.com/symfony/symfony/commit/c7b5802f8ae1bd901e2506bc9f2b4b72522eb817 in Symfony 6.2. Now, it's possible to do a $request->getSession() instanceof FlashBagAwareSessionInterface check. And, because of the stub, currently the stub is reported as always true which is not true...

2) Currently, RequestStack::getSession is returning SessionInterface when Request::getSession() is returning Session. This should be fixed by reverting the stub for request, or adding a stub for RequestStack.

3) The Request stub is loaded dynamically with the following extension https://github.com/phpstan/phpstan-symfony/blob/1.2.x/src/Symfony/InputBagStubFilesExtension.php#L19 which means that

Not sure this was expected

I'd like to improve the situation, what's your point of view about this ? The easiest thing to do would be at least to introduce a stub for RequestStack, but there is maybe something better to do ?

ondrejmirtes commented 1 year ago

I just spent a day figuring out how to work around some mess in DoctrineBundle. I'm not keen on spending more time on framework-specific problems anytime soon 😊

VincentLanglet commented 1 year ago

I just spent a day figuring out how to work around some mess in DoctrineBundle. I'm not keen on spending more time on framework-specific problems anytime soon 😊

I understand. I'm willing to do the PR I just wanted to know which choice you prefer.

If I can simplify: Stubs today:

Reality:

Solution 1:

Solution 2:

github-actions[bot] commented 1 year ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.