Skip to content

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

Closed
VincentLanglet opened this issue Jan 12, 2023 · 3 comments · Fixed by #324
Closed

Session/SessionInterface inconsistency #323

VincentLanglet opened this issue Jan 12, 2023 · 3 comments · Fixed by #324

Comments

@VincentLanglet
Copy link
Contributor

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

  • it was almost always true
  • there was no easy way to check for the getFlashBag method.
  1. 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...

  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

  • For user of SF <5.1, $request::getSession returns a SessionInterface.
  • For user of SF >=5.1, $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 ?

@ondrejmirtes
Copy link
Member

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
Copy link
Contributor Author

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:

  • Request::getSession return Session
  • RequestStack::getSessionInterface returns SessionInterface

Reality:

  • Request::getSession returns SessionInterface, which is Session in 99% of the time
  • RequestStack::getSession returns SessionInterface, which is Session in 99% of the time

Solution 1:

  • I add the same stub for RequestStack::getSession than for Request::getSession.

Solution 2:

  • I remove the stub for Request::getSession.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants