Incorrect calls to Connection::select() in MenuTreeStorage

Let's see if there's other places which using wrong arguments, it could add triggering error to catch it in contrib

Re-rolled patch #4. Attached interdiff for the same. Please review.

I'm not sure why patch #4 says ' 1148 | ERROR | Expected type hint "string"; found "?string" for $alias' in the code check output.

Patch #5 has formatting errors -- looks like an IDE's formatter has been let loose on it by accident!

+++ b/core/lib/Drupal/Core/Database/Connection.php
@@ -1145,7 +1145,7 @@ public function exceptionHandler() {
-  public function select($table, $alias = NULL, array $options = []) {
+  public function select($table, ?string $alias = NULL, array $options = []) {

Unfortunately this change is not allowed. It will break custom database driver implementations when and they do override the default implementation of this class.
What we can do is do a parameter test in the method __construct(). When the parameter $alias is not a string or is not NULL then throw a deprecation warning which will be replaced by an hard InvalidArgumentException in the next major release. Doing it this way will not break any sites and fixes the problem.

@mondrake: Good find!

Not sure we need to add a deprecation to every constructor but this single place looks enough

It looks good to me.
Contrib and custom module get a deprecation warning message, which will be replaced with a hard InvalidArgumentException in D10.
I do not think that a CR is needed or a deprecation message test.
Created the followup issue #3217961: Prepare for Connection::select() signature to be fully typehinted in D10.
For me it is RTBC.

Committed/pushed to 9.3.x and committed the same patch minus the deprecation to 9.2.x, thanks!

Nearly asked for a deprecation test and a change record, but read #10 and I think I agree - we're calls that were already buggy purely because it's not straightforward to add a type hint.