Add return types to SessionHandler

https://git.drupalcode.org/project/drupal/-/merge_requests/4751 should be transferred over here.

Also adapted the CR from the original issue.

Seemed to cause nightwatch failures.

Actually seeing this on other tickets. Wonder if head broke.

Yup was broken HEAD.

Thanks @andypost!

Hmm, this seems too disruptive for a minor to me, at least without additional process or tooling. We'd want to provide best-effort BC for typehint changes, or at least a tool to automatically fix all typehint changes, and I think if nothing else it should target 11.0.x with a single rector rule for all added typehints or something (if we can't come up with a BC way to document the typehint additions in a D10 minor, which I think we also could do somehow-or-other). We've never broken BC even for an internal API like this before.

It'd also need at least a CR if not a release note -- possibly one single CR or something that documents all the typehint additions in a given major-or-minor, and a single simple release note about typehint additions linking that CR. Or something.

Also, there are whole sprawling policy issues about how to go about adding typehints to existing code; this issue should have one of them as a parent, or at least related issues.

Tagging for RM review while I check with @catch about this.

Updated the issue summary with information from the original issue 🐛 Correctly implement SessionHandlerInterface Fixed and also linked the CR from over there.

Thanks @znerol. The stuff we need a CR for is not the original API, but the typehint specifically. It is not related to the API itself; I am talking about a generic CR for added return typehints. So retagging.

Adding this to the meta issue #3050720

We did 📌 Make DebugClassLoader ignored deprecations more accurate Fixed recently, this means that during tests, Symfony's debug classloader will issue deprecations for any parent method that has a documented type hint that it might add a return type hint in the future. So contrib will get notified when it's extending core classes that have documented type hints but haven't added them yet. I would prefer it if this was configurable via an attribute on methods or similar, but it's not, they just issue the deprecation for any random method on the basis that child classes can add type hints and it won't do any harm.

However, with this issue specifically, the parent interface, which is PHP's own interface, already has the return typehints, and you already get deprecation notices from PHP itself, if you implement those methods without a [#ReturnTypeWillChange] attribute (which is what's being removed here). So PHP is already doing the deprecation for us in this case.

https://www.php.net/manual/en/class.sessionhandlerinterface.php

Thanks @catch; that's compelling. I think maybe we can do the following:

1. Create a second CR about typehint changes in 10.2, and include this and any other issues.
2. Add a release note about typehint changes generally linking that first CR.
3. Update the CR to make it a little more clear about the typehint change and the consequences/mitigations.

Then we can use a similar pattern for the rest that we implement in D11.

Issue now targets 10.3, added a dedicated change record now.

Thank you, back to RTC

Committed/pushed to 11.x (which will become 10.3.x too), thanks!

Please close the MR

znerol → closed merge request !4785

Automatically closed - issue fixed for 2 weeks with no activity.