-
Notifications
You must be signed in to change notification settings - Fork 93
Session/SessionInterface inconsistency #323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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:
Reality:
Solution 1:
Solution 2:
|
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. |
Hi @ondrejmirtes,
After the merge of 012305d
(cf #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
getFlashBag
method.This is not the case after the merge of symfony/symfony@c7b5802 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...
Currently,
RequestStack::getSession
is returningSessionInterface
whenRequest::getSession()
is returningSession
.This should be fixed by reverting the stub for request, or adding a stub for RequestStack.
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
$request::getSession
returns a SessionInterface.$request::getSession
returns a Session.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 ?
The text was updated successfully, but these errors were encountered: